[C23] Add __builtin_c23_va_start

clang/docs/LanguageExtensions.rst

@@ -4283,6 +4283,17 @@ the ellipsis. This function initializes the given ``__builtin_va_list`` object.
 It is undefined behavior to call this function on an already initialized
 ``__builtin_va_list`` object.
 
+* ``void __builtin_c23_va_start(__builtin_va_list list, ...)``
+
+A builtin function for the target-specific ``va_start`` function-like macro,
+available only in C23 and later. The builtin accepts zero or one argument for
+the ellipsis (``...``). If such an argument is provided, it should be the name
+of the parameter preceeding the ellipsis, which is used for compatibility with
+C versions before C23. It is an error to provide two or more variadic arguments.
+This function initializes the given ``__builtin_va_list`` object. It is
+undefined behavior to call this function on an already initialized
+``__builtin_va_list`` object.
+
 * ``void __builtin_va_end(__builtin_va_list list)``
 
 A builtin function for the target-specific ``va_end`` function-like macro. This

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 BuiltinC23VaStart : 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/Basic/DiagnosticSemaKinds.td

@@ -10619,6 +10619,9 @@ def warn_second_arg_of_va_start_not_last_non_variadic_param : Warning<
 def warn_c17_compat_ellipsis_only_parameter : Warning<
   "'...' as the only parameter of a function is incompatible with C standards "
   "before C23">, DefaultIgnore, InGroup<CPre23Compat>;
+def warn_c17_compat_va_start_one_arg : Warning<
+  "passing only one argument to 'va_start' is incompatible with C standards "
+  "before C23">, DefaultIgnore, InGroup<CPre23Compat>;
 def warn_va_start_type_is_undefined : Warning<
   "passing %select{an object that undergoes default argument promotion|"
   "an object of reference type|a parameter declared with the 'register' "

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/CodeGen/CGBuiltin.cpp

@@ -3551,6 +3551,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   case Builtin::BI__builtin_stdarg_start:
   case Builtin::BI__builtin_va_start:
   case Builtin::BI__va_start:
+  case Builtin::BI__builtin_c23_va_start:
   case Builtin::BI__builtin_va_end:
     EmitVAStartEnd(BuiltinID == Builtin::BI__va_start
                        ? EmitScalarExpr(E->getArg(0))

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))
@@ -4863,23 +4876,34 @@ bool Sema::BuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall) {
   if (checkVAStartIsInVariadicFunction(*this, Fn, &LastParam))
     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.
-  bool SecondArgIsLastNamedArgument = false;
+  // Verify that the second argument to the builtin is the last non-variadic
+  // argument of the current function or method. In C23 mode, if the call is
+  // not to __builtin_c23_va_start, and 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) {
+    Diag(TheCall->getExprLoc(), diag::warn_c17_compat_va_start_one_arg);
+    return 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) {
+    Diag(TheCall->getExprLoc(), diag::warn_c17_compat_va_start_one_arg);
     return false;
+  }
 
   // These are valid if SecondArgIsLastNamedArgument is false after the next
   // block.
   QualType Type;
   SourceLocation ParamLoc;
   bool IsCRegister = false;
-
+  bool SecondArgIsLastNamedArgument = false;
   if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Arg)) {
     if (const ParmVarDecl *PV = dyn_cast<ParmVarDecl>(DR->getDecl())) {
       SecondArgIsLastNamedArgument = PV == LastParam;

clang/test/C/C23/n2975.c

@@ -6,28 +6,21 @@
 
 #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); // expected-warning {{passing only one argument to 'va_start' is incompatible with C standards before C23}}
   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__(__builtin_va_start(list, 0)), void), "");
+  _Static_assert(__builtin_types_compatible_p(__typeof__(va_start(list)), void), ""); // expected-warning {{passing only one argument to 'va_start' is incompatible with C standards before C23}}
+  _Static_assert(__builtin_types_compatible_p(__typeof__(__builtin_va_start(list, 0)), void), ""); // expected-warning {{passing only one argument to 'va_start' is incompatible with C standards before C23}}
 }
 
 // Show that function pointer types also don't need an argument before the
@@ -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 non-variadic 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 non-variadic parameter}}

clang/test/CodeGen/varargs.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-unknown -std=c2y -emit-llvm -o - %s | FileCheck %s
 
 // PR6433 - Don't crash on va_arg(typedef).
 typedef double gdouble;
@@ -21,3 +21,14 @@ void vla(int n, ...)
   void *p;
   p = __builtin_va_arg(ap, typeof (int (*)[++n])); // CHECK: add nsw i32 {{.*}}, 1
 }
+
+// Ensure that __builtin_va_start(list, 0) and __builtin_c23_va_start(list)
+// have the same codegen.
+void noargs(...) {
+  __builtin_va_list list;
+  // CHECK:   %list = alloca ptr
+  __builtin_va_start(list, 0);
+  // CHECK-NEXT: call void @llvm.va_start.p0(ptr %list)  
+  __builtin_c23_va_start(list);
+  // CHECK-NEXT: call void @llvm.va_start.p0(ptr %list)
+}

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 non-variadic parameter}}
+  __builtin_c23_va_start(list, x);    // expected-warning {{second argument to 'va_start' is not the last non-variadic 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 non-variadic parameter}}
+  va_start(list, x);    // both-warning {{second argument to 'va_start' is not the last non-variadic 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}}
+}