erik.pilkington created this revision.
erik.pilkington added reviewers: faisalv, rsmith.
erik.pilkington added a subscriber: cfe-commits.
Previously, clang emitted no diagnostic for the following:
```
auto f() {
int loc;
return [&] { return loc; };
}
```
The problem being that this returns a dangling reference to the local variable
'loc'. This patch warns on this by extending `-Wreturn-stack-address`.
This patch also warns on the following, where the lambda is stored in a
variable:
```
auto f() {
int loc;
auto lam = [&loc] {};
return lam; // warn
}
```
I believe that this is OK, I can't think of any valid way of mutating `lam`
that would make the code not return a dangling pointer to `loc`.
Also, this diagnoses the following, where a pointer to a local is captured via
an init-capture:
```
auto f() {
int local;
return [x = &local] {}; // warn
}
```
But not here, because we would have to verify that the pointer in `lam` wasn't
mutated in a previous call of the lambda:
```
auto f() {
int local;
auto lam = [x = &local] {};
return lam; // no warn
}
```
Thanks for taking a look!
https://reviews.llvm.org/D24639
Files:
include/clang/AST/LambdaCapture.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
test/SemaCXX/cxx1y-init-captures.cpp
test/SemaCXX/return-lambda-stack-addr.cpp
Index: test/SemaCXX/return-lambda-stack-addr.cpp
===================================================================
--- test/SemaCXX/return-lambda-stack-addr.cpp
+++ test/SemaCXX/return-lambda-stack-addr.cpp
@@ -0,0 +1,127 @@
+// RUN: %clang_cc1 -std=c++14 -verify -fsyntax-only %s
+
+auto basic() {
+ int local;
+
+ return [&]() {
+ (void)local; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+ };
+}
+
+auto by_copy() {
+ int local;
+
+ return [=]() {
+ (void)local; // no warning
+ };
+}
+
+auto nested() {
+ int local;
+
+ return [=] {
+ return [&]() { (void)local; }; // no warning
+ };
+}
+
+auto nested2() {
+ int local;
+
+ return [&] {
+ (void)[=] {
+ (void)local; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+ };
+ };
+}
+
+auto nested3() {
+ int local;
+ return [&] {
+ return [&] {
+ (void)local; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+ };
+ };
+}
+
+struct function {
+ template <class T>
+ function(T) {}
+};
+
+function to_function() {
+ int local;
+ return [&] { (void)local; };
+}
+
+auto lambda = [] {
+ int local;
+ return [&] { return local; }; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+};
+
+auto ref_copy() {
+ int x;
+ int &y = x;
+
+ return [=] { (void)y; };
+}
+
+auto ref_ref() {
+ int x;
+ int &y = x; // expected-note{{binding reference variable 'y' here}}
+ return [&] { (void)y; }; // expected-warning{{address of stack memory associated with local variable 'x' returned}}
+}
+
+auto param(int p) {
+ return [&] { (void)p; }; // expected-warning{{address of stack memory associated with local variable 'p' returned}}
+}
+
+auto return_var() {
+ int local;
+ auto lam = [&local] {}; // expected-note 4 {{capturing local variable in lambda 'lam' here}}
+
+ auto &lam_ref = lam; // expected-note 2 {{binding reference variable 'lam_ref' here}}
+
+ if (0)
+ return lam; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+ else if (0)
+ return *&lam; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+ else if (0)
+ return lam_ref; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+ else
+ return *&lam_ref; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+}
+
+auto return_ptr_no_warn() {
+ int local;
+ auto lam = [ptr = &local]{};
+ return lam; // no warning
+}
+
+auto return_ptr_warn() {
+ int local;
+ return [ptr = &local]{}; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+}
+
+auto mutable_lambda_warn() {
+ int local;
+ auto mut = [&local]() mutable {}; // expected-note{{capturing local variable in lambda 'mut' here}}
+ return mut; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+}
+
+auto mutable_lambda_warn2() {
+ int local;
+ return [cap = &local]() mutable {}; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+}
+
+auto mutable_lambda_no_warn() {
+ int local;
+ auto mut = [cap = &local]() mutable {};
+ return mut; // no warning
+}
+
+auto ref_ptr() {
+ int local;
+ auto lam = [&local] {}; // expected-note{{capturing local variable in lambda 'lam' here}}
+ auto *const &lolwhat = &lam; // expected-note{{binding reference variable 'lolwhat' here}}
+ return *lolwhat; // expected-warning{{address of stack memory associated with local variable 'local' returned}}
+}
Index: test/SemaCXX/cxx1y-init-captures.cpp
===================================================================
--- test/SemaCXX/cxx1y-init-captures.cpp
+++ test/SemaCXX/cxx1y-init-captures.cpp
@@ -117,10 +117,11 @@
{ // will need to capture x in outer lambda
const T x = 10; //expected-note {{declared}}
auto L = [z = x](char a) { //expected-note {{begins}}
- auto M = [&y = x](T b) { //expected-error {{cannot be implicitly captured}}
+ auto M = [&y = x](T b) { //expected-error {{cannot be implicitly captured}} expected-note{{capturing}}
return y;
};
- return M;
+ // FIXME: We shouldn't emit this warning here.
+ return M; // expected-warning{{stack memory}}
};
}
@@ -146,10 +147,11 @@
{ // will need to capture x in outer lambda
const int x = 10; //expected-note 2{{declared}}
auto L = [z = x](char a) { //expected-note 2{{begins}}
- auto M = [&y = x](T b) { //expected-error 2{{cannot be implicitly captured}}
+ auto M = [&y = x](T b) { //expected-error 2{{cannot be implicitly captured}} expected-note{{capturing local}}
return y;
};
- return M;
+ // FIXME: We shouldn't emit this warning here
+ return M; // expected-warning{{stack memory}}
};
}
{
Index: test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
===================================================================
--- test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
+++ test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
@@ -182,14 +182,14 @@
int d[sizeof(a) == sizeof(c) || sizeof(c) == sizeof(b) ? 2 : 1];
f(x, d);
f(y, d);
- f(z, d);
+ f(z, d); // expected-warning {{address of stack memory associated with local variable 'z' returned}}
decltype(a) A = a;
decltype(b) B = b;
const int &i = cx.i;
};
};
};
- auto M = L(3)(3.5);
+ auto M = L(3)(3.5); // expected-note{{in instantiation}}
M(3.14);
}
}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6585,8 +6585,13 @@
if (lhsType->isPointerType() ||
(!S.getLangOpts().ObjCAutoRefCount && lhsType->isBlockPointerType())) {
stackE = EvalAddr(RetValExp, refVars, /*ParentDecl=*/nullptr);
+
} else if (lhsType->isReferenceType()) {
stackE = EvalVal(RetValExp, refVars, /*ParentDecl=*/nullptr);
+
+ } else if (CXXRecordDecl *LambdaDecl = lhsType->getAsCXXRecordDecl()) {
+ if (LambdaDecl->isLambda())
+ stackE = EvalVal(RetValExp, refVars, /*ParentDecl=*/nullptr);
}
if (!stackE)
@@ -6641,8 +6646,12 @@
// show the range of the expression.
SourceRange range = (i < e - 1) ? refVars[i + 1]->getSourceRange()
: stackE->getSourceRange();
- S.Diag(VD->getLocation(), diag::note_ref_var_local_bind)
- << VD->getDeclName() << range;
+
+ unsigned Diag = VD->getType()->isReferenceType()
+ ? diag::note_ref_var_local_bind
+ : diag::note_ref_var_lambda_local_bind;
+
+ S.Diag(VD->getLocation(), Diag) << VD->getDeclName() << range;
}
}
@@ -6697,15 +6706,18 @@
if (DR->refersToEnclosingVariableOrCapture())
return nullptr;
- if (const VarDecl *V = dyn_cast<VarDecl>(DR->getDecl()))
+ if (const VarDecl *V = dyn_cast<VarDecl>(DR->getDecl())) {
// If this is a reference variable, follow through to the expression that
// it points to.
+ auto *RD = V->getType()->getAsCXXRecordDecl();
if (V->hasLocalStorage() &&
- V->getType()->isReferenceType() && V->hasInit()) {
+ (V->getType()->isReferenceType() || (RD && RD->isLambda())) &&
+ V->hasInit()) {
// Add the reference variable to the "trail".
refVars.push_back(DR);
return EvalAddr(V->getInit(), refVars, ParentDecl);
}
+ }
return nullptr;
}
@@ -6830,7 +6842,7 @@
static const Expr *EvalVal(const Expr *E,
SmallVectorImpl<const DeclRefExpr *> &refVars,
const Decl *ParentDecl) {
- do {
+ for (;;) {
// We should only be called for evaluating non-pointer expressions, or
// expressions with a pointer type that are not used as references but
// instead
@@ -6871,7 +6883,8 @@
return DR;
if (V->hasLocalStorage()) {
- if (!V->getType()->isReferenceType())
+ auto *RD = V->getType()->getAsCXXRecordDecl();
+ if (!V->getType()->isReferenceType() && !(RD && RD->isLambda()))
return DR;
// Reference variable, follow through to the expression that
@@ -6958,6 +6971,52 @@
return Result;
return E;
+ case Stmt::CXXConstructExprClass: {
+ auto *CE = cast<CXXConstructExpr>(E);
+
+ // We only check this node to find a construction of a C++ lambda.
+ if (!CE->getConstructor()->getParent()->isLambda())
+ return nullptr;
+
+ assert(CE->getNumArgs() == 1 && "Lambda constructor without 1 argument?");
+
+ E = CE->getArg(0)->IgnoreImplicit();
+ break;
+ }
+
+ case Stmt::LambdaExprClass: {
+ const LambdaExpr *Lambda = cast<LambdaExpr>(E);
+
+ SmallVector<const DeclRefExpr *, 8> oldRefVars(refVars.begin(),
+ refVars.end());
+
+ auto init = Lambda->capture_init_begin();
+ auto cap = Lambda->capture_begin(), cap_end = Lambda->capture_end();
+
+ for (; cap != cap_end; ++cap, ++init) {
+ if (!cap->capturesVariable())
+ continue;
+
+ // There are 2 cases we want to catch here:
+ // 1. Capturing a local by reference, ie:
+ // return [&local] {};
+ if (!cap->capturesByCopy() &&
+ cap->getCapturedVar()->hasLocalStorage()) {
+ if (auto *Res = EvalVal(*init, refVars, ParentDecl))
+ return Res;
+ }
+ // 2. Capturing a pointer to a local through an init-capture. ie:
+ // return [ptr = &local] {};
+ else if (refVars.empty() && (*init)->getType()->isPointerType())
+ if (auto *Res = EvalAddr(*init, refVars, ParentDecl))
+ return Res;
+
+ refVars = oldRefVars;
+ }
+
+ return nullptr;
+ }
+
default:
// Check that we don't return or take the address of a reference to a
// temporary. This is only useful in C++.
@@ -6967,7 +7026,7 @@
// Everything else: we simply don't reason about them.
return nullptr;
}
- } while (true);
+ }
}
void
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7406,6 +7406,8 @@
"returning block that lives on the local stack">;
def note_ref_var_local_bind : Note<
"binding reference variable %0 here">;
+def note_ref_var_lambda_local_bind : Note<
+ "capturing local variable in lambda %0 here">;
// Check for initializing a member variable with the address or a reference to
// a constructor parameter.
Index: include/clang/AST/LambdaCapture.h
===================================================================
--- include/clang/AST/LambdaCapture.h
+++ include/clang/AST/LambdaCapture.h
@@ -97,6 +97,8 @@
!(DeclAndBits.getInt() & Capture_This);
}
+ bool capturesByCopy() const { return DeclAndBits.getInt() & Capture_ByCopy; }
+
/// \brief Retrieve the declaration of the local variable being
/// captured.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
