r301968 - Simplify some va_start checking logic
Reid Kleckner via cfe-commits
cfe-commits at lists.llvm.org
Tue May 2 13:10:03 PDT 2017
Author: rnk
Date: Tue May 2 15:10:03 2017
New Revision: 301968
URL: http://llvm.org/viewvc/llvm-project?rev=301968&view=rev
Log:
Simplify some va_start checking logic
Combine the logic doing the ms_abi/sysv_abi checks into one function so
that each check and its logical opposite are near each other. Now we
don't need two Sema entry points for MS va_start and regular va_start.
Refactor the code that checks if the va_start caller is a function,
block, or obj-c method. We do this in three places, and they are all
buggy for variadic lambdas (PR32737). After this change, I have one
place to apply the functional fix.
NFC
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/varargs.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=301968&r1=301967&r2=301968&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 2 15:10:03 2017
@@ -8048,7 +8048,7 @@ def err_64_bit_builtin_32_bit_tgt : Erro
"this builtin is only available on 64-bit targets">;
def err_ppc_builtin_only_on_pwr7 : Error<
"this builtin is only valid on POWER7 or later CPUs">;
-def err_x86_builtin_32_bit_tgt : Error<
+def err_x86_builtin_64_only : Error<
"this builtin is only available on x86-64 targets">;
def err_x86_builtin_invalid_rounding : Error<
"invalid rounding argument">;
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=301968&r1=301967&r2=301968&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue May 2 15:10:03 2017
@@ -10068,9 +10068,7 @@ private:
bool CheckX86BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
bool CheckPPCBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
- bool SemaBuiltinVAStartImpl(CallExpr *TheCall);
- bool SemaBuiltinVAStart(CallExpr *TheCall);
- bool SemaBuiltinMSVAStart(CallExpr *TheCall);
+ bool SemaBuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall);
bool SemaBuiltinVAStartARM(CallExpr *Call);
bool SemaBuiltinUnorderedCompare(CallExpr *TheCall);
bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs);
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=301968&r1=301967&r2=301968&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue May 2 15:10:03 2017
@@ -759,7 +759,7 @@ Sema::CheckBuiltinFunctionCall(FunctionD
break;
case Builtin::BI__builtin_stdarg_start:
case Builtin::BI__builtin_va_start:
- if (SemaBuiltinVAStart(TheCall))
+ if (SemaBuiltinVAStart(BuiltinID, TheCall))
return ExprError();
break;
case Builtin::BI__va_start: {
@@ -770,7 +770,7 @@ Sema::CheckBuiltinFunctionCall(FunctionD
return ExprError();
break;
default:
- if (SemaBuiltinVAStart(TheCall))
+ if (SemaBuiltinVAStart(BuiltinID, TheCall))
return ExprError();
break;
}
@@ -2090,7 +2090,7 @@ bool Sema::CheckX86BuiltinFunctionCall(u
return SemaBuiltinCpuSupports(*this, TheCall);
if (BuiltinID == X86::BI__builtin_ms_va_start)
- return SemaBuiltinMSVAStart(TheCall);
+ return SemaBuiltinVAStart(BuiltinID, TheCall);
// If the intrinsic has rounding or SAE make sure its valid.
if (CheckX86BuiltinRoundingOrSAE(BuiltinID, TheCall))
@@ -3611,11 +3611,81 @@ ExprResult Sema::CheckOSLogFormatStringA
return Result;
}
+/// Check that the user is calling the appropriate va_start builtin for the
+/// target and calling convention.
+static bool checkVAStartABI(Sema &S, unsigned BuiltinID, Expr *Fn) {
+ const llvm::Triple &TT = S.Context.getTargetInfo().getTriple();
+ bool IsX64 = TT.getArch() == llvm::Triple::x86_64;
+ bool IsWindows = TT.isOSWindows();
+ bool IsMSVAStart = BuiltinID == X86::BI__builtin_ms_va_start;
+ if (IsX64) {
+ clang::CallingConv CC = CC_C;
+ if (const FunctionDecl *FD = S.getCurFunctionDecl())
+ CC = FD->getType()->getAs<FunctionType>()->getCallConv();
+ if (IsMSVAStart) {
+ // Don't allow this in System V ABI functions.
+ if (CC == CC_X86_64SysV || (!IsWindows && CC != CC_X86_64Win64))
+ return S.Diag(Fn->getLocStart(),
+ diag::err_ms_va_start_used_in_sysv_function);
+ } else {
+ // On x86-64 Unix, don't allow this in Win64 ABI functions.
+ // On x64 Windows, don't allow this in System V ABI functions.
+ // (Yes, that means there's no corresponding way to support variadic
+ // System V ABI functions on Windows.)
+ if ((IsWindows && CC == CC_X86_64SysV) ||
+ (!IsWindows && CC == CC_X86_64Win64))
+ return S.Diag(Fn->getLocStart(),
+ diag::err_va_start_used_in_wrong_abi_function)
+ << !IsWindows;
+ }
+ return false;
+ }
+
+ if (IsMSVAStart)
+ return S.Diag(Fn->getLocStart(), diag::err_x86_builtin_64_only);
+ 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<ParmVarDecl *> Params;
+ if (BlockScopeInfo *CurBlock = S.getCurBlock()) {
+ IsVariadic = CurBlock->TheDecl->isVariadic();
+ Params = CurBlock->TheDecl->parameters();
+ } else if (FunctionDecl *FD = S.getCurFunctionDecl()) {
+ IsVariadic = FD->isVariadic();
+ Params = FD->parameters();
+ } else if (ObjCMethodDecl *MD = S.getCurMethodDecl()) {
+ IsVariadic = MD->isVariadic();
+ // FIXME: This isn't correct for methods (results in bogus warning).
+ Params = MD->parameters();
+ } else {
+ llvm_unreachable("unknown va_start context");
+ }
+
+ if (!IsVariadic) {
+ S.Diag(Fn->getLocStart(), diag::err_va_start_used_in_non_variadic_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.
-bool Sema::SemaBuiltinVAStartImpl(CallExpr *TheCall) {
+bool Sema::SemaBuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall) {
Expr *Fn = TheCall->getCallee();
+
+ if (checkVAStartABI(*this, BuiltinID, Fn))
+ return true;
+
if (TheCall->getNumArgs() > 2) {
Diag(TheCall->getArg(2)->getLocStart(),
diag::err_typecheck_call_too_many_args)
@@ -3636,20 +3706,10 @@ bool Sema::SemaBuiltinVAStartImpl(CallEx
if (checkBuiltinArgument(*this, TheCall, 0))
return true;
- // Determine whether the current function is variadic or not.
- BlockScopeInfo *CurBlock = getCurBlock();
- bool isVariadic;
- if (CurBlock)
- isVariadic = CurBlock->TheDecl->isVariadic();
- else if (FunctionDecl *FD = getCurFunctionDecl())
- isVariadic = FD->isVariadic();
- else
- isVariadic = getCurMethodDecl()->isVariadic();
-
- if (!isVariadic) {
- Diag(Fn->getLocStart(), diag::err_va_start_used_in_non_variadic_function);
+ // Check that the current function is variadic, and get its last parameter.
+ ParmVarDecl *LastParam;
+ 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.
@@ -3664,16 +3724,7 @@ bool Sema::SemaBuiltinVAStartImpl(CallEx
if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Arg)) {
if (const ParmVarDecl *PV = dyn_cast<ParmVarDecl>(DR->getDecl())) {
- // FIXME: This isn't correct for methods (results in bogus warning).
- // Get the last formal in the current function.
- const ParmVarDecl *LastArg;
- if (CurBlock)
- LastArg = CurBlock->TheDecl->parameters().back();
- else if (FunctionDecl *FD = getCurFunctionDecl())
- LastArg = FD->parameters().back();
- else
- LastArg = getCurMethodDecl()->parameters().back();
- SecondArgIsLastNamedArgument = PV == LastArg;
+ SecondArgIsLastNamedArgument = PV == LastParam;
Type = PV->getType();
ParamLoc = PV->getLocation();
@@ -3708,48 +3759,6 @@ bool Sema::SemaBuiltinVAStartImpl(CallEx
return false;
}
-/// Check the arguments to '__builtin_va_start' for validity, and that
-/// it was called from a function of the native ABI.
-/// Emit an error and return true on failure; return false on success.
-bool Sema::SemaBuiltinVAStart(CallExpr *TheCall) {
- // On x86-64 Unix, don't allow this in Win64 ABI functions.
- // On x64 Windows, don't allow this in System V ABI functions.
- // (Yes, that means there's no corresponding way to support variadic
- // System V ABI functions on Windows.)
- if (Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86_64) {
- unsigned OS = Context.getTargetInfo().getTriple().getOS();
- clang::CallingConv CC = CC_C;
- if (const FunctionDecl *FD = getCurFunctionDecl())
- CC = FD->getType()->getAs<FunctionType>()->getCallConv();
- if ((OS == llvm::Triple::Win32 && CC == CC_X86_64SysV) ||
- (OS != llvm::Triple::Win32 && CC == CC_X86_64Win64))
- return Diag(TheCall->getCallee()->getLocStart(),
- diag::err_va_start_used_in_wrong_abi_function)
- << (OS != llvm::Triple::Win32);
- }
- return SemaBuiltinVAStartImpl(TheCall);
-}
-
-/// Check the arguments to '__builtin_ms_va_start' for validity, and that
-/// it was called from a Win64 ABI function.
-/// Emit an error and return true on failure; return false on success.
-bool Sema::SemaBuiltinMSVAStart(CallExpr *TheCall) {
- // This only makes sense for x86-64.
- const llvm::Triple &TT = Context.getTargetInfo().getTriple();
- Expr *Callee = TheCall->getCallee();
- if (TT.getArch() != llvm::Triple::x86_64)
- return Diag(Callee->getLocStart(), diag::err_x86_builtin_32_bit_tgt);
- // Don't allow this in System V ABI functions.
- clang::CallingConv CC = CC_C;
- if (const FunctionDecl *FD = getCurFunctionDecl())
- CC = FD->getType()->getAs<FunctionType>()->getCallConv();
- if (CC == CC_X86_64SysV ||
- (TT.getOS() != llvm::Triple::Win32 && CC != CC_X86_64Win64))
- return Diag(Callee->getLocStart(),
- diag::err_ms_va_start_used_in_sysv_function);
- return SemaBuiltinVAStartImpl(TheCall);
-}
-
bool Sema::SemaBuiltinVAStartARM(CallExpr *Call) {
// void __va_start(va_list *ap, const char *named_addr, size_t slot_size,
// const char *named_addr);
@@ -3761,26 +3770,14 @@ bool Sema::SemaBuiltinVAStartARM(CallExp
diag::err_typecheck_call_too_few_args_at_least)
<< 0 /*function call*/ << 3 << Call->getNumArgs();
- // Determine whether the current function is variadic or not.
- bool IsVariadic;
- if (BlockScopeInfo *CurBlock = getCurBlock())
- IsVariadic = CurBlock->TheDecl->isVariadic();
- else if (FunctionDecl *FD = getCurFunctionDecl())
- IsVariadic = FD->isVariadic();
- else if (ObjCMethodDecl *MD = getCurMethodDecl())
- IsVariadic = MD->isVariadic();
- else
- llvm_unreachable("unexpected statement type");
-
- if (!IsVariadic) {
- Diag(Func->getLocStart(), diag::err_va_start_used_in_non_variadic_function);
- return true;
- }
-
// Type-check the first argument normally.
if (checkBuiltinArgument(*this, Call, 0))
return true;
+ // Check that the current function is variadic.
+ if (checkVAStartIsInVariadicFunction(*this, Func))
+ return true;
+
const struct {
unsigned ArgNo;
QualType Type;
Modified: cfe/trunk/test/Sema/varargs.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/varargs.c?rev=301968&r1=301967&r2=301968&view=diff
==============================================================================
--- cfe/trunk/test/Sema/varargs.c (original)
+++ cfe/trunk/test/Sema/varargs.c Tue May 2 15:10:03 2017
@@ -27,7 +27,7 @@ void f3(float a, ...) { // expected-note
}
-// stdarg: PR3075
+// stdarg: PR3075 and PR2531
void f4(const char *msg, ...) {
__builtin_va_list ap;
__builtin_stdarg_start((ap), (msg));
More information about the cfe-commits
mailing list