public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Teresa Johnson <tejohnson@google.com>
To: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Cc: David Li <davidxl@google.com>
Subject: [PATCH] Fix ICE due to memory corruption in SRA
Date: Mon, 02 Jun 2014 21:21:00 -0000	[thread overview]
Message-ID: <CAAe5K+WJ6gVACAiZZUw06EyyEn-aLdo=obiO=SR4x6QVzMfGaA@mail.gmail.com> (raw)

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

             reply	other threads:[~2014-06-02 21:21 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-02 21:21 Teresa Johnson [this message]
2014-06-03  9:23 ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAAe5K+WJ6gVACAiZZUw06EyyEn-aLdo=obiO=SR4x6QVzMfGaA@mail.gmail.com' \
    --to=tejohnson@google.com \
    --cc=davidxl@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).