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.c (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.c (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.

diff --git a/gcc/c/ChangeLog.omp b/gcc/c/ChangeLog.omp
index 926d7cf3ffb70be73cb2f947f02b52dd6747c75b..c1f9e860d16bbd938b16b3c5062b97015422f07a 100644 (file)
--- a/gcc/c/ChangeLog.omp
+++ b/gcc/c/ChangeLog.omp
@@ -1,3 +1,14 @@
+2022-02-18  Kwok Cheung Yeung  <kcy@codesourcery.com>
+
+       * c-parser.c (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.
+
 2022-01-25  Kwok Cheung Yeung  <kcy@codesourcery.com>
 
        * c-parser.c (omp_target_device_selectors): New.

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 8b79ad3ba769512f972e2aabcb8d07ec55d55c71..bb463c6d8afc2bfe755448488a67f3062e238755 100644 (file)
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -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 *);
@@ -12489,6 +12488,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);
 
@@ -22943,7 +22946,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:
@@ -22981,10 +22984,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;
@@ -22996,13 +22997,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)
@@ -23210,6 +23212,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
@@ -23219,11 +23222,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;
@@ -23246,10 +23249,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;
@@ -23261,15 +23268,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.  */
@@ -23345,11 +23349,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/ChangeLog.omp b/gcc/cp/ChangeLog.omp
index 5acee473f5a7073ea63f4d3fe10241db3eccd833..03b72006567bb00ad4049fac0c1c44651e5d5a93 100644 (file)
--- a/gcc/cp/ChangeLog.omp
+++ b/gcc/cp/ChangeLog.omp
@@ -1,3 +1,14 @@
+2022-02-18  Kwok Cheung Yeung  <kcy@codesourcery.com>
+
+       * parser.c (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.
+
 2022-01-25  Kwok Cheung Yeung  <kcy@codesourcery.com>
 
        * parser.c (omp_target_device_selectors): New.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 30be7ddcb041a3631f818a123ea6d0d989f7097f..c910aafd50841c6ead8c60ab617b0e8fc79733eb 100644 (file)
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -45517,7 +45517,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,
@@ -45553,7 +45553,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:
@@ -45588,9 +45588,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;
@@ -45601,15 +45600,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))
     {
@@ -45828,6 +45826,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);
 
@@ -45862,8 +45861,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);
@@ -45875,8 +45876,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.  */
@@ -45884,7 +45884,6 @@ fail:
 
   /* Skip the metadirective body.  */
   cp_parser_skip_to_end_of_block_or_statement (parser);
-  return error_mark_node;
 }
 
 
@@ -47135,11 +47134,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,
@@ -47790,7 +47784,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:
@@ -47822,6 +47815,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/ChangeLog.omp b/gcc/testsuite/ChangeLog.omp
index 07bdb9d0645da33a7160bb28251dad2c1cabe9be..1bfbf8c9312300377ffb0d986297a11f2fccf6b8 100644 (file)
--- a/gcc/testsuite/ChangeLog.omp
+++ b/gcc/testsuite/ChangeLog.omp
@@ -1,3 +1,8 @@
+2022-02-18  Kwok Cheung Yeung  <kcy@codesourcery.com>
+
+       * c-c++-common/gomp/metadirective-1.c (f): Add test for
+       improperly nested metadirectives.
+
 2022-02-11  Kwok Cheung Yeung  <kcy@codesourcery.com>
 
        * gfortran.dg/gomp/metadirective-2.f90 (main): Test empty

diff --git a/gcc/testsuite/c-c++-common/gomp/metadirective-1.c b/gcc/testsuite/c-c++-common/gomp/metadirective-1.c
index 72cf0abbbd79c9011adfea5b2040ae03bb683710..543063a3324f52dfb8bee3c586a088e9b3fc0f28 100644 (file)
--- 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];
 }