> The failure with -flto is due to make_decl_local clearing
> DECL_COMDAT, so the check in ipa_devirt allows devirtualization to
> an implicitly declared destructor. It's not clear to me why
> make_decl_local needs to clear DECL_COMDAT, but it's simpler to just
> remove that check from ipa_devirt.
>
> Tested x86_64-pc-linux-gnu. OK for trunk and 4.9?
> commit f88473ffec00cd8b537f7d92102f99fbd855b685
> Author: Jason Merrill <ja...@redhat.com>
> Date: Fri Aug 29 12:44:54 2014 -0400
>
> PR c++/58678
> * ipa-devirt.c (ipa_devirt): Don't check DECL_COMDAT.
>
> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
> index f98a18e..948ae23 100644
> --- a/gcc/ipa-devirt.c
> +++ b/gcc/ipa-devirt.c
> @@ -3952,8 +3952,7 @@ ipa_devirt (void)
> /* Don't use an implicitly-declared destructor (c++/58678). */
> struct cgraph_node *non_thunk_target
> = likely_target->function_symbol ();
> - if (DECL_ARTIFICIAL (non_thunk_target->decl)
> - && DECL_COMDAT (non_thunk_target->decl))
> + if (DECL_ARTIFICIAL (non_thunk_target->decl))
If we know that we are going to output the destructor in current unit (i.e. it
is not COMDAT or EXTERNAL
and it have definition), I think we ought to devirtualize to it.
So (DECL_COMDAT || DECL_EXTERNAL), but then it won't helpif the function was
localized.
OK, I see:
_ZN1BD2Ev/3 (__base_dtor ) @0x7ffff756b000
Type: function definition analyzed
Visibility: externally_visible public weak comdat comdat_group:_ZN1BD5Ev
one_only artificial
Same comdat group as: _ZN1BD1Ev/4
Address is taken.
References: _ZTV1B/7 (addr)
Referring: _ZN1BD1Ev/4 (alias)
Read from file: t.o
First run: 0
Function flags:
Called by:
Calls: _ZN1AD2Ev/8 (0.00 per call) (can throw external)
so we have comdat destructor that we do not want to access. Next we do:
_ZN1BD2Ev/3 (__base_dtor ) @0x7ffff756b000
Type: function definition analyzed
Visibility: prevailing_def_ironly artificial
Address is taken.
References: _ZTV1B/7 (addr)
Referring: _ZN1BD1Ev/4 (alias)
Read from file: t.o
Availability: available
First run: 0
Function flags:
Called by:
Calls: _ZN1AD2Ev/8 (0.00 per call) (can throw external)
i.e. we bring the comdat local because we know we are only user of it in DSO.
>From this point on it is a normal static function and therefore everyone is
welcome to refer to it.
Again - I think main problem is that we provide a middle end a function body
that it is not allowed to use. We do not really have a concept of function
definitions that we may not use and worse yet, we are preventing that just in
one special case - i.e. in the speculative devirtualization. It is kind of semi
useful to have these because we can still analyze their side effects. Otherwise
i would say we should never get them out of FE.
I guess best approach would be avoid localizing those odd beasts, so add logic
into comdat_can_be_unshared_p_1 that will check for abstract destructors and
for those check if they do have some real references to them (other than from
another COMDAT/EXTERN symbols) and return false if they doesn't?
I wonder how many extra symbols we introduce this way, but for 4.9 it is
probably the way to go. For mainline we can play with localizing them later
if it turns out to be important for bigger C++ libraries.
(localization brings quite nice reductions to dynamic linker tables that
is always iportant)
Honza
> {
> if (dump_file)
> fprintf (dump_file, "Target is artificial\n\n");
> diff --git a/gcc/testsuite/g++.dg/ipa/devirt-28a.C
> b/gcc/testsuite/g++.dg/ipa/devirt-28a.C
> new file mode 100644
> index 0000000..bdd1682
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/devirt-28a.C
> @@ -0,0 +1,15 @@
> +// PR c++/58678
> +// { dg-options "-O3 -flto -shared -fPIC -Wl,--no-undefined" }
> +// { dg-do link { target { gld && fpic } } }
> +
> +struct A {
> + virtual ~A();
> +};
> +struct B : A {
> + virtual int m_fn1();
> +};
> +void fn1(B* b) {
> + delete b;
> +}
> +
> +int main() {}
