martong created this revision.
martong added a reviewer: steakhal.
Herald added subscribers: manas, ASDenysPetrov, gamesh411, dkrupp, donat.nagy,
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware,
xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
martong requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Depends on D126560 <https://reviews.llvm.org/D126560>. `getKnwonValue` has been
changed by the parent patch
in a way that simplification was removed. This is not correct when the
function is called by the Checkers. Thus, a new internal function is
introduced `getConstValue` which simply queries the constraint manager.
This `getConstValue` is used internally in the SimpleSValBuilder when a
binop is evaluated, this way we avoid the recursion into the `Simplifier`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127285
Files:
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
clang/test/Analysis/svalbuilder-simplify-no-crash.c
Index: clang/test/Analysis/svalbuilder-simplify-no-crash.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/svalbuilder-simplify-no-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=debug.ExprInspection \
+// RUN: -verify
+
+// Here, we test that svalbuilder simplification does not produce any
+// assertion failure.
+
+void crashing(long a, _Bool b) {
+ (void)(a & 1 && 0);
+ b = a & 1;
+ (void)(b << 1); // expected-warning{{core.UndefinedBinaryOperatorResult}}
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -22,6 +22,11 @@
namespace {
class SimpleSValBuilder : public SValBuilder {
+ // Query the constraint manager whether the SVal has only one possible
+ // (integer) value. If that is the case, the value is returned. Otherwise,
+ // returns NULL.
+ const llvm::APSInt *getConstValue(ProgramStateRef state, SVal V);
+
// With one `simplifySValOnce` call, a compound symbols might collapse to
// simpler symbol tree that is still possible to further simplify. Thus, we
// do the simplification on a new symbol tree until we reach the simplest
@@ -45,7 +50,7 @@
SVal simplifyUntilFixpoint(ProgramStateRef State, SVal Val);
// Recursively descends into symbolic expressions and replaces symbols
- // with their known values (in the sense of the getKnownValue() method).
+ // with their known values (in the sense of the getConstValue() method).
// We traverse the symbol tree and query the constraint values for the
// sub-trees and if a value is a constant we do the constant folding.
SVal simplifySValOnce(ProgramStateRef State, SVal V);
@@ -65,8 +70,9 @@
SVal evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op,
Loc lhs, NonLoc rhs, QualType resultTy) override;
- /// getKnownValue - evaluates a given SVal. If the SVal has only one possible
- /// (integer) value, that value is returned. Otherwise, returns NULL.
+ /// Evaluates a given SVal by recursively evaluating and
+ /// simplifying the children SVals. If the SVal has only one possible
+ /// (integer) value, that value is returned. Otherwise, returns NULL.
const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
SVal simplifySVal(ProgramStateRef State, SVal V) override;
@@ -532,7 +538,7 @@
llvm::APSInt LHSValue = lhs.castAs<nonloc::ConcreteInt>().getValue();
// If we're dealing with two known constants, just perform the operation.
- if (const llvm::APSInt *KnownRHSValue = getKnownValue(state, rhs)) {
+ if (const llvm::APSInt *KnownRHSValue = getConstValue(state, rhs)) {
llvm::APSInt RHSValue = *KnownRHSValue;
if (BinaryOperator::isComparisonOp(op)) {
// We're looking for a type big enough to compare the two values.
@@ -652,7 +658,7 @@
}
// For now, only handle expressions whose RHS is a constant.
- if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs)) {
+ if (const llvm::APSInt *RHSValue = getConstValue(state, rhs)) {
// If both the LHS and the current expression are additive,
// fold their constants and try again.
if (BinaryOperator::isAdditiveOp(op)) {
@@ -699,7 +705,7 @@
}
// Is the RHS a constant?
- if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
+ if (const llvm::APSInt *RHSValue = getConstValue(state, rhs))
return MakeSymIntVal(Sym, op, *RHSValue, resultTy);
if (Optional<NonLoc> V = tryRearrange(state, op, lhs, rhs, resultTy))
@@ -1187,8 +1193,8 @@
return UnknownVal();
}
-const llvm::APSInt *SimpleSValBuilder::getKnownValue(ProgramStateRef state,
- SVal V) {
+const llvm::APSInt *SimpleSValBuilder::getConstValue(ProgramStateRef state,
+ SVal V) {
if (V.isUnknownOrUndef())
return nullptr;
@@ -1204,6 +1210,11 @@
return nullptr;
}
+const llvm::APSInt *SimpleSValBuilder::getKnownValue(ProgramStateRef state,
+ SVal V) {
+ return getConstValue(state, simplifySVal(state, V));
+}
+
SVal SimpleSValBuilder::simplifyUntilFixpoint(ProgramStateRef State, SVal Val) {
SVal SimplifiedVal = simplifySValOnce(State, Val);
while (SimplifiedVal != Val) {
@@ -1270,7 +1281,7 @@
SVal VisitSymbolData(const SymbolData *S) {
// No cache here.
if (const llvm::APSInt *I =
- SVB.getKnownValue(State, SVB.makeSymbolVal(S)))
+ State->getConstraintManager().getSymVal(State, S))
return Loc::isLocType(S->getType()) ? (SVal)SVB.makeIntLocVal(*I)
: (SVal)SVB.makeIntVal(*I);
return SVB.makeSymbolVal(S);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
