Re: [PATCH, c-family] Fix a PCH thinko (and thus PR61250).

[Jeff Law <law at redhat dot com>]

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  <iain@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  <iain@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  <iain@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  <iain@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
jeff