Fix ipa-devirt ICE on virtual inheritance

Jan Hubicka hubicka@ucw.cz
Thu Jan 9 16:21:00 GMT 2014


Hi,
this patch fixes IPA-devirt testcase that gave me bad sleep for months.  The
problem turned out to be combination of three issues (that greatly confused
me). This patch fixes first two.  Here representation of BINFOs of multiple
inheritnace actually differs from my mental modem. For diamond shaped graph
    A
   / \
  B   C
   \ /
    D

here A is a common virtual base of B and C.  I assumed that there will be two
binfos representing A linked from binfos representing B/C both pointing to
virtual table of A. This did not work so I assumed that there is one shared
binfo.  In reality we however have two binfos but only first one has vtable
associated.

Second issue, also addressed in this patch is lookup of corresponding vtable.
I copied code from get_binfo_at_offset that dives into the structure and
tracks last base that has non-zero offset. This is becaus vtables of bases
starting at same offset are shared.

This however does not work with multiple inheritance.  A may have same
offset 0, while we may reach it over C that has non-zero offset.
In this case we really want D's vtable instead of C's.

So instead of tracking one vtable I now maintain stack where one
can look up corresponding base. Alternative is to mimick what
get_binfo_at_offset does by walking fields instead of bases.
Here the walk would bypass B/C and get dirrectly to A, but 
then I would have difficulties to lookup the A's binfo.

Final alternative is to use BINFO_INHERITANCE_CHAIN same way as C++ FE,
but we do not stream it and I would like to avoid using it unless really
necessary.

Bootstrapped/regtested ppc64-linux.

Honza

	PR ipa/58252
	PR ipa/59226
	* ipa-devirt.c record_target_from_binfo): Take as argument
	stack of binfos and lookup matching one for virtual inheritance.
	(possible_polymorphic_call_targets_1): Update.

	* g++.dg/ipa/devirt-20.C: New testcase.	
	* g++.dg/torture/pr58252.C: Likewise.
	* g++.dg/torture/pr59226.C: Likewise.

Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 206362)
+++ ipa-devirt.c	(working copy)
@@ -614,10 +614,8 @@ maybe_record_node (vec <cgraph_node *> &
    This match what get_binfo_at_offset does, but with offset
    being unknown.
 
-   TYPE_BINFO is binfo holding an virtual table matching
-   BINFO's type.  In the case of single inheritance, this
-   is binfo of BINFO's type ancestor (vtable is shared),
-   otherwise it is binfo of BINFO's type.
+   TYPE_BINFOS is a stack of BINFOS of types with defined
+   virtual table seen on way from class type to BINFO.
 
    MATCHED_VTABLES tracks virtual tables we already did lookup
    for virtual function in. INSERTED tracks nodes we already
@@ -630,7 +628,7 @@ static void
 record_target_from_binfo (vec <cgraph_node *> &nodes,
 			  tree binfo,
 			  tree otr_type,
-			  tree type_binfo,
+			  vec <tree> &type_binfos,
 			  HOST_WIDE_INT otr_token,
 			  tree outer_type,
 			  HOST_WIDE_INT offset,
@@ -642,10 +640,32 @@ record_target_from_binfo (vec <cgraph_no
   int i;
   tree base_binfo;
 
-  gcc_checking_assert (BINFO_VTABLE (type_binfo));
 
+  if (BINFO_VTABLE (binfo))
+    type_binfos.safe_push (binfo);
   if (types_same_for_odr (type, outer_type))
     {
+      int i;
+      tree type_binfo = NULL;
+
+      /* Lookup BINFO with virtual table.  For normal types it is always last
+	 binfo on stack.  */
+      for (i = type_binfos.length () - 1; i >= 0; i--)
+	if (BINFO_OFFSET (type_binfos[i]) == BINFO_OFFSET (binfo))
+	  {
+	    type_binfo = type_binfos[i];
+	    break;
+	  }
+      if (BINFO_VTABLE (binfo))
+	type_binfos.pop ();
+      /* If this is duplicated BINFO for base shared by virtual inheritance,
+	 we may not have its associated vtable.  This is not a problem, since
+	 we will walk it on the other path.  */
+      if (!type_binfo)
+	{
+	  gcc_assert (BINFO_VIRTUAL_P (binfo));
+	  return;
+	}
       tree inner_binfo = get_binfo_at_offset (type_binfo,
 					      offset, otr_type);
       /* For types in anonymous namespace first check if the respective vtable
@@ -676,12 +696,11 @@ record_target_from_binfo (vec <cgraph_no
     /* Walking bases that have no virtual method is pointless excercise.  */
     if (polymorphic_type_binfo_p (base_binfo))
       record_target_from_binfo (nodes, base_binfo, otr_type,
-				/* In the case of single inheritance,
-				   the virtual table is shared with
-				   the outer type.  */
-				BINFO_VTABLE (base_binfo) ? base_binfo : type_binfo,
+				type_binfos, 
 				otr_token, outer_type, offset, inserted,
 				matched_vtables, anonymous);
+  if (BINFO_VTABLE (binfo))
+    type_binfos.pop ();
 }
      
 /* Lookup virtual methods matching OTR_TYPE (with OFFSET and OTR_TOKEN)
@@ -701,11 +720,13 @@ possible_polymorphic_call_targets_1 (vec
 {
   tree binfo = TYPE_BINFO (type->type);
   unsigned int i;
+  vec <tree> type_binfos = vNULL;
 
-  record_target_from_binfo (nodes, binfo, otr_type, binfo, otr_token,
+  record_target_from_binfo (nodes, binfo, otr_type, type_binfos, otr_token,
 			    outer_type, offset,
 			    inserted, matched_vtables,
 			    type->anonymous_namespace);
+  type_binfos.release ();
   for (i = 0; i < type->derived_types.length (); i++)
     possible_polymorphic_call_targets_1 (nodes, inserted, 
 					 matched_vtables,
Index: testsuite/g++.dg/ipa/devirt-20.C
===================================================================
--- testsuite/g++.dg/ipa/devirt-20.C	(revision 0)
+++ testsuite/g++.dg/ipa/devirt-20.C	(working copy)
@@ -0,0 +1,31 @@
+#include <stdlib.h>
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-release_ssa"  } */
+namespace {
+struct A
+{ int a; virtual int foo() {return a;} void bar() {a=7;} };
+struct B
+{ int b; virtual int foo2() {return b;} void bar2() {b=9;} };
+struct C : public virtual A, public virtual B { };
+struct D : public virtual B, public virtual A { };
+struct E : public C, public D { void bar2() {b=9;} }; }
+int
+main(void)
+{
+  struct E e;
+  struct C *c = &e;
+  struct D *d = &e;
+  struct A *a = &e;
+  struct B *b = &e;
+  e.bar();
+  e.bar2();
+  if (e.foo() + e.foo2() != 16)
+    abort ();
+  if (c->foo() + d->foo2() != 16)
+    abort ();
+  if (a->foo() + b->foo2() != 16)
+    abort ();
+  return 0;
+}
+/* { dg-final { scan-tree-dump-not "abort" "release_ssa"  } } */
+/* { dg-final { cleanup-ipa-dump "release_ssa" } } */
Index: testsuite/g++.dg/torture/pr58252.C
===================================================================
--- testsuite/g++.dg/torture/pr58252.C	(revision 0)
+++ testsuite/g++.dg/torture/pr58252.C	(working copy)
@@ -0,0 +1,142 @@
+// { dg-do compile }
+// { dg-options "-fpermissive" }
+typedef long unsigned int size_t;
+       typedef bool _CORBA_Boolean;
+         typedef unsigned int _CORBA_ULong;
+             template <class T> class _CORBA_Sequence {
+     public:   typedef _CORBA_Sequence<T> T_seq;
+        inline T_seq &operator= (const T_seq &s)   {
+         for (unsigned long i=0;
+     i < pd_len;
+     i++) {
+       }
+       }
+       _CORBA_ULong pd_len;
+     };
+             template <class T> class _CORBA_Unbounded_Sequence : public _CORBA_Sequence<T> {
+        inline _CORBA_Unbounded_Sequence_WChar() {
+       }
+     };
+       class _CORBA_ObjRef_Var_base {
+     };
+         template <class T, class T_Helper> class _CORBA_ObjRef_Var : public _CORBA_ObjRef_Var_base {
+     public:   typedef T* ptr_t;
+       typedef T* T_ptr;
+        inline _CORBA_ObjRef_Var() : pd_objref(T_Helper::_nil()) {
+    }
+       inline _CORBA_ObjRef_Var(T_ptr p) : pd_objref(p) {
+       }
+      private:   T_ptr pd_objref;
+      };
+        class omniLocalIdentity;
+         class omniObjRef {
+     };
+            class omniServant {
+      public:   virtual ~omniServant();
+        virtual void* _ptrToInterface(const char* repoId);
+          };
+         namespace CORBA  {
+      class NVList {
+     };
+      class Object {
+     };
+      struct StructMember {
+     };
+      class StructMemberSeq : public _CORBA_Unbounded_Sequence< StructMember > {
+        };
+      class _objref_IRObject :   public virtual ::CORBA::Object,   public virtual omniObjRef {
+     };
+      class _impl_IRObject :   public virtual omniServant {
+      };
+     class _objref_Container;
+      typedef _objref_Container* Container_ptr;
+      class _impl_Contained :   public virtual _impl_IRObject {
+     };
+     class _objref_ExceptionDef;
+      typedef _objref_ExceptionDef* ExceptionDef_ptr;
+      class ExceptionDef_Helper {
+     public:   typedef ExceptionDef_ptr _ptr_type;
+        static _ptr_type _nil();
+     };
+      typedef _CORBA_ObjRef_Var<_objref_ExceptionDef, ExceptionDef_Helper> ExceptionDef_var;
+      class Container {
+     public:    typedef Container_ptr _ptr_type;
+        static const char* _PD_repoId;
+       };
+      class _objref_Container :   public virtual _objref_IRObject {
+       ExceptionDef_ptr create_exception(const char* id, const char* name, const char* version, const ::CORBA::StructMemberSeq& members);
+     };
+      class _impl_Container :   public virtual _impl_IRObject {
+     public:   virtual ~_impl_Container();
+       virtual ExceptionDef_ptr create_exception(const char* id, const char* name, const char* version, const ::CORBA::StructMemberSeq& members) = 0;
+     };
+      class _impl_IDLType :   public virtual _impl_IRObject {
+     };
+      class _impl_TypedefDef :   public virtual _impl_Contained,   public virtual _impl_IDLType {
+     };
+      class _impl_StructDef :   public virtual _impl_TypedefDef,   public virtual _impl_Container {
+      };
+           }
+          namespace PortableServer {
+            class ServantBase : public virtual omniServant {
+    };
+             }
+         namespace POA_CORBA {
+           class IRObject :   public virtual CORBA::_impl_IRObject,   public virtual ::PortableServer::ServantBase {
+     };
+      class Contained :   public virtual CORBA::_impl_Contained,   public virtual IRObject {
+     };
+      class Container :   public virtual CORBA::_impl_Container,   public virtual IRObject {
+     };
+      class IDLType :   public virtual CORBA::_impl_IDLType,   public virtual IRObject {
+     };
+      class TypedefDef :   public virtual CORBA::_impl_TypedefDef,   public virtual Contained,     public virtual IDLType {
+     };
+      class StructDef :   public virtual CORBA::_impl_StructDef,   public virtual TypedefDef,     public virtual Container {
+     public:   virtual ~StructDef();
+     };
+       }
+         namespace omni {
+     class omniOrbPOA;
+     class giopAddress;
+     }
+             class omniCallDescriptor {
+     public:   typedef void (*LocalCallFn)(omniCallDescriptor*, omniServant*);
+        inline omniCallDescriptor(LocalCallFn lcfn, const char* op_,        int op_len_, _CORBA_Boolean oneway,        const char*const* user_excns_,        int n_user_excns_,                             _CORBA_Boolean is_upcall_)     : pd_localCall(lcfn),       pd_op(op_), pd_oplen(op_len_),       pd_user_excns(user_excns_),       pd_n_user_excns(n_user_excns_),       pd_is_oneway(oneway),       pd_is_upcall(is_upcall_),       pd_contains_values(0),       pd_first_address_used(0),       pd_current_address(0),       pd_objref(0),       pd_poa(0),       pd_localId(0),       pd_deadline_secs(0),       pd_deadline_nanosecs(0) {
+    }
+      private:   LocalCallFn pd_localCall;
+       const char* pd_op;
+       size_t pd_oplen;
+       const char*const* pd_user_excns;
+       int pd_n_user_excns;
+       _CORBA_Boolean pd_is_oneway;
+       _CORBA_Boolean pd_is_upcall;
+       _CORBA_Boolean pd_contains_values;
+        const omni::giopAddress* pd_first_address_used;
+       const omni::giopAddress* pd_current_address;
+           omniObjRef* pd_objref;
+        omni::omniOrbPOA* pd_poa;
+       omniLocalIdentity* pd_localId;
+              unsigned long pd_deadline_secs;
+       unsigned long pd_deadline_nanosecs;
+      };
+          class _0RL_cd_7963219a43724a61_f2000000   : public omniCallDescriptor {
+     public:   inline _0RL_cd_7963219a43724a61_f2000000(LocalCallFn lcfn,const char* op_,size_t oplen,_CORBA_Boolean upcall=0):      omniCallDescriptor(lcfn, op_, oplen, 0, _user_exns, 0, upcall)   {
+        }
+         static const char* const _user_exns[];
+       const char* arg_0;
+       const char* arg_1;
+       const char* arg_2;
+       const CORBA::StructMemberSeq* arg_3;
+       CORBA::ExceptionDef_var result;
+     };
+          static void _0RL_lcfn_7963219a43724a61_03000000(omniCallDescriptor* cd, omniServant* svnt) {
+       _0RL_cd_7963219a43724a61_f2000000* tcd = (_0RL_cd_7963219a43724a61_f2000000*)cd;
+       CORBA::_impl_Container* impl = (CORBA::_impl_Container*) svnt->_ptrToInterface(CORBA::Container::_PD_repoId);
+       tcd->result = impl->create_exception(tcd->arg_0, tcd->arg_1, tcd->arg_2, *tcd->arg_3);
+       }
+         CORBA::ExceptionDef_ptr CORBA::_objref_Container::create_exception(const char* id, const char* name, const char* version, const ::CORBA::StructMemberSeq& members) {
+       _0RL_cd_7963219a43724a61_f2000000 _call_desc(_0RL_lcfn_7963219a43724a61_03000000, "create_exception", 17);
+       }
+         POA_CORBA::StructDef::~StructDef() {
+    }
Index: testsuite/g++.dg/torture/pr59226.C
===================================================================
--- testsuite/g++.dg/torture/pr59226.C	(revision 0)
+++ testsuite/g++.dg/torture/pr59226.C	(working copy)
@@ -0,0 +1,27 @@
+// { dg-do compile }
+struct A
+{
+  virtual void foo() {}
+};
+
+struct B
+{
+  virtual void foo() {}
+};
+
+struct C : virtual A {};
+
+struct D : virtual A, B
+{
+  virtual void foo() {}
+};
+
+struct E : C, D
+{
+  virtual void foo() {}
+};
+
+void bar(A* p)
+{
+  p->foo();
+}



More information about the Gcc-patches mailing list