On Thu, Aug 22, 2019 at 2:39 PM Jeff Law <l...@redhat.com> wrote: > > On 8/22/19 1:59 PM, Iain Sandoe wrote: > > 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 <i...@sandoe.co.uk> > > > > 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 <i...@sandoe.co.uk> > > > > 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 <i...@sandoe.co.uk> > > > > 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 <i...@sandoe.co.uk> > > > > 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. > OK
OK with me, too. Joseph recently mentioned being swamped with reviews, so I wouldn't worry about waiting for his review. Jason
