c: Diagnose unexpected va_start arguments in C23 [PR107980]

va_start macro was changed in C23 from the C17 va_start (va_list ap, parmN)
where parmN is the identifier of the last parameter into
va_start (va_list ap, ...) where arguments after ap aren't evaluated.
Late in the C23 development
"If any additional arguments expand to include unbalanced parentheses, or
a preprocessing token that does not convert to a token, the behavior is
undefined."
has been added, plus there is
"NOTE The macro allows additional arguments to be passed for va_start for
compatibility with older versions of the library only."
and
"Additional arguments beyond the first given to the va_start macro may be
expanded and used in unspecified contexts where they are unevaluated. For
example, an implementation diagnoses potentially erroneous input for an
invocation of va_start such as:"
...
va_start(vl, 1, 3.0, "12", xd); // diagnostic encouraged
...
"Simultaneously, va_start usage consistent with older revisions of this
document should not produce a diagnostic:"
...
void neigh (int last_arg, ...) {
va_list vl;
va_start(vl, last_arg); // no diagnostic

The following patch implements the recommended diagnostics.
Until now in C23 mode va_start(v, ...) was defined to
__builtin_va_start(v, 0)
and the extra arguments were silently ignored.
The following patch adds a new builtin in a form of a keyword which
parses the first argument, is silent about the __builtin_c23_va_start (ap)
form, for __builtin_c23_va_start (ap, identifier) looks the identifier up
and is silent if it is the last named parameter (except that it diagnoses
if it has register keyword), otherwise diagnoses it isn't the last one
but something else, and if there is just __builtin_c23_va_start (ap, )
or if __builtin_c23_va_start (ap, is followed by tokens other than
identifier followed by ), it skips over the tokens (with handling of
balanced ()s) until ) and diagnoses the extra tokens.
In all cases in a form of warnings.

2024-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR c/107980
gcc/
	* ginclude/stdarg.h (va_start): For C23+ change parameters from
	v, ... to just ... and define to __builtin_c23_va_start(__VA_ARGS__)
	rather than __builtin_va_start(v, 0).
gcc/c-family/
	* c-common.h (enum rid): Add RID_C23_VA_START.
	* c-common.cc (c_common_reswords): Add __builtin_c23_va_start.
gcc/c/
	* c-parser.cc (c_parser_postfix_expression): Handle RID_C23_VA_START.
gcc/testsuite/
	* gcc.dg/c23-stdarg-4.c: Expect extra warning.
	* gcc.dg/c23-stdarg-6.c: Likewise.
	* gcc.dg/c23-stdarg-7.c: Likewise.
	* gcc.dg/c23-stdarg-8.c: Likewise.
	* gcc.dg/c23-stdarg-10.c: New test.
	* gcc.dg/c23-stdarg-11.c: New test.
	* gcc.dg/torture/c23-stdarg-split-1a.c: Expect extra warning.
	* gcc.dg/torture/c23-stdarg-split-1b.c: Likewise.

diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index d21f2f9909c4f2f78bdb17fcfbf85713be210936..048952311f2ff950c92422208c773330973d7169 100644 (file)
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -459,6 +459,7 @@ const struct c_common_resword c_common_reswords[] =
   { "__builtin_tgmath", RID_BUILTIN_TGMATH, D_CONLY },
   { "__builtin_offsetof", RID_OFFSETOF, 0 },
   { "__builtin_types_compatible_p", RID_TYPES_COMPATIBLE_P, D_CONLY },
+  { "__builtin_c23_va_start", RID_C23_VA_START,        D_C23 },
   { "__builtin_va_arg",        RID_VA_ARG,     0 },
   { "__complex",       RID_COMPLEX,    0 },
   { "__complex__",     RID_COMPLEX,    0 },

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 7834e0d19590633be392201784522c5f8f168fec..e2195aa54b8b02007fc8601c94e48dc4897dd3f6 100644 (file)
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -105,7 +105,7 @@ enum rid
 
   /* C extensions */
   RID_ASM,       RID_TYPEOF,   RID_TYPEOF_UNQUAL, RID_ALIGNOF,  RID_ATTRIBUTE,
-  RID_VA_ARG,
+  RID_C23_VA_START, RID_VA_ARG,
   RID_EXTENSION, RID_IMAGPART, RID_REALPART, RID_LABEL,    RID_CHOOSE_EXPR,
   RID_TYPES_COMPATIBLE_P,      RID_BUILTIN_COMPLEX,       RID_BUILTIN_SHUFFLE,
   RID_BUILTIN_SHUFFLEVECTOR,   RID_BUILTIN_CONVERTVECTOR,  RID_BUILTIN_TGMATH,

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index e939767a6e62144c6c5bf42d065d545cf21341fb..a45e9f93ccfbbc221b6314c2dd524a80fd686395 100644 (file)
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -11478,6 +11478,101 @@ c_parser_postfix_expression (c_parser *parser)
              }
          }
          break;
+       case RID_C23_VA_START:
+         {
+           c_parser_consume_token (parser);
+           matching_parens parens;
+           if (!parens.require_open (parser))
+             {
+               expr.set_error ();
+               break;
+             }
+           e1 = c_parser_expr_no_commas (parser, NULL);
+           e1 = convert_lvalue_to_rvalue (e1.get_location (), e1, true, true);
+           if (!c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
+             {
+               location_t cloc = c_parser_peek_token (parser)->location;
+               if (!c_parser_require (parser, CPP_COMMA, "expected %<,%>"))
+                 {
+                   c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
+                   expr.set_error ();
+                   break;
+                 }
+               if (c_parser_next_token_is (parser, CPP_NAME)
+                   && c_parser_peek_token (parser)->id_kind == C_ID_ID
+                   && (c_parser_peek_2nd_token (parser)->type
+                       == CPP_CLOSE_PAREN))
+                 {
+                   tree name = c_parser_peek_token (parser)->value;
+                   location_t nloc = c_parser_peek_token (parser)->location;
+                   tree decl = lookup_name (name);
+                   tree last_parm
+                     = tree_last (DECL_ARGUMENTS (current_function_decl));
+                   if (!last_parm || decl != last_parm)
+                     warning_at (nloc, OPT_Wvarargs,
+                                 "optional second parameter of %<va_start%> "
+                                 "not last named argument");
+                   else if (DECL_REGISTER (decl))
+                     warning_at (nloc, OPT_Wvarargs,
+                                 "undefined behavior when second parameter "
+                                 "of %<va_start%> is declared with "
+                                 "%<register%> storage");
+                   c_parser_consume_token (parser);
+                 }
+               else
+                 {
+                   unsigned nesting_depth = 0;
+                   location_t sloc = c_parser_peek_token (parser)->location;
+                   location_t eloc = sloc;
+
+                   /* For va_start (ap,) the ) comes from stdarg.h.
+                      Use location of , in that case, otherwise without
+                      -Wsystem-headers nothing is reported.  After all,
+                      the problematic token is the comma in that case.  */
+                   if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
+                     sloc = eloc = cloc;
+                   while (true)
+                     {
+                       c_token *token = c_parser_peek_token (parser);
+                       if (token->type == CPP_CLOSE_PAREN && !nesting_depth)
+                         break;
+
+                       if (token->type == CPP_EOF)
+                         break;
+                       if (token->type == CPP_OPEN_PAREN)
+                         ++nesting_depth;
+                       else if (token->type == CPP_CLOSE_PAREN)
+                         --nesting_depth;
+                       eloc = token->location;
+                       c_parser_consume_token (parser);
+                     }
+                   if (sloc != eloc)
+                     sloc = make_location (sloc, sloc, eloc);
+                   warning_at (sloc, OPT_Wvarargs,
+                               "%<va_start%> macro used with additional "
+                               "arguments other than identifier of the "
+                               "last named argument");
+                 }
+             }
+           parens.skip_until_found_close (parser);
+           tree fndecl = builtin_decl_explicit (BUILT_IN_VA_START);
+           vec<tree, va_gc> *params;
+           vec_alloc (params, 2);
+           params->quick_push (e1.value);
+           params->quick_push (integer_zero_node);
+           auto_vec<location_t> arg_loc (2);
+           arg_loc.quick_push (e1.get_location ());
+           arg_loc.quick_push (UNKNOWN_LOCATION);
+           expr.value = c_build_function_call_vec (loc, arg_loc, fndecl,
+                                                   params, NULL);
+           set_c_expr_source_range (&expr, loc,
+                                    parser->tokens_buf[0].get_finish ());
+           expr.m_decimal = 0;
+           expr.original_code = ERROR_MARK;
+           expr.original_type = NULL;
+           release_tree_vector (params);
+           break;
+         }
        case RID_OFFSETOF:
          {
            c_parser_consume_token (parser);

diff --git a/gcc/ginclude/stdarg.h b/gcc/ginclude/stdarg.h
index d56fa81ee28caf8231b46e909bc85169dfd2c63b..f9d87dc62f2ad12ecffcddf46703a6dbfbcaf3e5 100644 (file)
--- a/gcc/ginclude/stdarg.h
+++ b/gcc/ginclude/stdarg.h
@@ -45,7 +45,7 @@ typedef __builtin_va_list __gnuc_va_list;
 #ifdef _STDARG_H
 
 #if defined __STDC_VERSION__ && __STDC_VERSION__ > 201710L
-#define va_start(v, ...)       __builtin_va_start(v, 0)
+#define va_start(...) __builtin_c23_va_start(__VA_ARGS__)
 #else
 #define va_start(v,l)  __builtin_va_start(v,l)
 #endif

diff --git a/gcc/testsuite/gcc.dg/c23-stdarg-10.c b/gcc/testsuite/gcc.dg/c23-stdarg-10.c
new file mode 100644 (file)
index 0000000..3414b81
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c23-stdarg-10.c
@@ -0,0 +1,112 @@
+/* PR c/107980 */
+/* { dg-do compile } */
+/* { dg-options "-std=c23 -pedantic-errors" } */
+
+#include <stdarg.h>
+
+int i;
+
+void
+f0 (...)
+{
+  va_list ap;
+  va_start (ap);
+  va_end (ap);
+}
+
+void
+f1 (...)
+{
+  va_list ap;
+  va_start (ap, i);                            /* { dg-warning "optional second parameter of 'va_start' not last named argument" } */
+  va_end (ap);
+}
+
+void
+f2 (...)
+{
+  int j = 0;
+  va_list ap;
+  va_start (ap, j);                            /* { dg-warning "optional second parameter of 'va_start' not last named argument" } */
+  va_end (ap);
+}
+
+void
+f3 (int k, int l, ...)
+{
+  va_list ap;
+  va_start (ap, k);                            /* { dg-warning "optional second parameter of 'va_start' not last named argument" } */
+  va_end (ap);
+}
+
+void
+f4 (int k, int l, ...)
+{
+  va_list ap;
+  va_start (ap, l);
+  va_end (ap);
+}
+
+void
+f5 (int k, int l, ...)
+{
+  va_list ap;
+  va_start (ap, (int) l);                      /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
+  va_end (ap);
+}
+
+void
+f6 (int k, int l, ...)
+{
+  va_list ap;
+  va_start (ap, l + 0);                                /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
+  va_end (ap);
+}
+
+void
+f7 (int k, int l, ...)
+{
+  va_list ap;
+  va_start (ap, ()()(), [][][], {}{}{}, *+-/1({[_*_]})%&&!?!?);        /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
+  va_end (ap);
+}
+
+void
+f8 (...)
+{
+  va_list ap;
+  va_start (ap,);                              /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
+  va_end (ap);
+}
+
+void
+f9 (int k, int l, ...)
+{
+  va_list ap;
+  va_start (ap, k+l+****2);                    /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
+  va_end (ap);
+}
+
+void
+f10 (register int m, ...)
+{
+  va_list ap;
+  va_start (ap, m);                            /* { dg-warning "undefined behavior when second parameter of 'va_start' is declared with 'register' storage" } */
+  va_end (ap);
+}
+
+void
+f11 (int k, int l, ...)
+{
+  va_list ap;
+  va_start (ap, ()()()[[[}}});                 /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
+  va_end (ap);
+}
+
+void
+f12 (int k, int l, ...)
+{
+  va_list ap;
+  va_start (ap, ]]]]]]{{{{{{);                 /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
+  va_end (ap);
+}

diff --git a/gcc/testsuite/gcc.dg/c23-stdarg-11.c b/gcc/testsuite/gcc.dg/c23-stdarg-11.c
new file mode 100644 (file)
index 0000000..f659c37
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c23-stdarg-11.c
@@ -0,0 +1,11 @@
+/* PR c/107980 */
+/* { dg-do compile } */
+/* { dg-options "-std=c23 -pedantic-errors" } */
+
+#include <stdarg.h>
+
+void
+f (...)
+{
+  va_start (); /* { dg-error "expected expression before '\\\)' token" } */
+}

diff --git a/gcc/testsuite/gcc.dg/c23-stdarg-4.c b/gcc/testsuite/gcc.dg/c23-stdarg-4.c
index cbbd9ef08ef1807aca5ce452b0966d56e33615a2..12cd442dc36aee058dbc0b2b396210e44316a3ef 100644 (file)
--- a/gcc/testsuite/gcc.dg/c23-stdarg-4.c
+++ b/gcc/testsuite/gcc.dg/c23-stdarg-4.c
@@ -25,7 +25,7 @@ void
 g (...)
 {
   va_list ap;
-  va_start (ap, random ! ignored, ignored ** text);
+  va_start (ap, random ! ignored, ignored ** text);    /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
   for (int i = 0; i < 10; i++)
     if (va_arg (ap, double) != i)
       abort ();

diff --git a/gcc/testsuite/gcc.dg/c23-stdarg-6.c b/gcc/testsuite/gcc.dg/c23-stdarg-6.c
index d5f08d0f5c2cd3c6628aed5b277c9d5ddc9802b8..721c2c489cef2dea74a684b4373f36f7333ae727 100644 (file)
--- a/gcc/testsuite/gcc.dg/c23-stdarg-6.c
+++ b/gcc/testsuite/gcc.dg/c23-stdarg-6.c
@@ -29,7 +29,7 @@ struct s
 g (...)
 {
   va_list ap;
-  va_start (ap, random ! ignored, ignored ** text);
+  va_start (ap, random ! ignored, ignored ** text);    /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
   for (int i = 0; i < 10; i++)
     if (va_arg (ap, double) != i)
       abort ();

diff --git a/gcc/testsuite/gcc.dg/c23-stdarg-7.c b/gcc/testsuite/gcc.dg/c23-stdarg-7.c
index b05a9623cd738430a9d2b80e0d6cf400abaffea8..8cc170f0351598470918f156b834bccaaad2158b 100644 (file)
--- a/gcc/testsuite/gcc.dg/c23-stdarg-7.c
+++ b/gcc/testsuite/gcc.dg/c23-stdarg-7.c
@@ -4,3 +4,5 @@
 /* { dg-options "-O2 -std=c23 -pedantic-errors" } */
 
 #include "c23-stdarg-4.c"
+
+/* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" "" { target *-*-* } 0 } */

diff --git a/gcc/testsuite/gcc.dg/c23-stdarg-8.c b/gcc/testsuite/gcc.dg/c23-stdarg-8.c
index 81140312731fc3ce2dd3b834194ba5d800811e5e..6b724cf082f3978b45129dd4b4376f1745fc71a9 100644 (file)
--- a/gcc/testsuite/gcc.dg/c23-stdarg-8.c
+++ b/gcc/testsuite/gcc.dg/c23-stdarg-8.c
@@ -4,3 +4,5 @@
 /* { dg-options "-O2 -std=c23 -pedantic-errors" } */
 
 #include "c23-stdarg-6.c"
+
+/* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" "" { target *-*-* } 0 } */

diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c
index 2dff79235b28506b6534a3c24c94612beb80205d..9aab941c6eb7ae421e27166c46e1fed1c10437be 100644 (file)
--- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c
+++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c
@@ -35,3 +35,5 @@ main ()
   h7 ((struct s) {}, 0.0, 1, 2.0, 3, 4.0, 5, 6.0, 7, 8.0, 9);
   exit (0);
 }
+
+/* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" "" { target *-*-* } 0 } */

diff --git a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c
index 064da121ec242ff2af1e6c93a73b103a8ddd853d..6bfe00f02854a2e1843f10000fe75043fbd38554 100644 (file)
--- a/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c
+++ b/gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c
@@ -25,7 +25,7 @@ void
 g (...)
 {
   va_list ap;
-  va_start (ap, random ! ignored, ignored ** text);
+  va_start (ap, random ! ignored, ignored ** text);    /* { dg-warning "'va_start' macro used with additional arguments other than identifier of the last named argument" } */
   for (int i = 0; i < 10; i++)
     if (va_arg (ap, double) != i)
       abort ();