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 cc530a6dd49f7780a66413de40581e23c516bf4e..ff9626d79da883611ec56004ad73d9d59a2c1e3f 100644 (file)
--- 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 a206100b89eba6bc59271cceb6205fbd293f1e14..14dd6267b8542efa53f8fa2d7f03fe44ac8df8ad 100644 (file)
--- 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<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;
   }
 

diff --git a/test/OpenMP/varargs.cpp b/test/OpenMP/varargs.cpp
new file mode 100644 (file)
index 0000000..7738f83
--- /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 6a1883786a33ac055354525a13e038dca31786f1..f9027c24799305264a54e640c061ef749f2c57d0 100644 (file)
--- 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