Bug 51573

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

Mine.

Reduced testcase:

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

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).

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.

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.

(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

:(

Created attachment 26108 [details]
patch #1

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

Created attachment 26109 [details]
patch #2

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

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 ...

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

Fixed.