diff mbox

[fortran] PR55827 ICE with multiple fortran modules.

Message ID 50E5F8B4.6080507@sfr.fr
State New
Headers show

Commit Message

Mikael Morin Jan. 3, 2013, 9:31 p.m. UTC 
Hello,

here is a fix for PR fortran/55827 where we had a function expression 
(loaded from a module) whose symtree pointer was NULL.  My attempt to 
have symtree properly set got me way too far, so this is fixing the 
unconditional usages of symtree based on Steve's patch in the PR.  As 
noted in the PR, it looks bogus to have a NULL expr->symtree, but in 
fact expr->value.function.esym is set, and it is what is solely looked 
at, apart for the cases fixed in this patch.

The trans-expr.c part initializes `sym' earlier and uses it instead of 
accessing `expr->symtree' directly.

The class.c part creates a similar construct to initialize `ts' without 
accessing `expr->symtree' directly.  I don't use Steve's way (returning 
early for a NULL symtree), because I remembered that `expr->symtree' 
could be a generic symbol, thus have invalid type for use to initialize 
`ts', contrary to `expr->value.function.esym'.  It's better to always 
use `esym' when it is available.  And then returning early on NULL 
symtree is not necessary any more.

Regression tested on x86_64-unknown-linux-gnu. OK for trunk?
Do we want it on the branches as well?

Mikael
2013-01-03  Steven G. Kargl  <kargl@gcc.gnu.org>
	    Mikael Morin  <mikael@gcc.gnu.org>

	* class.c (gfc_fix_class_refs): Adapt ts initialization for the case
	e->symtree == NULL.
	* trans-expr.c (gfc_conv_function_expr): Init sym earlier. Use it.

2013-01-03  Steven G. Kargl  <kargl@gcc.gnu.org>
	    Mikael Morin  <mikael@gcc.gnu.org>

	* gfortran.dg/use_10.f90: New test.

Comments

Paul Richard Thomas Jan. 4, 2013, 4:46 a.m. UTC | #1 
Dear Mikael and Steve,

From a quick read of your correspondence in the PR, you seem to have
covered all the angles between you.

OK for trunk.

As for the branches; yes, but.... how far back to go?  I guess that
4.6 and 4.7 should be patched, if possible, since they are in current
use in distributions.  However, I note that
class.c(gfc_fix_class_refs) is dated 2012-02-02 and was applied to
4.7.0 trunk.  Is the bug present 4.6?  I have neither tree loaded
since updating my system and so cannot check at the moment.

Thanks for the patch.

Paul


On 3 January 2013 22:31, Mikael Morin <mikael.morin@sfr.fr> wrote:
> Hello,
>
> here is a fix for PR fortran/55827 where we had a function expression
> (loaded from a module) whose symtree pointer was NULL.  My attempt to have
> symtree properly set got me way too far, so this is fixing the unconditional
> usages of symtree based on Steve's patch in the PR.  As noted in the PR, it
> looks bogus to have a NULL expr->symtree, but in fact
> expr->value.function.esym is set, and it is what is solely looked at, apart
> for the cases fixed in this patch.
>
> The trans-expr.c part initializes `sym' earlier and uses it instead of
> accessing `expr->symtree' directly.
>
> The class.c part creates a similar construct to initialize `ts' without
> accessing `expr->symtree' directly.  I don't use Steve's way (returning
> early for a NULL symtree), because I remembered that `expr->symtree' could
> be a generic symbol, thus have invalid type for use to initialize `ts',
> contrary to `expr->value.function.esym'.  It's better to always use `esym'
> when it is available.  And then returning early on NULL symtree is not
> necessary any more.
>
> Regression tested on x86_64-unknown-linux-gnu. OK for trunk?
> Do we want it on the branches as well?
>
> Mikael
>
>
>
>
>
>
>
diff mbox

Patch

diff --git a/class.c b/class.c
index 8a8a54a..8b00a2c 100644
--- a/class.c
+++ b/class.c
@@ -166,7 +166,23 @@  gfc_fix_class_refs (gfc_expr *e)
 	  && e->value.function.isym != NULL))
     return;
 
-  ts = &e->symtree->n.sym->ts;
+  if (e->expr_type == EXPR_VARIABLE)
+    ts = &e->symtree->n.sym->ts;
+  else
+    {
+      gfc_symbol *func;
+
+      gcc_assert (e->expr_type == EXPR_FUNCTION);
+      if (e->value.function.esym != NULL)
+	func = e->value.function.esym;
+      else
+	func = e->symtree->n.sym;
+
+      if (func->result != NULL)
+	ts = &func->result->ts;
+      else
+	ts = &func->ts;
+    }
 
   for (ref = &e->ref; *ref != NULL; ref = &(*ref)->next)
     {
diff --git a/trans-expr.c b/trans-expr.c
index 42f6e0c..d36b466 100644
--- a/trans-expr.c
+++ b/trans-expr.c
@@ -5420,20 +5420,20 @@  gfc_conv_function_expr (gfc_se * se, gfc_expr * expr)
       return;
     }
 
+  /* expr.value.function.esym is the resolved (specific) function symbol for
+     most functions.  However this isn't set for dummy procedures.  */
+  sym = expr->value.function.esym;
+  if (!sym)
+    sym = expr->symtree->n.sym;
+
   /* We distinguish statement functions from general functions to improve
      runtime performance.  */
-  if (expr->symtree->n.sym->attr.proc == PROC_ST_FUNCTION)
+  if (sym->attr.proc == PROC_ST_FUNCTION)
     {
       gfc_conv_statement_function (se, expr);
       return;
     }
 
-  /* expr.value.function.esym is the resolved (specific) function symbol for
-     most functions.  However this isn't set for dummy procedures.  */
-  sym = expr->value.function.esym;
-  if (!sym)
-    sym = expr->symtree->n.sym;
-
   gfc_conv_procedure_call (se, sym, expr->value.function.actual, expr,
 			   NULL);
 }