From patchwork Thu Aug 22 19:59:32 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 1151775 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-507547-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="WuMmvcVI"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 46DwNm3H01z9sNF for ; Fri, 23 Aug 2019 05:59:50 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :content-type:content-transfer-encoding:mime-version:subject :message-id:date:cc:to; q=dns; s=default; b=s605N7u3g03AsQIm6mgQ ERSLeWAHydbpx2DL08WoP4E+mLYPH3dMPvl4br7gmhMzs/VRaJXn/A8fqenx3e23 ho9A4DLfB1PNbR0gM/b1VXautsa5x2a1D0KpAg/BjRJnqDUuxXkAhHXA9758ns+3 NPxnZ4YHw2qehnH/YZQj2oY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :content-type:content-transfer-encoding:mime-version:subject :message-id:date:cc:to; s=default; bh=RsvSsbmdluQo3W2TNXF4rnSH3Q g=; b=WuMmvcVI1xhsYNNSO3CUi344seOuj4pD+mBnrdokpavsVF6txJ69TUTjBn 2vbGv8G1qwuBgD4N5r4VUafcMsj4UIqaSwLeUe/YQnXy6AxbsYUji6HJ5et+pQvo HooCWGNSK/i9eMDbrfr2jPFKAhiKfdQqDkVdihOm5AO4ZUvpY= Received: (qmail 25986 invoked by alias); 22 Aug 2019 19:59:41 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 25978 invoked by uid 89); 22 Aug 2019 19:59:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_COUK, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=reasonably, collected, everywhere X-HELO: smtp2.wavenetuk.net Received: from smtp.wavenetuk.net (HELO smtp2.wavenetuk.net) (195.26.37.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 22 Aug 2019 19:59:38 +0000 Received: from euterpe-sie.home (host81-138-1-83.in-addr.btopenworld.com [81.138.1.83]) by smtp2.wavenetuk.net (Postfix) with ESMTPA id BC5496002A4; Thu, 22 Aug 2019 20:59:35 +0100 (BST) From: Iain Sandoe Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [PATCH, c-family] Fix a PCH thinko (and thus PR61250). Message-Id: Date: Thu, 22 Aug 2019 20:59:32 +0100 Cc: Jason Merrill , Joseph Myers To: GCC Patches Hi, When we are parsing a source file, the very first token might be a PRAGMA_GCC_PCH_PREPROCESS. This indicates that we are going read in a PCH file (named as the value of the pragma). If we don't see this pragma, then we know that it's OK to release any resources that the host might have set aside for the PCH file. There is a thinko in the current implementation, in that the decision to release resources is happening unconditionally right after the first token is extracted but before it's been checked or acted upon. This leads to the pch bug on Darwin, because we actually do release resources - which are subsequently (reasonably) assumed to be available when reading a PCH file. We then get random crashes or hangs depending on the interaction between unmmap and malloc. The bug is present everywhere but doesn't show on (say) Linux, since the release of PCH resources is a NOP there. This effects all the c-family front ends, because they all use c_lex_with_flags () to implement this. The solution is to check for the PRAGMA_GCC_PCH_PREPROCESS and only call c_common_no_more_pch () when that is not the first token. A secondary effect of the collection is that the name of the PCH file can be collected during the ggc_pch_read() reset of state. Therefore we should issue any diagnostic that might name the file before the collections are triggered. Tested on x86-64-darwin16/18 (where the pch.exp tests fail roughly half the time for any parallel testing) and pass reliably without it. Tested on x86_64,powerpc-linux-gnu with no regressions (and no expectation of any progression either). Since the fix is in common code, it needs the ack of both C and C++ to apply (I’m obviously OK with applying it from the Objective-C/C++ PoV) OK for trunk? given that this is a show-stopper for PCH + -save-temps I would also like to fix it on the open branches? thanks Iain gcc/c-family/ 2019-08-22 Iain Sandoe PR pch/61250 * c-lex.c (c_lex_with_flags): Don't call c_common_no_more_pch () from here. gcc/c/ 2019-08-22 Iain Sandoe PR pch/61250 * c-parser.c (c_parse_file): Call c_common_no_more_pch () after determining that the first token is not PRAGMA_GCC_PCH_PREPROCESS. gcc/cp/ 2019-08-22 Iain Sandoe PR pch/61250 * parser.c (cp_parser_initial_pragma): Call c_common_no_more_pch () after determining that the first token is not PRAGMA_GCC_PCH_PREPROCESS. gcc/ 2019-08-22 Iain Sandoe PR pch/61250 * ggc-page.c (ggc_pch_read): Read the ggc_pch_ondisk structure and issue any diagnostics needed before collecting the pre-PCH state. --- gcc/c-family/c-lex.c | 7 ------- gcc/c/c-parser.c | 2 ++ gcc/cp/parser.c | 5 ++++- gcc/ggc-page.c | 5 +++-- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c index 851fd704e5..e3c602fbb8 100644 --- a/gcc/c-family/c-lex.c +++ b/gcc/c-family/c-lex.c @@ -394,7 +394,6 @@ enum cpp_ttype c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags, int lex_flags) { - static bool no_more_pch; const cpp_token *tok; enum cpp_ttype type; unsigned char add_flags = 0; @@ -628,12 +627,6 @@ c_lex_with_flags (tree *value, location_t *loc, unsigned char *cpp_flags, if (cpp_flags) *cpp_flags = tok->flags | add_flags; - if (!no_more_pch) - { - no_more_pch = true; - c_common_no_more_pch (); - } - timevar_pop (TV_CPP); return type; diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 81919a89cc..cf973b3c8d 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -20232,6 +20232,8 @@ c_parse_file (void) if (c_parser_peek_token (&tparser)->pragma_kind == PRAGMA_GCC_PCH_PREPROCESS) c_parser_pragma_pch_preprocess (&tparser); + else + c_common_no_more_pch (); the_parser = ggc_alloc (); *the_parser = tparser; diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index dbbfe1dbc2..ab8a0cabb4 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -41353,7 +41353,10 @@ cp_parser_initial_pragma (cp_token *first_token) cp_lexer_get_preprocessor_token (NULL, first_token); if (cp_parser_pragma_kind (first_token) != PRAGMA_GCC_PCH_PREPROCESS) - return; + { + c_common_no_more_pch (); + return; + } cp_lexer_get_preprocessor_token (NULL, first_token); if (first_token->type == CPP_STRING) diff --git a/gcc/ggc-page.c b/gcc/ggc-page.c index a2736bc1df..220f20c5cf 100644 --- a/gcc/ggc-page.c +++ b/gcc/ggc-page.c @@ -2556,6 +2556,9 @@ ggc_pch_read (FILE *f, void *addr) count_old_page_tables = G.by_depth_in_use; + if (fread (&d, sizeof (d), 1, f) != 1) + fatal_error (input_location, "cannot read PCH file: %m"); + /* We've just read in a PCH file. So, every object that used to be allocated is now free. */ clear_marks (); @@ -2584,8 +2587,6 @@ ggc_pch_read (FILE *f, void *addr) /* Allocate the appropriate page-table entries for the pages read from the PCH file. */ - if (fread (&d, sizeof (d), 1, f) != 1) - fatal_error (input_location, "cannot read PCH file: %m"); for (i = 0; i < NUM_ORDERS; i++) {