baloghadamsoftware updated this revision to Diff 210498.
baloghadamsoftware added a comment.
Fix wrong upload.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63279/new/
https://reviews.llvm.org/D63279
Files:
lib/StaticAnalyzer/Core/LoopUnrolling.cpp
test/Analysis/loop-unrolling.cpp
Index: test/Analysis/loop-unrolling.cpp
===================================================================
--- test/Analysis/loop-unrolling.cpp
+++ test/Analysis/loop-unrolling.cpp
@@ -499,3 +499,27 @@
clang_analyzer_numTimesReached(); // expected-warning {{6}}
}
}
+
+int unroll_known_value_of_variable1() {
+ int a[9];
+ int k = 42;
+ int n = 9;
+ for (int i = 0; i < n; i++) {
+ clang_analyzer_numTimesReached(); // expected-warning {{9}}
+ a[i] = 42;
+ }
+ int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+ return 0;
+}
+
+int unroll_known_value_of_variable2() {
+ int a[9];
+ int k = 42;
+ int n = 9;
+ for (int i = 0; n > i; i++) {
+ clang_analyzer_numTimesReached(); // expected-warning {{9}}
+ a[i] = 42;
+ }
+ int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===================================================================
--- lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -12,11 +12,11 @@
///
//===----------------------------------------------------------------------===//
-#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h"
using namespace clang;
using namespace ento;
@@ -80,13 +80,14 @@
}
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"))))
+ return binaryOperator(
+ anyOf(hasOperatorName("<"), hasOperatorName(">"),
+ hasOperatorName("<="), hasOperatorName(">="),
+ hasOperatorName("!=")),
+ hasEitherOperand(ignoringParenImpCasts(
+ declRefExpr(to(varDecl(allOf(hasType(isInteger()),
+ equalsBoundNode(BindName)))))
+ .bind("boundVarOperand"))))
.bind("conditionOperator");
}
@@ -138,17 +139,17 @@
static internal::Matcher<Stmt> forLoopMatcher() {
return forStmt(
- hasCondition(simpleCondition("initVarName")),
// Initialization should match the form: 'int i = 6' or 'i = 42'.
- hasLoopInit(
- anyOf(declStmt(hasSingleDecl(
- varDecl(allOf(hasInitializer(ignoringParenImpCasts(
- integerLiteral().bind("initNum"))),
- equalsBoundNode("initVarName"))))),
- binaryOperator(hasLHS(declRefExpr(to(varDecl(
- equalsBoundNode("initVarName"))))),
- hasRHS(ignoringParenImpCasts(
- integerLiteral().bind("initNum")))))),
+ hasLoopInit(anyOf(
+ declStmt(hasSingleDecl(
+ varDecl(hasInitializer(ignoringParenImpCasts(
+ integerLiteral().bind("initNum"))))
+ .bind("initVarName"))),
+ binaryOperator(
+ hasLHS(declRefExpr(to(varDecl().bind("initVarName")))),
+ hasRHS(ignoringParenImpCasts(
+ integerLiteral().bind("initNum")))))),
+ hasCondition(simpleCondition("initVarName")),
// Incrementation should be a simple increment or decrement
// operator call.
hasIncrement(unaryOperator(
@@ -156,7 +157,8 @@
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) {
@@ -196,22 +198,43 @@
bool shouldCompletelyUnroll(const Stmt *LoopStmt, ASTContext &ASTCtx,
ExplodedNode *Pred, unsigned &maxStep) {
-
if (!isLoopStmt(LoopStmt))
return false;
- // TODO: Match the cases where the bound is not a concrete literal but an
- // integer with known value
auto Matches = match(forLoopMatcher(), *LoopStmt, ASTCtx);
if (Matches.empty())
return false;
+ const auto State = Pred->getState();
+ auto &SVB = State->getStateManager().getSValBuilder();
+
auto CounterVar = Matches[0].getNodeAs<VarDecl>("initVarName");
- llvm::APInt BoundNum =
- Matches[0].getNodeAs<IntegerLiteral>("boundNum")->getValue();
llvm::APInt InitNum =
Matches[0].getNodeAs<IntegerLiteral>("initNum")->getValue();
auto CondOp = Matches[0].getNodeAs<BinaryOperator>("conditionOperator");
+
+ const Expr *BoundExpr = CondOp->getLHS()->IgnoreParenImpCasts();
+ if (BoundExpr == Matches[0].getNodeAs<Expr>("boundVarOperand"))
+ BoundExpr = CondOp->getRHS()->IgnoreParenImpCasts();
+ SVal BoundNumVal = Pred->getSVal(BoundExpr);
+
+ // If the value of the expression is unknown and it is a declaration
+ // reference then try to get the value of the declaration instead
+ if (BoundNumVal.isUnknown()) {
+ if (const auto *BoundDeclRefExpr = dyn_cast<DeclRefExpr>(BoundExpr)) {
+ // FIXME: Add other declarations such as Objective-C fields
+ if (const auto *BoundVarDecl =
+ dyn_cast<VarDecl>(BoundDeclRefExpr->getDecl())) {
+ BoundNumVal = State->getSVal(
+ State->getLValue(BoundVarDecl, Pred->getLocationContext()));
+ }
+ }
+ }
+ const llvm::APSInt *BoundNumPtr = SVB.getKnownValue(State, BoundNumVal);
+ if (!BoundNumPtr)
+ return false;
+ llvm::APInt BoundNum = *BoundNumPtr;
+
if (InitNum.getBitWidth() != BoundNum.getBitWidth()) {
InitNum = InitNum.zextOrSelf(BoundNum.getBitWidth());
BoundNum = BoundNum.zextOrSelf(InitNum.getBitWidth());
@@ -289,5 +312,5 @@
return false;
return true;
}
-}
-}
+} // namespace ento
+} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
