C++ PATCH for c++/91416 - GC during late parsing collects live data

[Jason Merrill]

On 8/12/19 4:37 AM, Richard Biener wrote:
> On Sun, Aug 11, 2019 at 7:12 PM Marek Polacek <polacek@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?

It's already a significant issue for C++ that we only collect between 
function declarations, I'm concerned that this change will cause more 
memory problems.

Perhaps if we start parsing a class-specifier we could push the 
decl_specifiers onto a vec that is a GC root?

Jason

>> 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.

...and is defined in the declaration of something else?  Does it happen 
without declaring the variable 's'?

Jason