diff mbox

PR 78534 Change character length from int to size_t

Message ID CAO9iq9GNtY8fXTUCV5CtxTtMNVx2zLXaFPPJV8pOxqhTA_0fGQ@mail.gmail.com
State New
Headers show

Commit Message

Janne Blomqvist Dec. 13, 2016, 7:08 p.m. UTC
On Mon, Dec 12, 2016 at 9:39 PM, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
> On Mon, Dec 12, 2016 at 7:39 PM, Andre Vehreschild <vehre@gmx.de> wrote:
>> Hi Janne,
>>
>> I found that you are favoring build_int_cst (size_type_node, 0) over
>> size_zero_node. Is there a reason to this?
>
> Yes. AFAIU size_zero_node is a zero constant for sizetype which is not
> the same as size_type_node.
>
> AFAIK the difference is that size_type_node is the C size_t type,
> whereas sizetype is a GCC internal type used for address expressions.
> On a "normal" target I understand that they are the same size, but
> there are some slight differences in semantics, e.g. size_type_node
> like C unsigned integer types is defined to wrap around on overflow
> whereas sizetype is undefined on overflow.
>
> I don't know if GCC supports some strange targets with some kind of
> segmented memory where the size of sizetype would be different from
> size_type_node, but I guess it's possible in theory at least.
>
> That being said, now that you mention in I should be using
> build_zero_cst (some_type_node) instead of
> build_int_cst(some_type_node, 0). There's also build_one_cst that I
> should use.
>
>> Furthermore did I have to patch this:
>>
>> diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
>> index 585f25d..f374558 100644
>> --- a/gcc/fortran/dump-parse-tree.c
>> +++ b/gcc/fortran/dump-parse-tree.c
>> @@ -465,7 +465,7 @@ show_expr (gfc_expr *p)
>>           break;
>>
>>         case BT_HOLLERITH:
>> -         fprintf (dumpfile, "%dH", p->representation.length);
>> +         fprintf (dumpfile, "%zdH", p->representation.length);
>>           c = p->representation.string;
>>           for (i = 0; i < p->representation.length; i++, c++)
>>             {
>>
>> to bootstrap on x86_64-linux/f23.
>
> Ah, thanks for the catch. I'll fix it by using HOST_WIDE_INT_PRINT_DEC
> since I'll have to change gfc_charlen_t to be a typedef form
> HOST_WIDE_INT (see my answer to FX).
>
>> And I have this regression:
>>
>> FAIL: gfortran.dg/allocate_deferred_char_scalar_1.f03   -O1  (test for excess
>> errors)
>>
>> allocate_deferred_char_scalar_1.f03:184:0:
>>
>>      p = '12345679'
>>
>> Warning: '__builtin_memcpy' writing 8 bytes into a region of size 5 overflows
>>      the destination [-Wstringop-overflow=]
>>      allocate_deferred_char_scalar_1.f03:242:0:
>>
>>      p = 4_'12345679'
>>
>> Warning: '__builtin_memcpy' writing 32 bytes into a region of size 20 overflows
>> the destination [-Wstringop-overflow=]
>
> I'm seeing that too, but I assumed they would be fixed by Paul's
> recent patch which I don't yet have in my tree yet due to the git
> mirror being stuck..
>
>> Btw, the patch for changing the ABI of the coarray-libs is already nearly done.
>> I just need to figure that what the state of regressions is with and without my
>> change.
>
> Thanks.
>
> I'll produce an updated patch with the changes discussed so far.
>
>
> --
> Janne Blomqvist

Hi,

attached is the updated patch that applies on top of the original. I
didn't do the charlen_zero_node etc, I just fixed the relatively few
places in my previous patch rather than everywhere in the entire
frontend.

Now that the git mirror is working again, I see though those warnings
with -O1 from gfortran.dg/allocate_deferred_char_scalar_1.f03 are
still there, and Paul's patch didn't get rid of them. :(. I have
looked at the tree dumps, however, and AFAICS it's a bogus warning.

Ok for trunk?
diff mbox

Patch

diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
index 585f25d..7aafe54 100644
--- a/gcc/fortran/dump-parse-tree.c
+++ b/gcc/fortran/dump-parse-tree.c
@@ -348,12 +348,10 @@  show_constructor (gfc_constructor_base base)
 
 
 static void
-show_char_const (const gfc_char_t *c, int length)
+show_char_const (const gfc_char_t *c, gfc_charlen_t length)
 {
-  int i;
-
   fputc ('\'', dumpfile);
-  for (i = 0; i < length; i++)
+  for (gfc_charlen_t i = 0; i < length; i++)
     {
       if (c[i] == '\'')
 	fputs ("''", dumpfile);
@@ -465,7 +463,8 @@  show_expr (gfc_expr *p)
 	  break;
 
 	case BT_HOLLERITH:
-	  fprintf (dumpfile, "%dH", p->representation.length);
+	  fprintf (dumpfile, HOST_WIDE_INT_PRINT_DEC "H",
+		   p->representation.length);
 	  c = p->representation.string;
 	  for (i = 0; i < p->representation.length; i++, c++)
 	    {
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 6a05e9e..e82c320 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2067,10 +2067,12 @@  gfc_intrinsic_sym;
 typedef splay_tree gfc_constructor_base;
 
 
-/* This should be a size_t. But occasionally the string length field
-   is used as a flag with values -1 and -2, see
-   e.g. gfc_add_assign_aux_vars.  */
-typedef ptrdiff_t gfc_charlen_t;
+/* This should be an unsigned variable of type size_t.  But to handle
+   compiling to a 64-bit target from a 32-bit host, we need to use a
+   HOST_WIDE_INT.  Also, occasionally the string length field is used
+   as a flag with values -1 and -2, see e.g. gfc_add_assign_aux_vars.
+   So it needs to be signed.  */
+typedef HOST_WIDE_INT gfc_charlen_t;
 
 typedef struct gfc_expr
 {
diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
index 5e2a750..5b05a3d 100644
--- a/gcc/fortran/gfortran.texi
+++ b/gcc/fortran/gfortran.texi
@@ -3812,10 +3812,10 @@  or GCC's Ada compiler for @code{Boolean}.)
 For arguments of @code{CHARACTER} type, the character length is passed
 as hidden argument.  For deferred-length strings, the value is passed
 by reference, otherwise by value.  The character length has the type
-@code{INTEGER(kind=4)}.  Note with C binding, @code{CHARACTER(len=1)}
-result variables are returned according to the platform ABI and no
-hidden length argument is used for dummy arguments; with @code{VALUE},
-those variables are passed by value.
+@code{INTEGER(kind=C_SIZE_T)}.  Note with C binding,
+@code{CHARACTER(len=1)} result variables are returned according to the
+platform ABI and no hidden length argument is used for dummy
+arguments; with @code{VALUE}, those variables are passed by value.
 
 For @code{OPTIONAL} dummy arguments, an absent argument is denoted
 by a NULL pointer, except for scalar dummy arguments of type
diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index 6184289..aaeb4a7 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -5782,8 +5782,8 @@  select_intrinsic_set_tmp (gfc_typespec *ts)
     sprintf (name, "__tmp_%s_%d", gfc_basic_typename (ts->type),
 	     ts->kind);
   else
-    sprintf (name, "__tmp_%s_%ld_%d", gfc_basic_typename (ts->type),
-	     charlen, ts->kind);
+    sprintf (name, "__tmp_%s_" HOST_WIDE_INT_PRINT_DEC "_%d",
+	     gfc_basic_typename (ts->type), charlen, ts->kind);
 
   gfc_get_sym_tree (name, gfc_current_ns, &tmp, false);
   gfc_add_type (tmp->n.sym, ts, NULL);
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 150f0c6..fac531f 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -8727,8 +8727,8 @@  resolve_select_type (gfc_code *code, gfc_namespace *old_ns)
 	  if (c->ts.u.cl && c->ts.u.cl->length
 	      && c->ts.u.cl->length->expr_type == EXPR_CONSTANT)
 	    charlen = mpz_get_si (c->ts.u.cl->length->value.integer);
-	  sprintf (name, "__tmp_%s_%ld_%d", gfc_basic_typename (c->ts.type),
-	           charlen, c->ts.kind);
+	  sprintf (name, "__tmp_%s_" HOST_WIDE_INT_PRINT_DEC "_%d",
+		   gfc_basic_typename (c->ts.type), charlen, c->ts.kind);
 	}
       else
 	sprintf (name, "__tmp_%s_%d", gfc_basic_typename (c->ts.type),
diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c
index 5b4bb2e..179e136 100644
--- a/gcc/fortran/trans-expr.c
+++ b/gcc/fortran/trans-expr.c
@@ -250,7 +250,7 @@  gfc_class_len_or_zero_get (tree decl)
   return len != NULL_TREE ? fold_build3_loc (input_location, COMPONENT_REF,
 					     TREE_TYPE (len), decl, len,
 					     NULL_TREE)
-    : build_int_cst (gfc_charlen_type_node, 0);
+    : build_zero_cst (gfc_charlen_type_node);
 }
 
 
@@ -1042,7 +1042,7 @@  gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, gfc_typespec class_ts,
       if (DECL_LANG_SPECIFIC (tmp) && GFC_DECL_SAVED_DESCRIPTOR (tmp))
 	tmp = GFC_DECL_SAVED_DESCRIPTOR (tmp);
 
-      slen = build_int_cst (size_type_node, 0);
+      slen = build_zero_cst (size_type_node);
     }
   else
     {
@@ -1089,7 +1089,7 @@  gfc_conv_class_to_class (gfc_se *parmse, gfc_expr *e, gfc_typespec class_ts,
 	  tmp = slen;
 	}
       else
-	tmp = build_int_cst (size_type_node, 0);
+	tmp = build_zero_cst (size_type_node);
       gfc_add_modify (&parmse->pre, ctree,
 		      fold_convert (TREE_TYPE (ctree), tmp));
 
@@ -1228,7 +1228,7 @@  gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited)
       if (from != NULL_TREE && unlimited)
 	from_len = gfc_class_len_or_zero_get (from);
       else
-	from_len = build_int_cst (size_type_node, 0);
+	from_len = build_zero_cst (size_type_node);
     }
 
   if (GFC_CLASS_TYPE_P (TREE_TYPE (to)))
@@ -1340,7 +1340,7 @@  gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited)
 
 	  tmp = fold_build2_loc (input_location, GT_EXPR,
 				 boolean_type_node, from_len,
-				 build_int_cst (TREE_TYPE (from_len), 0));
+				 build_zero_cst (TREE_TYPE (from_len)));
 	  tmp = fold_build3_loc (input_location, COND_EXPR,
 				 void_type_node, tmp, extcopy, stdcopy);
 	  gfc_add_expr_to_block (&body, tmp);
@@ -1368,7 +1368,7 @@  gfc_copy_class_to_class (tree from, tree to, tree nelems, bool unlimited)
 	  extcopy = build_call_vec (fcn_type, fcn, args);
 	  tmp = fold_build2_loc (input_location, GT_EXPR,
 				 boolean_type_node, from_len,
-				 build_int_cst (TREE_TYPE (from_len), 0));
+				 build_zero_cst (TREE_TYPE (from_len)));
 	  tmp = fold_build3_loc (input_location, COND_EXPR,
 				 void_type_node, tmp, extcopy, stdcopy);
 	}
@@ -2196,7 +2196,7 @@  gfc_conv_string_length (gfc_charlen * cl, gfc_expr * expr, stmtblock_t * pblock)
 
   gfc_conv_expr_type (&se, cl->length, gfc_charlen_type_node);
   se.expr = fold_build2_loc (input_location, MAX_EXPR, gfc_charlen_type_node,
-			     se.expr, build_int_cst (TREE_TYPE (se.expr), 0));
+			     se.expr, build_zero_cst (TREE_TYPE (se.expr)));
   gfc_add_block_to_block (pblock, &se.pre);
 
   if (cl->backend_decl)
@@ -2268,7 +2268,7 @@  gfc_conv_substring (gfc_se * se, gfc_ref * ref, int kind,
       /* Check lower bound.  */
       fault = fold_build2_loc (input_location, LT_EXPR, boolean_type_node,
 			       start.expr,
-			       build_int_cst (TREE_TYPE (start.expr), 1));
+			       build_one_cst (TREE_TYPE (start.expr)));
       fault = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR,
 			       boolean_type_node, nonempty, fault);
       if (name)
@@ -5877,7 +5877,7 @@  gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym,
 	  tmp = fold_convert (gfc_charlen_type_node, parmse.expr);
 	  tmp = fold_build2_loc (input_location, MAX_EXPR,
 				 gfc_charlen_type_node, tmp,
-				 build_int_cst (TREE_TYPE (tmp), 0));
+				 build_zero_cst (TREE_TYPE (tmp)));
 	  cl.backend_decl = tmp;
 	}
 
@@ -8111,7 +8111,7 @@  trans_class_vptr_len_assignment (stmtblock_t *block, gfc_expr * le,
 		  from_len = gfc_evaluate_now (se.expr, block);
 		}
 	      else
-		from_len = build_int_cst (gfc_charlen_type_node, 0);
+		from_len = build_zero_cst (gfc_charlen_type_node);
 	    }
 	  gfc_add_modify (pre, to_len, fold_convert (TREE_TYPE (to_len),
 						     from_len));
@@ -8240,7 +8240,7 @@  gfc_trans_pointer_assignment (gfc_expr * expr1, gfc_expr * expr2)
 	    gfc_add_modify (&block, lse.string_length, rse.string_length);
 	  else if (lse.string_length != NULL)
 	    gfc_add_modify (&block, lse.string_length,
-			    build_int_cst (TREE_TYPE (lse.string_length), 0));
+			    build_zero_cst (TREE_TYPE (lse.string_length)));
 	}
 
       gfc_add_modify (&block, lse.expr,
@@ -9676,7 +9676,7 @@  trans_class_assignment (stmtblock_t *block, gfc_expr *lhs, gfc_expr *rhs,
 
 	  tmp = fold_build2_loc (input_location, GT_EXPR,
 				 boolean_type_node, from_len,
-				 build_int_cst (TREE_TYPE (from_len), 0));
+				 build_zero_cst (TREE_TYPE (from_len)));
 	  return fold_build3_loc (input_location, COND_EXPR,
 				  void_type_node, tmp,
 				  extcopy, stdcopy);
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index df414ee..5461666 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -7495,9 +7495,8 @@  gfc_conv_associated (gfc_se *se, gfc_expr *expr)
 	  = fold_build2_loc (input_location, NE_EXPR,
 			     boolean_type_node,
 			     arg1->expr->ts.u.cl->backend_decl,
-			     build_int_cst
-			     (TREE_TYPE (arg1->expr->ts.u.cl->backend_decl),
-			      0));
+			     build_zero_cst
+			     (TREE_TYPE (arg1->expr->ts.u.cl->backend_decl)));
       if (scalar)
         {
 	  /* A pointer to a scalar.  */
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index 3285ee0..663bb42 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -2955,7 +2955,7 @@  gfc_trans_character_select (gfc_code *code)
       if (d->low == NULL)
         {
           CONSTRUCTOR_APPEND_ELT (node, ss_string1[k], null_pointer_node);
-          CONSTRUCTOR_APPEND_ELT (node, ss_string1_len[k], build_int_cst (gfc_charlen_type_node, 0));
+          CONSTRUCTOR_APPEND_ELT (node, ss_string1_len[k], build_zero_cst (gfc_charlen_type_node));
         }
       else
         {
@@ -2968,7 +2968,7 @@  gfc_trans_character_select (gfc_code *code)
       if (d->high == NULL)
         {
           CONSTRUCTOR_APPEND_ELT (node, ss_string2[k], null_pointer_node);
-          CONSTRUCTOR_APPEND_ELT (node, ss_string2_len[k], build_int_cst (gfc_charlen_type_node, 0));
+          CONSTRUCTOR_APPEND_ELT (node, ss_string2_len[k], build_zero_cst (gfc_charlen_type_node));
         }
       else
         {
@@ -5640,7 +5640,7 @@  gfc_trans_allocate (gfc_code * code)
 	{
 	  gfc_init_se (&se, NULL);
 	  temp_var_needed = false;
-	  expr3_len = build_int_cst (gfc_charlen_type_node, 0);
+	  expr3_len = build_zero_cst (gfc_charlen_type_node);
 	  e3_is = E3_MOLD;
 	}
       /* Prevent aliasing, i.e., se.expr may be already a
@@ -6036,8 +6036,8 @@  gfc_trans_allocate (gfc_code * code)
 		     e.g., a string.  */
 		  memsz = fold_build2_loc (input_location, GT_EXPR,
 					   boolean_type_node, expr3_len,
-					   build_int_cst
-					   (TREE_TYPE (expr3_len), 0));
+					   build_zero_cst
+					   (TREE_TYPE (expr3_len)));
 		  memsz = fold_build3_loc (input_location, COND_EXPR,
 					 TREE_TYPE (expr3_esize),
 					 memsz, tmp, expr3_esize);