Message ID | 4DCAE1ED.6040203@codesourcery.com |
---|---|
State | New |
Headers | show |
On Wed, May 11, 2011 at 9:22 PM, Nathan Froyd <froydnj@codesourcery.com> wrote: > The comment for pointer_map_traverse says: > > /* Pass each pointer in PMAP to the function in FN, together with the pointer > to the value and the fixed parameter DATA. If FN returns false, the > iteration stops. */ > > However, the code in tree-cfg:edge_to_cases_cleanup does: > > static bool > edge_to_cases_cleanup (const void *key ATTRIBUTE_UNUSED, void **value, > void *data ATTRIBUTE_UNUSED) > { > tree t, next; > > for (t = (tree) *value; t; t = next) > { > next = CASE_CHAIN (t); > CASE_CHAIN (t) = NULL; > } > > *value = NULL; > return false; > } > > ... > pointer_map_traverse (edge_to_cases, edge_to_cases_cleanup, NULL); > > which means that we're only cleaning up one of the case chains stored in > EDGE_TO_CASES. Since it's a pointer_map, we can potentially get > different chains selected each time. Under -fcompare-debug, this leads > to problems later on when we walk the function body and collect all the > DECLs referenced therein; we might walk non-NULL CASE_CHAINs and reach > more DECLs, depending on the memory layout. > > This wouldn't have shown up previously, since TREE_CHAIN was used, and > we wouldn't walk TREE_CHAIN of expressions to find DECLs. > > The fix is simple: return true from the above function! I've added > logic to verify_expr to catch this sort of thing. > > Tested on x86_64-unknown-linux-gnu. OK to commit? Ok. Thanks, Richard. > -Nathan > > gcc/ > PR middle-end/48965 > * tree-cfg.c (edge_to_cases_cleanup): Return true. > (verify_expr) [CASE_LABEL_EXPR]: Add checking. > > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c > index e1f8707..e2e84a2 100644 > --- a/gcc/tree-cfg.c > +++ b/gcc/tree-cfg.c > @@ -843,7 +843,7 @@ edge_to_cases_cleanup (const void *key > ATTRIBUTE_UNUSED, void **value, > } > > *value = NULL; > - return false; > + return true; > } > > /* Start recording information mapping edges to case labels. */ > @@ -2830,6 +2830,14 @@ verify_expr (tree *tp, int *walk_subtrees, void > *data ATTRIBUTE_UNUSED) > *walk_subtrees = 0; > break; > > + case CASE_LABEL_EXPR: > + if (CASE_CHAIN (t)) > + { > + error ("invalid CASE_CHAIN"); > + return t; > + } > + break; > + > default: > break; > } >
different chains selected each time. Under -fcompare-debug, this leads to problems later on when we walk the function body and collect all the DECLs referenced therein; we might walk non-NULL CASE_CHAINs and reach more DECLs, depending on the memory layout. This wouldn't have shown up previously, since TREE_CHAIN was used, and we wouldn't walk TREE_CHAIN of expressions to find DECLs. The fix is simple: return true from the above function! I've added logic to verify_expr to catch this sort of thing. Tested on x86_64-unknown-linux-gnu. OK to commit? -Nathan gcc/ PR middle-end/48965 * tree-cfg.c (edge_to_cases_cleanup): Return true. (verify_expr) [CASE_LABEL_EXPR]: Add checking. diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index e1f8707..e2e84a2 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -843,7 +843,7 @@ edge_to_cases_cleanup (const void *key ATTRIBUTE_UNUSED, void **value, } *value = NULL; - return false; + return true; }