* Fix two ICEs with C++ destructors and LTO
@ 2019-03-18 15:48 Jan Hubicka
2019-03-19 8:02 ` Richard Biener
2019-03-19 22:08 ` Rainer Orth
0 siblings, 2 replies; 3+ messages in thread
From: Jan Hubicka @ 2019-03-18 15:48 UTC (permalink / raw)
To: gcc-patches, rguenther
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?
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;
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Fix two ICEs with C++ destructors and LTO
2019-03-18 15:48 Fix two ICEs with C++ destructors and LTO Jan Hubicka
@ 2019-03-19 8:02 ` Richard Biener
2019-03-19 22:08 ` Rainer Orth
1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2019-03-19 8:02 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches
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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Fix two ICEs with C++ destructors and LTO
2019-03-18 15:48 Fix two ICEs with C++ destructors and LTO Jan Hubicka
2019-03-19 8:02 ` Richard Biener
@ 2019-03-19 22:08 ` Rainer Orth
1 sibling, 0 replies; 3+ messages in thread
From: Rainer Orth @ 2019-03-19 22:08 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches, rguenther
Hi Jan,
> 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?
>
> 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.
this test FAILs on Solaris with the native ld:
FAIL: g++.dg/lto/pr89335 cp_lto_pr89335_0.o assemble, -O2 -flto -Wsuggest-final-methods
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/lto/pr89335_0.C:9:7: warning: Declaring virtual destructor of 'struct List' final would enable devirtualization of 1 call [-Wsuggest-final-methods]
The same failure can be seen on Linux/x86_64 with -fno-use-linker-plugin.
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-19 21:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-18 15:48 Fix two ICEs with C++ destructors and LTO Jan Hubicka
2019-03-19 8:02 ` Richard Biener
2019-03-19 22:08 ` Rainer Orth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).