From f717d4795ac42bf51b52d9cbff70b7a85dbdd9bd Mon Sep 17 00:00:00 2001
From: Henry Wong <movietravelcode@outlook.com>
Date: Sat, 31 Mar 2018 12:46:46 +0000
Subject: [PATCH] [analyzer] Unroll the loop when it has a unsigned counter.

Summary:
The original implementation in the `LoopUnrolling.cpp` didn't consider the case where the counter is unsigned. This case is only handled in `simpleCondition()`, but this is not enough, we also need to deal with the unsinged counter with the counter initialization.

Since `IntegerLiteral` is `signed`, there is a `ImplicitCastExpr<IntegralCast>` in `unsigned counter = IntergerLiteral`. This patch add the `ignoringParenImpCasts()` in the `IntegerLiteral` matcher.

Reviewers: szepet, a.sidorin, NoQ, george.karpenkov

Reviewed By: szepet, george.karpenkov

Subscribers: xazax.hun, rnkovacs, cfe-commits, MTC

Differential Revision: https://reviews.llvm.org/D45086

llvm-svn: 328919
---
 clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp | 16 +++++++++-------
 clang/test/Analysis/loop-unrolling.cpp          | 23 +++++++++++++++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
index b32e934..da4574c 100644
--- a/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ b/clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -141,13 +141,15 @@ 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(integerLiteral().bind("initNum")),
-                           equalsBoundNode("initVarName"))))),
-                 binaryOperator(hasLHS(declRefExpr(to(
-                                    varDecl(equalsBoundNode("initVarName"))))),
-                                hasRHS(integerLiteral().bind("initNum"))))),
+             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")))))),
              // Incrementation should be a simple increment or decrement
              // operator call.
              hasIncrement(unaryOperator(
diff --git a/clang/test/Analysis/loop-unrolling.cpp b/clang/test/Analysis/loop-unrolling.cpp
index aa145ac..ce7ada8 100644
--- a/clang/test/Analysis/loop-unrolling.cpp
+++ b/clang/test/Analysis/loop-unrolling.cpp
@@ -36,6 +36,29 @@ int simple_unroll2() {
   return 0;
 }
 
+int simple_unroll3_unsigned() {
+  int a[9];
+  int k = 42;
+  for (unsigned i = 0; i < 9; i++) {
+    clang_analyzer_numTimesReached(); // expected-warning {{9}}
+    a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
+int simple_unroll4_unsigned() {
+  int a[9];
+  int k = 42;
+  unsigned i;
+  for (i = (0); i < 9; i++) {
+    clang_analyzer_numTimesReached(); // expected-warning {{9}}
+    a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
 int simple_no_unroll1() {
   int a[9];
   int k = 42;
-- 
2.7.4