| Message ID | CAAe5K+WJ6gVACAiZZUw06EyyEn-aLdo=obiO=SR4x6QVzMfGaA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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");
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.