https://gcc.gnu.org/g:fca04028d7075a6eaae350774a3916f14d4004ae

commit r15-5939-gfca04028d7075a6eaae350774a3916f14d4004ae
Author: Jakub Jelinek <ja...@redhat.com>
Date:   Thu Dec 5 12:57:44 2024 +0100

    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  <ja...@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:
---
 gcc/c-family/c-common.cc                           |   1 +
 gcc/c-family/c-common.h                            |   2 +-
 gcc/c/c-parser.cc                                  |  95 +++++++++++++++++
 gcc/ginclude/stdarg.h                              |   2 +-
 gcc/testsuite/gcc.dg/c23-stdarg-10.c               | 112 +++++++++++++++++++++
 gcc/testsuite/gcc.dg/c23-stdarg-11.c               |  11 ++
 gcc/testsuite/gcc.dg/c23-stdarg-4.c                |   2 +-
 gcc/testsuite/gcc.dg/c23-stdarg-6.c                |   2 +-
 gcc/testsuite/gcc.dg/c23-stdarg-7.c                |   2 +
 gcc/testsuite/gcc.dg/c23-stdarg-8.c                |   2 +
 gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1a.c |   2 +
 gcc/testsuite/gcc.dg/torture/c23-stdarg-split-1b.c |   2 +-
 12 files changed, 230 insertions(+), 5 deletions(-)

diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index d21f2f9909c4..048952311f2f 100644
--- 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 7834e0d19590..e2195aa54b8b 100644
--- 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 e939767a6e62..a45e9f93ccfb 100644
--- 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 d56fa81ee28c..f9d87dc62f2a 100644
--- 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
index 000000000000..3414b81dcad4
--- /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
index 000000000000..f659c374ee82
--- /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 cbbd9ef08ef1..12cd442dc36a 100644
--- 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 d5f08d0f5c2c..721c2c489cef 100644
--- 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 b05a9623cd73..8cc170f03515 100644
--- 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 81140312731f..6b724cf082f3 100644
--- 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 2dff79235b28..9aab941c6eb7 100644
--- 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 064da121ec24..6bfe00f02854 100644
--- 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 ();

Reply via email to