Author: Gabor Marton Date: 2022-06-09T16:13:57+02:00 New Revision: bc2c759aeee7d1560840a24365ce18b8e8b63ded
URL: https://github.com/llvm/llvm-project/commit/bc2c759aeee7d1560840a24365ce18b8e8b63ded DIFF: https://github.com/llvm/llvm-project/commit/bc2c759aeee7d1560840a24365ce18b8e8b63ded.diff LOG: [analyzer] Fix assertion failure after getKnownValue call Depends on D126560. `getKnownValue` 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`. Differential Revision: https://reviews.llvm.org/D127285 Added: clang/test/Analysis/svalbuilder-simplify-no-crash.c Modified: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp index 392251109e7da..785039a186486 100644 --- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -22,6 +22,13 @@ using namespace ento; 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. + // This is an implementation detail. Checkers should use `getKnownValue()` + // instead. + 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 +52,7 @@ class SimpleSValBuilder : public SValBuilder { 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 +72,9 @@ class SimpleSValBuilder : public SValBuilder { 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 +540,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state, 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 +660,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state, } // 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 +707,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state, } // 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 +1195,8 @@ SVal SimpleSValBuilder::evalBinOpLN(ProgramStateRef state, 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 +1212,11 @@ const llvm::APSInt *SimpleSValBuilder::getKnownValue(ProgramStateRef state, 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 +1283,7 @@ SVal SimpleSValBuilder::simplifySValOnce(ProgramStateRef State, SVal V) { 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); diff --git a/clang/test/Analysis/svalbuilder-simplify-no-crash.c b/clang/test/Analysis/svalbuilder-simplify-no-crash.c new file mode 100644 index 0000000000000..b43ccbdbfd100 --- /dev/null +++ b/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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits