[PATCH] Create internally nul terminated string literals in fortan FE

[PATCH] Create internally nul terminated string literals in fortan FE

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

Hi,

this patch changes the Fortan FE to create NUL terminated STRING_CST
objects.  This is a cleanup in preparation of a more thorough check
on the STRING_CST objects in the middle-end.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-fortran-fe.diff --]
[-- Type: text/x-patch; name="patch-fortran-fe.diff", Size: 3624 bytes --]

2018-08-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* trans-array.c (gfc_conv_array_initializer): Remove excess precision
	from overlength string initializers.
	* trans-const.c (gfc_build_wide_string_const): Make the internal
	representation of STRING_CST properly NUL terminated.
	(gfc_build_hollerith_string_const): New helper function.
	(gfc_conv_constant_to_tree): Use it.
	

diff -pur gcc/fortran/trans-array.c gcc/fortran/trans-array.c
--- gcc/fortran/trans-array.c	2018-07-02 09:24:43.000000000 +0200
+++ gcc/fortran/trans-array.c	2018-08-01 06:45:20.529923246 +0200
@@ -5964,6 +5964,32 @@ gfc_conv_array_initializer (tree type, g
 	    {
 	    case EXPR_CONSTANT:
 	      gfc_conv_constant (&se, c->expr);
+
+	      /* See gfortran.dg/charlen_15.f90 for instance.  */
+	      if (TREE_CODE (se.expr) == STRING_CST
+		  && TREE_CODE (type) == ARRAY_TYPE)
+		{
+		  tree atype = type;
+		  while (TREE_CODE (TREE_TYPE (atype)) == ARRAY_TYPE)
+		    atype = TREE_TYPE (atype);
+		  if (TREE_CODE (TREE_TYPE (atype)) == INTEGER_TYPE
+		      && tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (se.expr)))
+			 > tree_to_uhwi (TYPE_SIZE_UNIT (atype)))
+		    {
+		      unsigned HOST_WIDE_INT size
+			= tree_to_uhwi (TYPE_SIZE_UNIT (atype));
+		      unsigned unit
+			= TYPE_PRECISION (TREE_TYPE (atype)) / BITS_PER_UNIT;
+		      const char *p = TREE_STRING_POINTER (se.expr);
+		      char *q = (char *)xmalloc (size + unit);
+
+		      memcpy (q, p, size);
+		      memset (q + size, 0, unit);
+		      se.expr = build_string (size + unit, q);
+		      TREE_TYPE (se.expr) = atype;
+		      free (q);
+		    }
+		}
 	      break;
 
 	    case EXPR_STRUCTURE:
diff -pur gcc/fortran/trans-const.c gcc/fortran/trans-const.c
--- gcc/fortran/trans-const.c	2018-06-10 14:50:03.000000000 +0200
+++ gcc/fortran/trans-const.c	2018-07-31 20:15:21.721153877 +0200
@@ -93,13 +93,16 @@ gfc_build_wide_string_const (int kind, s
   int i;
   tree str, len;
   size_t size;
+  size_t elem;
   char *s;
 
   i = gfc_validate_kind (BT_CHARACTER, kind, false);
-  size = length * gfc_character_kinds[i].bit_size / 8;
+  elem = gfc_character_kinds[i].bit_size / 8;
+  size = (length + 1) * elem;
 
   s = XCNEWVAR (char, size);
   gfc_encode_character (kind, length, string, (unsigned char *) s, size);
+  memset (s + size - elem, 0, elem);
 
   str = build_string (size, s);
   free (s);
@@ -131,6 +134,30 @@ gfc_build_localized_cstring_const (const
 }
 
 
+/* Build a hollerith string constant.  */
+
+static
+tree
+gfc_build_hollerith_string_const (size_t length, const char *s)
+{
+  tree str;
+  tree len;
+  char *p;
+
+  p = XCNEWVAR (char, length + 1);
+  memcpy (p, s, length);
+  p[length] = '\0';
+  str = build_string (length + 1, p);
+  free (p);
+  len = size_int (length);
+  TREE_TYPE (str) =
+    build_array_type (gfc_character1_type_node,
+		      build_range_type (gfc_charlen_type_node,
+					size_one_node, len));
+  TYPE_STRING_FLAG (TREE_TYPE (str)) = 1;
+  return str;
+}
+
 /* Return a string constant with the given length.  Used for static
    initializers.  The constant will be padded or truncated to match
    length.  */
@@ -363,8 +390,8 @@ gfc_conv_constant_to_tree (gfc_expr * ex
       return res;
 
     case BT_HOLLERITH:
-      return gfc_build_string_const (expr->representation.length,
-				     expr->representation.string);
+      return gfc_build_hollerith_string_const (expr->representation.length,
+					       expr->representation.string);
 
     default:
       gcc_unreachable ();

[PING] [PATCH] Create internally nul terminated string literals in fortan FE

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

Hi,

I'd like to ping this patch: https://gcc.gnu.org/ml/fortran/2018-08/msg00000.html

I attach a new version, which contains only a minor white-space change from
the previous version, in the function header of gfc_build_hollerith_string_const
to contain "static tree" on one line instead of two.

Thanks
Bernd.

On 08/01/18 13:32, Bernd Edlinger wrote:
> Hi,
> 
> this patch changes the Fortan FE to create NUL terminated STRING_CST
> objects.  This is a cleanup in preparation of a more thorough check
> on the STRING_CST objects in the middle-end.
> 
> 
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-fortran-fe.diff --]
[-- Type: text/x-patch; name="patch-fortran-fe.diff", Size: 3638 bytes --]

2018-08-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* trans-array.c (gfc_conv_array_initializer): Remove excess precision
	from overlength string initializers.
	* trans-const.c (gfc_build_wide_string_const): Make the internal
	representation of STRING_CST properly NUL terminated.
	(gfc_build_hollerith_string_const): New helper function.
	(gfc_conv_constant_to_tree): Use it.
	

diff -pur gcc/fortran/trans-array.c gcc/fortran/trans-array.c
--- gcc/fortran/trans-array.c	2018-07-02 09:24:43.000000000 +0200
+++ gcc/fortran/trans-array.c	2018-08-01 06:45:20.529923246 +0200
@@ -5964,6 +5964,32 @@ gfc_conv_array_initializer (tree type, g
 	    {
 	    case EXPR_CONSTANT:
 	      gfc_conv_constant (&se, c->expr);
+
+	      /* See gfortran.dg/charlen_15.f90 for instance.  */
+	      if (TREE_CODE (se.expr) == STRING_CST
+		  && TREE_CODE (type) == ARRAY_TYPE)
+		{
+		  tree atype = type;
+		  while (TREE_CODE (TREE_TYPE (atype)) == ARRAY_TYPE)
+		    atype = TREE_TYPE (atype);
+		  if (TREE_CODE (TREE_TYPE (atype)) == INTEGER_TYPE
+		      && tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (se.expr)))
+			 > tree_to_uhwi (TYPE_SIZE_UNIT (atype)))
+		    {
+		      unsigned HOST_WIDE_INT size
+			= tree_to_uhwi (TYPE_SIZE_UNIT (atype));
+		      unsigned unit
+			= TYPE_PRECISION (TREE_TYPE (atype)) / BITS_PER_UNIT;
+		      const char *p = TREE_STRING_POINTER (se.expr);
+		      char *q = (char *)xmalloc (size + unit);
+
+		      memcpy (q, p, size);
+		      memset (q + size, 0, unit);
+		      se.expr = build_string (size + unit, q);
+		      TREE_TYPE (se.expr) = atype;
+		      free (q);
+		    }
+		}
 	      break;
 
 	    case EXPR_STRUCTURE:
diff -pur gcc/fortran/trans-const.c gcc/fortran/trans-const.c
--- gcc/fortran/trans-const.c	2018-06-10 14:50:03.000000000 +0200
+++ gcc/fortran/trans-const.c	2018-07-31 20:15:21.721153877 +0200
@@ -93,13 +93,16 @@ gfc_build_wide_string_const (int kind, s
   int i;
   tree str, len;
   size_t size;
+  size_t elem;
   char *s;
 
   i = gfc_validate_kind (BT_CHARACTER, kind, false);
-  size = length * gfc_character_kinds[i].bit_size / 8;
+  elem = gfc_character_kinds[i].bit_size / 8;
+  size = (length + 1) * elem;
 
   s = XCNEWVAR (char, size);
   gfc_encode_character (kind, length, string, (unsigned char *) s, size);
+  memset (s + size - elem, 0, elem);
 
   str = build_string (size, s);
   free (s);
@@ -131,6 +134,30 @@ gfc_build_localized_cstring_const (const char *msg
 }
 
 
+/* Build a hollerith string constant.  */
+
+static tree
+gfc_build_hollerith_string_const (size_t length, const char *s)
+{
+  tree str;
+  tree len;
+  char *p;
+
+  p = XCNEWVAR (char, length + 1);
+  memcpy (p, s, length);
+  p[length] = '\0';
+  str = build_string (length + 1, p);
+  free (p);
+  len = size_int (length);
+  TREE_TYPE (str) =
+    build_array_type (gfc_character1_type_node,
+		      build_range_type (gfc_charlen_type_node,
+					size_one_node, len));
+  TYPE_STRING_FLAG (TREE_TYPE (str)) = 1;
+  return str;
+}
+
+
 /* Return a string constant with the given length.  Used for static
    initializers.  The constant will be padded or truncated to match
    length.  */
@@ -363,8 +390,8 @@ gfc_conv_constant_to_tree (gfc_expr * expr)
       return res;
 
     case BT_HOLLERITH:
-      return gfc_build_string_const (expr->representation.length,
-				     expr->representation.string);
+      return gfc_build_hollerith_string_const (expr->representation.length,
+					       expr->representation.string);
 
     default:
       gcc_unreachable ();

[PATCHv2] Handle overlength string literals in the fortan FE

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

Hi!


This is an alternative approach to handle overlength strings in the Fortran FE.

The difference to the previous version is that overlength
STRING_CST never have a longer TREE_STRING_LENGTH than the TYPE_DOMAIN.
And those STRING_CSTs are thus no longer zero terminated.

And the requirement to have all sting constants internally zero-terminated
is dropped.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-fortran-fe-v2.diff --]
[-- Type: text/x-patch; name="patch-fortran-fe-v2.diff", Size: 1267 bytes --]

2018-08-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* trans-array.c (gfc_conv_array_initializer): Remove excess precision
	from overlength string initializers.

Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c	(revision 263807)
+++ gcc/fortran/trans-array.c	(working copy)
@@ -5964,6 +5964,26 @@ gfc_conv_array_initializer (tree type, gfc_expr *
 	    {
 	    case EXPR_CONSTANT:
 	      gfc_conv_constant (&se, c->expr);
+
+	      /* See gfortran.dg/charlen_15.f90 for instance.  */
+	      if (TREE_CODE (se.expr) == STRING_CST
+		  && TREE_CODE (type) == ARRAY_TYPE)
+		{
+		  tree atype = type;
+		  while (TREE_CODE (TREE_TYPE (atype)) == ARRAY_TYPE)
+		    atype = TREE_TYPE (atype);
+		  if (TREE_CODE (TREE_TYPE (atype)) == INTEGER_TYPE
+		      && tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (se.expr)))
+			 > tree_to_uhwi (TYPE_SIZE_UNIT (atype)))
+		    {
+		      unsigned HOST_WIDE_INT size
+			= tree_to_uhwi (TYPE_SIZE_UNIT (atype));
+		      const char *p = TREE_STRING_POINTER (se.expr);
+
+		      se.expr = build_string (size, p);
+		      TREE_TYPE (se.expr) = atype;
+		    }
+		}
 	      break;
 
 	    case EXPR_STRUCTURE:

Re: [PATCHv2] Handle overlength string literals in the fortan FE

[Janne Blomqvist]

On Fri, Aug 24, 2018 at 11:06 PM Bernd Edlinger <bernd.edlinger@hotmail.de>
wrote:

> Hi!
>
>
> This is an alternative approach to handle overlength strings in the
> Fortran FE.
>

Hi,

can you explain a little more what the problem that this patch tries to
solve is? What is an "overlength" string?


-- 
Janne Blomqvist

Re: [PATCHv2] Handle overlength string literals in the fortan FE

[Bernd Edlinger]

On 03/09/2018, 21:25 Janne Blomqvist wrote:
> On Fri, Aug 24, 2018 at 11:06 PM Bernd Edlinger <bernd.edlinger@hotmail.de>
> wrote:
>
>> Hi!
>>
>>
>> This is an alternative approach to handle overlength strings in the
>> Fortran FE.
>>
>
> Hi,
>
> can you explain a little more what the problem that this patch tries to
> solve is? What is an "overlength" string?

In the middle-end STRING_CST objects have a TYPE_DOMAIN
which specifies how much memory the string constant uses,
and what kind of characters the string constant consists of,
and a TREE_STRING_LENGTH which specifies how many
bytes the string value contains.

Everything is fine, if both sizes agree, or the memory size
is larger than the string length, in which case the string is simply
padded with zero bytes to the full length.

But things get unnecessarily complicated if the memory size
is smaller than the string length.

In this situation we have two different use cases of STRING_CST
which have contradicting rules:

For string literals and flexible arrays the memory size is ignored
and the TREE_STRING_LENGTH is used to specify both the
string length and the memory size.  Fortran does not use those.

For STRING_CST used in a CONSTRUCTOR of a string object
the TREE_STRING_LENGTH is ignored, and only the part of the
string value is used that fits into the memory size, the situation
is similar to excess precision floating point values.

Now it happens that the middle-end sees a STRING_CST with
overlength and wants to know if the string constant is properly
zero-terminated, and it is impossible to tell, since any nul byte
at the end of the string value might be part of the ignored excess
precision, but this depends on where the string constant actually
came from.

Therefore I started an effort to sanitize the STRING_CST via
an assertion in the varasm.c where most of the string constants
finally come along, and it triggered in two fortran test cases,
and a few other languages of course.

This is what this patch tries to fix.

Bernd.

Re: [PATCHv2] Handle overlength string literals in the fortan FE

[Janne Blomqvist]

On Tue, Sep 4, 2018 at 10:05 AM Bernd Edlinger <bernd.edlinger@hotmail.de>
wrote:

> On 03/09/2018, 21:25 Janne Blomqvist wrote:
> > On Fri, Aug 24, 2018 at 11:06 PM Bernd Edlinger <
> bernd.edlinger@hotmail.de>
> > wrote:
> >
> >> Hi!
> >>
> >>
> >> This is an alternative approach to handle overlength strings in the
> >> Fortran FE.
> >>
> >
> > Hi,
> >
> > can you explain a little more what the problem that this patch tries to
> > solve is? What is an "overlength" string?
>
> In the middle-end STRING_CST objects have a TYPE_DOMAIN
> which specifies how much memory the string constant uses,
> and what kind of characters the string constant consists of,
> and a TREE_STRING_LENGTH which specifies how many
> bytes the string value contains.
>
> Everything is fine, if both sizes agree, or the memory size
> is larger than the string length, in which case the string is simply
> padded with zero bytes to the full length.
>
> But things get unnecessarily complicated if the memory size
> is smaller than the string length.
>
> In this situation we have two different use cases of STRING_CST
> which have contradicting rules:
>
> For string literals and flexible arrays the memory size is ignored
> and the TREE_STRING_LENGTH is used to specify both the
> string length and the memory size.  Fortran does not use those.
>
> For STRING_CST used in a CONSTRUCTOR of a string object
> the TREE_STRING_LENGTH is ignored, and only the part of the
> string value is used that fits into the memory size, the situation
> is similar to excess precision floating point values.
>
> Now it happens that the middle-end sees a STRING_CST with
> overlength and wants to know if the string constant is properly
> zero-terminated, and it is impossible to tell, since any nul byte
> at the end of the string value might be part of the ignored excess
> precision, but this depends on where the string constant actually
> came from.
>
> Therefore I started an effort to sanitize the STRING_CST via
> an assertion in the varasm.c where most of the string constants
> finally come along, and it triggered in two fortran test cases,
> and a few other languages of course.
>
> This is what this patch tries to fix.
>
> Bernd.
>

I guess, I'm slightly confused why this mismatch happens in the first place
(does the Fortran frontend do something dumb wrt string declarations, or?),
but, Ok for trunk.


-- 
Janne Blomqvist

Re: [PATCHv2] Handle overlength string literals in the fortan FE

[Bernd Edlinger]

On 09/05/18 20:16, Janne Blomqvist wrote:
> On Tue, Sep 4, 2018 at 10:05 AM Bernd Edlinger <bernd.edlinger@hotmail.de <mailto:bernd.edlinger@hotmail.de>> wrote:
> 
>     On 03/09/2018, 21:25 Janne Blomqvist wrote:
>      > On Fri, Aug 24, 2018 at 11:06 PM Bernd Edlinger <bernd.edlinger@hotmail.de <mailto:bernd.edlinger@hotmail.de>>
>      > wrote:
>      >
>      >> Hi!
>      >>
>      >>
>      >> This is an alternative approach to handle overlength strings in the
>      >> Fortran FE.
>      >>
>      >
>      > Hi,
>      >
>      > can you explain a little more what the problem that this patch tries to
>      > solve is? What is an "overlength" string?
> 
>     In the middle-end STRING_CST objects have a TYPE_DOMAIN
>     which specifies how much memory the string constant uses,
>     and what kind of characters the string constant consists of,
>     and a TREE_STRING_LENGTH which specifies how many
>     bytes the string value contains.
> 
>     Everything is fine, if both sizes agree, or the memory size
>     is larger than the string length, in which case the string is simply
>     padded with zero bytes to the full length.
> 
>     But things get unnecessarily complicated if the memory size
>     is smaller than the string length.
> 
>     In this situation we have two different use cases of STRING_CST
>     which have contradicting rules:
> 
>     For string literals and flexible arrays the memory size is ignored
>     and the TREE_STRING_LENGTH is used to specify both the
>     string length and the memory size.  Fortran does not use those.
> 
>     For STRING_CST used in a CONSTRUCTOR of a string object
>     the TREE_STRING_LENGTH is ignored, and only the part of the
>     string value is used that fits into the memory size, the situation
>     is similar to excess precision floating point values.
> 
>     Now it happens that the middle-end sees a STRING_CST with
>     overlength and wants to know if the string constant is properly
>     zero-terminated, and it is impossible to tell, since any nul byte
>     at the end of the string value might be part of the ignored excess
>     precision, but this depends on where the string constant actually
>     came from.
> 
>     Therefore I started an effort to sanitize the STRING_CST via
>     an assertion in the varasm.c where most of the string constants
>     finally come along, and it triggered in two fortran test cases,
>     and a few other languages of course.
> 
>     This is what this patch tries to fix.
> 
>     Bernd.
> 
> 
> I guess, I'm slightly confused why this mismatch happens in the first place (does the Fortran frontend do something dumb wrt string declarations, or?), but, Ok for trunk.
> 
> 

This is something that happens only on the test case that is mentioned in the comment.
If I remember correctly the string constant is 3 characters long, as well as the
type info on the STRING_CST itself, but the type of the object has only 2 byte
space for the string.  Therefore make the string shorter, and use the original type from
the declaration.

I am going to apply this together with the rest of the STRING_CST semantic patches,
once those are approved.


Thanks
Bernd.