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}};