diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp index 01d87b02fcdbd..5803cbd9f7229 100644 --- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -23,6 +23,8 @@ using namespace clang; using namespace ento; using namespace clang::ast_matchers; +using ast_matchers::internal::Matcher; + static const int MAXIMUM_STEP_UNROLLED = 128; namespace { @@ -69,6 +71,11 @@ struct LoopState { REGISTER_LIST_WITH_PROGRAMSTATE(LoopStack, LoopState) namespace clang { +namespace { +AST_MATCHER(QualType, isIntegralOrEnumerationType) { + return Node->isIntegralOrEnumerationType(); +} +} // namespace namespace ento { static bool isLoopStmt(const Stmt *S) { @@ -82,22 +89,23 @@ ProgramStateRef processLoopEnd(const Stmt *LoopStmt, ProgramStateRef State) { return State; } -static internal::Matcher simpleCondition(StringRef BindName, - StringRef RefName) { +static Matcher simpleCondition(StringRef BindName, StringRef RefName) { + auto LoopVariable = ignoringParenImpCasts( + declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName))) + .bind(RefName)); + auto UpperBound = ignoringParenImpCasts( + expr(hasType(isIntegralOrEnumerationType())).bind("boundNum")); + return binaryOperator( anyOf(hasOperatorName("<"), hasOperatorName(">"), hasOperatorName("<="), hasOperatorName(">="), hasOperatorName("!=")), - hasEitherOperand(ignoringParenImpCasts( - declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName))) - .bind(RefName))), - hasEitherOperand( - ignoringParenImpCasts(integerLiteral().bind("boundNum")))) + anyOf(binaryOperator(hasLHS(LoopVariable), hasRHS(UpperBound)), + binaryOperator(hasRHS(LoopVariable), hasLHS(UpperBound)))) .bind("conditionOperator"); } -static internal::Matcher -changeIntBoundNode(internal::Matcher VarNodeMatcher) { +static Matcher changeIntBoundNode(Matcher VarNodeMatcher) { return anyOf( unaryOperator(anyOf(hasOperatorName("--"), hasOperatorName("++")), hasUnaryOperand(ignoringParenImpCasts( @@ -107,15 +115,13 @@ changeIntBoundNode(internal::Matcher VarNodeMatcher) { declRefExpr(to(varDecl(VarNodeMatcher))))))); } -static internal::Matcher -callByRef(internal::Matcher VarNodeMatcher) { +static Matcher callByRef(Matcher VarNodeMatcher) { return callExpr(forEachArgumentWithParam( declRefExpr(to(varDecl(VarNodeMatcher))), parmVarDecl(hasType(references(qualType(unless(isConstQualified()))))))); } -static internal::Matcher -assignedToRef(internal::Matcher VarNodeMatcher) { +static Matcher assignedToRef(Matcher VarNodeMatcher) { return declStmt(hasDescendant(varDecl( allOf(hasType(referenceType()), hasInitializer(anyOf( @@ -123,14 +129,13 @@ assignedToRef(internal::Matcher VarNodeMatcher) { declRefExpr(to(varDecl(VarNodeMatcher))))))))); } -static internal::Matcher -getAddrTo(internal::Matcher VarNodeMatcher) { +static Matcher getAddrTo(Matcher VarNodeMatcher) { return unaryOperator( hasOperatorName("&"), hasUnaryOperand(declRefExpr(hasDeclaration(VarNodeMatcher)))); } -static internal::Matcher hasSuspiciousStmt(StringRef NodeName) { +static Matcher hasSuspiciousStmt(StringRef NodeName) { return hasDescendant(stmt( anyOf(gotoStmt(), switchStmt(), returnStmt(), // Escaping and not known mutation of the loop counter is handled @@ -142,7 +147,7 @@ static internal::Matcher hasSuspiciousStmt(StringRef NodeName) { assignedToRef(equalsBoundNode(std::string(NodeName)))))); } -static internal::Matcher forLoopMatcher() { +static Matcher forLoopMatcher() { return forStmt( hasCondition(simpleCondition("initVarName", "initVarRef")), // Initialization should match the form: 'int i = 6' or 'i = 42'. @@ -271,23 +276,26 @@ static bool shouldCompletelyUnroll(const Stmt *LoopStmt, ASTContext &ASTCtx, 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 *CounterVarRef = Matches[0].getNodeAs("initVarRef"); - llvm::APInt BoundNum = - Matches[0].getNodeAs("boundNum")->getValue(); + const Expr *BoundNumExpr = Matches[0].getNodeAs("boundNum"); + + Expr::EvalResult BoundNumResult; + if (!BoundNumExpr || !BoundNumExpr->EvaluateAsInt(BoundNumResult, ASTCtx, + Expr::SE_NoSideEffects)) { + return false; + } llvm::APInt InitNum = Matches[0].getNodeAs("initNum")->getValue(); auto CondOp = Matches[0].getNodeAs("conditionOperator"); - unsigned MaxWidth = std::max(InitNum.getBitWidth(), BoundNum.getBitWidth()); + unsigned MaxWidth = std::max(InitNum.getBitWidth(), + BoundNumResult.Val.getInt().getBitWidth()); InitNum = InitNum.zext(MaxWidth); - BoundNum = BoundNum.zext(MaxWidth); - + llvm::APInt BoundNum = BoundNumResult.Val.getInt().zext(MaxWidth); if (CondOp->getOpcode() == BO_GE || CondOp->getOpcode() == BO_LE) maxStep = (BoundNum - InitNum + 1).abs().getZExtValue(); else diff --git a/clang/test/Analysis/loop-unrolling.cpp b/clang/test/Analysis/loop-unrolling.cpp index ebae81e000c7a..8a4690b9b6c98 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=expected,default -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=expected,dfs -std=c++14 %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify=expected,default -std=c++17 -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=expected,dfs -std=c++17 %s void clang_analyzer_numTimesReached(); void clang_analyzer_warnIfReached(); @@ -580,3 +580,21 @@ void test_escaping_on_var_before_switch_case_no_crash(int c) { } } } + +template struct Integer { + static constexpr int value = Val; +}; + +void complicated_compile_time_upper_bound() { + static_assert((sizeof(char) * Integer<4>::value + 3) == 7); + for (int i = 0; i < (sizeof(char) * Integer<4>::value + (((3)))); ++i) { + clang_analyzer_numTimesReached(); // expected-warning {{7}} + } +} + +void complicated_compile_time_upper_bound_indirect() { + using Seven = Integer<(sizeof(char) * Integer<4>::value + 3)>; + for (int i = 0; i < ((Seven::value)); ++i) { + clang_analyzer_numTimesReached(); // expected-warning {{7}} + } +}