[Analyzer] More accurate modeling about the increment operator of the operand with...
authorHenry Wong <movietravelcode@outlook.com>
Tue, 6 Mar 2018 12:29:09 +0000 (12:29 +0000)
committerHenry Wong <movietravelcode@outlook.com>
Tue, 6 Mar 2018 12:29:09 +0000 (12:29 +0000)
Summary:
There is a problem with analyzer that a wrong value is given when modeling the increment operator of the operand with type bool. After `rL307604` is applied, a unsigned overflow may occur.

Example:
```
void func() {
  bool b = true;
  // unsigned overflow occur, 2 -> 0 U1b
  b++;
}
```

The use of an operand of type bool with the ++ operators is deprecated but valid untill C++17. And if the operand of the increment operator is of type bool, it is set to true.

This patch includes two parts:

  - If the operand of the increment operator is of type bool or type _Bool, set to true.
  - Modify `BasicValueFactory::getTruthValue()`, use `getIntWidth()` instead `getTypeSize()` and use `unsigned` instead `signed`.

Reviewers: alexshap, NoQ, dcoughlin, george.karpenkov

Reviewed By: NoQ

Subscribers: xazax.hun, szepet, a.sidorin, cfe-commits, MTC

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

llvm-svn: 326776

clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
clang/test/Analysis/_Bool-increment-decrement.c [new file with mode: 0644]
clang/test/Analysis/bool-increment.cpp [new file with mode: 0644]

index d449afc..0bbc650 100644 (file)
@@ -211,7 +211,7 @@ public:
   }
 
   const llvm::APSInt &getTruthValue(bool b, QualType T) {
-    return getValue(b ? 1 : 0, Ctx.getTypeSize(T), false);
+    return getValue(b ? 1 : 0, Ctx.getIntWidth(T), true);
   }
 
   const llvm::APSInt &getTruthValue(bool b) {
index c3c1cb9..55ee2ce 100644 (file)
@@ -1066,6 +1066,7 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U,
     // constant value. If the UnaryOperator has location type, create the
     // constant with int type and pointer width.
     SVal RHS;
+    SVal Result;
 
     if (U->getType()->isAnyPointerType())
       RHS = svalBuilder.makeArrayIndex(1);
@@ -1074,7 +1075,14 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U,
     else
       RHS = UnknownVal();
 
-    SVal Result = evalBinOp(state, Op, V2, RHS, U->getType());
+    // The use of an operand of type bool with the ++ operators is deprecated
+    // but valid until C++17. And if the operand of the ++ operator is of type
+    // bool, it is set to true until C++17. Note that for '_Bool', it is also
+    // set to true when it encounters ++ operator.
+    if (U->getType()->isBooleanType() && U->isIncrementOp())
+      Result = svalBuilder.makeTruthVal(true, U->getType());
+    else
+      Result = evalBinOp(state, Op, V2, RHS, U->getType());
 
     // Conjure a new symbol if necessary to recover precision.
     if (Result.isUnknown()){
@@ -1096,7 +1104,6 @@ void ExprEngine::VisitIncrementDecrementOperator(const UnaryOperator* U,
           Constraint = svalBuilder.evalEQ(state, SymVal,
                                        svalBuilder.makeZeroVal(U->getType()));
 
-
           state = state->assume(Constraint, false);
           assert(state);
         }
diff --git a/clang/test/Analysis/_Bool-increment-decrement.c b/clang/test/Analysis/_Bool-increment-decrement.c
new file mode 100644 (file)
index 0000000..477b6ed
--- /dev/null
@@ -0,0 +1,140 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify -std=c99 -Dbool=_Bool -Dtrue=1 -Dfalse=0 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify -std=c11 -Dbool=_Bool -Dtrue=1 -Dfalse=0 %s
+extern void clang_analyzer_eval(bool);
+
+void test__Bool_value() {
+  {
+    bool b = true;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = false;
+    clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = -10;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 10;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 10;
+    b++;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 0;
+    b++;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+}
+
+void test__Bool_increment() {
+  {
+    bool b = true;
+    b++;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = false;
+    b++;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = true;
+    ++b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = false;
+    ++b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 0;
+    ++b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 10;
+    ++b;
+    ++b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = -10;
+    ++b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = -1;
+    ++b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+}
+
+void test__Bool_decrement() {
+  {
+    bool b = true;
+    b--;
+    clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = false;
+    b--;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = true;
+    --b;
+    clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = false;
+    --b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 0;
+    --b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 10;
+    --b;
+    clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
+    --b;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = -10;
+    --b;
+    clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 1;
+    --b;
+    clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
+  }
+}
diff --git a/clang/test/Analysis/bool-increment.cpp b/clang/test/Analysis/bool-increment.cpp
new file mode 100644 (file)
index 0000000..93002d3
--- /dev/null
@@ -0,0 +1,84 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify -std=c++98 -Wno-deprecated %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify -std=c++11 -Wno-deprecated %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify -std=c++14 -Wno-deprecated %s
+
+extern void clang_analyzer_eval(bool);
+
+void test_bool_value() {
+  {
+    bool b = true;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = false;
+    clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = -10;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 10;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 10;
+    b++;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 0;
+    b++;
+    clang_analyzer_eval(b == 1); // expected-warning{{TRUE}}
+  }
+}
+
+void test_bool_increment() {
+  {
+    bool b = true;
+    b++;
+    clang_analyzer_eval(b); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = false;
+    b++;
+    clang_analyzer_eval(b); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = true;
+    ++b;
+    clang_analyzer_eval(b); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = false;
+    ++b;
+    clang_analyzer_eval(b); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 0;
+    ++b;
+    clang_analyzer_eval(b); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = 10;
+    ++b;
+    ++b;
+    clang_analyzer_eval(b); // expected-warning{{TRUE}}
+  }
+
+  {
+    bool b = -10;
+    ++b;
+    clang_analyzer_eval(b); // expected-warning{{TRUE}}
+  }
+}