* Re: Small refactoring of cgraph_node::release_body
@ 2021-03-31 18:45 David Edelsohn
2021-04-01 4:45 ` Martin Liška
0 siblings, 1 reply; 6+ messages in thread
From: David Edelsohn @ 2021-03-31 18:45 UTC (permalink / raw)
To: GCC Patches, Jan Hubicka
This patch is causing new crashes in the testsuite.
ICE in release_body, at graph.c:1863
ranges offset out of range
Thanks, David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Small refactoring of cgraph_node::release_body
2021-03-31 18:45 Small refactoring of cgraph_node::release_body David Edelsohn
@ 2021-04-01 4:45 ` Martin Liška
2021-04-01 15:12 ` David Edelsohn
0 siblings, 1 reply; 6+ messages in thread
From: Martin Liška @ 2021-04-01 4:45 UTC (permalink / raw)
To: David Edelsohn, GCC Patches, Jan Hubicka
On 3/31/21 8:45 PM, David Edelsohn via Gcc-patches wrote:
> This patch is causing new crashes in the testsuite.
>
> ICE in release_body, at graph.c:1863
> ranges offset out of range
Hello.
Should be fixed with 23ce9945d5efa77c96161443f68e03664705ada3.
Martin
>
> Thanks, David
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Small refactoring of cgraph_node::release_body
2021-04-01 4:45 ` Martin Liška
@ 2021-04-01 15:12 ` David Edelsohn
0 siblings, 0 replies; 6+ messages in thread
From: David Edelsohn @ 2021-04-01 15:12 UTC (permalink / raw)
To: Martin Liška; +Cc: GCC Patches, Jan Hubicka
On Thu, Apr 1, 2021 at 12:45 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 3/31/21 8:45 PM, David Edelsohn via Gcc-patches wrote:
> > This patch is causing new crashes in the testsuite.
> >
> > ICE in release_body, at graph.c:1863
> > ranges offset out of range
>
> Hello.
>
> Should be fixed with 23ce9945d5efa77c96161443f68e03664705ada3.
Yes, thanks fixed. I bootstrapped during the window.
Thanks, David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Small refactoring of cgraph_node::release_body
2021-04-01 9:02 ` Christophe Lyon
@ 2021-04-01 9:29 ` Jan Hubicka
0 siblings, 0 replies; 6+ messages in thread
From: Jan Hubicka @ 2021-04-01 9:29 UTC (permalink / raw)
To: Christophe Lyon; +Cc: gcc Patches
> This patch is causing ICEs on arm and aarch64, and others according to
> gcc-testresults:
> on aarch64:
> g++.dg/ipa/devirt-7.C -std=gnu++14 (internal compiler error)
> g++.dg/ipa/devirt-7.C -std=gnu++17 (internal compiler error)
> g++.dg/ipa/devirt-7.C -std=gnu++2a (internal compiler error)
> g++.dg/ipa/devirt-7.C -std=gnu++98 (internal compiler error)
> g++.dg/ipa/pr71146.C -std=gnu++14 (internal compiler error)
> g++.dg/ipa/pr71146.C -std=gnu++17 (internal compiler error)
> g++.dg/ipa/pr71146.C -std=gnu++2a (internal compiler error)
> g++.dg/ipa/pr71146.C -std=gnu++98 (internal compiler error)
> g++.dg/ipa/pr85421.C (internal compiler error)
> g++.dg/ipa/pr92528.C (internal compiler error)
> g++.dg/lto/pr89330 cp_lto_pr89330_0.o-cp_lto_pr89330_1.o link,
> -O3 -g -flto -shared -fPIC -Wno-odr (internal compiler error)
> g++.dg/torture/covariant-1.C -O2 -flto -fno-use-linker-plugin
> -flto-partition=none (internal compiler error)
> g++.dg/torture/covariant-1.C -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects (internal compiler error)
> g++.dg/torture/pr46287.C -O2 (internal compiler error)
> g++.dg/torture/pr46287.C -O2 -flto -fno-use-linker-plugin
> -flto-partition=none (internal compiler error)
> g++.dg/torture/pr46287.C -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects (internal compiler error)
> g++.dg/torture/pr46287.C -O3 -fomit-frame-pointer -funroll-loops
> -fpeel-loops -ftracer -finline-functions (internal compiler error)
> g++.dg/torture/pr46287.C -O3 -g (internal compiler error)
> g++.dg/torture/pr78692.C -O2 (internal compiler error)
> g++.dg/torture/pr78692.C -O2 -flto -fno-use-linker-plugin
> -flto-partition=none (internal compiler error)
> g++.dg/torture/pr78692.C -O3 -g (internal compiler error)
> g++.dg/torture/pr83619.C -O2 (internal compiler error)
> g++.dg/torture/pr83619.C -O2 -flto -fno-use-linker-plugin
> -flto-partition=none (internal compiler error)
> g++.dg/torture/pr83619.C -O3 -g (internal compiler error)
>
> The backtrace includes:
> FAIL: g++.dg/ipa/devirt-7.C -std=gnu++98 (internal compiler error)
> FAIL: g++.dg/ipa/devirt-7.C -std=gnu++98 (test for excess errors)
> Excess errors:
> /gcc/testsuite/g++.dg/ipa/devirt-7.C:85:1: internal compiler error: in
> release_body, at cgraph.c:1863
> 0xb98d4f cgraph_node::release_body(bool)
> /gcc/cgraph.c:1863
> 0xba7f3c expand_all_functions
> /gcc/cgraphunit.c:1994
> 0xba7f3c symbol_table::compile()
> /gcc/cgraphunit.c:2358
> 0xbab204 symbol_table::finalize_compilation_unit()
> /gcc/cgraphunit.c:2539
>
> Can you check/fix?
I checked in a fix for most of the yesterday (I added last minute check
that was too restrictive and apparently did not fully re-test by
mistake). In some setups there is one failure
libgomp.c/declare-variant-1.c remaining that is different, I am looking
into it now and will fix it soon.
Honza
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Small refactoring of cgraph_node::release_body
2021-03-31 9:38 Jan Hubicka
@ 2021-04-01 9:02 ` Christophe Lyon
2021-04-01 9:29 ` Jan Hubicka
0 siblings, 1 reply; 6+ messages in thread
From: Christophe Lyon @ 2021-04-01 9:02 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc Patches
Hi,
On Wed, 31 Mar 2021 at 11:38, Jan Hubicka <hubicka@ucw.cz> wrote:
>
> Hi,
> in the dicussion on PR 99447 there was some confusion about release_body
> being used in context where call edges/references survive. This is not
> a valid use because it would leave stale pointers to ggc_freed memory
> location. By auditing code I did not find any however this patch moves
> the callees/references removal into the function itself that makes it
> bit more robust.
>
> Some code paths calling release_body already free these earlier, but
> checking poitners for being NULL is not that expensive.
>
> Bootstrapped/regtested x86_64-linux, comitted.
This patch is causing ICEs on arm and aarch64, and others according to
gcc-testresults:
on aarch64:
g++.dg/ipa/devirt-7.C -std=gnu++14 (internal compiler error)
g++.dg/ipa/devirt-7.C -std=gnu++17 (internal compiler error)
g++.dg/ipa/devirt-7.C -std=gnu++2a (internal compiler error)
g++.dg/ipa/devirt-7.C -std=gnu++98 (internal compiler error)
g++.dg/ipa/pr71146.C -std=gnu++14 (internal compiler error)
g++.dg/ipa/pr71146.C -std=gnu++17 (internal compiler error)
g++.dg/ipa/pr71146.C -std=gnu++2a (internal compiler error)
g++.dg/ipa/pr71146.C -std=gnu++98 (internal compiler error)
g++.dg/ipa/pr85421.C (internal compiler error)
g++.dg/ipa/pr92528.C (internal compiler error)
g++.dg/lto/pr89330 cp_lto_pr89330_0.o-cp_lto_pr89330_1.o link,
-O3 -g -flto -shared -fPIC -Wno-odr (internal compiler error)
g++.dg/torture/covariant-1.C -O2 -flto -fno-use-linker-plugin
-flto-partition=none (internal compiler error)
g++.dg/torture/covariant-1.C -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects (internal compiler error)
g++.dg/torture/pr46287.C -O2 (internal compiler error)
g++.dg/torture/pr46287.C -O2 -flto -fno-use-linker-plugin
-flto-partition=none (internal compiler error)
g++.dg/torture/pr46287.C -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects (internal compiler error)
g++.dg/torture/pr46287.C -O3 -fomit-frame-pointer -funroll-loops
-fpeel-loops -ftracer -finline-functions (internal compiler error)
g++.dg/torture/pr46287.C -O3 -g (internal compiler error)
g++.dg/torture/pr78692.C -O2 (internal compiler error)
g++.dg/torture/pr78692.C -O2 -flto -fno-use-linker-plugin
-flto-partition=none (internal compiler error)
g++.dg/torture/pr78692.C -O3 -g (internal compiler error)
g++.dg/torture/pr83619.C -O2 (internal compiler error)
g++.dg/torture/pr83619.C -O2 -flto -fno-use-linker-plugin
-flto-partition=none (internal compiler error)
g++.dg/torture/pr83619.C -O3 -g (internal compiler error)
The backtrace includes:
FAIL: g++.dg/ipa/devirt-7.C -std=gnu++98 (internal compiler error)
FAIL: g++.dg/ipa/devirt-7.C -std=gnu++98 (test for excess errors)
Excess errors:
/gcc/testsuite/g++.dg/ipa/devirt-7.C:85:1: internal compiler error: in
release_body, at cgraph.c:1863
0xb98d4f cgraph_node::release_body(bool)
/gcc/cgraph.c:1863
0xba7f3c expand_all_functions
/gcc/cgraphunit.c:1994
0xba7f3c symbol_table::compile()
/gcc/cgraphunit.c:2358
0xbab204 symbol_table::finalize_compilation_unit()
/gcc/cgraphunit.c:2539
Can you check/fix?
Thanks,
Christophe
>
> PR lto/99447
> * cgraph.c (cgraph_node::release_body): Remove all callers and
> references.
> * cgraphclones.c (cgraph_node::materialize_clone): Do not do it here.
> * cgraphunit.c (cgraph_node::expand): And here.
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 80140757d16..b77c676a58a 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -1860,6 +1860,9 @@ cgraph_node::release_body (bool keep_arguments)
> lto_free_function_in_decl_state_for_node (this);
> lto_file_data = NULL;
> }
> + gcc_assert (!clones);
> + remove_callees ();
> + remove_all_references ();
> }
>
> /* Remove function from symbol table. */
> diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
> index 95103a423f7..9f86463b42d 100644
> --- a/gcc/cgraphclones.c
> +++ b/gcc/cgraphclones.c
> @@ -1143,11 +1143,7 @@ cgraph_node::materialize_clone ()
> /* Function is no longer clone. */
> remove_from_clone_tree ();
> if (!this_clone_of->analyzed && !this_clone_of->clones)
> - {
> - this_clone_of->release_body ();
> - this_clone_of->remove_callees ();
> - this_clone_of->remove_all_references ();
> - }
> + this_clone_of->release_body ();
> }
>
> #include "gt-cgraphclones.h"
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 1c74cee69ac..0b70e4d4fde 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -1892,10 +1892,6 @@ cgraph_node::expand (void)
> comdat groups. */
> assemble_thunks_and_aliases ();
> release_body ();
> - /* Eliminate all call edges. This is important so the GIMPLE_CALL no longer
> - points to the dead function body. */
> - remove_callees ();
> - remove_all_references ();
> }
>
> /* Node comparator that is responsible for the order that corresponds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Small refactoring of cgraph_node::release_body
@ 2021-03-31 9:38 Jan Hubicka
2021-04-01 9:02 ` Christophe Lyon
0 siblings, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2021-03-31 9:38 UTC (permalink / raw)
To: gcc-patches
Hi,
in the dicussion on PR 99447 there was some confusion about release_body
being used in context where call edges/references survive. This is not
a valid use because it would leave stale pointers to ggc_freed memory
location. By auditing code I did not find any however this patch moves
the callees/references removal into the function itself that makes it
bit more robust.
Some code paths calling release_body already free these earlier, but
checking poitners for being NULL is not that expensive.
Bootstrapped/regtested x86_64-linux, comitted.
PR lto/99447
* cgraph.c (cgraph_node::release_body): Remove all callers and
references.
* cgraphclones.c (cgraph_node::materialize_clone): Do not do it here.
* cgraphunit.c (cgraph_node::expand): And here.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 80140757d16..b77c676a58a 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1860,6 +1860,9 @@ cgraph_node::release_body (bool keep_arguments)
lto_free_function_in_decl_state_for_node (this);
lto_file_data = NULL;
}
+ gcc_assert (!clones);
+ remove_callees ();
+ remove_all_references ();
}
/* Remove function from symbol table. */
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index 95103a423f7..9f86463b42d 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -1143,11 +1143,7 @@ cgraph_node::materialize_clone ()
/* Function is no longer clone. */
remove_from_clone_tree ();
if (!this_clone_of->analyzed && !this_clone_of->clones)
- {
- this_clone_of->release_body ();
- this_clone_of->remove_callees ();
- this_clone_of->remove_all_references ();
- }
+ this_clone_of->release_body ();
}
#include "gt-cgraphclones.h"
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 1c74cee69ac..0b70e4d4fde 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1892,10 +1892,6 @@ cgraph_node::expand (void)
comdat groups. */
assemble_thunks_and_aliases ();
release_body ();
- /* Eliminate all call edges. This is important so the GIMPLE_CALL no longer
- points to the dead function body. */
- remove_callees ();
- remove_all_references ();
}
/* Node comparator that is responsible for the order that corresponds
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-01 15:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31 18:45 Small refactoring of cgraph_node::release_body David Edelsohn
2021-04-01 4:45 ` Martin Liška
2021-04-01 15:12 ` David Edelsohn
-- strict thread matches above, loose matches on Subject: below --
2021-03-31 9:38 Jan Hubicka
2021-04-01 9:02 ` Christophe Lyon
2021-04-01 9:29 ` Jan Hubicka
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).