diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index 387251804fa76..cc12ff5bad353 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/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 diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 6456acfcc2ada..b5966f9fa1c63 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/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 ```` to use + the new builtin. Fixes #GH124031. Non-comprehensive list of changes in this release ------------------------------------------------- diff --git a/clang/include/clang/Basic/Builtins.h b/clang/include/clang/Basic/Builtins.h index 12c250afb4c61..3a5e31de2bc50 100644 --- a/clang/include/clang/Basic/Builtins.h +++ b/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. diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td index 2fbdfaea57ccd..72a5e495c4059 100644 --- a/clang/include/clang/Basic/Builtins.td +++ b/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]; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 135d3bd161356..86c9c955c1c78 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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; +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; 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' " diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b6bf1e468ef2c..414f3c6eff038 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/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); diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp index e7829a461bbc5..885abdc152e3a 100644 --- a/clang/lib/Basic/Builtins.cpp +++ b/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; } diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index a5ed2595bad4d..c126f88b9e3a5 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/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)) diff --git a/clang/lib/Headers/__stdarg_va_arg.h b/clang/lib/Headers/__stdarg_va_arg.h index 89bd2f65d3bea..ebdb6f9d4b1eb 100644 --- a/clang/lib/Headers/__stdarg_va_arg.h +++ b/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) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index e99e30d75df94..500e7be84f9fa 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/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), - // 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), + // 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 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(Arg)) { if (const ParmVarDecl *PV = dyn_cast(DR->getDecl())) { SecondArgIsLastNamedArgument = PV == LastParam; diff --git a/clang/test/C/C23/n2975.c b/clang/test/C/C23/n2975.c index 338f987f6a65d..6e7c936855e51 100644 --- a/clang/test/C/C23/n2975.c +++ b/clang/test/C/C23/n2975.c @@ -6,28 +6,21 @@ #include -#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}} diff --git a/clang/test/CodeGen/varargs.c b/clang/test/CodeGen/varargs.c index 625399b87f7ad..71ca88c0d635d 100644 --- a/clang/test/CodeGen/varargs.c +++ b/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) +} diff --git a/clang/test/Sema/c23-varargs.c b/clang/test/Sema/c23-varargs.c new file mode 100644 index 0000000000000..1700d5a429160 --- /dev/null +++ b/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 +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}} +}