From: Reid Kleckner Date: Thu, 4 May 2017 19:51:05 +0000 (+0000) Subject: Fix bugs checking va_start in lambdas and erroneous contexts X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=42925a0581982545cf101d7d2051ef09d2eaf970;p=clang Fix bugs checking va_start in lambdas and erroneous contexts Summary: First, getCurFunction looks through blocks and lambdas, which is wrong. Inside a lambda, va_start should refer to the lambda call operator prototype. This fixes PR32737. Second, we shouldn't use any of the getCur* methods, because they look through contexts that we don't want to look through (EnumDecl, CapturedStmtDecl). We can use CurContext directly as the calling context. Finally, this code assumed that CallExprs would never appear outside of code contexts (block, function, obj-c method), which is wrong. Struct member initializers are an easy way to create and parse exprs in a non-code context. Reviewers: rsmith Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D32761 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@302188 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index cc530a6dd4..ff9626d79d 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -7920,7 +7920,11 @@ def warn_empty_switch_body : Warning< def note_empty_body_on_separate_line : Note< "put the semicolon on a separate line to silence this warning">; -def err_va_start_used_in_non_variadic_function : Error< +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_start_used_in_wrong_abi_function : Error< "'va_start' used in %select{System V|Win64}0 ABI function">; diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index a206100b89..14dd6267b8 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -3652,22 +3652,29 @@ static bool checkVAStartIsInVariadicFunction(Sema &S, Expr *Fn, // and get its parameter list. bool IsVariadic = false; ArrayRef Params; - if (BlockScopeInfo *CurBlock = S.getCurBlock()) { - IsVariadic = CurBlock->TheDecl->isVariadic(); - Params = CurBlock->TheDecl->parameters(); - } else if (FunctionDecl *FD = S.getCurFunctionDecl()) { + 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 (ObjCMethodDecl *MD = S.getCurMethodDecl()) { + } 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->getLocStart(), diag::err_va_start_captured_stmt); + return true; } else { - llvm_unreachable("unknown va_start context"); + // This must be some other declcontext that parses exprs. + S.Diag(Fn->getLocStart(), diag::err_va_start_outside_function); + return true; } if (!IsVariadic) { - S.Diag(Fn->getLocStart(), diag::err_va_start_used_in_non_variadic_function); + S.Diag(Fn->getLocStart(), diag::err_va_start_fixed_function); return true; } diff --git a/test/OpenMP/varargs.cpp b/test/OpenMP/varargs.cpp new file mode 100644 index 0000000000..7738f83fab --- /dev/null +++ b/test/OpenMP/varargs.cpp @@ -0,0 +1,8 @@ +// RUN: %clang_cc1 -verify -fopenmp %s +void f(int a, ...) { +#pragma omp parallel for + for (int i = 0; i < 100; ++i) { + __builtin_va_list ap; + __builtin_va_start(ap, a); // expected-error {{'va_start' cannot be used in a captured statement}} + } +}; diff --git a/test/SemaCXX/varargs.cpp b/test/SemaCXX/varargs.cpp index 6a1883786a..f9027c2479 100644 --- a/test/SemaCXX/varargs.cpp +++ b/test/SemaCXX/varargs.cpp @@ -1,12 +1,59 @@ -// RUN: %clang_cc1 -fsyntax-only -std=c++03 -verify %s +// RUN: %clang_cc1 -std=c++03 -verify %s +// RUN: %clang_cc1 -std=c++11 -verify %s + +__builtin_va_list ap; class string; void f(const string& s, ...) { // expected-note {{parameter of type 'const string &' is declared here}} - __builtin_va_list ap; __builtin_va_start(ap, s); // expected-warning {{passing an object of reference type to 'va_start' has undefined behavior}} } void g(register int i, ...) { - __builtin_va_list ap; - __builtin_va_start(ap, i); // okay + __builtin_va_start(ap, i); // UB in C, OK in C++ +} + +// Don't crash when there is no last parameter. +void no_params(...) { + int a; + __builtin_va_start(ap, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}} +} + +// Reject this. The __builtin_va_start would execute in Foo's non-variadic +// default ctor. +void record_context(int a, ...) { + struct Foo { + // expected-error@+1 {{'va_start' cannot be used outside a function}} + void meth(int a, int b = (__builtin_va_start(ap, a), 0)) {} + }; +} + +#if __cplusplus >= 201103L +// We used to have bugs identifying the correct enclosing function scope in a +// lambda. + +void fixed_lambda_varargs_function(int a, ...) { + [](int b) { + __builtin_va_start(ap, b); // expected-error {{'va_start' used in function with fixed args}} + }(42); +} +void varargs_lambda_fixed_function(int a) { + [](int b, ...) { + __builtin_va_start(ap, b); // correct + }(42); +} + +auto fixed_lambda_global = [](int f) { + __builtin_va_start(ap, f); // expected-error {{'va_start' used in function with fixed args}} +}; +auto varargs_lambda_global = [](int f, ...) { + __builtin_va_start(ap, f); // correct +}; + +void record_member_init(int a, ...) { + struct Foo { + int a = 0; + // expected-error@+1 {{'va_start' cannot be used outside a function}} + int b = (__builtin_va_start(ap, a), 0); + }; } +#endif