Bug 60899 - undef reference generated with -fdevirtualize-speculatively
Summary: undef reference generated with -fdevirtualize-speculatively
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 5.0
: P3 normal
Target Milestone: ---
Assignee: Jan Hubicka
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2014-04-20 05:34 UTC by davidxl
Modified: 2016-01-10 10:56 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2014-04-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description davidxl 2014-04-20 05:34:49 UTC
Build the following code with the following command line:

g++ -O2 -fdisable-tree-einline a.cc a_m.cc 

results in:

/tmp/cci31j3N.o: In function `D::doit()':
a.cc:(.text._ZN1D4doitEv[_ZN1D4doitEv]+0x5): undefined reference to `A::foo()'
collect2: error: ld returned 1 exit status

It builds fine when devirtualization is disabled:

-O2 -fno-devirtualization-speculatively -fdisable-tree-einline 


The problem is there is no instantiation of any class A instances (final or subclass) in the program, so vtables and A::foo are all eliminated. The reference to A::foo is from D::doit. In a successful build, there are no D instances either, so D::doit won't be emitted. However with speculative devirtualization, D::doit may be speculatively referenced even though there are no D instances.

What happens is that during ipa-inline, goo is inlined into D::doit, the virtual call to foo should become an direct call to A::foo, but the new edge is not discovered. Since there is no call edge to A::foo, A::foo gets removed right after ipa-inline (before inline transform). However during inline transform, gimple-fold-call converts the virtual call into a direct call.


The test case is extracted from a very large real program. The explicit reference to D::doit in bar is to demonstrate the problem -- in the real program, the reference is from spec-devirt.


//a.h
struct B {
 virtual int foo() = 0;
 int goo() { return foo(); }
 int i;
};

struct A  : public B {
 A() : i(0)  {}
 int foo() { return 1;}
 int i;
};
struct A2  : public B {
 int foo() { return 2;}
};

struct DI {
  virtual int doit() = 0;
};

struct D : public DI {
   virtual int doit () { return m.goo(); }

   A m;
};

// a.cc

#include "a.h"

int cond;
int bar (DI* ap) {
   if (cond) return static_cast<D*>(ap)->D::doit();  // Mimic speculative devirtualization
   return ap->doit();
}


// a_m.cc

#include "a.h"

int cond;
int bar (DI* ap) {
   if (cond) return static_cast<D*>(ap)->D::doit();  // Mimic speculative devirtualization
   return ap->doit();
}
Comment 1 Jan Hubicka 2014-04-20 06:26:07 UTC
David,
can you check if can_refer_decl_in_current_unit_p
Comment 2 Jan Hubicka 2014-04-20 06:26:22 UTC
David,
it seems a_m.C should be different form a.C.  From chain of events you describe I think
we need to figure out why the last folding happens.  Does the function pass 
can_refer_decl_in_current_unit_p and if so, how does cgraph node look at that time?

Honza
Comment 3 davidxl 2014-04-20 06:52:43 UTC
(In reply to Jan Hubicka from comment #2)
> David,
> it seems a_m.C should be different form a.C.  From chain of events you
> describe I think
> we need to figure out why the last folding happens.  Does the function pass 
> can_refer_decl_in_current_unit_p and if so, how does cgraph node look at
> that time?
> 
> Honza

Cut & paste error:

// a_m.cc

#include "a.h"
struct D2: public DI {
  virtual int doit () { return 3; }
};

extern int bar(DI*);
int main()
{
  D2 d2;
  return bar(&d2);
}
Comment 4 davidxl 2014-04-20 07:10:06 UTC
(In reply to davidxl from comment #3)
> (In reply to Jan Hubicka from comment #2)
> > David,
> > it seems a_m.C should be different form a.C.  From chain of events you
> > describe I think
> > we need to figure out why the last folding happens.  Does the function pass 
> > can_refer_decl_in_current_unit_p and if so, how does cgraph node look at
> > that time?
> > 
> > Honza
> 
> Cut & paste error:
> 
> // a_m.cc
> 
> #include "a.h"
> struct D2: public DI {
>   virtual int doit () { return 3; }
> };
> 
> extern int bar(DI*);
> int main()
> {
>   D2 d2;
>   return bar(&d2);
> }


stepping into can_refer_decl_in_current_unit_p indicates it returns true (for A::foo and A::vtable) at the condition @line 106.
Comment 5 Jan Hubicka 2014-04-20 07:23:24 UTC
I am running benchmarks I do not want to disturb, but the following should fix the problem.
$ svn diff ../../gcc/gimple-fold.c
Index: ../../gcc/gimple-fold.c
===================================================================
--- ../../gcc/gimple-fold.c     (revision 209526)
+++ ../../gcc/gimple-fold.c     (working copy)
@@ -105,7 +105,9 @@ can_refer_decl_in_current_unit_p (tree d
      external var.  */
   if (!from_decl
       || TREE_CODE (from_decl) != VAR_DECL
-      || !DECL_EXTERNAL (from_decl)
+      || (!DECL_EXTERNAL (from_decl)
+         && (vnode = varpool_get_node (from_decl)) != NULL
+         && vnode->definition)
       || (flag_ltrans
          && symtab_get_node (from_decl)->in_other_partition))
     return true;
Comment 6 Paul Pluzhnikov 2014-04-20 15:20:32 UTC
Google ref: b/13453242
Comment 7 davidxl 2014-04-21 03:33:04 UTC
(In reply to Paul Pluzhnikov from comment #6)
> Google ref: b/13453242

Verified that the proposed patch fixed the problem in b/1345242.
Comment 8 Jan Hubicka 2014-04-21 03:48:19 UTC
> Verified that the proposed patch fixed the problem in b/1345242.

Great, thanks! I still would preffer to see DECL_EXTERNAL bit on vtable that is
not emit in the current unit. But C++ visibility code is bit of mess, so if
Jason think it is impractical to arrange it, lets go with this patch.
Comment 9 Richard Biener 2014-04-22 08:51:48 UTC
Confirmed.
Comment 10 Jan Hubicka 2014-05-21 06:16:35 UTC
Author: hubicka
Date: Wed May 21 06:16:03 2014
New Revision: 210676

URL: http://gcc.gnu.org/viewcvs?rev=210676&root=gcc&view=rev
Log:

	PR tree-optimization/60899
	* gimple-fold.c (can_refer_decl_in_current_unit_p): Cleanup;
	assume all static symbols will have definition wile parsing and
	check the do have definition later in compilation; check that
	variable referring symbol will be output before concluding that
	reference is safe; be conservative for referring local statics;
	be more precise about when comdat is output in other partition.

	g++.dg/ipa/devirt-11.C: Update template.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-fold.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/ipa/devirt-11.C
Comment 11 Hristo Venev 2014-06-27 13:06:40 UTC
This bug also affects gcc 4.9.1. It causes LLVM to fail to build.
Comment 12 Markus Trippelsdorf 2014-06-27 13:36:05 UTC
(In reply to Hristo Venev from comment #11)
> This bug also affects gcc 4.9.1. It causes LLVM to fail to build.

LLVM build failure is a different issue, see: http://llvm.org/bugs/show_bug.cgi?id=20067
Comment 13 Jan Hubicka 2016-01-10 10:56:10 UTC
I believe this can be closed.