| Message ID | 53330512.2090408@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/26/2014 12:49 PM, Jason Merrill wrote: > But it seems odd to me that it > isn't happening on path through a non-virtual call to ~MultiTermDocs, so > I'm filing a bug about that. 60674. Jason
> My earlier patch for 58678 caused this problem: even if we aren't > going to refer to the dtor thunks from the vtable, we need to emit > them in case other translation units use them. > > I'm adding xfails to two testcases: devirt-21.C and devirt-23.C. > The fails aren't a new bug; we are no longer seeing the > devirtualization because we remove the path to wrap() through a > virtual call to ~MultiTermDocs that it occurred along. But it seems > odd to me that it isn't happening on path through a non-virtual call > to ~MultiTermDocs, so I'm filing a bug about that. MultiTermDocs::~MultiTermDocs() (struct MultiTermDocs * const this, const void * * __vtt_parm) { unsigned int i; int (*__vtbl_ptr_type) () * iftmp.6_4; struct A * _8; unsigned int _10; <bb 2>: iftmp.6_4 = *__vtt_parm_3(D); this_5(D)->_vptr.MultiTermDocs = iftmp.6_4; MultiTermDocs::wrap (this_5(D)); I belive the problem here is the _vptr.MultiTermDocs vtable is initialized from VTT that is not understood by ipa-prop jump functions. The other path has direct store of VPTR in it. Honza
Jason Merrill <jason@redhat.com> writes: > diff --git a/gcc/testsuite/g++.dg/abi/thunk6.C b/gcc/testsuite/g++.dg/abi/thunk6.C > new file mode 100644 > index 0000000..e3d07f2 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/abi/thunk6.C > @@ -0,0 +1,18 @@ > +// PR c++/60566 > +// We need to emit the construction vtable thunk for ~C even if we aren't > +// going to use it. > + > +struct A > +{ > + virtual void f() = 0; > + virtual ~A() {} > +}; > + > +struct B: virtual A { int i; }; > +struct C: virtual A { int i; ~C(); }; > + > +C::~C() {} > + > +int main() {} > + > +// { dg-final { scan-assembler "_ZTv0_n32_N1CD1Ev" } } FAIL: g++.dg/abi/thunk6.C -std=c++11 scan-assembler _ZTv0_n32_N1CD1Ev $ grep _ZTv0_ thunk6.s .globl _ZTv0_n16_N1CD1Ev .type _ZTv0_n16_N1CD1Ev, @function _ZTv0_n16_N1CD1Ev: .size _ZTv0_n16_N1CD1Ev, .-_ZTv0_n16_N1CD1Ev .globl _ZTv0_n16_N1CD0Ev .type _ZTv0_n16_N1CD0Ev, @function _ZTv0_n16_N1CD0Ev: .size _ZTv0_n16_N1CD0Ev, .-_ZTv0_n16_N1CD0Ev Andreas.
On 03/27/2014 01:42 AM, Jan Hubicka wrote: > I belive the problem here is the _vptr.MultiTermDocs vtable is initialized from > VTT that is not understood by ipa-prop jump functions. Makes sense. It would be good to update those functions to understand that the initialization is always setting the vptr to a construction vtable for MultiTermDocs (in some derived class). Jason
Andreas Schwab <schwab@suse.de> writes: > Jason Merrill <jason@redhat.com> writes: > >> diff --git a/gcc/testsuite/g++.dg/abi/thunk6.C >> b/gcc/testsuite/g++.dg/abi/thunk6.C >> new file mode 100644 >> index 0000000..e3d07f2 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/abi/thunk6.C >> @@ -0,0 +1,18 @@ >> +// PR c++/60566 >> +// We need to emit the construction vtable thunk for ~C even if we aren't >> +// going to use it. >> + >> +struct A >> +{ >> + virtual void f() = 0; >> + virtual ~A() {} >> +}; >> + >> +struct B: virtual A { int i; }; >> +struct C: virtual A { int i; ~C(); }; >> + >> +C::~C() {} >> + >> +int main() {} >> + >> +// { dg-final { scan-assembler "_ZTv0_n32_N1CD1Ev" } } > > FAIL: g++.dg/abi/thunk6.C -std=c++11 scan-assembler _ZTv0_n32_N1CD1Ev > > $ grep _ZTv0_ thunk6.s > .globl _ZTv0_n16_N1CD1Ev > .type _ZTv0_n16_N1CD1Ev, @function > _ZTv0_n16_N1CD1Ev: > .size _ZTv0_n16_N1CD1Ev, .-_ZTv0_n16_N1CD1Ev > .globl _ZTv0_n16_N1CD0Ev > .type _ZTv0_n16_N1CD0Ev, @function > _ZTv0_n16_N1CD0Ev: > .size _ZTv0_n16_N1CD0Ev, .-_ZTv0_n16_N1CD0Ev It would help to state which target this is... Same for the 32-bit multilib on Solaris/SPARC and x86 (i386-pc-solaris2.11, sparc-sun-solaris2.11). Rainer
commit 38f48ae422064906ffc4acb3db6eaa962c702b39
Author: Jason Merrill <jason@redhat.com>
Date: Wed Mar 26 00:02:35 2014 -0400
PR c++/60566
PR c++/58678
* class.c (build_vtbl_initializer): Handle abstract dtors here.
* search.c (get_pure_virtuals): Not here.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index b46391b..d277e07 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -9017,6 +9017,16 @@ build_vtbl_initializer (tree binfo,
if (!TARGET_VTABLE_USES_DESCRIPTORS)
init = fold_convert (vfunc_ptr_type_node,
build_fold_addr_expr (fn));
+ /* Don't refer to a virtual destructor from a constructor
+ vtable or a vtable for an abstract class, since destroying
+ an object under construction is undefined behavior and we
+ don't want it to be considered a candidate for speculative
+ devirtualization. But do create the thunk for ABI
+ compliance. */
+ if (DECL_DESTRUCTOR_P (fn_original)
+ && (CLASSTYPE_PURE_VIRTUALS (DECL_CONTEXT (fn_original))
+ || orig_binfo != binfo))
+ init = size_zero_node;
}
}
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index d99e182..c3eed90 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -2115,22 +2115,6 @@ get_pure_virtuals (tree type)
which it is a primary base will contain vtable entries for the
pure virtuals in the base class. */
dfs_walk_once (TYPE_BINFO (type), NULL, dfs_get_pure_virtuals, type);
-
- /* Treat a virtual destructor in an abstract class as pure even if it
- isn't declared as pure; there is no way it would be called through the
- vtable except during construction, which causes undefined behavior. */
- if (CLASSTYPE_PURE_VIRTUALS (type)
- && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
- {
- tree dtor = CLASSTYPE_DESTRUCTORS (type);
- if (dtor && DECL_VIRTUAL_P (dtor) && !DECL_PURE_VIRTUAL_P (dtor))
- {
- tree clone;
- DECL_PURE_VIRTUAL_P (dtor) = true;
- FOR_EACH_CLONE (clone, dtor)
- DECL_PURE_VIRTUAL_P (clone) = true;
- }
- }
}
/* Debug info for C++ classes can get very large; try to avoid
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 2f84f17..6fb1449 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1261,7 +1261,7 @@ get_polymorphic_call_info (tree fndecl,
}
/* If the function is constructor or destructor, then
- the type is possibly in consturction, but we know
+ the type is possibly in construction, but we know
it is not derived type. */
if (DECL_CXX_CONSTRUCTOR_P (fndecl)
|| DECL_CXX_DESTRUCTOR_P (fndecl))
diff --git a/gcc/testsuite/g++.dg/abi/thunk6.C b/gcc/testsuite/g++.dg/abi/thunk6.C
new file mode 100644
index 0000000..e3d07f2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/thunk6.C
@@ -0,0 +1,18 @@
+// PR c++/60566
+// We need to emit the construction vtable thunk for ~C even if we aren't
+// going to use it.
+
+struct A
+{
+ virtual void f() = 0;
+ virtual ~A() {}
+};
+
+struct B: virtual A { int i; };
+struct C: virtual A { int i; ~C(); };
+
+C::~C() {}
+
+int main() {}
+
+// { dg-final { scan-assembler "_ZTv0_n32_N1CD1Ev" } }