| Message ID | 5310EB44.60602@redhat.com |
|---|---|
| State | New |
| Headers | show |
> Multiple large C++ projects (KDE and libreoffice, at least) have > been breaking when GCC speculatively devirtualizes a call to an > implicitly-declared virtual destructor, because this leads to > references to base destructors and vtables that might be hidden in > another DSO. This patch avoids this problem by avoiding speculative > devirtualization of calls to implicitly-declared functions. > > Tested x86_64-pc-linux-gnu. OK for trunk? > > commit 94eb5df9fb20c796d09151d7293ae89ac012ae79 > Author: Jason Merrill <jason@redhat.com> > Date: Fri Feb 28 14:03:19 2014 -0500 > > PR c++/58678 > * ipa-devirt.c (ipa_devirt): Don't choose an implicitly-declared > function. > > diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c > index 21649cb..27dc27d 100644 > --- a/gcc/ipa-devirt.c > +++ b/gcc/ipa-devirt.c > @@ -1710,7 +1710,7 @@ ipa_devirt (void) > > int npolymorphic = 0, nspeculated = 0, nconverted = 0, ncold = 0; > int nmultiple = 0, noverwritable = 0, ndevirtualized = 0, nnotdefined = 0; > - int nwrong = 0, nok = 0, nexternal = 0;; > + int nwrong = 0, nok = 0, nexternal = 0, nartificial = 0; > > FOR_EACH_DEFINED_FUNCTION (n) > { > @@ -1820,6 +1820,16 @@ ipa_devirt (void) > nexternal++; > continue; > } > + /* Don't use an implicitly-declared destructor (c++/58678). */ > + struct cgraph_node *real_target > + = cgraph_function_node (likely_target); > + if (DECL_ARTIFICIAL (real_target->decl)) I think we can safely test here DECL_ARTIFICIAL && (DECL_EXTERNAL || DECL_COMDAT). If the dtor is going to be output anyway, we are safe to use it. Are those programs valid by C++ standard? (I believe it is not valid to include sutff whose implementation you do not link with.). If we just want to avoid breaking python and libreoffice (I fixed libreoffice part however), we may just go with the ipa-devirt change as you propose (with external&comdat check). If this is an correcness issue, I think we want to be safe that other optimizations won't do the same. In that case your check seems misplaced. If DECL_ARTIFICIAL destructors are not safe to inline, I would add it into function_attribute_inlinable_p. If the dtor is not safe to refer, then I would add it into can_refer_decl_in_current_unit_p Both such changes would however inhibit quite some potimization, since artificial destructors are quite common case, right? Or is there some reason why only speculative devirtualiztaion count possibly work out reference to these? Honza
commit 94eb5df9fb20c796d09151d7293ae89ac012ae79
Author: Jason Merrill <jason@redhat.com>
Date: Fri Feb 28 14:03:19 2014 -0500
PR c++/58678
* ipa-devirt.c (ipa_devirt): Don't choose an implicitly-declared
function.
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 21649cb..27dc27d 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1710,7 +1710,7 @@ ipa_devirt (void)
int npolymorphic = 0, nspeculated = 0, nconverted = 0, ncold = 0;
int nmultiple = 0, noverwritable = 0, ndevirtualized = 0, nnotdefined = 0;
- int nwrong = 0, nok = 0, nexternal = 0;;
+ int nwrong = 0, nok = 0, nexternal = 0, nartificial = 0;
FOR_EACH_DEFINED_FUNCTION (n)
{
@@ -1820,6 +1820,16 @@ ipa_devirt (void)
nexternal++;
continue;
}
+ /* Don't use an implicitly-declared destructor (c++/58678). */
+ struct cgraph_node *real_target
+ = cgraph_function_node (likely_target);
+ if (DECL_ARTIFICIAL (real_target->decl))
+ {
+ if (dump_file)
+ fprintf (dump_file, "Target is implicitly declared\n\n");
+ nartificial++;
+ continue;
+ }
if (cgraph_function_body_availability (likely_target)
<= AVAIL_OVERWRITABLE
&& symtab_can_be_discarded (likely_target))
@@ -1862,10 +1872,10 @@ ipa_devirt (void)
" %i speculatively devirtualized, %i cold\n"
"%i have multiple targets, %i overwritable,"
" %i already speculated (%i agree, %i disagree),"
- " %i external, %i not defined\n",
+ " %i external, %i not defined, %i artificial\n",
npolymorphic, ndevirtualized, nconverted, ncold,
nmultiple, noverwritable, nspeculated, nok, nwrong,
- nexternal, nnotdefined);
+ nexternal, nnotdefined, nartificial);
return ndevirtualized ? TODO_remove_functions : 0;
}
diff --git a/gcc/testsuite/g++.dg/ipa/devirt-28.C b/gcc/testsuite/g++.dg/ipa/devirt-28.C
new file mode 100644
index 0000000..35c8df1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/devirt-28.C
@@ -0,0 +1,17 @@
+// PR c++/58678
+// { dg-options "-O3 -fdump-ipa-devirt" }
+
+struct A {
+ virtual ~A();
+};
+struct B : A {
+ virtual int m_fn1();
+};
+void fn1(B* b) {
+ delete b;
+}
+
+// { dg-final { scan-assembler-not "_ZN1AD2Ev" } }
+// { dg-final { scan-assembler-not "_ZN1BD0Ev" } }
+// { dg-final { scan-ipa-dump "Target is implicitly declared" "devirt" } }
+// { dg-final { cleanup-ipa-dump "devirt" } }