erik.pilkington updated this revision to Diff 77717.
erik.pilkington added a comment.
This new patch rewrites the check to just use the captures in the lambda's
CXXRecordDecl, instead of searching through the returned expression to find the
LambdaExpr.
Thanks!
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/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 [&]() { // expected-warning{{reference to stack memory associated with local variable 'local' returned through lambda capture}}
+ (void)local; // expected-note{{capturing reference to local variable 'local' in lambda here}}
+ };
+}
+
+auto extern_lambda() {
+ return basic(); // no warn
+}
+
+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 [&] { // expected-warning{{reference to stack memory associated with local variable 'local' returned through lambda capture}}
+ (void)[=] {
+ (void)local; // expected-note{{capturing reference to local variable 'local' in lambda here}}
+ };
+ };
+}
+
+auto nested3() {
+ int local;
+ return [&] { // expected-warning{{reference to stack memory associated with local variable 'local' returned through lambda capture}}
+ return [&] {
+ (void)local; // expected-note{{capturing reference to local variable 'local' in lambda here}}
+ };
+ };
+}
+
+auto nested4() {
+ int local;
+ (void)[&] {
+ return [&] { (void)local; }; // no warn
+ };
+}
+
+auto lambda = [] {
+ int local;
+ return [&] { return local; }; // expected-warning{{reference to stack memory associated with local variable 'local' returned through lambda capture}} expected-note{{capturing reference to local variable 'local' in lambda here}}
+};
+
+auto ref_copy() {
+ int x;
+ int &y = x;
+
+ return [=] { (void)y; }; // no warn
+}
+
+auto ref_ref() {
+ int x;
+ int &y = x;
+ return [&] { (void)y; }; // expected-warning{{reference to stack memory associated with local variable 'x' returned through lambda capture}} expected-note{{capturing reference to local}}
+}
+
+auto ref_ref_ok(int &r) {
+ int &y = r;
+ return [&] { return y; }; // no warning
+}
+
+auto &return_ref() {
+ auto lam = [] {};
+ return lam; // expected-warning{{reference to stack memory}}
+}
+
+auto param(int p) {
+ return [&] { (void)p; }; // expected-warning{{reference to stack memory associated with local variable 'p' returned}} expected-note{{capturing reference to local}}
+}
+
+auto return_var() {
+ int local;
+ auto lam = [&local] {}; // expected-note 2 {{capturing reference to local variable 'local' in lambda here}}
+
+ auto &lam_ref = lam;
+
+ if (0)
+ return lam; // expected-warning{{reference to stack memory associated with local variable 'local' returned}}
+ else
+ return lam_ref; // expected-warning{{reference to stack memory associated with local variable 'local' returned}}
+}
+
+auto return_ptr_in_var_warn() {
+ int local;
+ auto lam = [ptr = &local]{}; // expected-note{{capturing address of local variable 'local' in lambda here}}
+ return lam; // expected-warning{{address of stack memory associated with local variable 'local' returned through lambda capture}}
+}
+
+auto return_ptr_warn() {
+ int local;
+ return [ptr = &local]{}; // expected-warning{{address of stack memory associated with local variable 'local' returned through lambda capture}} expected-note{{capturing address of local variable 'local' in lambda here}}
+}
+
+auto mutable_lambda_warn() {
+ int local;
+ auto mut = [&local]() mutable {}; // expected-note{{capturing reference to local variable 'local' in lambda here}}
+ return mut; // expected-warning{{reference to stack memory associated with local variable 'local' returned through lambda capture}}
+}
+
+auto mutable_lambda_warn2() {
+ int local;
+ return [cap = &local]() mutable {}; // expected-warning{{address of stack memory associated with local variable 'local' returned through lambda capture}} expected-note{{capturing address of local variable 'local' in lambda here}}
+}
+
+auto mutable_lambda_no_warn() {
+ int local;
+ auto mut = [cap = &local]() mutable {};
+ return mut; // no warning
+}
Index: test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
===================================================================
--- test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
+++ test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
@@ -176,20 +176,20 @@
sample::X cx{5};
auto L = [=](auto a) {
const int z = 3;
- return [&,a](auto b) {
+ return [&,a](auto b) { // expected-warning {{reference to stack memory associated with local variable 'z' returned}}
const int y = 5;
return [=](auto c) {
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-note{{capturing reference to local variable 'z' in lambda here}}
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
@@ -7236,6 +7236,82 @@
SmallVectorImpl<const DeclRefExpr *> &refVars,
const Decl *ParentDecl);
+/// \brief Diagnose if the lambda \c RD, initialized by the returned expression
+/// \c E refers to a local variable.
+static void CheckReturnStackAddrLambda(Sema &S, const CXXRecordDecl *RD,
+ const Expr *E,
+ SourceLocation ReturnLoc) {
+ assert(RD->isLambda() && "Expected a lambda here!");
+
+ // We diagnose returning a lambda that captures a pointer to a local if it's
+ // not mutable or if it's being returned directly, because in either case we
+ // know that the pointer in the closure has not been updated.
+ bool ShouldDiagCapPtr = true;
+ if (!RD->getLambdaCallOperator()->isConst()) {
+ ShouldDiagCapPtr = false;
+ E = E->IgnoreParenImpCasts();
+ if (auto *CE = dyn_cast<CXXConstructExpr>(E))
+ if (CE->getNumArgs() && isa<LambdaExpr>(CE->getArg(0)->IgnoreImplicit()))
+ ShouldDiagCapPtr = true;
+ }
+
+ SmallVector<const DeclRefExpr *, 8> RefVars;
+ const NamedDecl *OffendingVar = nullptr;
+ const Expr *OffendingExpr = nullptr;
+ SourceLocation CaptureLoc;
+ bool IsReference;
+
+ for (const LambdaCapture &Cap : RD->captures()) {
+ if (!Cap.capturesVariable())
+ continue;
+
+ CaptureLoc = Cap.getLocation();
+ IsReference = !Cap.capturesByCopy();
+ const VarDecl *VD = Cap.getCapturedVar();
+ bool IsInitCapture = RD->getLambdaCallOperator() == VD->getDeclContext();
+ RefVars.clear();
+
+ // There are 3 cases that we want to catch here:
+ // 1. An init-capture whose initializing expression refers to a stack
+ // address. Here, VD is the variable that stores the capture, ie 'l' in:
+ // return [&l = local] {};
+ if (IsReference && IsInitCapture)
+ OffendingExpr = EvalVal(VD->getInit(), RefVars, nullptr);
+
+ // 2. Capturing a reference to a local. Here, VD is declaration of the
+ // variable we're capturing.
+ else if (IsReference && !IsInitCapture && VD->isLocalVarDeclOrParm() &&
+ S.CurContext == VD->getDeclContext()) {
+ // If the captured variable is also a reference, follow through to the
+ // initializing expression.
+ if (VD->getType()->isReferenceType()) {
+ if (VD->hasInit())
+ OffendingExpr = EvalVal(VD->getInit(), RefVars, nullptr);
+ } else
+ OffendingVar = VD;
+ }
+ // 3. Explicitly capturing a pointer to a local by copy. ie:
+ // return [ptr = &local] {};
+ else if (!IsReference && IsInitCapture && ShouldDiagCapPtr &&
+ VD->getType()->isPointerType() && VD->hasInit())
+ OffendingExpr = EvalAddr(VD->getInit(), RefVars, nullptr);
+
+ if (OffendingExpr || OffendingVar)
+ break;
+ }
+
+ if (auto *DRE = dyn_cast_or_null<DeclRefExpr>(OffendingExpr))
+ OffendingVar = DRE->getDecl();
+
+ if (!OffendingVar || OffendingVar->getDeclContext() != S.CurContext)
+ return;
+
+ S.Diag(ReturnLoc, diag::warn_ret_stack_addr_ref_lambda) << IsReference
+ << OffendingVar;
+ S.Diag(CaptureLoc, diag::note_ref_var_lambda_local_bind) << IsReference
+ << OffendingVar;
+}
+
/// CheckReturnStackAddr - Check if a return statement returns the address
/// of a stack variable.
static void
@@ -7252,6 +7328,9 @@
stackE = EvalAddr(RetValExp, refVars, /*ParentDecl=*/nullptr);
} else if (lhsType->isReferenceType()) {
stackE = EvalVal(RetValExp, refVars, /*ParentDecl=*/nullptr);
+ } else if (auto *LambdaDecl = lhsType->getAsCXXRecordDecl()) {
+ if (LambdaDecl->isLambda())
+ return CheckReturnStackAddrLambda(S, LambdaDecl, RetValExp, ReturnLoc);
}
if (!stackE)
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7507,6 +7507,12 @@
"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 %select{address of|reference to}0 local variable %1 in lambda here">;
+def warn_ret_stack_addr_ref_lambda : Warning<
+ "%select{address of|reference to}0 stack memory associated with local "
+ "variable %1 returned through lambda capture">,
+ InGroup<ReturnStackAddress>;
// 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
