On Mon, 18 Mar 2019, Jan Hubicka wrote:

> Hi,
> these two PRs are about C++ destructors that are not virtual but have
> virtual alias. The devirtualization machinery walks from alias to symbol
> and is then confused by not knowing the class symbol belongs to.
> 
> I think it would make more sense to avoid these walks but that require
> more of surgery, so for GCC9 I think it is best to stop freeing contexts
> for desctructors. This does not save any stremaing because THIS pointer
> of the destructor has the same type.
> 
> Bootstrapped/regtested x86_64-linux, OK?

OK.

Richard.

> Honza
> 
>       PR lto/87809
>       PR lto/89335
>       * tree.c (free_lang_data_in_decl): Do not free context of C++
>       destrutors.
> 
>       * g++.dg/lto/pr87089_0.C: New testcase.
>       * g++.dg/lto/pr87089_1.C: New testcase.
>       * g++.dg/lto/pr89335_0.C: New testcase.
> Index: tree.c
> ===================================================================
> --- tree.c    (revision 269704)
> +++ tree.c    (working copy)
> @@ -5772,10 +5772,16 @@ free_lang_data_in_decl (tree decl, struc
>       not do well with TREE_CHAIN pointers linking them.
>  
>       Also do not drop containing types for virtual methods and tables because
> -     these are needed by devirtualization.  */
> +     these are needed by devirtualization.
> +     C++ destructors are special because C++ frontends sometimes produces
> +     virtual destructor as an alias of non-virtual destructor.  In
> +     devirutalization code we always walk through aliases and we need
> +     context to be preserved too.  See PR89335  */
>    if (TREE_CODE (decl) != FIELD_DECL
>        && ((TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL)
> -          || !DECL_VIRTUAL_P (decl)))
> +          || (!DECL_VIRTUAL_P (decl)
> +           && (TREE_CODE (decl) != FUNCTION_DECL
> +               || !DECL_CXX_DESTRUCTOR_P (decl)))))
>      DECL_CONTEXT (decl) = fld_decl_context (DECL_CONTEXT (decl));
>  }
>  
> Index: testsuite/g++.dg/lto/pr87089_0.C
> ===================================================================
> --- testsuite/g++.dg/lto/pr87089_0.C  (nonexistent)
> +++ testsuite/g++.dg/lto/pr87089_0.C  (working copy)
> @@ -0,0 +1,21 @@
> +// { dg-lto-do link }
> +// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" }
> +namespace itpp {
> +template <class a> void b(a *c) { c[0].~a(); }
> +class CFix;
> +template <class> class d {
> +  void e(const char *);
> +  CFix *data;
> +};
> +class CFix {
> +public:
> +  virtual ~CFix();
> +};
> +template <> void d<int>::e(const char *) { b(data); }
> +} // namespace itpp
> +
> +int
> +main (void)
> +{
> +  return 0;
> +}
> Index: testsuite/g++.dg/lto/pr87089_1.C
> ===================================================================
> --- testsuite/g++.dg/lto/pr87089_1.C  (nonexistent)
> +++ testsuite/g++.dg/lto/pr87089_1.C  (working copy)
> @@ -0,0 +1,12 @@
> +namespace itpp {
> +enum a { b };
> +class CFix {
> +public:
> +  virtual ~CFix();
> +};
> +template <a = b> class c : CFix {
> +  ~c() {}
> +};
> +template class c<>;
> +} // namespace itpp
> +
> Index: testsuite/g++.dg/lto/pr89335_0.C
> ===================================================================
> --- testsuite/g++.dg/lto/pr89335_0.C  (nonexistent)
> +++ testsuite/g++.dg/lto/pr89335_0.C  (working copy)
> @@ -0,0 +1,16 @@
> +// { dg-lto-do link }
> +// { dg-lto-options {{-O2 -flto -Wsuggest-final-methods}} }
> +// { dg-extra-ld-options "-r -nostdlib -flinker-output=nolto-rel" }
> +class Container
> +{
> +public:
> +  virtual ~Container ();
> +};
> +class List : public Container // { dg-lto-message "final would enable 
> devirtualization" }
> +{
> +};
> +static List cache[256];
> +int main (void)
> +{
> +  return 0;
> +}
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Reply via email to