From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 130697 invoked by alias); 12 Aug 2019 08:37:16 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 130688 invoked by uid 89); 12 Aug 2019 08:37:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=tentative, HX-HELO:sk:mail-lf X-HELO: mail-lf1-f68.google.com Received: from mail-lf1-f68.google.com (HELO mail-lf1-f68.google.com) (209.85.167.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Aug 2019 08:37:14 +0000 Received: by mail-lf1-f68.google.com with SMTP id s19so11000102lfb.9 for ; Mon, 12 Aug 2019 01:37:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=C3xhi7QLk2HLWN3flgS/23IIs4BZCQqSMP4v1s0Nez0=; b=QTfjFWkcO1brCRzT8Xxb+9OqMOmJ5VjUIf8SVh+fkD507XE58L4CCg5h9o/V7oBUMl 36ZZSOtgcjbh3B9Z5Xd3ntZSqAYCnfkPdNfh9sidTizfCYxpA6uoKHthSdu/GgfdJ0ad F+wDaTFWTAntIJzrMLo19vhh3GlLDRXIZoM1sQlK+UFl9dR8zg72V4zXF99/SUiMiEOX LBhGAyEWZR/WVOezyFeQSjdIqVDqL+X7Ya3nuhekMopU2vfAXimJ3hTI5sXO/ncAvEgA hDvj2eWQRMq5Wj85aUPYLlKbrLh+Jos7nyzgnfZXCzZ6q3FnXX/oKehoqRIfm2ruvNAF UqLA== MIME-Version: 1.0 References: <20190811171206.GB14737@redhat.com> In-Reply-To: <20190811171206.GB14737@redhat.com> From: Richard Biener Date: Mon, 12 Aug 2019 08:55:00 -0000 Message-ID: Subject: Re: C++ PATCH for c++/91416 - GC during late parsing collects live data To: Marek Polacek Cc: GCC Patches , Jason Merrill Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg00696.txt.bz2 On Sun, Aug 11, 2019 at 7:12 PM Marek Polacek 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 > > 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;