r302188 - Fix bugs checking va_start in lambdas and erroneous contexts

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Thu May 4 12:51:06 PDT 2017


Author: rnk
Date: Thu May  4 14:51:05 2017
New Revision: 302188

URL: http://llvm.org/viewvc/llvm-project?rev=302188&view=rev
Log:
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

Added:
    cfe/trunk/test/OpenMP/varargs.cpp
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/SemaCXX/varargs.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=302188&r1=302187&r2=302188&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu May  4 14:51:05 2017
@@ -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">;

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=302188&r1=302187&r2=302188&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu May  4 14:51:05 2017
@@ -3652,22 +3652,29 @@ static bool checkVAStartIsInVariadicFunc
   // 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()) {
+  DeclContext *Caller = S.CurContext;
+  if (auto *Block = dyn_cast<BlockDecl>(Caller)) {
+    IsVariadic = Block->isVariadic();
+    Params = Block->parameters();
+  } else if (auto *FD = dyn_cast<FunctionDecl>(Caller)) {
     IsVariadic = FD->isVariadic();
     Params = FD->parameters();
-  } else if (ObjCMethodDecl *MD = S.getCurMethodDecl()) {
+  } else if (auto *MD = dyn_cast<ObjCMethodDecl>(Caller)) {
     IsVariadic = MD->isVariadic();
     // FIXME: This isn't correct for methods (results in bogus warning).
     Params = MD->parameters();
+  } else if (isa<CapturedDecl>(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;
   }
 

Added: cfe/trunk/test/OpenMP/varargs.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/varargs.cpp?rev=302188&view=auto
==============================================================================
--- cfe/trunk/test/OpenMP/varargs.cpp (added)
+++ cfe/trunk/test/OpenMP/varargs.cpp Thu May  4 14:51:05 2017
@@ -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}}
+  }
+};

Modified: cfe/trunk/test/SemaCXX/varargs.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/varargs.cpp?rev=302188&r1=302187&r2=302188&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/varargs.cpp (original)
+++ cfe/trunk/test/SemaCXX/varargs.cpp Thu May  4 14:51:05 2017
@@ -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 at +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 at +1 {{'va_start' cannot be used outside a function}}
+    int b = (__builtin_va_start(ap, a), 0);
+  };
 }
+#endif




More information about the cfe-commits mailing list