Bug 51809 - [4.7 Regression][OOP] ICE (segfault) depending on USE statements order
Summary: [4.7 Regression][OOP] ICE (segfault) depending on USE statements order
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.7.0
: P4 normal
Target Milestone: 4.7.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2012-01-10 09:52 UTC by Kacper Kowalik
Modified: 2012-01-16 19:53 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
reduced testcase (275 bytes, text/x-fortran)
2012-01-10 09:52 UTC, Kacper Kowalik
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kacper Kowalik 2012-01-10 09:52:16 UTC
Created attachment 26289 [details]
reduced testcase

Attached code ICEs with gfortran-4.7.0 (I've checked snapshots from 20111126 till 20120106)
Works fine with gcc-4.6.2
Comment 1 Tobias Burnus 2012-01-10 10:40:59 UTC
Unless I made a mistake when bisecting:
Working
  4.7.0 20111109 - Rev. 181198
Failing
  4.7.0 20111109 - Rev. 181199

That's
    http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181199

    2011-11-09  Janus Weil  <janus@gcc.gnu.org>

        PR fortran/50960
        * class.c (gfc_find_derived_vtab): Make the vtab symbols FL_PARAMETER.
        * expr.c (gfc_simplify_expr): Prevent vtabs from being replaced with
        their value.
        * resolve.c (resolve_values): Use-associated symbols do not need to
        be resolved again.
        (resolve_fl_parameter): Make sure the symbol has a value.


Valgrind shows:

==2177== Invalid read of size 8
==2177==    at 0x553580: mio_pointer_ref(void*) (module.c:2463)
==2177==    by 0x5555A8: mio_symbol_ref(gfc_symbol**) (module.c:2726)
==2177==    by 0x5560E8: mio_expr(gfc_expr**) (module.c:3305)
==2177==    by 0x55643D: mio_expr(gfc_expr**) (module.c:2855)
==2177==    by 0x55703B: mio_symbol(gfc_symbol*) (module.c:3782)
==2177==    by 0x5572B5: write_symbol(int, gfc_symbol*) (module.c:5027)
==2177==    by 0x5591B5: write_symbol0(gfc_symtree*) (module.c:5067)
==2177==    by 0x559158: write_symbol0(gfc_symtree*) (module.c:5046)
==2177==    by 0x559158: write_symbol0(gfc_symtree*) (module.c:5046)
==2177==    by 0x559158: write_symbol0(gfc_symtree*) (module.c:5046)
==2177==    by 0x559804: gfc_dump_module(char const*, int) (module.c:5243)
==2177==    by 0x564CCA: gfc_parse_file() (parse.c:4603)
Comment 2 Tobias Burnus 2012-01-10 15:49:52 UTC
Some debugging. To make it easier, I split the test case into two files and I look at the one which only consists of "module merry_ICE". 

The ICE occurs for writing the .mod file of module merry_ICE. However, the actual issue already occurs with reading. If one sets a break point in mio_symbol, one sees that GCC reads the following symbols:
  $1 = 0x2aaaacbf01e0 "__vtab_foo_Foo_t"
  $2 = 0x2aaaacbf01c8 "__def_init_foo_Foo_t"
  $3 = 0x2aaaacbf0258 "__vtab_bar_Bar_t"
  $4 = 0x2aaaacbf0240 "__def_init_bar_Bar_t"

The issue is for the PARAMETER "__vtab_bar_Bar_t": its initializer (sym->value) is a structure constructor consisting of _hash, _size, _extends etc. However, during read in the _extends gets only a c->expr->symtree == NULL. Thus, during write out, it crashes in mio_symtree_ref as that tries to access symtree->n.sym.

If one reverts the order of the USE statements, one first reads the __vtab of "bar" - again with symtree == NULL - and only then one reads __vtab of "foo". 

Seemingly, only if the order is reversed the symtree fixing (fixup/pointer_info) works.


The main change of the patch in comment 1 is that it changes FL_VARIABLE to FL_PARAMETER. If one looks at mio_symbol, one sees:

  if (sym->attr.flavor == FL_PARAMETER)
    mio_expr (&sym->value);

One possibility is to revert the patch of comment 1 - and use the other variant to set TREE_READONLY: http://gcc.gnu.org/ml/fortran/2011-11/msg00099.html

Alternatively - or additionally [as it can also affect other code] - one should find out why the symtree is not fixed up.
Comment 3 Tobias Burnus 2012-01-16 12:54:31 UTC
I have not quite found why it works in one order and not the other. There is some reading/fixup oddness. In particular, when reading in 'bar', in module_read's fixup setup, one already has a symtree for __vtab_foo_Foo_t available - which in principle is needed for _extension.

In any case, what we do for __vtab (as PARAMETER) is not possible in Fortran itself. Namely, having a PARAMETER with a (non-NULL) pointer. In Fortran, it's not allowed as one could use the parameter's pointer target in a TRANSFER. Thus, one had to know the address at compile time. However, the address is only known at link time. See PR 51266 and related PR 45290 and PR 51076.

While accessing __vtab_...%_extension is not possible within Fortran, the module reading does not seem to be prepared for it.

A simple revert of comment 1 is not possible as one runs into similar problems for __def_init - causing an ICE. Ditto for a simple revert of only class.c.

 * * *

In any case, I planing to submit the following (regtested) patch, which is a revert of comment 1 plus FL_PARAMETER->FL_VARIABLE for __def_init plus a patch to trans-decl.c.

--- a/gcc/fortran/class.c
+++ b/gcc/fortran/class.c
@@ -590,3 +590,3 @@ gfc_find_derived_vtab (gfc_symbol *derived)
          vtab->ts.type = BT_DERIVED;
-         if (gfc_add_flavor (&vtab->attr, FL_PARAMETER, NULL,
+         if (gfc_add_flavor (&vtab->attr, FL_VARIABLE, NULL,
                              &gfc_current_locus) == FAILURE)
@@ -684,3 +684,3 @@ gfc_find_derived_vtab (gfc_symbol *derived)
                  def_init->attr.access = ACCESS_PUBLIC;
-                 def_init->attr.flavor = FL_PARAMETER;
+                 def_init->attr.flavor = FL_VARIABLE;
                  gfc_set_sym_referenced (def_init);
--- a/gcc/fortran/expr.c
+++ b/gcc/fortran/expr.c
@@ -1885,4 +1885,3 @@ gfc_simplify_expr (gfc_expr *p, int type)
          && (gfc_init_expr_flag || p->ref
-             || p->symtree->n.sym->value->expr_type != EXPR_ARRAY)
-         && !p->symtree->n.sym->attr.vtab)
+             || p->symtree->n.sym->value->expr_type != EXPR_ARRAY))
        {
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -9639,3 +9639,3 @@ resolve_values (gfc_symbol *sym)

-  if (sym->value == NULL || sym->attr.use_assoc)
+  if (sym->value == NULL)
     return;
@@ -12197,3 +12197,3 @@ resolve_fl_parameter (gfc_symbol *sym)
      refer to a derived type from the host.  */
-  if (sym->ts.type == BT_DERIVED && sym->value
+  if (sym->ts.type == BT_DERIVED
       && !gfc_compare_types (&sym->ts, &sym->value->ts))
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -1487,3 +1487,6 @@ gfc_get_symbol_decl (gfc_symbol * sym)
       || (sym->name[0] == '_' && strncmp ("__def_init", sym->name, 10) == 0))
-    GFC_DECL_PUSH_TOPLEVEL (decl) = 1;
+    {
+      TREE_READONLY (decl) = 1;
+      GFC_DECL_PUSH_TOPLEVEL (decl) = 1;
+    }
Comment 4 Tobias Burnus 2012-01-16 19:50:16 UTC
Author: burnus
Date: Mon Jan 16 19:50:11 2012
New Revision: 183219

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183219
Log:
2012-01-16  Tobias Burnus  <burnus@net-b.de>

        PR fortran/51809
        * class.c (gfc_find_derived_vtab): Mark __vtab and
        __def_init as FL_VARIABLE not as FL_PARAMETER.
        * expr.c (gfc_simplify_expr): Remove special
        handling of __vtab.
        * resolve.c (resolve_values): Ditto.
        * trans-decl.c (gfc_get_symbol_decl): Mark __vtab
        and __def_init as TREE_READONLY.

2012-01-16  Tobias Burnus  <burnus@net-b.de>

        PR fortran/51809
        * gfortran.dg/use_20.f90: New


Added:
    trunk/gcc/testsuite/gfortran.dg/use_20.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/class.c
    trunk/gcc/fortran/expr.c
    trunk/gcc/fortran/resolve.c
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/testsuite/ChangeLog
Comment 5 Tobias Burnus 2012-01-16 19:53:27 UTC
FIXED on the trunk (4.7).

Thanks for the report!