[Bug lto/51573] New: [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[Bug lto/51573] New: [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

             Bug #: 51573
           Summary: [4.7 Regression] ICE (segfault) in
                    lto_varpool_encoder_encode_initializer_p
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: markus@trippelsdorf.de


ICE happened during a Firefox build.

 % cat test.ii
extern "C" struct JSObject;

template < class T > class HeapPtr
{
};

struct PendingProxyOperation {
    JSObject *object;
};
struct ThreadData {
    PendingProxyOperation *pendingProxyOperation;
};
struct JSThread {
    ThreadData data;
};
struct JSContext {
    JSThread *thread_;
};
class TypeConstraint
{
    virtual void newObjectState () {
    }
};
class TypeSet
{
    TypeConstraint *constraintList;
};
struct Property {
    TypeSet types;
};
struct Cell {
    Property **propertySet;
};
struct JSObject:Cell {
};
bool
IndexToId ()
{
    return 0;
    bool IndexToIdSlow (JSContext);
}

 % c++ -flto test.ii
test.ii: In function ‘IndexToId()’:
test.ii:41:1: internal compiler error: Segmentation fault
Please submit a full bug report

Program received signal SIGSEGV, Segmentation fault.
[Switching to process 16149]
0x0000000000b89c40 in
lto_varpool_encoder_encode_initializer_p(lto_varpool_encoder_d*, varpool_node*)
()
(gdb) bt
#0  0x0000000000b89c40 in
lto_varpool_encoder_encode_initializer_p(lto_varpool_encoder_d*, varpool_node*)
()
#1  0x0000000000957790 in lto_output_tree(output_block*, tree_node*, bool) ()
#2  0x0000000000a1beab in streamer_write_tree_body(output_block*, tree_node*,
bool) ()
#3  0x0000000000957506 in lto_output_tree(output_block*, tree_node*, bool) ()
...

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[markus at trippelsdorf dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

Markus Trippelsdorf <markus at trippelsdorf dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #1 from Markus Trippelsdorf <markus at trippelsdorf dot de> 2011-12-15 18:35:36 UTC ---
Caused by rev182358:
c75a2739c2dd84336557e95cf655eceb163fc341 is the first bad commit
commit c75a2739c2dd84336557e95cf655eceb163fc341
Author: rguenth <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Thu Dec 15 09:44:11 2011 +0000

    2012-12-15  Richard Guenther  <rguenther@suse.de>

        Revert
        PR lto/48437
        * lto-streamer-out.c (tree_is_indexable): Exclude block-local
        extern declarations.

        PR lto/48508
        PR lto/48437
        * tree-streamer-out.c (streamer_write_chain): Stream DECL_EXTERNAL
        VAR_DECLs and FUNCTION_DECLs locally.

        * g++.dg/lto/pr48508-1_0.C: New testcase.
        * g++.dg/lto/pr48508-1_1.C: Likewise.


    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@182358
138bc75d-0d04-0410-961f-82ee72b054a4

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2011-12-16
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |
   Target Milestone|---                         |4.7.0
     Ever Confirmed|0                           |1

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 08:34:41 UTC ---
Mine.

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

--- Comment #3 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 09:32:10 UTC ---
Reduced testcase:

struct T
{
  virtual void m () { } 
};
int
main ()
{
  bool fn (T);
  return 0;
}

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 09:41:06 UTC ---
We are streaming TYPE_BINFO of 'T', of that BINFO_VTABLE which is &_ZTV1T + 16
and when processing _ZTV1T:

 <var_decl 0x7ffff5a2c140 _ZTV1T
    type <array_type 0x7ffff5b8e9d8
        type <pointer_type 0x7ffff5b83f18 __vtbl_ptr_type type <function_type
0x7ffff5b83e70>
            unsigned DI
            size <integer_cst 0x7ffff5a20f40 constant 64>
            unit size <integer_cst 0x7ffff5a20f60 constant 8>
            align 64 symtab 0 alias set -1 canonical type 0x7ffff5b83f18
            pointer_to_this <pointer_type 0x7ffff5b870a8>>
...
    readonly public static ignored external weak virtual decl_5 BLK file t.ii
line 1 col 8 size <integer_cst 0x7ffff5a3c720 192> unit size <integer_cst
0x7ffff5a3c6e0 24>
    align 64 context <record_type 0x7ffff5b8e348 T> initial <constructor
0x7ffff5b64dc8>
    not-really-extern>

we want to encode the initializer.

That all seems ok - the issue is that we ultimately come from streaming
BLOCK_VARS of the main function outermost scope, which refers to 'fn'
whose type refers to 'T'.  And indeed - we shouldn't make ref_p false
for all streams streamed by streamer_write_chain but only the immediate
tree which contains the TREE_CHAIN pointer ... (we can be lucky if
those trees were all streamed already - we'd get references nevertheless,
so it's now dependent on the streaming order whether we happen to trigger
this bug).

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dnovillo at gcc dot
                   |                            |gnu.org, jason at gcc dot
                   |                            |gnu.org
             Blocks|                            |48508, 48437

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 10:21:16 UTC ---
Because of the way C++ treats things (look at PR48508) it is impossible to
move fixing PRs 48508 and 48437 to the point where we handle decl merging.
Thus, while

Index: gcc/lto-symtab.c
===================================================================
--- gcc/lto-symtab.c    (revision 182398)
+++ gcc/lto-symtab.c    (working copy)
@@ -785,6 +785,12 @@ lto_symtab_prevailing_decl (tree decl)
   if (TREE_CODE (decl) == FUNCTION_DECL && DECL_ABSTRACT (decl))
     return decl;

+  /* Extern decls with function context should not be merged, they
+     appear in BLOCK_VARS.  */
+  if (DECL_EXTERNAL (decl)
+      && decl_function_context (decl))
+    return decl;
+
   /* Ensure DECL_ASSEMBLER_NAME will not set assembler name.  */
   gcc_assert (DECL_ASSEMBLER_NAME_SET_P (decl));

Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c       (revision 182398)
+++ gcc/lto/lto.c       (working copy)
@@ -867,9 +867,13 @@ uniquify_nodes (struct data_in *data_in,
       if (t == NULL_TREE)
        continue;

-      if (TREE_CODE (t) == VAR_DECL)
+      if (TREE_CODE (t) == VAR_DECL
+         && (!DECL_EXTERNAL (t)
+             || !decl_function_context (t)))
        lto_register_var_decl_in_symtab (data_in, t);
-      else if (TREE_CODE (t) == FUNCTION_DECL && !DECL_BUILT_IN (t))
+      else if (TREE_CODE (t) == FUNCTION_DECL && !DECL_BUILT_IN (t)
+              && (!DECL_EXTERNAL (t)
+                  || !decl_function_context (t)))
        lto_register_function_decl_in_symtab (data_in, t);
       else if (TYPE_P (t) && !TYPE_CANONICAL (t))
        TYPE_CANONICAL (t) = gimple_register_canonical_type (t);


works great for C it does not work for C++ at all because in

static void
bar (void)
{
  extern void foo (int);
  foo (0);
}

the function-local extern declaration of 'foo' has a toplevel DECL_CONTEXT.

Thus the only way I see to fix this bug is to introduce either a new
streaming hook or add an argument to the stream_write_tree that
indicates whether the actual tree should be streamed by reference,
not affecting its siblings.

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 12:13:55 UTC ---
It's a really messed up situation as with the C++ units

void
bar1 (void)
{
  extern void foo (int);
  foo (0);
}

---

void
bar2 (void)
{
  extern void foo (int);
  foo (0);
}

---

void foo (int) {}

we _do_ need to enter the function local foo's for decl/cgraph merging.
But OTOH we cannot, as the merged decl can only appear in one BLOCK_VARS
list.  ISTM that either the frontends should move those over to
BLOCK_NONLOCALIZED_DECLS or all frontends consistently need to put a
decl copy in BLOCK_VARS for the sake of debuginfo (still using the
global-scope decl for the actual call) - that is what the C frontend
is doing and that works quite well.

Jason?  Can we please change the C++ frontend to mimic what the C
frontend does here?  See c-decl.c:1199ff.

I'm testing another (temporary?) workaround, still prone to the above
issue.

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

--- Comment #7 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 12:50:05 UTC ---
(In reply to comment #6)
> It's a really messed up situation as with the C++ units
> 
> void
> bar1 (void)
> {
>   extern void foo (int);
>   foo (0);
> }
> 
> ---
> 
> void
> bar2 (void)
> {
>   extern void foo (int);
>   foo (0);
> }
> 
> ---
> 
> void foo (int) {}
> 
> we _do_ need to enter the function local foo's for decl/cgraph merging.
> But OTOH we cannot, as the merged decl can only appear in one BLOCK_VARS
> list.  ISTM that either the frontends should move those over to
> BLOCK_NONLOCALIZED_DECLS or all frontends consistently need to put a
> decl copy in BLOCK_VARS for the sake of debuginfo (still using the
> global-scope decl for the actual call) - that is what the C frontend
> is doing and that works quite well.
> 
> Jason?  Can we please change the C++ frontend to mimic what the C
> frontend does here?  See c-decl.c:1199ff.
> 
> I'm testing another (temporary?) workaround, still prone to the above
> issue.

And it exactly shows the issue during LTO bootstrap with

`is_cpp_driver' referenced in section `.text' of
/tmp/ccNdxpcJ.ltrans15.ltrans.o: defined in discarded section `.text' of gcc.o
(symbol from plugin)
collect2: error: ld returned 1 exit status
make[3]: *** [cpp] Error 1

:(

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 13:21:40 UTC ---
Created attachment 26109
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26109
patch #2

Second patch that looks cleaner to me but runs into the C++ issue I mentioned.

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 13:20:52 UTC ---
Created attachment 26108
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26108
patch #1

First patch that doesn't work (runs into PR51572, once that is fixed I'll
re-try).

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-16 15:31:02 UTC ---
Patch #1 now passes LTO bootstrap, I'll schedule a SPEC2k6 build and commit
early next week if that succeeds.  Feels more like baby-steps incrementally
improving the -flto -g situation ...

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-19 15:57:25 UTC ---
Fixed.

[Bug lto/51573] [4.7 Regression] ICE (segfault) in lto_varpool_encoder_encode_initializer_p

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51573

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-12-19 15:57:06 UTC ---
Author: rguenth
Date: Mon Dec 19 15:57:02 2011
New Revision: 182487

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=182487
Log:
2011-12-19  Richard Guenther  <rguenther@suse.de>

    PR lto/51573
    * streamer-hooks.h (struct streamer_hooks): Add second
    ref_p parameter to write_tree.
    (stream_write_tree): Adjust.
    (stream_write_tree_shallow_non_ref): New define.
    * lto-streamer.h (lto_output_tree): Adjust.
    * lto-streamer-out.c (lto_output_tree): Likewise.
    * tree-streamer-out.c (streamer_write_chain): Only
    force the immediate tree to be streamed as non-reference.

    * gcc.dg/lto/20111207-2_0.c: Adjust.
    * g++.dg/lto/pr51573-1_0.C: New testcase.

Added:
    trunk/gcc/testsuite/g++.dg/lto/pr51573-1_0.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/lto-streamer-out.c
    trunk/gcc/lto-streamer.h
    trunk/gcc/streamer-hooks.h
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/lto/20111207-2_0.c
    trunk/gcc/tree-streamer-out.c