This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1af97c9d0b02: [analyzer] LoopUnrolling: fix crash when a
loop counter is captured in a lambda… (authored by AbbasSabra, committed by
vsavchenko).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102273/new/
https://reviews.llvm.org/D102273
Files:
clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
clang/test/Analysis/loop-unrolling.cpp
Index: clang/test/Analysis/loop-unrolling.cpp
===================================================================
--- clang/test/Analysis/loop-unrolling.cpp
+++ 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 @@
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}}
+ }
+ };
+}
Index: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -79,14 +79,17 @@
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> 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 @@
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 @@
// 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 @@
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 @@
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 @@
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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
