Index: clang/include/clang/AST/Expr.h =================================================================== --- clang/include/clang/AST/Expr.h +++ clang/include/clang/AST/Expr.h @@ -2656,6 +2656,9 @@ FD->getIdentifier() && FD->getIdentifier()->isStr("move"); } + /// If this call's last argument is __builtin_va_arg_pack(). + bool isInlineVAArgPackStrawmanCall() const; + static bool classof(const Stmt *T) { return T->getStmtClass() >= firstCallExprConstant && T->getStmtClass() <= lastCallExprConstant; Index: clang/include/clang/Basic/Builtins.def =================================================================== --- clang/include/clang/Basic/Builtins.def +++ clang/include/clang/Basic/Builtins.def @@ -446,6 +446,8 @@ BUILTIN(__builtin_va_start, "vA.", "nt") BUILTIN(__builtin_va_end, "vA", "n") BUILTIN(__builtin_va_copy, "vAA", "n") +BUILTIN(__builtin_va_arg_pack, "i", "n") +BUILTIN(__builtin_va_arg_pack_len, "i", "n") BUILTIN(__builtin_stdarg_start, "vA.", "n") BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc") BUILTIN(__builtin_bcmp, "iv*v*z", "Fn") Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8179,12 +8179,17 @@ def note_empty_body_on_separate_line : Note< "put the semicolon on a separate line to silence this warning">; -def err_va_start_captured_stmt : Error< - "'va_start' cannot be used in a captured statement">; -def err_va_start_outside_function : Error< - "'va_start' cannot be used outside a function">; -def err_va_start_fixed_function : Error< - "'va_start' used in function with fixed args">; +def err_va_builtin_captured_stmt : Error< + "'%0' cannot be used in a captured statement">; +def err_va_builtin_outside_function : Error< + "'%0' cannot be used outside a function">; +def err_va_builtin_fixed_function : Error< + "'%0' used in function with fixed args">; +def err_builtin_va_arg_pack_bad_context : Error< + "'%0' can't be used in a function that might be emitted">; +def err_builtin_va_arg_pack_bad_call : Error< + "'__builtin_va_arg_pack' must appear as the last argument " + "to a variadic function">; def err_va_start_used_in_wrong_abi_function : Error< "'va_start' used in %select{System V|Win64}0 ABI function">; def err_ms_va_start_used_in_sysv_function : Error< Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -10589,6 +10589,11 @@ bool CheckObjCString(Expr *Arg); ExprResult CheckOSLogFormatStringArg(Expr *Arg); + /// If this context permits use of __builtin_va_arg_pack{,len}. These builtins + /// are only usable in a function that is only available for inlining, never + /// emitted as a standalone definition ('available_externally' in LLVM-speak). + bool isVAArgPackBuiltinUsableHere(DeclContext *Ctx); + ExprResult CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, CallExpr *TheCall); Index: clang/lib/AST/Expr.cpp =================================================================== --- clang/lib/AST/Expr.cpp +++ clang/lib/AST/Expr.cpp @@ -3075,6 +3075,15 @@ Arg->EvaluateAsBooleanCondition(ArgVal, Ctx) && !ArgVal; } +bool CallExpr::isInlineVAArgPackStrawmanCall() const { + if (getNumArgs() != 0) { + const Expr *LastArg = getArg(getNumArgs() - 1)->IgnoreParens(); + if (auto *CallArg = dyn_cast(LastArg)) + return CallArg->getBuiltinCallee() == Builtin::BI__builtin_va_arg_pack; + } + return false; +} + namespace { /// Look for any side effects within a Stmt. class SideEffectFinder : public ConstEvaluatedExprVisitor { Index: clang/lib/CodeGen/CGBuiltin.cpp =================================================================== --- clang/lib/CodeGen/CGBuiltin.cpp +++ clang/lib/CodeGen/CGBuiltin.cpp @@ -1698,6 +1698,12 @@ return RValue::get(Builder.CreateCall(CGM.getIntrinsic(Intrinsic::vacopy), {DstPtr, SrcPtr})); } + case Builtin::BI__builtin_va_arg_pack: + llvm_unreachable("builtin_va_arg_pack outside of call?"); + case Builtin::BI__builtin_va_arg_pack_len: { + return RValue::get( + Builder.CreateCall(CGM.getIntrinsic(Intrinsic::va_arg_pack_len))); + } case Builtin::BI__builtin_abs: case Builtin::BI__builtin_labs: case Builtin::BI__builtin_llabs: { Index: clang/lib/CodeGen/CGExpr.cpp =================================================================== --- clang/lib/CodeGen/CGExpr.cpp +++ clang/lib/CodeGen/CGExpr.cpp @@ -4763,7 +4763,15 @@ } } - EmitCallArgs(Args, dyn_cast(FnType), E->arguments(), + auto ArgRange = E->arguments(); + bool InlineVAArgPackCall = false; + if (E->isInlineVAArgPackStrawmanCall()) { + // If the last argument is __builtin_va_arg_pack(), drop it. It's + // represented as an attribute on the call instruction in LLVM IR. + ArgRange = CallExpr::const_arg_range(ArgRange.begin(), ArgRange.end() - 1); + InlineVAArgPackCall = true; + } + EmitCallArgs(Args, dyn_cast(FnType), ArgRange, E->getDirectCallee(), /*ParamsToSkip*/ 0, Order); const CGFunctionInfo &FnInfo = CGM.getTypes().arrangeFreeFunctionCall( @@ -4798,7 +4806,14 @@ Callee.setFunctionPointer(CalleePtr); } - return EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr, E->getExprLoc()); + llvm::CallBase *TheCallInst = nullptr; + RValue Result = EmitCall(FnInfo, Callee, ReturnValue, Args, + &TheCallInst, E->getExprLoc()); + + if (E->isInlineVAArgPackStrawmanCall()) + TheCallInst->setIsInlineVAArgPackCall(true); + + return Result; } LValue CodeGenFunction:: Index: clang/lib/Sema/SemaChecking.cpp =================================================================== --- clang/lib/Sema/SemaChecking.cpp +++ clang/lib/Sema/SemaChecking.cpp @@ -953,6 +953,61 @@ return true; } +static bool +checkVABuiltinIsInVariadicFunction(Sema &S, Expr *Fn, StringRef BuiltinName, + ParmVarDecl **LastParam = nullptr) { + // Determine whether the current function, block, or obj-c method is variadic + // and get its parameter list. + bool IsVariadic = false; + ArrayRef Params; + DeclContext *Caller = S.CurContext; + if (auto *Block = dyn_cast(Caller)) { + IsVariadic = Block->isVariadic(); + Params = Block->parameters(); + } else if (auto *FD = dyn_cast(Caller)) { + IsVariadic = FD->isVariadic(); + Params = FD->parameters(); + } else if (auto *MD = dyn_cast(Caller)) { + IsVariadic = MD->isVariadic(); + // FIXME: This isn't correct for methods (results in bogus warning). + Params = MD->parameters(); + } else if (isa(Caller)) { + // We don't support va_start in a CapturedDecl. + S.Diag(Fn->getBeginLoc(), diag::err_va_builtin_captured_stmt) + << BuiltinName; + return true; + } else { + // This must be some other declcontext that parses exprs. + S.Diag(Fn->getBeginLoc(), diag::err_va_builtin_outside_function) + << BuiltinName; + return true; + } + + if (!IsVariadic) { + S.Diag(Fn->getBeginLoc(), diag::err_va_builtin_fixed_function) + << BuiltinName; + return true; + } + + if (LastParam) + *LastParam = Params.empty() ? nullptr : Params.back(); + + return false; +} + +/// If this context permits use of __builtin_va_arg_pack{,len}. These builtins +/// are only usable in a function that is only available for inlining, never +/// emitted as a standalone definition ('available_externally' in LLVM-speak). +bool Sema::isVAArgPackBuiltinUsableHere(DeclContext *Ctx) { + auto *FD = dyn_cast(Ctx); + if (!FD) return false; + + return FD->isInlined() && + (!getLangOpts().CPlusPlus || getLangOpts().GNUInline || + FD->hasAttr()) && + !FD->isInlineDefinitionExternallyVisible(); +} + ExprResult Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, CallExpr *TheCall) { @@ -1004,7 +1059,22 @@ } break; } + case Builtin::BI__builtin_va_arg_pack: + case Builtin::BI__builtin_va_arg_pack_len: { + StringRef BuiltinName = BuiltinID == Builtin::BI__builtin_va_arg_pack + ? StringRef("__builtin_va_arg_pack") + : "__builtin_va_arg_pack_len"; + if (checkVABuiltinIsInVariadicFunction(*this, TheCall, BuiltinName)) + return ExprError(); + + if (isVAArgPackBuiltinUsableHere(CurContext)) + break; + + Diag(TheCall->getBeginLoc(), diag::err_builtin_va_arg_pack_bad_context) + << BuiltinName; + return ExprError(); + } // The acquire, release, and no fence variants are ARM and AArch64 only. case Builtin::BI_interlockedbittestandset_acq: case Builtin::BI_interlockedbittestandset_rel: @@ -5339,44 +5409,6 @@ return false; } -static bool checkVAStartIsInVariadicFunction(Sema &S, Expr *Fn, - ParmVarDecl **LastParam = nullptr) { - // Determine whether the current function, block, or obj-c method is variadic - // and get its parameter list. - bool IsVariadic = false; - ArrayRef Params; - DeclContext *Caller = S.CurContext; - if (auto *Block = dyn_cast(Caller)) { - IsVariadic = Block->isVariadic(); - Params = Block->parameters(); - } else if (auto *FD = dyn_cast(Caller)) { - IsVariadic = FD->isVariadic(); - Params = FD->parameters(); - } else if (auto *MD = dyn_cast(Caller)) { - IsVariadic = MD->isVariadic(); - // FIXME: This isn't correct for methods (results in bogus warning). - Params = MD->parameters(); - } else if (isa(Caller)) { - // We don't support va_start in a CapturedDecl. - S.Diag(Fn->getBeginLoc(), diag::err_va_start_captured_stmt); - return true; - } else { - // This must be some other declcontext that parses exprs. - S.Diag(Fn->getBeginLoc(), diag::err_va_start_outside_function); - return true; - } - - if (!IsVariadic) { - S.Diag(Fn->getBeginLoc(), diag::err_va_start_fixed_function); - return true; - } - - if (LastParam) - *LastParam = Params.empty() ? nullptr : Params.back(); - - return false; -} - /// 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. @@ -5408,7 +5440,7 @@ // Check that the current function is variadic, and get its last parameter. ParmVarDecl *LastParam; - if (checkVAStartIsInVariadicFunction(*this, Fn, &LastParam)) + if (checkVABuiltinIsInVariadicFunction(*this, Fn, "va_start", &LastParam)) return true; // Verify that the second argument to the builtin is the last argument of the @@ -5475,7 +5507,7 @@ return true; // Check that the current function is variadic. - if (checkVAStartIsInVariadicFunction(*this, Func)) + if (checkVABuiltinIsInVariadicFunction(*this, Func, "va_start")) return true; // __va_start on Windows does not validate the parameter qualifiers Index: clang/lib/Sema/SemaDecl.cpp =================================================================== --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -13090,6 +13090,62 @@ return ActOnFinishFunctionBody(D, BodyArg, false); } +/// Visit the body of a function to verify that all calls to +/// __builtin_va_arg_pack appear in the last argument to a call to a variadic +/// function. Any other use of this builtin is an error. +class DiagMisplacedVAArgPacks + : public ConstStmtVisitor { + Sema &S; + + static bool isBuiltinVAArgCall(const Expr *E) { + E = E->IgnoreParens(); + if (const auto *CE = dyn_cast(E)) + return CE->getBuiltinCallee() == Builtin::BI__builtin_va_arg_pack; + return false; + } + +public: + explicit DiagMisplacedVAArgPacks(Sema &S) : S(S) {} + + void VisitStmt(const Stmt *S) { + for (const Stmt *C : S->children()) + if (C) Visit(C); + } + + void VisitCallExpr(const CallExpr *CE) { + if (isBuiltinVAArgCall(CE)) { + S.Diag(CE->getBeginLoc(), diag::err_builtin_va_arg_pack_bad_call); + return; + } + + if (!CE->isInlineVAArgPackStrawmanCall()) { + VisitStmt(CE); + return; + } + + // The last argument to this call is __builtin_va_arg_pack(). + + QualType CalleeTy = CE->getCallee()->getType(); + if (auto *PtrTy = CalleeTy->getAs()) + CalleeTy = PtrTy->getPointeeType(); + else if (CalleeTy->isSpecificPlaceholderType(BuiltinType::BoundMember)) + CalleeTy = Expr::findBoundMemberType(CE->getCallee()); + + if (auto *FPT = CalleeTy->getAs()) { + if (CE->getNumArgs() == FPT->getNumParams()) + S.Diag(CE->getBeginLoc(), diag::err_builtin_va_arg_pack_bad_call); + } else if (!CalleeTy->getAs()) { + S.Diag(CE->getBeginLoc(), diag::err_builtin_va_arg_pack_bad_call); + } + + // Recuse to the children, ignoring the builtin call. + const Expr *LastArg = CE->getArg(CE->getNumArgs() - 1); + for (const Stmt *Sub : CE->children()) + if (Sub != LastArg) + Visit(Sub); + } +}; + /// RAII object that pops an ExpressionEvaluationContext when exiting a function /// body. class ExitFunctionBodyRAII { @@ -13404,6 +13460,13 @@ "Leftover expressions for odr-use checking"); } + // Check if there are any misplaced __builtin_va_arg_pack{,_len}s in this + // function. Only do this traversal if this function permits theses builtins, + // otherwise it'll be diagnosed earlier. + if (Body && FD && !FD->isDependentContext() && + isVAArgPackBuiltinUsableHere(FD)) + DiagMisplacedVAArgPacks(*this).Visit(Body); + if (!IsInstantiation) PopDeclContext(); Index: clang/test/CodeGen/builtin-va-arg-pack.c =================================================================== --- /dev/null +++ clang/test/CodeGen/builtin-va-arg-pack.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 %s -O1 -emit-llvm -disable-llvm-passes -triple x86_64-apple-macosx10.14.0 -o - | FileCheck %s +// RUN: %clang_cc1 -xc++ %s -O1 -emit-llvm -disable-llvm-passes -triple x86_64-apple-macosx10.14.0 -o - | FileCheck %s + +#ifdef __cplusplus +extern "C" +#endif +int printf_like(const char *, ...); + +// CHECK-LABEL: define available_externally void @ +extern inline __attribute__((gnu_inline)) +void check(const char *x, ...) { + // CHECK: call{{.*}}@printf_like(i8* %{{[a-zA-Z0-9]+}}){{.*}}[[CALL_ATTRS:#[0-9]+]] + printf_like(x, __builtin_va_arg_pack()); + + // CHECK: call{{.*}}@printf_like(i8* %{{[a-zA-Z0-9]+}}, i32 1, i32 2, i32 3){{.*}}[[CALL_ATTRS:#[0-9]+]] + printf_like(x, 1, 2, 3, __builtin_va_arg_pack()); + + // CHECK: call i32 @llvm.va_arg_pack_len + __builtin_va_arg_pack_len(); +} + +// CHECK: attributes [[CALL_ATTRS]] = { inline_va_arg_pack } + +int main() { + (void)check; +} Index: clang/test/Sema/builtin-va-arg-pack.c =================================================================== --- /dev/null +++ clang/test/Sema/builtin-va-arg-pack.c @@ -0,0 +1,129 @@ +// RUN: %clang_cc1 %s -verify +// RUN: %clang_cc1 %s -fgnu89-inline -verify -DGNU89 +// RUN: %clang_cc1 -xc++ -fblocks %s -verify + +int printf_like(const char *, ...); + +extern inline __attribute__((gnu_inline)) +void correct(const char *msg, ...) { + printf_like(msg, __builtin_va_arg_pack()); + __builtin_va_arg_pack_len(); +} + +void incorrect1(const char *msg, ...) { + printf_like(msg, __builtin_va_arg_pack()); // expected-error {{'__builtin_va_arg_pack' can't be used in a function that might be emitted}} + __builtin_va_arg_pack_len(); // expected-error {{'__builtin_va_arg_pack_len' can't be used in a function that might be emitted}} +} + +extern inline +void incorrect2(const char *msg, ...) { + printf_like(msg, __builtin_va_arg_pack()); + __builtin_va_arg_pack_len(); +#ifndef GNU89 + // expected-error@-3 {{'__builtin_va_arg_pack' can't be used in a function that might be emitted}} + // expected-error@-3 {{'__builtin_va_arg_pack_len' can't be used in a function that might be emitted}} +#endif +} + +static inline __attribute__((gnu_inline)) +void incorrect3(const char *msg, ...) { + printf_like(msg, __builtin_va_arg_pack()); // expected-error {{'__builtin_va_arg_pack' can't be used in a function that might be emitted}} + __builtin_va_arg_pack_len(); // expected-error {{'__builtin_va_arg_pack_len' can't be used in a function that might be emitted}} +} + +extern inline __attribute__((gnu_inline)) +void bad_position(int c, ...) { + __builtin_va_arg_pack(); // expected-error {{'__builtin_va_arg_pack' must appear as the last argument to a variadic function}} +} + +extern inline __attribute__((gnu_inline)) +void many_packs(int x, ...) { + // GCC accepts this, but the docs claim that its ill-formed. Whatever, it + // probably doesn't matter. + printf_like("", __builtin_va_arg_pack(), __builtin_va_arg_pack()); // expected-error {{'__builtin_va_arg_pack' must appear as the last argument to a variadic function}} +} + +extern inline __attribute__((gnu_inline)) +void merge_packs(const char* x, ...) { + printf_like(x, 1, 2, 3, __builtin_va_arg_pack()); +} + +#ifdef __cplusplus +int empty_varargs(...); + +extern inline __attribute__((gnu_inline)) +void call_empty(...) { + empty_varargs(__builtin_va_arg_pack()); +} + +struct S { + void member1(const char *, ...); + virtual void member2(const char *, ...); + static void member3(const char *, ...); + + void call_em(const char *x, ...); +}; + +extern inline __attribute__((gnu_inline)) +void S::call_em(const char *x, ...){ + member1(x, __builtin_va_arg_pack()); + member2(x, __builtin_va_arg_pack()); + member3(x, __builtin_va_arg_pack()); +} + +extern inline __attribute__((gnu_inline)) +void call_em(S &s, const char *x, ...) { + s.member1(x, __builtin_va_arg_pack()); + s.member2(x, __builtin_va_arg_pack()); + s.member3(x, __builtin_va_arg_pack()); +} + +template +extern inline __attribute__((gnu_inline)) +int call_em_dep(const char *x, ...) { + T s; + s.member1(x, __builtin_va_arg_pack()); + s.member2(x, __builtin_va_arg_pack()); + s.member3(x, __builtin_va_arg_pack()); + __builtin_va_arg_pack(); // expected-error {{'__builtin_va_arg_pack' must appear as the last argument to a variadic function}} + return 0; +} + +int p = call_em_dep("", 1, 2, 3); // expected-note {{in instantiation}} + +extern inline __attribute__((gnu_inline)) +void S::member1(const char *x, ...) { + printf_like(x, __builtin_va_arg_pack()); +} + +extern inline __attribute__((gnu_inline)) +void S::member2(const char *x, ...) { + printf_like(x, __builtin_va_arg_pack()); +} + +extern inline __attribute__((gnu_inline)) +void S::member3(const char *x, ...) { + printf_like(x, __builtin_va_arg_pack()); +} + +struct VACtor { + VACtor(const char *, ...); +}; + +// FIXME: Should we bother supporting these cases? +extern inline __attribute__((gnu_inline)) +void doesnt_work(const char *x, ...) { + VACtor s(x, __builtin_va_arg_pack()); // expected-error{{'__builtin_va_arg_pack' must appear as the last argument to a variadic function}} + void (^blk)(const char *, ...); + blk(x, __builtin_va_arg_pack()); // expected-error{{'__builtin_va_arg_pack' must appear as the last argument to a variadic function}} +} +#endif + +extern inline __attribute__((gnu_inline)) +void nonsensical() { + __builtin_va_arg_pack(0); // expected-error{{too many arguments to function call, expected 0, have 1}} + __builtin_va_arg_pack_len(0); // expected-error{{too many arguments to function call, expected 0, have 1}} + + __builtin_va_arg_pack(); // expected-error{{'__builtin_va_arg_pack' used in function with fixed args}} + __builtin_va_arg_pack_len(); // expected-error{{'__builtin_va_arg_pack_len' used in function with fixed args}} +}