| Message ID | 56AB4F9C.3090702@redhat.com |
|---|---|
| State | New |
| Headers | show |
Ping. On 01/29/2016 12:40 PM, Bernd Schmidt wrote: > Let's say we have > > struct a { > int x[1]; > int y[1]; > } x = { 0, { 0 } }; > ^ > > When we reach the marked brace, we call into push_init_level, where we > notice that we have implicit initializers (for x[]) lying around that we > should deal with now that we've seen another open brace. The problem is > that we've created a new obstack for the initializer of y, and this is > where we also put data for the inits of x, freeing it when we see the > close brace for the initialization of y. > > In the actual testcase, which is a little more complex to actually > demonstrate the issue, we end up allocating two init elts at the same > address (because of premature freeing) and place them in the same tree, > which ends up containing a cycle because of this. Then we hang. > > Fixed by this patch, which splits off a new function > finish_implicit_inits from push_init_level and ensures it's called with > the outer obstack instead of the new one in the problematic case. > > Bootstrapped and tested on x86_64-linux, ok? > > > Bernd
On 01/29/2016 04:40 AM, Bernd Schmidt wrote: > Let's say we have > > struct a { > int x[1]; > int y[1]; > } x = { 0, { 0 } }; > ^ > > When we reach the marked brace, we call into push_init_level, where we > notice that we have implicit initializers (for x[]) lying around that we > should deal with now that we've seen another open brace. The problem is > that we've created a new obstack for the initializer of y, and this is > where we also put data for the inits of x, freeing it when we see the > close brace for the initialization of y. > > In the actual testcase, which is a little more complex to actually > demonstrate the issue, we end up allocating two init elts at the same > address (because of premature freeing) and place them in the same tree, > which ends up containing a cycle because of this. Then we hang. > > Fixed by this patch, which splits off a new function > finish_implicit_inits from push_init_level and ensures it's called with > the outer obstack instead of the new one in the problematic case. > > Bootstrapped and tested on x86_64-linux, ok? > > > Bernd > > braced-init.diff > > > c/ > PR c/69522 > * c-parser.c (c_parser_braced_init): New arg outer_obstack. All > callers changed. If nested_p is true, use it to call > finish_implicit_inits. > * c-tree.h (finish_implicit_inits): Declare. > * c-typeck.c (finish_implicit_inits): New function. Move code > from ... > (push_init_level): ... here. > (set_designator, process_init_element): Call finish_implicit_inits. > > testsuite/ > PR c/69522 > gcc.dg/pr69522.c: New test. OK. Thanks for tracking this down. Thanks, Jeff
FAIL: gcc.dg/pr69522.c (test for excess errors) Excess errors: /daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:2:8: error: struct has no members [-Wpedantic] /daten/aranym/gcc/gcc-20160212/gcc/testsuite/gcc.dg/pr69522.c:9:8: error: ISO C forbids empty initializer braces [-Wpedantic] Andreas.
On 02/08/2016 05:30 PM, Jeff Law wrote: > On 01/29/2016 04:40 AM, Bernd Schmidt wrote: >> c/ >> PR c/69522 >> * c-parser.c (c_parser_braced_init): New arg outer_obstack. All >> callers changed. If nested_p is true, use it to call >> finish_implicit_inits. >> * c-tree.h (finish_implicit_inits): Declare. >> * c-typeck.c (finish_implicit_inits): New function. Move code >> from ... >> (push_init_level): ... here. >> (set_designator, process_init_element): Call finish_implicit_inits. >> >> testsuite/ >> PR c/69522 >> gcc.dg/pr69522.c: New test. > OK. Thanks for tracking this down. Ok for branches too? Bernd
On 02/16/2016 07:21 AM, Bernd Schmidt wrote: > On 02/08/2016 05:30 PM, Jeff Law wrote: >> On 01/29/2016 04:40 AM, Bernd Schmidt wrote: >>> c/ >>> PR c/69522 >>> * c-parser.c (c_parser_braced_init): New arg outer_obstack. All >>> callers changed. If nested_p is true, use it to call >>> finish_implicit_inits. >>> * c-tree.h (finish_implicit_inits): Declare. >>> * c-typeck.c (finish_implicit_inits): New function. Move code >>> from ... >>> (push_init_level): ... here. >>> (set_designator, process_init_element): Call finish_implicit_inits. >>> >>> testsuite/ >>> PR c/69522 >>> gcc.dg/pr69522.c: New test. >> OK. Thanks for tracking this down. > > Ok for branches too? Yes. Definitely given it's a memory management issue. jeff
On 16 February 2016 at 16:37, Jeff Law <law@redhat.com> wrote: > On 02/16/2016 07:21 AM, Bernd Schmidt wrote: >> >> On 02/08/2016 05:30 PM, Jeff Law wrote: >>> >>> On 01/29/2016 04:40 AM, Bernd Schmidt wrote: >>>> >>>> c/ >>>> PR c/69522 >>>> * c-parser.c (c_parser_braced_init): New arg outer_obstack. All >>>> callers changed. If nested_p is true, use it to call >>>> finish_implicit_inits. >>>> * c-tree.h (finish_implicit_inits): Declare. >>>> * c-typeck.c (finish_implicit_inits): New function. Move code >>>> from ... >>>> (push_init_level): ... here. >>>> (set_designator, process_init_element): Call finish_implicit_inits. >>>> >>>> testsuite/ >>>> PR c/69522 >>>> gcc.dg/pr69522.c: New test. >>> >>> OK. Thanks for tracking this down. >> >> >> Ok for branches too? > Hi, FWIW, I'm seeing the new test failing on the 4.9 branch: gcc/testsuite/gcc.dg/pr69522.c:2:8: error: struct has no members [-Wpedantic] gcc/testsuite/gcc.dg/pr69522.c:9:8: error: ISO C forbids empty initializer braces [-Wpedantic] > Yes. Definitely given it's a memory management issue. > > jeff
On Thu, Feb 18, 2016 at 11:53:04AM +0100, Christophe Lyon wrote: > FWIW, I'm seeing the new test failing on the 4.9 branch: > gcc/testsuite/gcc.dg/pr69522.c:2:8: error: struct has no members [-Wpedantic] > gcc/testsuite/gcc.dg/pr69522.c:9:8: error: ISO C forbids empty > initializer braces [-Wpedantic] 4.9 branch is missing https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00867.html I'll fix. Marek
On 02/18/2016 12:22 PM, Marek Polacek wrote: > On Thu, Feb 18, 2016 at 11:53:04AM +0100, Christophe Lyon wrote: >> FWIW, I'm seeing the new test failing on the 4.9 branch: >> gcc/testsuite/gcc.dg/pr69522.c:2:8: error: struct has no members [-Wpedantic] >> gcc/testsuite/gcc.dg/pr69522.c:9:8: error: ISO C forbids empty >> initializer braces [-Wpedantic] > > 4.9 branch is missing https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00867.html > I'll fix. I suck. Bernd
c/ PR c/69522 * c-parser.c (c_parser_braced_init): New arg outer_obstack. All callers changed. If nested_p is true, use it to call finish_implicit_inits. * c-tree.h (finish_implicit_inits): Declare. * c-typeck.c (finish_implicit_inits): New function. Move code from ... (push_init_level): ... here. (set_designator, process_init_element): Call finish_implicit_inits. testsuite/ PR c/69522 gcc.dg/pr69522.c: New test. diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 43c26ae..eac0b1c 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -1284,7 +1284,8 @@ static tree c_parser_simple_asm_expr (c_parser *); static tree c_parser_attributes (c_parser *); static struct c_type_name *c_parser_type_name (c_parser *); static struct c_expr c_parser_initializer (c_parser *); -static struct c_expr c_parser_braced_init (c_parser *, tree, bool); +static struct c_expr c_parser_braced_init (c_parser *, tree, bool, + struct obstack *); static void c_parser_initelt (c_parser *, struct obstack *); static void c_parser_initval (c_parser *, struct c_expr *, struct obstack *); @@ -4289,7 +4290,7 @@ static struct c_expr c_parser_initializer (c_parser *parser) { if (c_parser_next_token_is (parser, CPP_OPEN_BRACE)) - return c_parser_braced_init (parser, NULL_TREE, false); + return c_parser_braced_init (parser, NULL_TREE, false, NULL); else { struct c_expr ret; @@ -4309,7 +4310,8 @@ c_parser_initializer (c_parser *parser) top-level initializer in a declaration. */ static struct c_expr -c_parser_braced_init (c_parser *parser, tree type, bool nested_p) +c_parser_braced_init (c_parser *parser, tree type, bool nested_p, + struct obstack *outer_obstack) { struct c_expr ret; struct obstack braced_init_obstack; @@ -4318,7 +4320,10 @@ c_parser_braced_init (c_parser *parser, tree type, bool nested_p) gcc_assert (c_parser_next_token_is (parser, CPP_OPEN_BRACE)); c_parser_consume_token (parser); if (nested_p) - push_init_level (brace_loc, 0, &braced_init_obstack); + { + finish_implicit_inits (brace_loc, outer_obstack); + push_init_level (brace_loc, 0, &braced_init_obstack); + } else really_start_incremental_init (type); if (c_parser_next_token_is (parser, CPP_CLOSE_BRACE)) @@ -4576,7 +4581,8 @@ c_parser_initval (c_parser *parser, struct c_expr *after, location_t loc = c_parser_peek_token (parser)->location; if (c_parser_next_token_is (parser, CPP_OPEN_BRACE) && !after) - init = c_parser_braced_init (parser, NULL_TREE, true); + init = c_parser_braced_init (parser, NULL_TREE, true, + braced_init_obstack); else { init = c_parser_expr_no_commas (parser, after); @@ -8060,7 +8066,7 @@ c_parser_postfix_expression_after_paren_type (c_parser *parser, error_at (type_loc, "compound literal has variable size"); type = error_mark_node; } - init = c_parser_braced_init (parser, type, false); + init = c_parser_braced_init (parser, type, false, NULL); finish_init (); maybe_warn_string_init (type_loc, type, init); diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index 00e72b1..5902fd2 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -625,6 +625,7 @@ extern void maybe_warn_string_init (location_t, tree, struct c_expr); extern void start_init (tree, tree, int); extern void finish_init (void); extern void really_start_incremental_init (tree); +extern void finish_implicit_inits (location_t, struct obstack *); extern void push_init_level (location_t, int, struct obstack *); extern struct c_expr pop_init_level (location_t, int, struct obstack *); extern void set_init_index (location_t, tree, tree, struct obstack *); diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index a147ac6..e4e2944 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -7447,6 +7447,30 @@ really_start_incremental_init (tree type) } } +/* Called when we see an open brace for a nested initializer. Finish + off any pending levels with implicit braces. */ +void +finish_implicit_inits (location_t loc, struct obstack *braced_init_obstack) +{ + while (constructor_stack->implicit) + { + if (RECORD_OR_UNION_TYPE_P (constructor_type) + && constructor_fields == 0) + process_init_element (input_location, + pop_init_level (loc, 1, braced_init_obstack), + true, braced_init_obstack); + else if (TREE_CODE (constructor_type) == ARRAY_TYPE + && constructor_max_index + && tree_int_cst_lt (constructor_max_index, + constructor_index)) + process_init_element (input_location, + pop_init_level (loc, 1, braced_init_obstack), + true, braced_init_obstack); + else + break; + } +} + /* Push down into a subobject, for initialization. If this is for an explicit set of braces, IMPLICIT is 0. If it is because the next element belongs at a lower level, @@ -7459,33 +7483,6 @@ push_init_level (location_t loc, int implicit, struct constructor_stack *p; tree value = NULL_TREE; - /* If we've exhausted any levels that didn't have braces, - pop them now. If implicit == 1, this will have been done in - process_init_element; do not repeat it here because in the case - of excess initializers for an empty aggregate this leads to an - infinite cycle of popping a level and immediately recreating - it. */ - if (implicit != 1) - { - while (constructor_stack->implicit) - { - if (RECORD_OR_UNION_TYPE_P (constructor_type) - && constructor_fields == 0) - process_init_element (input_location, - pop_init_level (loc, 1, braced_init_obstack), - true, braced_init_obstack); - else if (TREE_CODE (constructor_type) == ARRAY_TYPE - && constructor_max_index - && tree_int_cst_lt (constructor_max_index, - constructor_index)) - process_init_element (input_location, - pop_init_level (loc, 1, braced_init_obstack), - true, braced_init_obstack); - else - break; - } - } - /* Unless this is an explicit brace, we need to preserve previous content if any. */ if (implicit) @@ -7912,6 +7909,7 @@ set_designator (location_t loc, int array, } constructor_designated = 1; + finish_implicit_inits (loc, braced_init_obstack); push_init_level (loc, 2, braced_init_obstack); return 0; } @@ -9295,6 +9293,7 @@ process_init_element (location_t loc, struct c_expr value, bool implicit, p = p->next; if (!p) break; + finish_implicit_inits (loc, braced_init_obstack); push_init_level (loc, 2, braced_init_obstack); p->stack = constructor_stack; if (p->range_end && tree_int_cst_equal (p->index, p->range_end)) Index: gcc/testsuite/gcc.dg/pr69522.c =================================================================== --- gcc/testsuite/gcc.dg/pr69522.c (revision 0) +++ gcc/testsuite/gcc.dg/pr69522.c (working copy) @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +struct str {}; +struct { + struct str b; + float c[1]; + int d[1]; + float e[2]; + int f[1]; +} a = {{}, 0, {0.5}, 0, 0, {0}};
Let's say we have struct a { int x[1]; int y[1]; } x = { 0, { 0 } }; ^ When we reach the marked brace, we call into push_init_level, where we notice that we have implicit initializers (for x[]) lying around that we should deal with now that we've seen another open brace. The problem is that we've created a new obstack for the initializer of y, and this is where we also put data for the inits of x, freeing it when we see the close brace for the initialization of y. In the actual testcase, which is a little more complex to actually demonstrate the issue, we end up allocating two init elts at the same address (because of premature freeing) and place them in the same tree, which ends up containing a cycle because of this. Then we hang. Fixed by this patch, which splits off a new function finish_implicit_inits from push_init_level and ensures it's called with the outer obstack instead of the new one in the problematic case. Bootstrapped and tested on x86_64-linux, ok? Bernd