[C23] Add __builtin_c23_va_start

clang/docs/ReleaseNotes.rst

@@ -123,6 +123,10 @@ C2y Feature Support
 
 C23 Feature Support
 ^^^^^^^^^^^^^^^^^^^
+- Added ``__builtin_c23_va_start()`` for compatibility with GCC and to enable
+  better diagnostic behavior for the ``va_start()`` macro in C23 and later.
+  This also updates the definition of ``va_start()`` in ``<stdarg.h>`` to use
+  the new builtin. Fixes #GH124031.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------

clang/include/clang/Basic/Builtins.h

@@ -44,6 +44,7 @@ enum LanguageID : uint16_t {
   OCL_DSE = 0x400,           // builtin requires OpenCL device side enqueue.
   ALL_OCL_LANGUAGES = 0x800, // builtin for OCL languages.
   HLSL_LANG = 0x1000,        // builtin requires HLSL.
+  C23_LANG = 0x2000,         // builtin requires C23 or later.
   ALL_LANGUAGES = C_LANG | CXX_LANG | OBJC_LANG, // builtin for all languages.
   ALL_GNU_LANGUAGES = ALL_LANGUAGES | GNU_LANG,  // builtin requires GNU mode.
   ALL_MS_LANGUAGES = ALL_LANGUAGES | MS_LANG     // builtin requires MS mode.

clang/include/clang/Basic/Builtins.td

@@ -833,6 +833,12 @@ def BuiltinVaStart : Builtin {
   let Prototype = "void(__builtin_va_list_ref, ...)";
 }
 
+def BuiltinC32VaStart : LangBuiltin<"C23_LANG"> {
+  let Spellings = ["__builtin_c23_va_start"];
+  let Attributes = [NoThrow, CustomTypeChecking];
+  let Prototype = "void(__builtin_va_list_ref, ...)";
+}
+
 def BuiltinStdargStart : Builtin {
   let Spellings = ["__builtin_stdarg_start"];
   let Attributes = [NoThrow, CustomTypeChecking];

clang/include/clang/Sema/Sema.h

@@ -2499,9 +2499,9 @@ class Sema final : public SemaBase {
 
   void checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, CallExpr *TheCall);
 
-  /// Check the arguments to '__builtin_va_start' or '__builtin_ms_va_start'
-  /// for validity.  Emit an error and return true on failure; return false
-  /// on success.
+  /// Check the arguments to '__builtin_va_start', '__builtin_ms_va_start',
+  /// or '__builtin_c23_va_start' for validity. Emit an error and return true
+  /// on failure; return false on success.
   bool BuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall);
   bool BuiltinVAStartARMMicrosoft(CallExpr *Call);
 

clang/lib/Basic/Builtins.cpp

@@ -191,6 +191,9 @@ static bool builtinIsSupported(const llvm::StringTable &Strings,
   /* consteval Unsupported */
   if (!LangOpts.CPlusPlus20 && strchr(AttributesStr.data(), 'G') != nullptr)
     return false;
+  /* C23 unsupported */
+  if (!LangOpts.C23 && BuiltinInfo.Langs == C23_LANG)
+    return false;
   return true;
 }
 

clang/lib/Headers/__stdarg_va_arg.h

@@ -10,8 +10,8 @@
 #ifndef va_arg
 
 #if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
-/* C23 does not require the second parameter for va_start. */
-#define va_start(ap, ...) __builtin_va_start(ap, 0)
+/* C23 uses a special builtin. */
+#define va_start(...) __builtin_c23_va_start(__VA_ARGS__)
 #else
 /* Versions before C23 do require the second parameter. */
 #define va_start(ap, param) __builtin_va_start(ap, param)

clang/lib/Sema/SemaChecking.cpp

@@ -2161,6 +2161,7 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
   case Builtin::BI__builtin_ms_va_start:
   case Builtin::BI__builtin_stdarg_start:
   case Builtin::BI__builtin_va_start:
+  case Builtin::BI__builtin_c23_va_start:
     if (BuiltinVAStart(BuiltinID, TheCall))
       return ExprError();
     break;
@@ -4844,15 +4845,27 @@ static bool checkVAStartIsInVariadicFunction(Sema &S, Expr *Fn,
 
 bool Sema::BuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall) {
   Expr *Fn = TheCall->getCallee();
-
   if (checkVAStartABI(*this, BuiltinID, Fn))
     return true;
 
-  // In C23 mode, va_start only needs one argument. However, the builtin still
-  // requires two arguments (which matches the behavior of the GCC builtin),
-  // <stdarg.h> passes `0` as the second argument in C23 mode.
-  if (checkArgCount(TheCall, 2))
-    return true;
+  if (BuiltinID == Builtin::BI__builtin_c23_va_start) {
+    // This builtin requires one argument (the va_list), allows two arguments,
+    // but diagnoses more than two arguments. e.g.,
+    //   __builtin_c23_va_start(); // error
+    //   __builtin_c23_va_start(list); // ok
+    //   __builtin_c23_va_start(list, param); // ok
+    //   __builtin_c23_va_start(list, anything, anything); // error
+    // This differs from the GCC behavior in that they accept the last case
+    // with a warning, but it doesn't seem like a useful behavior to allow.
+    if (checkArgCountRange(TheCall, 1, 2))
+      return true;
+  } else {
+    // In C23 mode, va_start only needs one argument. However, the builtin still
+    // requires two arguments (which matches the behavior of the GCC builtin),
+    // <stdarg.h> passes `0` as the second argument in C23 mode.
+    if (checkArgCount(TheCall, 2))
+      return true;
+  }
 
   // Type-check the first argument normally.
   if (checkBuiltinArgument(*this, TheCall, 0))
@@ -4864,14 +4877,22 @@ bool Sema::BuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall) {
     return true;
 
   // Verify that the second argument to the builtin is the last argument of the
-  // current function or method. In C23 mode, if the second argument is an
-  // integer constant expression with value 0, then we don't bother with this
-  // check.
+  // current function or method. In C23 mode and the call is not to
+  // __builtin_c23_va_start, if the second argument is an integer constant
+  // expression with value 0, then we don't bother with this check. For
+  // __builtin_c23_va_start, we only perform the check for the second argument
+  // being the last argument to the current function if there is a second
+  // argument present.
+  if (BuiltinID == Builtin::BI__builtin_c23_va_start &&
+      TheCall->getNumArgs() < 2)
+    return false;
+
   bool SecondArgIsLastNamedArgument = false;
   const Expr *Arg = TheCall->getArg(1)->IgnoreParenCasts();
   if (std::optional<llvm::APSInt> Val =
           TheCall->getArg(1)->getIntegerConstantExpr(Context);
-      Val && LangOpts.C23 && *Val == 0)
+      Val && LangOpts.C23 && *Val == 0 &&
+      BuiltinID != Builtin::BI__builtin_c23_va_start)
     return false;
 
   // These are valid if SecondArgIsLastNamedArgument is false after the next

clang/test/C/C23/n2975.c

@@ -6,27 +6,20 @@
 
 #include <stdarg.h>
 
-#define DERP this is an error
-
 void func(...) { // expected-warning {{'...' as the only parameter of a function is incompatible with C standards before C23}}
   // Show that va_start doesn't require the second argument in C23 mode.
   va_list list;
-  va_start(list); // expected-warning {{passing no argument for the '...' parameter of a variadic macro is incompatible with C standards before C23}} expected-note@* {{macro 'va_start' defined here}}
-  va_end(list);
-
-  // Show that va_start doesn't expand or evaluate the second argument.
-  va_start(list, DERP);
+  va_start(list);
   va_end(list);
 
-  // FIXME: it would be kinder to diagnose this instead of silently accepting it.
-  va_start(list, 1, 2);
+  va_start(list, 1, 2); // expected-error {{too many arguments to function call, expected at most 2, have 3}}
   va_end(list);
 
   // We didn't change the behavior of __builtin_va_start (and neither did GCC).
   __builtin_va_start(list); // expected-error {{too few arguments to function call, expected 2, have 1}}
 
   // Verify that the return type of a call to va_start is 'void'.
-  _Static_assert(__builtin_types_compatible_p(__typeof__(va_start(list)), void), ""); // expected-warning {{passing no argument for the '...' parameter of a variadic macro is incompatible with C standards before C23}} expected-note@* {{macro 'va_start' defined here}}
+  _Static_assert(__builtin_types_compatible_p(__typeof__(va_start(list)), void), "");
   _Static_assert(__builtin_types_compatible_p(__typeof__(__builtin_va_start(list, 0)), void), "");
 }
 
@@ -37,13 +30,7 @@ typedef void (*fp)(...); // expected-warning {{'...' as the only parameter of a
 // Passing something other than the argument before the ... is still not valid.
 void diag(int a, int b, ...) {
   va_list list;
-  // FIXME: the call to va_start should also diagnose the same way as the call
-  // to __builtin_va_start. However, because va_start is not allowed to expand
-  // or evaluate the second argument, we can't pass it along to
-  // __builtin_va_start to get that diagnostic. So in C17 and earlier, we will
-  // diagnose this use through the macro, but in C23 and later we've lost the
-  // diagnostic entirely. GCC has the same issue currently.
-  va_start(list, a);
+  va_start(list, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
   // However, the builtin itself is under no such constraints regarding
   // expanding or evaluating the second argument, so it can still diagnose.
   __builtin_va_start(list, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}}

clang/test/Sema/c23-varargs.c

@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -std=c23 -fsyntax-only -ffreestanding -verify=expected,both %s -triple i386-pc-unknown
+// RUN: %clang_cc1 -std=c23 -fsyntax-only -ffreestanding -verify=expected,both %s -triple x86_64-apple-darwin9
+// RUN: %clang_cc1 -std=c23 -fsyntax-only -ffreestanding -fms-compatibility -verify=expected,both %s -triple x86_64-pc-win32
+// RUN: %clang_cc1 -std=c17 -fsyntax-only -ffreestanding -verify=both,pre-c23 %s
+
+void foo(int x, int y, ...) {
+  __builtin_va_list list;
+  __builtin_c23_va_start();           // pre-c23-error {{use of unknown builtin '__builtin_c23_va_start'}} \
+                                         expected-error{{too few arguments to function call, expected 1, have 0}}
+  // Note, the unknown builtin diagnostic is only issued once per function,
+  // which is why the rest of the lines do not get the same diagonstic.
+  __builtin_c23_va_start(list);       // ok
+  __builtin_c23_va_start(list, 0);    // expected-warning {{second argument to 'va_start' is not the last named parameter}}
+  __builtin_c23_va_start(list, x);    // expected-warning {{second argument to 'va_start' is not the last named parameter}}
+  __builtin_c23_va_start(list, y);    // ok
+  __builtin_c23_va_start(list, 0, 1); // expected-error {{too many arguments to function call, expected at most 2, have 3}}
+  __builtin_c23_va_start(list, y, y); // expected-error {{too many arguments to function call, expected at most 2, have 3}}
+}
+
+// Test the same thing as above, only with the macro from stdarg.h. This will
+// not have the unknown builtin diagnostics, but will have different
+// diagnostics between C23 and earlier modes.
+#include <stdarg.h>
+void bar(int x, int y, ...) {
+  // FIXME: the "use of undeclared identifier 'va_start'" diagnostics is an odd
+  // follow-on diagnostic that should be silenced.
+  va_list list;
+  va_start();           // pre-c23-error {{too few arguments provided to function-like macro invocation}} \
+                           pre-c23-error {{use of undeclared identifier 'va_start'}} \
+                           expected-error{{too few arguments to function call, expected 1, have 0}}
+  va_start(list);       // pre-c23-error {{too few arguments provided to function-like macro invocation}} \
+                           pre-c23-error {{use of undeclared identifier 'va_start'}}
+  va_start(list, 0);    // both-warning {{second argument to 'va_start' is not the last named parameter}}
+  va_start(list, x);    // both-warning {{second argument to 'va_start' is not the last named parameter}}
+  va_start(list, y);    // ok
+  va_start(list, 0, 1); // pre-c23-error {{too many arguments provided to function-like macro invocation}} \
+                           pre-c23-error {{use of undeclared identifier 'va_start'}} \
+                           expected-error {{too many arguments to function call, expected at most 2, have 3}}
+  va_start(list, y, y); // pre-c23-error {{too many arguments provided to function-like macro invocation}} \
+                           pre-c23-error {{use of undeclared identifier 'va_start'}} \
+                           expected-error {{too many arguments to function call, expected at most 2, have 3}}	
+  // pre-c23-note@__stdarg_va_arg.h:* 4 {{macro 'va_start' defined here}}
+}