[clang] 1af97c9 - [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference
Valeriy Savchenko via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 12 07:06:29 PDT 2021
Author: Abbas Sabra
Date: 2021-07-12T17:06:07+03:00
New Revision: 1af97c9d0b02002586473b4b9845b0c390504a27
URL: https://github.com/llvm/llvm-project/commit/1af97c9d0b02002586473b4b9845b0c390504a27
DIFF: https://github.com/llvm/llvm-project/commit/1af97c9d0b02002586473b4b9845b0c390504a27.diff
LOG: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference
Reviewed By: vsavchenko
Differential Revision: https://reviews.llvm.org/D102273
Added:
Modified:
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
clang/test/Analysis/loop-unrolling.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index dc268e562237f..e5f4e9ea30c97 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -79,14 +79,17 @@ ProgramStateRef processLoopEnd(const Stmt *LoopStmt, ProgramStateRef State) {
return State;
}
-static internal::Matcher<Stmt> simpleCondition(StringRef BindName) {
- return binaryOperator(anyOf(hasOperatorName("<"), hasOperatorName(">"),
- hasOperatorName("<="), hasOperatorName(">="),
- hasOperatorName("!=")),
- hasEitherOperand(ignoringParenImpCasts(declRefExpr(
- to(varDecl(hasType(isInteger())).bind(BindName))))),
- hasEitherOperand(ignoringParenImpCasts(
- integerLiteral().bind("boundNum"))))
+static internal::Matcher<Stmt> simpleCondition(StringRef BindName,
+ StringRef RefName) {
+ return binaryOperator(
+ anyOf(hasOperatorName("<"), hasOperatorName(">"),
+ hasOperatorName("<="), hasOperatorName(">="),
+ hasOperatorName("!=")),
+ hasEitherOperand(ignoringParenImpCasts(
+ declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName)))
+ .bind(RefName))),
+ hasEitherOperand(
+ ignoringParenImpCasts(integerLiteral().bind("boundNum"))))
.bind("conditionOperator");
}
@@ -138,7 +141,7 @@ static internal::Matcher<Stmt> hasSuspiciousStmt(StringRef NodeName) {
static internal::Matcher<Stmt> forLoopMatcher() {
return forStmt(
- hasCondition(simpleCondition("initVarName")),
+ hasCondition(simpleCondition("initVarName", "initVarRef")),
// Initialization should match the form: 'int i = 6' or 'i = 42'.
hasLoopInit(
anyOf(declStmt(hasSingleDecl(
@@ -156,17 +159,52 @@ static internal::Matcher<Stmt> forLoopMatcher() {
hasUnaryOperand(declRefExpr(
to(varDecl(allOf(equalsBoundNode("initVarName"),
hasType(isInteger())))))))),
- unless(hasBody(hasSuspiciousStmt("initVarName")))).bind("forLoop");
+ unless(hasBody(hasSuspiciousStmt("initVarName"))))
+ .bind("forLoop");
}
-static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
- // Global variables assumed as escaped variables.
+static bool isCapturedByReference(ExplodedNode *N, const DeclRefExpr *DR) {
+
+ // Get the lambda CXXRecordDecl
+ assert(DR->refersToEnclosingVariableOrCapture());
+ const LocationContext *LocCtxt = N->getLocationContext();
+ const Decl *D = LocCtxt->getDecl();
+ const auto *MD = cast<CXXMethodDecl>(D);
+ assert(MD && MD->getParent()->isLambda() &&
+ "Captured variable should only be seen while evaluating a lambda");
+ const CXXRecordDecl *LambdaCXXRec = MD->getParent();
+
+ // Lookup the fields of the lambda
+ llvm::DenseMap<const VarDecl *, FieldDecl *> LambdaCaptureFields;
+ FieldDecl *LambdaThisCaptureField;
+ LambdaCXXRec->getCaptureFields(LambdaCaptureFields, LambdaThisCaptureField);
+
+ // Check if the counter is captured by reference
+ const VarDecl *VD = cast<VarDecl>(DR->getDecl()->getCanonicalDecl());
+ assert(VD);
+ const FieldDecl *FD = LambdaCaptureFields[VD];
+ assert(FD && "Captured variable without a corresponding field");
+ return FD->getType()->isReferenceType();
+}
+
+// A loop counter is considered escaped if:
+// case 1: It is a global variable.
+// case 2: It is a reference parameter or a reference capture.
+// case 3: It is assigned to a non-const reference variable or parameter.
+// case 4: Has its address taken.
+static bool isPossiblyEscaped(ExplodedNode *N, const DeclRefExpr *DR) {
+ const VarDecl *VD = cast<VarDecl>(DR->getDecl()->getCanonicalDecl());
+ assert(VD);
+ // Case 1:
if (VD->hasGlobalStorage())
return true;
- const bool isParm = isa<ParmVarDecl>(VD);
- // Reference parameters are assumed as escaped variables.
- if (isParm && VD->getType()->isReferenceType())
+ const bool IsRefParamOrCapture =
+ isa<ParmVarDecl>(VD) || DR->refersToEnclosingVariableOrCapture();
+ // Case 2:
+ if ((DR->refersToEnclosingVariableOrCapture() &&
+ isCapturedByReference(N, DR)) ||
+ (IsRefParamOrCapture && VD->getType()->isReferenceType()))
return true;
while (!N->pred_empty()) {
@@ -189,6 +227,7 @@ static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
// on VD and reference initialized by VD.
ASTContext &ASTCtx =
N->getLocationContext()->getAnalysisDeclContext()->getASTContext();
+ // Case 3 and 4:
auto Match =
match(stmt(anyOf(callByRef(equalsNode(VD)), getAddrTo(equalsNode(VD)),
assignedToRef(equalsNode(VD)))),
@@ -199,8 +238,8 @@ static bool isPossiblyEscaped(const VarDecl *VD, ExplodedNode *N) {
N = N->getFirstPred();
}
- // Parameter declaration will not be found.
- if (isParm)
+ // Reference parameter and reference capture will not be found.
+ if (IsRefParamOrCapture)
return false;
llvm_unreachable("Reached root without finding the declaration of VD");
@@ -218,7 +257,7 @@ bool shouldCompletelyUnroll(const Stmt *LoopStmt, ASTContext &ASTCtx,
if (Matches.empty())
return false;
- auto CounterVar = Matches[0].getNodeAs<VarDecl>("initVarName");
+ const auto *CounterVarRef = Matches[0].getNodeAs<DeclRefExpr>("initVarRef");
llvm::APInt BoundNum =
Matches[0].getNodeAs<IntegerLiteral>("boundNum")->getValue();
llvm::APInt InitNum =
@@ -235,7 +274,7 @@ bool shouldCompletelyUnroll(const Stmt *LoopStmt, ASTContext &ASTCtx,
maxStep = (BoundNum - InitNum).abs().getZExtValue();
// Check if the counter of the loop is not escaped before.
- return !isPossiblyEscaped(CounterVar->getCanonicalDecl(), Pred);
+ return !isPossiblyEscaped(Pred, CounterVarRef);
}
bool madeNewBranch(ExplodedNode *N, const Stmt *LoopStmt) {
diff --git a/clang/test/Analysis/loop-unrolling.cpp b/clang/test/Analysis/loop-unrolling.cpp
index e8ba8b9476ae8..fc1fb06cdc014 100644
--- a/clang/test/Analysis/loop-unrolling.cpp
+++ b/clang/test/Analysis/loop-unrolling.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++11 -analyzer-config exploration_strategy=unexplored_first_queue %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++11 -DDFS=1 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++14 -analyzer-config exploration_strategy=unexplored_first_queue %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true,exploration_strategy=dfs -verify -std=c++14 -DDFS=1 %s
void clang_analyzer_numTimesReached();
void clang_analyzer_warnIfReached();
@@ -511,3 +511,39 @@ void parm_by_ref_as_loop_counter(int &i) {
clang_analyzer_numTimesReached(); // expected-warning {{4}}
}
}
+
+void capture_by_value_as_loop_counter() {
+ int out = 0;
+ auto l = [i = out]() mutable {
+ for (i = 0; i < 10; ++i) {
+ clang_analyzer_numTimesReached(); // expected-warning {{10}}
+ }
+ };
+}
+
+void capture_by_ref_as_loop_counter() {
+ int out = 0;
+ auto l = [&i = out]() {
+ for (i = 0; i < 10; ++i) {
+ clang_analyzer_numTimesReached(); // expected-warning {{4}}
+ }
+ };
+}
+
+void capture_implicitly_by_value_as_loop_counter() {
+ int i = 0;
+ auto l = [=]() mutable {
+ for (i = 0; i < 10; ++i) {
+ clang_analyzer_numTimesReached(); // expected-warning {{10}}
+ }
+ };
+}
+
+void capture_implicitly_by_ref_as_loop_counter() {
+ int i = 0;
+ auto l = [&]() mutable {
+ for (i = 0; i < 10; ++i) {
+ clang_analyzer_numTimesReached(); // expected-warning {{4}}
+ }
+ };
+}
More information about the cfe-commits
mailing list