Re: RFA: ipa-devirt PATCH for c++/58678 (devirt causes KDE build failure)

[Jan Hubicka <hubicka at ucw dot cz>]

> 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