From: Henry Wong Date: Tue, 6 Mar 2018 12:29:09 +0000 (+0000) Subject: [Analyzer] More accurate modeling about the increment operator of the operand with... X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e47b89d1f866ae78034026c6477638dbcd48f025;p=platform%2Fupstream%2Fllvm.git [Analyzer] More accurate modeling about the increment operator of the operand with type bool. 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 --- diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h index d449afc..0bbc650 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h @@ -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) { diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp index c3c1cb9..55ee2ce 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -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 index 0000000..477b6ed --- /dev/null +++ b/clang/test/Analysis/_Bool-increment-decrement.c @@ -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 index 0000000..93002d3 --- /dev/null +++ b/clang/test/Analysis/bool-increment.cpp @@ -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}} + } +}