* [PATCH] Fix ICE due to memory corruption in SRA
@ 2014-06-02 21:21 Teresa Johnson
2014-06-03 9:23 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Teresa Johnson @ 2014-06-02 21:21 UTC (permalink / raw)
To: gcc-patches; +Cc: David Li
This patch fixes an ICE due to memory corruption discovered while building a
large application with FDO and LIPO on the google branch. I don't have a small
reproducer, but the same code appears on trunk, and I believe it could also
silently result in incorrect code generation.
The problem occurs if SRA is applied on a recursive call. In this case,
the redirect_callers vec below contains the recursive edge from node->node.
When rebuild_cgraph_edges is invoked, it will free the callee edges of node,
including the one recorded in redirect_callers. In the case I looked at,
after rebuilding the cgraph edges for node, the address recorded in
redirect_callers now pointed to a different cgraph edge, and we later
got an ICE because the (incorrect) callee that we tried to modify had
the wrong number of arguments.
To fix, I simply moved the collection of caller edges to after the cgraph
edge rebuilding.
Google ref b/15383777.
Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?
Thanks,
Teresa
2014-06-02 Teresa Johnson <tejohnson@google.com>
* tree-sra.c (modify_function): Record caller nodes after rebuild.
Index: tree-sra.c
===================================================================
--- tree-sra.c (revision 211139)
+++ tree-sra.c (working copy)
@@ -4925,12 +4925,15 @@ modify_function (struct cgraph_node *node, ipa_par
{
struct cgraph_node *new_node;
bool cfg_changed;
- vec<cgraph_edge_p> redirect_callers = collect_callers_of_node (node);
rebuild_cgraph_edges ();
free_dominance_info (CDI_DOMINATORS);
pop_cfun ();
+ /* This must be done after rebuilding cgraph edges for node above.
+ Otherwise any recursive calls to node that are recorded in
+ redirect_callers will be corrupted. */
+ vec<cgraph_edge_p> redirect_callers = collect_callers_of_node (node);
new_node = cgraph_function_versioning (node, redirect_callers,
NULL,
NULL, false, NULL, NULL, "isra");
--
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix ICE due to memory corruption in SRA
2014-06-02 21:21 [PATCH] Fix ICE due to memory corruption in SRA Teresa Johnson
@ 2014-06-03 9:23 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2014-06-03 9:23 UTC (permalink / raw)
To: Teresa Johnson; +Cc: gcc-patches, David Li
On Mon, Jun 2, 2014 at 11:21 PM, Teresa Johnson <tejohnson@google.com> wrote:
> This patch fixes an ICE due to memory corruption discovered while building a
> large application with FDO and LIPO on the google branch. I don't have a small
> reproducer, but the same code appears on trunk, and I believe it could also
> silently result in incorrect code generation.
>
> The problem occurs if SRA is applied on a recursive call. In this case,
> the redirect_callers vec below contains the recursive edge from node->node.
> When rebuild_cgraph_edges is invoked, it will free the callee edges of node,
> including the one recorded in redirect_callers. In the case I looked at,
> after rebuilding the cgraph edges for node, the address recorded in
> redirect_callers now pointed to a different cgraph edge, and we later
> got an ICE because the (incorrect) callee that we tried to modify had
> the wrong number of arguments.
>
> To fix, I simply moved the collection of caller edges to after the cgraph
> edge rebuilding.
>
> Google ref b/15383777.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?
Ok.
Thanks,
Richard.
> Thanks,
> Teresa
>
> 2014-06-02 Teresa Johnson <tejohnson@google.com>
>
> * tree-sra.c (modify_function): Record caller nodes after rebuild.
>
> Index: tree-sra.c
> ===================================================================
> --- tree-sra.c (revision 211139)
> +++ tree-sra.c (working copy)
> @@ -4925,12 +4925,15 @@ modify_function (struct cgraph_node *node, ipa_par
> {
> struct cgraph_node *new_node;
> bool cfg_changed;
> - vec<cgraph_edge_p> redirect_callers = collect_callers_of_node (node);
>
> rebuild_cgraph_edges ();
> free_dominance_info (CDI_DOMINATORS);
> pop_cfun ();
>
> + /* This must be done after rebuilding cgraph edges for node above.
> + Otherwise any recursive calls to node that are recorded in
> + redirect_callers will be corrupted. */
> + vec<cgraph_edge_p> redirect_callers = collect_callers_of_node (node);
> new_node = cgraph_function_versioning (node, redirect_callers,
> NULL,
> NULL, false, NULL, NULL, "isra");
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-06-03 9:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-02 21:21 [PATCH] Fix ICE due to memory corruption in SRA Teresa Johnson
2014-06-03 9:23 ` Richard Biener
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).