public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Kwok Cheung Yeung <kcy@codesourcery.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>, Jakub Jelinek <jakub@redhat.com>
Subject: [PATCH] openmp: Improve handling of nested OpenMP metadirectives in C and C++ (was: Re: [PATCH 1/7] openmp: Add C support for parsing metadirectives)
Date: Fri, 18 Feb 2022 21:09:01 +0000	[thread overview]
Message-ID: <0090d882-4c17-3c12-ab54-54670bf32ab0@codesourcery.com> (raw)
In-Reply-To: <4a277a5e-6070-b287-7bc8-c2bcc72532a0@codesourcery.com>

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

This patch (to be applied on top of the metadirective patch series) 
addresses issues found in the C/C++ parsers when nested metadirectives 
are used.

analyze_metadirective_body when encountering code like:

#pragma omp metadirective when {set={...}: A)
   #pragma omp metadirective when (set={...}: B)

would stop just before ': B' before it naively assumes that the '}' 
marks the end of the body associated with the first metadirective, when 
it needs to include the whole of the second metadirective plus its 
associated body. This is fixed by checking that the nesting level of 
parentheses is zero as well before stopping the gathering of tokens.

The assert on the remaining tokens after parsing a clause can fail 
(resulting in an ICE) if there is a parse error in the directive or the 
body, since in that case not all tokens may be processed before parsing 
aborts early. The assert is therefore not enforced if any parse errors 
occur in the clause.

I have also moved the handling of the metadirective pragma from 
c_parser_omp_construct to c_parser_pragma (and their C++ equivalents), 
since c_parser_omp_construct has some checks that do not apply to 
metadirectives.

Kwok

[-- Attachment #2: 0001-openmp-Improve-handling-of-nested-OpenMP-metadirecti.patch --]
[-- Type: text/plain, Size: 13354 bytes --]

From a9e4936b8476b97f11bb81b416ef3d28fa60cd37 Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung <kcy@codesourcery.com>
Date: Fri, 18 Feb 2022 19:00:57 +0000
Subject: [PATCH] openmp: Improve handling of nested OpenMP metadirectives in C
 and C++

This patch fixes a misparsing issue when encountering code like:

  #pragma omp metadirective when {<selector_set>={...}: A)
    #pragma omp metadirective when (<selector_set>={...}: B)

When called for the first metadirective, analyze_metadirective_body would
stop just before the colon in the second metadirective because it naively
assumes that the '}' marks the end of a code block.

The assertion for clauses to end parsing at the same point is now disabled
if a parse error has occurred during the parsing of the clause, since some
tokens may not be consumed if a parse error cuts parsing short.

2022-02-18  Kwok Cheung Yeung  <kcy@codesourcery.com>

	gcc/c/
	* c-parser.cc (c_parser_omp_construct): Move handling of
	PRAGMA_OMP_METADIRECTIVE from here...
	(c_parser_pragma): ...to here.
	(analyze_metadirective_body): Check that the bracket nesting level
	is also zero before stopping the adding of tokens on encountering a
	close brace.
	(c_parser_omp_metadirective): Modify function signature and update.
	Do not assert on remaining tokens if there has been a parse error.

	gcc/cp/
	* parser.cc (cp_parser_omp_construct): Move handling of
	PRAGMA_OMP_METADIRECTIVE from here...
	(cp_parser_pragma): ...to here.
	(analyze_metadirective_body): Check that the bracket
	nesting level is also zero before stopping the adding of tokens on
	encountering a close brace.
	(cp_parser_omp_metadirective): Modify function signature and update.
	Do not assert on remaining tokens if there has been a parse error.

	gcc/testsuite/
	* c-c++-common/gomp/metadirective-1.c (f): Add test for
	improperly nested metadirectives.
---
 gcc/c/c-parser.cc                             | 47 +++++++++----------
 gcc/cp/parser.cc                              | 33 ++++++-------
 .../c-c++-common/gomp/metadirective-1.c       | 13 +++++
 3 files changed, 51 insertions(+), 42 deletions(-)

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 58fcbb398ee..6a134e0fb50 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -1592,6 +1592,7 @@ static void c_parser_omp_taskwait (c_parser *);
 static void c_parser_omp_taskyield (c_parser *);
 static void c_parser_omp_cancel (c_parser *);
 static void c_parser_omp_nothing (c_parser *);
+static void c_parser_omp_metadirective (c_parser *, bool *);
 
 enum pragma_context { pragma_external, pragma_struct, pragma_param,
 		      pragma_stmt, pragma_compound };
@@ -1600,8 +1601,6 @@ static bool c_parser_omp_cancellation_point (c_parser *, enum pragma_context);
 static bool c_parser_omp_target (c_parser *, enum pragma_context, bool *);
 static void c_parser_omp_end_declare_target (c_parser *);
 static bool c_parser_omp_declare (c_parser *, enum pragma_context);
-static tree c_parser_omp_metadirective (location_t, c_parser *, char *,
-					omp_clause_mask, tree *, bool *);
 static void c_parser_omp_requires (c_parser *);
 static bool c_parser_omp_error (c_parser *, enum pragma_context);
 static bool c_parser_omp_ordered (c_parser *, enum pragma_context, bool *);
@@ -12551,6 +12550,10 @@ c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p)
       c_parser_omp_nothing (parser);
       return false;
 
+    case PRAGMA_OMP_METADIRECTIVE:
+      c_parser_omp_metadirective (parser, if_p);
+      return true;
+
     case PRAGMA_OMP_ERROR:
       return c_parser_omp_error (parser, context);
 
@@ -23020,7 +23023,7 @@ analyze_metadirective_body (c_parser *parser,
 	  ++nesting_depth;
 	  goto add;
 	case CPP_CLOSE_BRACE:
-	  if (--nesting_depth == 0)
+	  if (--nesting_depth == 0 && bracket_depth == 0)
 	    stop = true;
 	  goto add;
 	case CPP_OPEN_PAREN:
@@ -23058,10 +23061,8 @@ analyze_metadirective_body (c_parser *parser,
   # pragma omp metadirective [clause[, clause]]
 */
 
-static tree
-c_parser_omp_metadirective (location_t loc, c_parser *parser,
-			    char *p_name, omp_clause_mask, tree *,
-			    bool *if_p)
+static void
+c_parser_omp_metadirective (c_parser *parser, bool *if_p)
 {
   tree ret;
   auto_vec<c_token> directive_tokens;
@@ -23073,13 +23074,14 @@ c_parser_omp_metadirective (location_t loc, c_parser *parser,
   bool default_seen = false;
   int directive_token_idx = 0;
   tree standalone_body = NULL_TREE;
+  location_t pragma_loc = c_parser_peek_token (parser)->location;
 
   ret = make_node (OMP_METADIRECTIVE);
-  SET_EXPR_LOCATION (ret, loc);
+  SET_EXPR_LOCATION (ret, pragma_loc);
   TREE_TYPE (ret) = void_type_node;
   OMP_METADIRECTIVE_CLAUSES (ret) = NULL_TREE;
-  strcat (p_name, " metadirective");
 
+  c_parser_consume_pragma (parser);
   while (c_parser_next_token_is_not (parser, CPP_PRAGMA_EOL))
     {
       if (c_parser_next_token_is_not (parser, CPP_NAME)
@@ -23287,6 +23289,7 @@ c_parser_omp_metadirective (location_t loc, c_parser *parser,
       parser->tokens = tokens.address ();
       parser->tokens_avail = tokens.length ();
 
+      int prev_errorcount = errorcount;
       tree directive = c_begin_compound_stmt (true);
 
       /* Declare all non-local labels that occur within the directive body
@@ -23296,11 +23299,11 @@ c_parser_omp_metadirective (location_t loc, c_parser *parser,
 	  tree label = declare_label (body_labels[j]);
 
 	  C_DECLARED_LABEL_FLAG (label) = 1;
-	  add_stmt (build_stmt (loc, DECL_EXPR, label));
+	  add_stmt (build_stmt (pragma_loc, DECL_EXPR, label));
 	}
 
       c_parser_pragma (parser, pragma_compound, if_p);
-      directive = c_end_compound_stmt (loc, directive, true);
+      directive = c_end_compound_stmt (pragma_loc, directive, true);
       bool standalone_p
 	= directives[i]->kind == C_OMP_DIR_STANDALONE
 	  || directives[i]->kind == C_OMP_DIR_UTILITY;
@@ -23323,10 +23326,14 @@ c_parser_omp_metadirective (location_t loc, c_parser *parser,
       OMP_METADIRECTIVE_CLAUSES (ret)
 	= chainon (OMP_METADIRECTIVE_CLAUSES (ret), variant);
 
-      /* Check that all valid tokens have been consumed.  */
-      gcc_assert (parser->tokens_avail == 2);
-      gcc_assert (c_parser_next_token_is (parser, CPP_EOF));
-      gcc_assert (c_parser_peek_2nd_token (parser)->type == CPP_EOF);
+      /* Check that all valid tokens have been consumed if no parse errors
+	 encountered.  */
+      if (errorcount == prev_errorcount)
+	{
+	  gcc_assert (parser->tokens_avail == 2);
+	  gcc_assert (c_parser_next_token_is (parser, CPP_EOF));
+	  gcc_assert (c_parser_peek_2nd_token (parser)->type == CPP_EOF);
+	}
 
       parser->tokens = old_tokens;
       parser->tokens_avail = old_tokens_avail;
@@ -23338,15 +23345,12 @@ c_parser_omp_metadirective (location_t loc, c_parser *parser,
     ret = c_omp_expand_metadirective (candidates);
 
   add_stmt (ret);
-
-  return ret;
+  return;
 
 error:
   if (parser->in_pragma)
     c_parser_skip_to_pragma_eol (parser);
   c_parser_skip_to_end_of_block_or_statement (parser);
-
-  return NULL_TREE;
 }
 
 /* Main entry point to parsing most OpenMP pragmas.  */
@@ -23422,11 +23426,6 @@ c_parser_omp_construct (c_parser *parser, bool *if_p)
       strcpy (p_name, "#pragma omp");
       stmt = c_parser_omp_master (loc, parser, p_name, mask, NULL, if_p);
       break;
-    case PRAGMA_OMP_METADIRECTIVE:
-      strcpy (p_name, "#pragma omp");
-      stmt = c_parser_omp_metadirective (loc, parser, p_name, mask, NULL,
-					 if_p);
-      break;
     case PRAGMA_OMP_PARALLEL:
       strcpy (p_name, "#pragma omp");
       stmt = c_parser_omp_parallel (loc, parser, p_name, mask, NULL, if_p);
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index aa23688814a..ef33cb1611f 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -45985,7 +45985,7 @@ cp_parser_omp_end_declare_target (cp_parser *parser, cp_token *pragma_tok)
 }
 
 
-/* Helper function for c_parser_omp_metadirective.  */
+/* Helper function for cp_parser_omp_metadirective.  */
 
 static void
 analyze_metadirective_body (cp_parser *parser,
@@ -46021,7 +46021,7 @@ analyze_metadirective_body (cp_parser *parser,
 	  ++nesting_depth;
 	  goto add;
 	case CPP_CLOSE_BRACE:
-	  if (--nesting_depth == 0)
+	  if (--nesting_depth == 0 && bracket_depth == 0)
 	    stop = true;
 	  goto add;
 	case CPP_OPEN_PAREN:
@@ -46056,9 +46056,8 @@ analyze_metadirective_body (cp_parser *parser,
   # pragma omp metadirective [clause[, clause]]
 */
 
-static tree
+static void
 cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok,
-			     char *p_name, omp_clause_mask, tree *,
 			     bool *if_p)
 {
   tree ret;
@@ -46069,15 +46068,14 @@ cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok,
   auto_vec<tree> ctxs;
   bool default_seen = false;
   int directive_token_idx = 0;
-  location_t loc = cp_lexer_peek_token (parser->lexer)->location;
+  location_t pragma_loc = pragma_tok->location;
   tree standalone_body = NULL_TREE;
   vec<struct omp_metadirective_variant> candidates;
 
   ret = make_node (OMP_METADIRECTIVE);
-  SET_EXPR_LOCATION (ret, loc);
+  SET_EXPR_LOCATION (ret, pragma_loc);
   TREE_TYPE (ret) = void_type_node;
   OMP_METADIRECTIVE_CLAUSES (ret) = NULL_TREE;
-  strcat (p_name, " metadirective");
 
   while (cp_lexer_next_token_is_not (parser->lexer, CPP_PRAGMA_EOL))
     {
@@ -46296,6 +46294,7 @@ cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok,
       parser->lexer = lexer;
       cp_lexer_set_source_position_from_token (lexer->next_token);
 
+      int prev_errorcount = errorcount;
       tree directive = push_stmt_list ();
       tree directive_stmt = begin_compound_stmt (0);
 
@@ -46330,8 +46329,10 @@ cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok,
       OMP_METADIRECTIVE_CLAUSES (ret)
 	= chainon (OMP_METADIRECTIVE_CLAUSES (ret), variant);
 
-      /* Check that all valid tokens have been consumed.  */
-      gcc_assert (cp_lexer_next_token_is (parser->lexer, CPP_EOF));
+      /* Check that all valid tokens have been consumed if no parse errors
+	 encountered.  */
+      gcc_assert (errorcount != prev_errorcount
+		  || cp_lexer_next_token_is (parser->lexer, CPP_EOF));
 
       parser->lexer = old_lexer;
       cp_lexer_set_source_position_from_token (old_lexer->next_token);
@@ -46343,8 +46344,7 @@ cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok,
     ret = c_omp_expand_metadirective (candidates);
 
   add_stmt (ret);
-
-  return ret;
+  return;
 
 fail:
   /* Skip the metadirective pragma.  */
@@ -46352,7 +46352,6 @@ fail:
 
   /* Skip the metadirective body.  */
   cp_parser_skip_to_end_of_block_or_statement (parser);
-  return error_mark_node;
 }
 
 
@@ -47602,11 +47601,6 @@ cp_parser_omp_construct (cp_parser *parser, cp_token *pragma_tok, bool *if_p)
       stmt = cp_parser_omp_master (parser, pragma_tok, p_name, mask, NULL,
 				   if_p);
       break;
-    case PRAGMA_OMP_METADIRECTIVE:
-      strcpy (p_name, "#pragma omp");
-      stmt = cp_parser_omp_metadirective (parser, pragma_tok, p_name, mask,
-					  NULL, if_p);
-      break;
     case PRAGMA_OMP_PARALLEL:
       strcpy (p_name, "#pragma omp");
       stmt = cp_parser_omp_parallel (parser, pragma_tok, p_name, mask, NULL,
@@ -48257,7 +48251,6 @@ cp_parser_pragma (cp_parser *parser, enum pragma_context context, bool *if_p)
     case PRAGMA_OMP_LOOP:
     case PRAGMA_OMP_MASKED:
     case PRAGMA_OMP_MASTER:
-    case PRAGMA_OMP_METADIRECTIVE:
     case PRAGMA_OMP_PARALLEL:
     case PRAGMA_OMP_SCOPE:
     case PRAGMA_OMP_SECTIONS:
@@ -48289,6 +48282,10 @@ cp_parser_pragma (cp_parser *parser, enum pragma_context context, bool *if_p)
       cp_parser_omp_nothing (parser, pragma_tok);
       return false;
 
+    case PRAGMA_OMP_METADIRECTIVE:
+      cp_parser_omp_metadirective (parser, pragma_tok, if_p);
+      return true;
+
     case PRAGMA_OMP_ERROR:
       return cp_parser_omp_error (parser, pragma_tok, context);
 
diff --git a/gcc/testsuite/c-c++-common/gomp/metadirective-1.c b/gcc/testsuite/c-c++-common/gomp/metadirective-1.c
index 72cf0abbbd7..543063a3324 100644
--- a/gcc/testsuite/c-c++-common/gomp/metadirective-1.c
+++ b/gcc/testsuite/c-c++-common/gomp/metadirective-1.c
@@ -4,6 +4,8 @@
 
 void f (int a[], int b[], int c[])
 {
+  int i;
+
   #pragma omp metadirective \
       default (teams loop) \
       default (parallel loop) /* { dg-error "there can only be one default clause in a metadirective before '\\(' token" } */
@@ -26,4 +28,15 @@ void f (int a[], int b[], int c[])
   #pragma omp metadirective \
 	default (metadirective default (flush))	/* { dg-error "metadirectives cannot be used as directive variants before 'default'" } */
     for (i = 0; i < N; i++) c[i] = a[i] * b[i];
+
+  /* Test improperly nested metadirectives - even though the second
+     metadirective resolves to 'omp nothing', that is not the same as there
+     being literally nothing there.  */
+  #pragma omp metadirective \
+      when (implementation={vendor("gnu")}: parallel for)
+    #pragma omp metadirective \
+	when (implementation={vendor("cray")}: parallel for)
+	/* { dg-error "for statement expected before '#pragma'" "" { target c } .-2 } */
+	/* { dg-error "'#pragma' is not allowed here" "" { target c++ } .-3 } */
+      for (i = 0; i < N; i++) c[i] = a[i] * b[i];
 }
-- 
2.25.1


  reply	other threads:[~2022-02-18 21:09 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09 11:16 [WIP, OpenMP] OpenMP metadirectives support Kwok Cheung Yeung
2021-07-26 11:38 ` Kwok Cheung Yeung
2021-07-26 14:29 ` Jakub Jelinek
2021-07-26 19:28   ` Kwok Cheung Yeung
2021-07-26 19:56     ` Jakub Jelinek
2021-07-26 21:19       ` Kwok Cheung Yeung
2021-07-26 21:23         ` Jakub Jelinek
2021-07-26 21:27           ` Kwok Cheung Yeung
2022-01-28 16:33           ` [PATCH] openmp: Add warning when functions containing metadirectives with 'construct={target}' called directly Kwok Cheung Yeung
2021-12-10 17:27   ` [WIP, OpenMP] OpenMP metadirectives support Kwok Cheung Yeung
2021-12-10 17:29 ` [PATCH 0/7] openmp: " Kwok Cheung Yeung
2021-12-10 17:31   ` [PATCH 1/7] openmp: Add C support for parsing metadirectives Kwok Cheung Yeung
2022-02-18 21:09     ` Kwok Cheung Yeung [this message]
2022-02-18 21:26       ` [og11][committed] openmp: Improve handling of nested OpenMP metadirectives in C and C++ Kwok Cheung Yeung
2022-05-27 17:44     ` [PATCH 1/7] openmp: Add C support for parsing metadirectives Jakub Jelinek
2021-12-10 17:33   ` [PATCH 2/7] openmp: Add middle-end support for metadirectives Kwok Cheung Yeung
2022-05-30 10:54     ` Jakub Jelinek
2021-12-10 17:35   ` [PATCH 3/7] openmp: Add support for resolving metadirectives during parsing and Gimplification Kwok Cheung Yeung
2022-05-30 11:13     ` Jakub Jelinek
2021-12-10 17:36   ` [PATCH 4/7] openmp: Add support for streaming metadirectives and resolving them after LTO Kwok Cheung Yeung
2022-05-30 11:33     ` Jakub Jelinek
2021-12-10 17:37   ` [PATCH 5/7] openmp: Add C++ support for parsing metadirectives Kwok Cheung Yeung
2022-05-30 11:52     ` Jakub Jelinek
2021-12-10 17:39   ` [PATCH 6/7] openmp, fortran: Add Fortran " Kwok Cheung Yeung
2022-02-14 15:09     ` Kwok Cheung Yeung
2022-02-14 15:17     ` Kwok Cheung Yeung
2021-12-10 17:40   ` [PATCH 7/7] openmp: Add testcases for metadirectives Kwok Cheung Yeung
2022-05-27 13:42     ` Jakub Jelinek
2022-01-24 21:28   ` [PATCH] openmp: Metadirective patch fixes Kwok Cheung Yeung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0090d882-4c17-3c12-ab54-54670bf32ab0@codesourcery.com \
    --to=kcy@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).