From 4df7572a49e7cfe16328060c827c8501c3a52700 Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Fri, 20 Nov 2015 01:53:44 +0000 Subject: [PATCH] [analyzer] DeadStoresChecker: Treat locals captured by reference in C++ lambdas as escaped. The analyzer currently reports dead store false positives when a local variable is captured by reference in a C++ lambda. For example: int local = 0; auto lambda = [&local]() { local++; }; local = 7; // False Positive: Value stored to 'local' is never read lambda(); In this case, the assignment setting `local` to 7 is not a dead store because the called lambda will later read that assigned value. This commit silences this source of false positives by treating locals captured by reference in C++ lambdas as escaped, similarly to how the DeadStoresChecker deals with locals whose address is taken. rdar://problem/22165179 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@253630 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/DeadStoresChecker.cpp | 27 +++++++++++ test/Analysis/lambdas.cpp | 45 ++++++++++++++++++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp index 5d8baf6ba7..f2a269a333 100644 --- a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp @@ -401,6 +401,11 @@ public: // Check for '&'. Any VarDecl whose address has been taken we treat as // escaped. // FIXME: What about references? + if (auto *LE = dyn_cast(S)) { + findLambdaReferenceCaptures(LE); + return; + } + const UnaryOperator *U = dyn_cast(S); if (!U) return; @@ -412,6 +417,28 @@ public: if (const VarDecl *VD = dyn_cast(DR->getDecl())) Escaped.insert(VD); } + + // Treat local variables captured by reference in C++ lambdas as escaped. + void findLambdaReferenceCaptures(const LambdaExpr *LE) { + const CXXRecordDecl *LambdaClass = LE->getLambdaClass(); + llvm::DenseMap CaptureFields; + FieldDecl *ThisCaptureField; + LambdaClass->getCaptureFields(CaptureFields, ThisCaptureField); + + for (const LambdaCapture &C : LE->captures()) { + if (!C.capturesVariable()) + continue; + + VarDecl *VD = C.getCapturedVar(); + const FieldDecl *FD = CaptureFields[VD]; + if (!FD) + continue; + + // If the capture field is a reference type, it is capture-by-reference. + if (FD->getType()->isReferenceType()) + Escaped.insert(VD); + } + } }; } // end anonymous namespace diff --git a/test/Analysis/lambdas.cpp b/test/Analysis/lambdas.cpp index a906cedb3a..ae4febbb42 100644 --- a/test/Analysis/lambdas.cpp +++ b/test/Analysis/lambdas.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,deadcode,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s // RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.DumpCFG -analyzer-config inline-lambdas=true %s > %t 2>&1 // RUN: FileCheck --input-file=%t %s @@ -281,6 +281,49 @@ void captureStructReference(const StructPR24914& s) { }(); } +// Lambda capture counts as use for dead-store checking. + +int returnsValue(); + +void captureByCopyCausesUse() { + int local1 = returnsValue(); // no-warning + int local2 = returnsValue(); // no-warning + int local3 = returnsValue(); // expected-warning{{Value stored to 'local3' during its initialization is never read}} + + (void)[local1, local2]() { }; // Explicit capture by copy counts as use. + + int local4 = returnsValue(); // no-warning + int local5 = returnsValue(); // expected-warning{{Value stored to 'local5' during its initialization is never read}} + + (void)[=]() { + (void)local4; // Implicit capture by copy counts as use + }; +} + +void captureByReference() { + int local1 = returnsValue(); // no-warning + + auto lambda1 = [&local1]() { // Explicit capture by reference + local1++; + }; + + // Don't treat as a dead store because local1 was was captured by reference. + local1 = 7; // no-warning + + lambda1(); + + int local2 = returnsValue(); // no-warning + + auto lambda2 = [&]() { + local2++; // Implicit capture by reference + }; + + // Don't treat as a dead store because local2 was was captured by reference. + local2 = 7; // no-warning + + lambda2(); +} + // CHECK: [B2 (ENTRY)] // CHECK: Succs (1): B1 -- 2.40.0