public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Fix up cp_parser_skip_to_pragma_eol [PR104623]
@ 2022-03-12 19:22 Jakub Jelinek
  2022-03-14 22:39 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2022-03-12 19:22 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

We ICE on the following testcase, because we tentatively parse it multiple
times and the erroneous attribute syntax results in
cp_parser_skip_to_end_of_statement, which when seeing CPP_PRAGMA (can be
any deferred one, OpenMP/OpenACC/ivdep etc.) it calls
cp_parser_skip_to_pragma_eol, which calls cp_lexer_purge_tokens_after.
That call purges all the tokens from CPP_PRAGMA until CPP_PRAGMA_EOL,
excluding the initial CPP_PRAGMA though (but including the final
CPP_PRAGMA_EOL).  This means the second time we parse this, we see
CPP_PRAGMA with no tokens after it from the pragma, most importantly
not the CPP_PRAGMA_EOL, so either if it is the last pragma in the TU,
we ICE, or if there are other pragmas we treat everything in between
as a pragma.

I've tried various things, including making the CPP_PRAGMA token
itself also purged, or changing the cp_parser_skip_to_end_of_statement
(and cp_parser_skip_to_end_of_block_or_statement) to call it with
NULL instead of token, so that this purging isn't done there,
but each patch resulted in lots of regressions.
But removing the purging altogether surprisingly doesn't regress anything,
and I think it is the right thing, if we e.g. parse tentatively, why can't
we parse the pragma multiple times or at least skip over it?

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2022-03-12  Jakub Jelinek  <jakub@redhat.com>

	PR c++/104623
	* parser.cc (cp_parser_skip_to_pragma_eol): Don't purge any tokens.

	* g++.dg/gomp/pr104623.C: New test.

--- gcc/cp/parser.cc.jj	2022-03-11 13:11:53.622094878 +0100
+++ gcc/cp/parser.cc	2022-03-11 14:45:36.877647173 +0100
@@ -4111,8 +4111,6 @@ cp_parser_skip_to_pragma_eol (cp_parser*
 
   if (pragma_tok)
     {
-      /* Ensure that the pragma is not parsed again.  */
-      cp_lexer_purge_tokens_after (parser->lexer, pragma_tok);
       parser->lexer->in_pragma = false;
       if (parser->lexer->in_omp_attribute_pragma
 	  && cp_lexer_next_token_is (parser->lexer, CPP_EOF))
--- gcc/testsuite/g++.dg/gomp/pr104623.C.jj	2022-03-11 14:22:15.724288282 +0100
+++ gcc/testsuite/g++.dg/gomp/pr104623.C	2022-03-11 14:22:06.746413835 +0100
@@ -0,0 +1,9 @@
+// PR c++/104623
+// { dg-do compile }
+
+void
+foo ()
+{
+  struct __attribute__() a	// { dg-error "expected primary-expression before" }
+  #pragma omp task
+}

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] c++: Fix up cp_parser_skip_to_pragma_eol [PR104623]
  2022-03-12 19:22 [PATCH] c++: Fix up cp_parser_skip_to_pragma_eol [PR104623] Jakub Jelinek
@ 2022-03-14 22:39 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2022-03-14 22:39 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 3/12/22 14:22, Jakub Jelinek wrote:
> Hi!
> 
> We ICE on the following testcase, because we tentatively parse it multiple
> times and the erroneous attribute syntax results in
> cp_parser_skip_to_end_of_statement, which when seeing CPP_PRAGMA (can be
> any deferred one, OpenMP/OpenACC/ivdep etc.) it calls
> cp_parser_skip_to_pragma_eol, which calls cp_lexer_purge_tokens_after.
> That call purges all the tokens from CPP_PRAGMA until CPP_PRAGMA_EOL,
> excluding the initial CPP_PRAGMA though (but including the final
> CPP_PRAGMA_EOL).  This means the second time we parse this, we see
> CPP_PRAGMA with no tokens after it from the pragma, most importantly
> not the CPP_PRAGMA_EOL, so either if it is the last pragma in the TU,
> we ICE, or if there are other pragmas we treat everything in between
> as a pragma.
> 
> I've tried various things, including making the CPP_PRAGMA token
> itself also purged, or changing the cp_parser_skip_to_end_of_statement
> (and cp_parser_skip_to_end_of_block_or_statement) to call it with
> NULL instead of token, so that this purging isn't done there,
> but each patch resulted in lots of regressions.
> But removing the purging altogether surprisingly doesn't regress anything,
> and I think it is the right thing, if we e.g. parse tentatively, why can't
> we parse the pragma multiple times or at least skip over it?
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

The purging seems to date back to rth's r109336 that introduced CPP_PRAGMA.

> 2022-03-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/104623
> 	* parser.cc (cp_parser_skip_to_pragma_eol): Don't purge any tokens.
> 
> 	* g++.dg/gomp/pr104623.C: New test.
> 
> --- gcc/cp/parser.cc.jj	2022-03-11 13:11:53.622094878 +0100
> +++ gcc/cp/parser.cc	2022-03-11 14:45:36.877647173 +0100
> @@ -4111,8 +4111,6 @@ cp_parser_skip_to_pragma_eol (cp_parser*
>   
>     if (pragma_tok)
>       {
> -      /* Ensure that the pragma is not parsed again.  */
> -      cp_lexer_purge_tokens_after (parser->lexer, pragma_tok);
>         parser->lexer->in_pragma = false;
>         if (parser->lexer->in_omp_attribute_pragma
>   	  && cp_lexer_next_token_is (parser->lexer, CPP_EOF))
> --- gcc/testsuite/g++.dg/gomp/pr104623.C.jj	2022-03-11 14:22:15.724288282 +0100
> +++ gcc/testsuite/g++.dg/gomp/pr104623.C	2022-03-11 14:22:06.746413835 +0100
> @@ -0,0 +1,9 @@
> +// PR c++/104623
> +// { dg-do compile }
> +
> +void
> +foo ()
> +{
> +  struct __attribute__() a	// { dg-error "expected primary-expression before" }
> +  #pragma omp task
> +}
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-03-14 22:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-12 19:22 [PATCH] c++: Fix up cp_parser_skip_to_pragma_eol [PR104623] Jakub Jelinek
2022-03-14 22:39 ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).