diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp --- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -79,14 +79,17 @@ return State; } -static internal::Matcher 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 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 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,54 @@ 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(D); + assert(MD && MD->getParent()->isLambda() && + "Captured variable should only be seen while evaluating a lambda"); + const CXXRecordDecl *LambdaCXXRec = MD->getParent(); + + // Lookup the field of the lambda + llvm::DenseMap LambdaCaptureFields; + FieldDecl *LambdaThisCaptureField; + LambdaCXXRec->getCaptureFields(LambdaCaptureFields, LambdaThisCaptureField); + + // Check if the counter is captured by reference + const VarDecl *VD = cast(DR->getDecl()->getCanonicalDecl()); + assert(VD); + // Sema follows a sequence of complex rules to determine whether the + // variable should be captured. + 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(DR->getDecl()->getCanonicalDecl()); + assert(VD); + // case 1: if (VD->hasGlobalStorage()) return true; - const bool isParm = isa(VD); - // Reference parameters are assumed as escaped variables. - if (isParm && VD->getType()->isReferenceType()) + const bool IsRefParamOrCapture = + isa(VD) || DR->refersToEnclosingVariableOrCapture(); + // case 2: + if ((DR->refersToEnclosingVariableOrCapture() && + isCapturedByReference(N, DR)) || + (IsRefParamOrCapture && VD->getType()->isReferenceType())) return true; while (!N->pred_empty()) { @@ -189,6 +229,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 +240,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 +259,7 @@ if (Matches.empty()) return false; - auto CounterVar = Matches[0].getNodeAs("initVarName"); + const auto *CounterVarRef = Matches[0].getNodeAs("initVarRef"); llvm::APInt BoundNum = Matches[0].getNodeAs("boundNum")->getValue(); llvm::APInt InitNum = @@ -235,7 +276,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) { diff --git a/clang/test/Analysis/loop-unrolling.cpp b/clang/test/Analysis/loop-unrolling.cpp --- 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 @@ 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}} + } + }; +}