Re: [Patch, Fortran, 4.5 Regression] PR 40822: Internal compiler error when Fortran intrinsic LEN referenced before explicit declaration

[Janus Weil <janus at gcc dot gnu dot org>]

2009/7/23 Steve Kargl <sgk@troutmask.apl.washington.edu>:
> On Thu, Jul 23, 2009 at 05:53:37PM +0200, Janus Weil wrote:
>> 2009/7/23 Tobias Burnus <burnus@net-b.de>:
>> > Janus Weil wrote:
>> >> here is a close-to-obvious two-line patch, fixing a regression which
>> >> was introduced by one of my earlier ProcPtr patches.
>> > I have not fully thought about the patch. But if I read
>> >
>> > + ? ? ?if (formal_arg->sym->ts.type == BT_CHARACTER)
>> > + ? ? ? formal_arg->sym->ts.cl = gfc_get_charlen ();
>> > +
>> >
>> >
>> > I wonder whether this does not just allocate space for the character
>> > length data, without actually properly initializing the struct.
>>
>> I guess the cl structure here is at least initialized in the sense
>> that it is zeroed, and there is no junk data in there.
>>
>> The reason that I don't initialize cl->length is that
>> "cl->length==NULL" is equivalent to an assumed-length string, right?
>
> I believe that you're correct. ?But, with the deferred type
> parameter patch that I've been work on, cl->length==NULL
> also means the CHARACTER(len=:) entity hasn't been allocated
> (or assigned to).

Thanks for the remark. However, I think in this special case there
should be no ambiguity (wrt assumed length vs deferred type
parameter), since my patch only applies to formal args of intrinsic
functions.

[ In general: How do you distinguish the two, if both come with
cl->length==NULL? Is attr.dummy enough? ]

By now, I have also extended my patch to put the charlen into a
namespace. For this I invented a little function "gfc_new_charlen",
which is also useful in many other places to replace gfc_get_charlen.

At the same time I have spotted a few places where a new charlen is
not put into a namespace (potentially causing a memleak):

* expr.c (simplify_const_ref)
* symbol.c (gfc_set_default_type, generate_isocbinding_symbol)
* trans-decl.c (create_function_arglist)

I'm not quite sure yet if any of them is actually problematic. Maybe
someone who is more of a charlen expert than me can give some advice.

The new patch is again regtested on x86_64-unknown-linux-gnu. Ok now?

Cheers,
Janus


2009-07-23  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/40822
	* array.c (gfc_resolve_character_array_constructor): Use new function
	gfc_new_charlen.
	* decl.c (add_init_expr_to_sym,variable_decl,match_char_spec,
	gfc_match_implicit): Ditto.
	* expr.c (gfc_simplify_expr): Ditto.
	* gfortran.h (gfc_new_charlen): New prototype.
	* iresolve.c (check_charlen_present,gfc_resolve_char_achar): Use new
	function gfc_new_charlen.
	* module.c (mio_charlen): Ditto.
	* resolve.c (gfc_resolve_substring_charlen,
	gfc_resolve_character_operator,fixup_charlen,resolve_fl_derived,
	resolve_symbol): Ditto.
	* symbol.c (gfc_new_charlen): New function to create a new gfc_charlen
	structure and add it to a namespace.
	(gfc_copy_formal_args_intr): Make sure ts.cl is present
	for CHARACTER variables.


2009-07-23  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/40822
	* gfortran.dg/char_length_16.f90: New.

Index: gcc/fortran/symbol.c
===================================================================
--- gcc/fortran/symbol.c	(revision 150008)
+++ gcc/fortran/symbol.c	(working copy)
@@ -3071,6 +3071,19 @@ gfc_free_finalizer_list (gfc_finalizer* 
 }
 
 
+/* Create a new gfc_charlen structure and add it to a namespace.  */
+
+gfc_charlen*
+gfc_new_charlen (gfc_namespace *ns)
+{
+  gfc_charlen *cl;
+  cl = gfc_get_charlen ();
+  cl->next = ns->cl_list;
+  ns->cl_list = cl;
+  return cl;
+}
+
+
 /* Free the charlen list from cl to end (end is not freed). 
    Free the whole list if end is NULL.  */
 
@@ -3927,6 +3940,9 @@ gfc_copy_formal_args_intr (gfc_symbol *d
       formal_arg->sym->attr.flavor = FL_VARIABLE;
       formal_arg->sym->attr.dummy = 1;
 
+      if (formal_arg->sym->ts.type == BT_CHARACTER)
+	formal_arg->sym->ts.cl = gfc_new_charlen (gfc_current_ns);
+
       /* If this isn't the first arg, set up the next ptr.  For the
         last arg built, the formal_arg->next will never get set to
         anything other than NULL.  */
Index: gcc/fortran/decl.c
===================================================================
--- gcc/fortran/decl.c	(revision 150008)
+++ gcc/fortran/decl.c	(working copy)
@@ -1258,9 +1258,7 @@ add_init_expr_to_sym (const char *name, 
 	      int clen;
 	      /* If there are multiple CHARACTER variables declared on the
 		 same line, we don't want them to share the same length.  */
-	      sym->ts.cl = gfc_get_charlen ();
-	      sym->ts.cl->next = gfc_current_ns->cl_list;
-	      gfc_current_ns->cl_list = sym->ts.cl;
+	      sym->ts.cl = gfc_new_charlen (gfc_current_ns);
 
 	      if (sym->attr.flavor == FL_PARAMETER)
 		{
@@ -1292,9 +1290,7 @@ add_init_expr_to_sym (const char *name, 
 		{
 		  /* Build a new charlen to prevent simplification from
 		     deleting the length before it is resolved.  */
-		  init->ts.cl = gfc_get_charlen ();
-		  init->ts.cl->next = gfc_current_ns->cl_list;
-		  gfc_current_ns->cl_list = sym->ts.cl;
+		  init->ts.cl = gfc_new_charlen (gfc_current_ns);
 		  init->ts.cl->length = gfc_copy_expr (sym->ts.cl->length);
 
 		  for (p = init->value.constructor; p; p = p->next)
@@ -1597,9 +1593,7 @@ variable_decl (int elem)
       switch (match_char_length (&char_len))
 	{
 	case MATCH_YES:
-	  cl = gfc_get_charlen ();
-	  cl->next = gfc_current_ns->cl_list;
-	  gfc_current_ns->cl_list = cl;
+	  cl = gfc_new_charlen (gfc_current_ns);
 
 	  cl->length = char_len;
 	  break;
@@ -1611,9 +1605,7 @@ variable_decl (int elem)
 	      && (current_ts.cl->length == NULL
 		  || current_ts.cl->length->expr_type != EXPR_CONSTANT))
 	    {
-	      cl = gfc_get_charlen ();
-	      cl->next = gfc_current_ns->cl_list;
-	      gfc_current_ns->cl_list = cl;
+	      cl = gfc_new_charlen (gfc_current_ns);
 	      cl->length = gfc_copy_expr (current_ts.cl->length);
 	    }
 	  else
@@ -2235,9 +2227,7 @@ done:
     }
 
   /* Do some final massaging of the length values.  */
-  cl = gfc_get_charlen ();
-  cl->next = gfc_current_ns->cl_list;
-  gfc_current_ns->cl_list = cl;
+  cl = gfc_new_charlen (gfc_current_ns);
 
   if (seen_length == 0)
     cl->length = gfc_int_expr (1);
@@ -2611,9 +2601,7 @@ gfc_match_implicit (void)
 	      if (ts.type == BT_CHARACTER && !ts.cl)
 		{
 		  ts.kind = gfc_default_character_kind;
-		  ts.cl = gfc_get_charlen ();
-		  ts.cl->next = gfc_current_ns->cl_list;
-		  gfc_current_ns->cl_list = ts.cl;
+		  ts.cl = gfc_new_charlen (gfc_current_ns);
 		  ts.cl->length = gfc_int_expr (1);
 		}
 
Index: gcc/fortran/array.c
===================================================================
--- gcc/fortran/array.c	(revision 150008)
+++ gcc/fortran/array.c	(working copy)
@@ -1599,9 +1599,7 @@ gfc_resolve_character_array_constructor 
 	    goto got_charlen;
 	  }
 
-      expr->ts.cl = gfc_get_charlen ();
-      expr->ts.cl->next = gfc_current_ns->cl_list;
-      gfc_current_ns->cl_list = expr->ts.cl;
+      expr->ts.cl = gfc_new_charlen (gfc_current_ns);
     }
 
 got_charlen:
Index: gcc/fortran/gfortran.h
===================================================================
--- gcc/fortran/gfortran.h	(revision 150008)
+++ gcc/fortran/gfortran.h	(working copy)
@@ -2415,6 +2415,7 @@ int gfc_symbols_could_alias (gfc_symbol 
 void gfc_undo_symbols (void);
 void gfc_commit_symbols (void);
 void gfc_commit_symbol (gfc_symbol *);
+gfc_charlen *gfc_new_charlen (gfc_namespace *);
 void gfc_free_charlen (gfc_charlen *, gfc_charlen *);
 void gfc_free_namespace (gfc_namespace *);
 
Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(revision 150008)
+++ gcc/fortran/expr.c	(working copy)
@@ -1681,9 +1681,7 @@ gfc_simplify_expr (gfc_expr *p, int type
 	  gfc_free (p->value.character.string);
 	  p->value.character.string = s;
 	  p->value.character.length = end - start;
-	  p->ts.cl = gfc_get_charlen ();
-	  p->ts.cl->next = gfc_current_ns->cl_list;
-	  gfc_current_ns->cl_list = p->ts.cl;
+	  p->ts.cl = gfc_new_charlen (gfc_current_ns);
 	  p->ts.cl->length = gfc_int_expr (p->value.character.length);
 	  gfc_free_ref_list (p->ref);
 	  p->ref = NULL;
Index: gcc/fortran/module.c
===================================================================
--- gcc/fortran/module.c	(revision 150008)
+++ gcc/fortran/module.c	(working copy)
@@ -2000,13 +2000,9 @@ mio_charlen (gfc_charlen **clp)
     {
       if (peek_atom () != ATOM_RPAREN)
 	{
-	  cl = gfc_get_charlen ();
+	  cl = gfc_new_charlen (gfc_current_ns);
 	  mio_expr (&cl->length);
-
 	  *clp = cl;
-
-	  cl->next = gfc_current_ns->cl_list;
-	  gfc_current_ns->cl_list = cl;
 	}
     }
 
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 150008)
+++ gcc/fortran/resolve.c	(working copy)
@@ -4012,11 +4012,7 @@ gfc_resolve_substring_charlen (gfc_expr 
   e->ts.kind = gfc_default_character_kind;
 
   if (!e->ts.cl)
-    {
-      e->ts.cl = gfc_get_charlen ();
-      e->ts.cl->next = gfc_current_ns->cl_list;
-      gfc_current_ns->cl_list = e->ts.cl;
-    }
+    e->ts.cl = gfc_new_charlen (gfc_current_ns);
 
   if (char_ref->u.ss.start)
     start = gfc_copy_expr (char_ref->u.ss.start);
@@ -4489,9 +4485,7 @@ gfc_resolve_character_operator (gfc_expr
   else if (op2->expr_type == EXPR_CONSTANT)
     e2 = gfc_int_expr (op2->value.character.length);
 
-  e->ts.cl = gfc_get_charlen ();
-  e->ts.cl->next = gfc_current_ns->cl_list;
-  gfc_current_ns->cl_list = e->ts.cl;
+  e->ts.cl = gfc_new_charlen (gfc_current_ns);
 
   if (!e1 || !e2)
     return;
@@ -4530,11 +4524,7 @@ fixup_charlen (gfc_expr *e)
 
     default:
       if (!e->ts.cl)
-	{
-	  e->ts.cl = gfc_get_charlen ();
-	  e->ts.cl->next = gfc_current_ns->cl_list;
-	  gfc_current_ns->cl_list = e->ts.cl;
-	}
+	e->ts.cl = gfc_new_charlen (gfc_current_ns);
 
       break;
     }
@@ -9085,16 +9075,10 @@ resolve_fl_derived (gfc_symbol *sym)
 	      /* Copy char length.  */
 	      if (ifc->ts.cl)
 		{
-		  c->ts.cl = gfc_get_charlen();
+		  c->ts.cl = gfc_new_charlen (sym->ns);
 	          c->ts.cl->resolved = ifc->ts.cl->resolved;
 		  c->ts.cl->length = gfc_copy_expr (ifc->ts.cl->length);
 		  /* TODO: gfc_expr_replace_symbols (c->ts.cl->length, c);*/
-		  /* Add charlen to namespace.  */
-		  /*if (c->formal_ns)
-		    {
-		      c->ts.cl->next = c->formal_ns->cl_list;
-		      c->formal_ns->cl_list = c->ts.cl;
-		    }*/
 		}
 	    }
 	  else if (c->ts.interface->name[0] != '\0')
@@ -9490,16 +9474,10 @@ resolve_symbol (gfc_symbol *sym)
 	  /* Copy char length.  */
 	  if (ifc->ts.cl)
 	    {
-	      sym->ts.cl = gfc_get_charlen();
+	      sym->ts.cl = gfc_new_charlen (sym->ns);
 	      sym->ts.cl->resolved = ifc->ts.cl->resolved;
 	      sym->ts.cl->length = gfc_copy_expr (ifc->ts.cl->length);
 	      gfc_expr_replace_symbols (sym->ts.cl->length, sym);
-	      /* Add charlen to namespace.  */
-	      if (sym->formal_ns)
-		{
-		  sym->ts.cl->next = sym->formal_ns->cl_list;
-		  sym->formal_ns->cl_list = sym->ts.cl;
-		}
 	    }
 	}
       else if (sym->ts.interface->name[0] != '\0')
Index: gcc/fortran/iresolve.c
===================================================================
--- gcc/fortran/iresolve.c	(revision 150008)
+++ gcc/fortran/iresolve.c	(working copy)
@@ -63,11 +63,7 @@ static void
 check_charlen_present (gfc_expr *source)
 {
   if (source->ts.cl == NULL)
-    {
-      source->ts.cl = gfc_get_charlen ();
-      source->ts.cl->next = gfc_current_ns->cl_list;
-      gfc_current_ns->cl_list = source->ts.cl;
-    }
+    source->ts.cl = gfc_new_charlen (gfc_current_ns);
 
   if (source->expr_type == EXPR_CONSTANT)
     {
@@ -165,9 +161,7 @@ gfc_resolve_char_achar (gfc_expr *f, gfc
   f->ts.type = BT_CHARACTER;
   f->ts.kind = (kind == NULL)
 	     ? gfc_default_character_kind : mpz_get_si (kind->value.integer);
-  f->ts.cl = gfc_get_charlen ();
-  f->ts.cl->next = gfc_current_ns->cl_list;
-  gfc_current_ns->cl_list = f->ts.cl;
+  f->ts.cl = gfc_new_charlen (gfc_current_ns);
   f->ts.cl->length = gfc_int_expr (1);
 
   f->value.function.name = gfc_get_string (name, f->ts.kind,

Attachment: char_length_16.f90
Description: Binary data