On Sun, Aug 11, 2019 at 7:12 PM Marek Polacek <pola...@redhat.com> wrote:
>
> This is a crash that points to a GC problem. Consider this test:
>
> __attribute__ ((unused)) struct S {
> S() { }
> } s;
>
> We're parsing a simple-declaration. While parsing the decl specs, we parse
> the
> attribute, which means creating a TREE_LIST using ggc_alloc_*.
>
> A function body is a complete-class context so when parsing the
> member-specification of this class-specifier, we parse the bodies of the
> functions we'd queued in cp_parser_late_parsing_for_member. This then leads
> to
> this call chain:
> cp_parser_function_definition_after_declarator -> expand_or_defer_fn ->
> expand_or_defer_fn_1 -> maybe_clone_body -> expand_or_defer_fn ->
> cgraph_node::finalize_function -> ggc_collect.
>
> In this test, the ggc_collect call collects the TREE_LIST we had allocated,
> and
> a crash duly ensues. We can't avoid late parsing of members in this context,
> so my fix is to bump function_depth, exactly following cp_parser_lambda_body.
> Since we are performing late parsing, we know we have to be nested in a class.
> (We still ggc_collect later, in c_parse_final_cleanups.)
So the struct S itself is properly referenced by a GC root? If so why not
attach the attribute list to the tentative struct instead? Or do we
fear we have
other non-rooted data live at the point we collect? If so shouldn't we instead
bump function_depth when parsing a declaration in general?
>
> But here's the thing. This goes back to ancient r104500, at least. How has
> this not broken before? All you need to trigger it is to enable GC checking
> and have a class with a ctor/member function, that has an attribute. You'd
> think that since we've got hundreds of those in the testsuite, at least one of
> them would trigger this crash. Or that there'd be several reports about this
> in
> the BZ already. Yet I didn't find any. Truly, I'm perplexed.
>
> Anyway, bootstrapped/regtested on x86_64-linux, ok for trunk? How about 9.3?
> I vote yes.
>
> 2019-08-11 Marek Polacek <pola...@redhat.com>
>
> PR c++/91416 - GC during late parsing collects live data.
> * parser.c (cp_parser_late_parsing_for_member): Increment
> function_depth
> around call to cp_parser_function_definition_after_declarator.
>
> * g++.dg/other/gc6.C: New test.
>
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index b56cc6924f4..0d4d32e9670 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -28934,6 +28934,8 @@ cp_parser_late_parsing_for_member (cp_parser* parser,
> tree member_function)
> function_scope = current_function_decl;
> if (function_scope)
> push_function_context ();
> + else
> + ++function_depth;
>
> /* Push the body of the function onto the lexer stack. */
> cp_parser_push_lexer_for_tokens (parser, tokens);
> @@ -28966,6 +28968,9 @@ cp_parser_late_parsing_for_member (cp_parser* parser,
> tree member_function)
> /* Leave the scope of the containing function. */
> if (function_scope)
> pop_function_context ();
> + else
> + --function_depth;
> +
> cp_parser_pop_lexer (parser);
> }
>
> diff --git gcc/testsuite/g++.dg/other/gc6.C gcc/testsuite/g++.dg/other/gc6.C
> new file mode 100644
> index 00000000000..385be5c945e
> --- /dev/null
> +++ gcc/testsuite/g++.dg/other/gc6.C
> @@ -0,0 +1,7 @@
> +// PR c++/91416 - GC during late parsing collects live data.
> +// { dg-do compile }
> +// { dg-options "--param ggc-min-heapsize=0 --param ggc-min-expand=0" }
> +
> +__attribute__ ((unused)) struct S {
> + S() { }
> +} s;
