MTC created this revision.
MTC added reviewers: alexshap, NoQ, dcoughlin.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.
Herald added a reviewer: george.karpenkov.

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, set to true.
- Modify `BasicValueFactory::getTruthValue()`, use `getIntWidth()` instead 
`getTypeSize()` and use `unsigned` instead `signed`.


Repository:
  rC Clang

https://reviews.llvm.org/D43741

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/bool.cpp

Index: test/Analysis/bool.cpp
===================================================================
--- /dev/null
+++ test/Analysis/bool.cpp
@@ -0,0 +1,84 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -analyzer-store=region -verify -std=c++11 -Wno-deprecated %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -analyzer-store=region -verify -std=c++14 -Wno-deprecated %s
+
+extern void clang_analyzer_eval(bool);
+extern void clang_analyzer_dump(bool);
+
+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}}
+  }
+}
+
+void test_bool_value() {
+  {
+    bool b = true;
+    clang_analyzer_dump(b); // expected-warning{{1 U1b}}
+  }
+
+  {
+    bool b = false;
+    clang_analyzer_dump(b); // expected-warning{{0 U1b}}
+  }
+
+  {
+    bool b = -10;
+    clang_analyzer_dump(b); // expected-warning{{1 U1b}}
+  }
+
+  {
+    bool b = 10;
+    clang_analyzer_dump(b); // expected-warning{{1 U1b}}
+  }
+
+  {
+    bool b = 10;
+    b++;
+    clang_analyzer_dump(b); // expected-warning{{1 U1b}}
+  }
+
+  {
+    bool b = 0;
+    b++;
+    clang_analyzer_dump(b); // expected-warning{{1 U1b}}
+  }
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -1066,15 +1066,23 @@
     // 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);
     else if (U->getType()->isIntegralOrEnumerationType())
       RHS = svalBuilder.makeIntVal(1, U->getType());
     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 untill C++17. And if the operand of the increment operator is
+    // of type bool, it is set to true untill C++17.
+    if (U->getType()->isBooleanType() && U->isIncrementOp() &&
+        AMgr.getLangOpts().CPlusPlus)
+      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 @@
           Constraint = svalBuilder.evalEQ(state, SymVal,
                                        svalBuilder.makeZeroVal(U->getType()));
 
-
           state = state->assume(Constraint, false);
           assert(state);
         }
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -194,7 +194,7 @@
   }
 
   inline 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);
   }
 
   inline const llvm::APSInt& getTruthValue(bool b) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to