https://github.com/balazs-benics-sonarsource updated
https://github.com/llvm/llvm-project/pull/144327
From bc7dfc2b4f143c2caa1189db096bf9e4ea76f053 Mon Sep 17 00:00:00 2001
From: Balazs Benics <balazs.ben...@sonarsource.com>
Date: Mon, 16 Jun 2025 12:14:06 +0200
Subject: [PATCH 1/5] [analyzer] Enforce not making overly complicated symbols
CPP-6182
---
.../Core/PathSensitive/SValBuilder.h | 8 +-
.../Core/PathSensitive/SymExpr.h | 19 +--
.../Core/PathSensitive/SymbolManager.h | 127 +++++++++++++-----
.../Core/PathSensitive/Symbols.def | 1 +
clang/lib/StaticAnalyzer/Checkers/Taint.cpp | 2 +-
.../Checkers/TrustNonnullChecker.cpp | 2 +-
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 2 +-
.../Core/RangeConstraintManager.cpp | 11 +-
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp | 43 +++---
.../StaticAnalyzer/Core/SimpleSValBuilder.cpp | 6 +-
.../lib/StaticAnalyzer/Core/SymbolManager.cpp | 13 ++
.../ensure-capped-symbol-complexity.cpp | 58 ++++++++
12 files changed, 211 insertions(+), 81 deletions(-)
create mode 100644 clang/test/Analysis/ensure-capped-symbol-complexity.cpp
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 2911554de9d97..fae78c564e0ab 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -57,6 +57,8 @@ class SValBuilder {
protected:
ASTContext &Context;
+ const AnalyzerOptions &AnOpts;
+
/// Manager of APSInt values.
BasicValueFactory BasicVals;
@@ -68,8 +70,6 @@ class SValBuilder {
ProgramStateManager &StateMgr;
- const AnalyzerOptions &AnOpts;
-
/// The scalar type to use for array indices.
const QualType ArrayIndexTy;
@@ -326,8 +326,8 @@ class SValBuilder {
nonloc::SymbolVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
const SymExpr *rhs, QualType type);
- NonLoc makeNonLoc(const SymExpr *operand, UnaryOperator::Opcode op,
- QualType type);
+ nonloc::SymbolVal makeNonLoc(const SymExpr *operand, UnaryOperator::Opcode
op,
+ QualType type);
/// Create a NonLoc value for cast.
nonloc::SymbolVal makeNonLoc(const SymExpr *operand, QualType fromTy,
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
index 6233a22d2ca2b..11d0a22a31c46 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
@@ -51,9 +51,11 @@ class SymExpr : public llvm::FoldingSetNode {
/// Note, however, that it can't be used in Profile because SymbolManager
/// needs to compute Profile before allocating SymExpr.
const SymbolID Sym;
+ const unsigned Complexity;
protected:
- SymExpr(Kind k, SymbolID Sym) : K(k), Sym(Sym) {}
+ SymExpr(Kind k, SymbolID Sym, unsigned Complexity)
+ : K(k), Sym(Sym), Complexity(Complexity) {}
static bool isValidTypeForSymbol(QualType T) {
// FIXME: Depending on whether we choose to deprecate structural symbols,
@@ -61,8 +63,6 @@ class SymExpr : public llvm::FoldingSetNode {
return !T.isNull() && !T->isVoidType();
}
- mutable unsigned Complexity = 0;
-
public:
virtual ~SymExpr() = default;
@@ -108,7 +108,7 @@ class SymExpr : public llvm::FoldingSetNode {
return llvm::make_range(symbol_iterator(this), symbol_iterator());
}
- virtual unsigned computeComplexity() const = 0;
+ unsigned complexity() const { return Complexity; }
/// Find the region from which this symbol originates.
///
@@ -136,10 +136,15 @@ using SymbolRefSmallVectorTy = SmallVector<SymbolRef, 2>;
/// A symbol representing data which can be stored in a memory location
/// (region).
class SymbolData : public SymExpr {
+ friend class SymbolManager;
void anchor() override;
protected:
- SymbolData(Kind k, SymbolID sym) : SymExpr(k, sym) { assert(classof(this)); }
+ SymbolData(Kind k, SymbolID sym) : SymExpr(k, sym, computeComplexity()) {
+ assert(classof(this));
+ }
+
+ static unsigned computeComplexity(...) { return 1; }
public:
~SymbolData() override = default;
@@ -147,10 +152,6 @@ class SymbolData : public SymExpr {
/// Get a string representation of the kind of the region.
virtual StringRef getKindStr() const = 0;
- unsigned computeComplexity() const override {
- return 1;
- };
-
// Implement isa<T> support.
static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
static constexpr bool classof(Kind K) {
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 7af86cd721e8e..9c27669a61ba9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -15,9 +15,11 @@
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SYMBOLMANAGER_H
#include "clang/AST/Expr.h"
+#include "clang/AST/OperationKinds.h"
#include "clang/AST/Type.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/LLVM.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h"
@@ -26,9 +28,11 @@
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/FoldingSet.h"
#include "llvm/ADT/ImmutableSet.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/Allocator.h"
#include <cassert>
+#include <type_traits>
namespace clang {
@@ -133,6 +137,47 @@ class SymbolConjured : public SymbolData {
static constexpr bool classof(Kind K) { return K == ClassKind; }
};
+/// A symbol representing the result of an expression that became too
+/// complicated. In other words, its complexity surpassed the
+/// MaxSymbolComplexity threshold.
+/// TODO: When the MaxSymbolComplexity is reached, we should propagate the
taint
+/// info to it.
+class SymbolOverlyComplex final : public SymbolData {
+ const SymExpr *OverlyComplicatedSymbol;
+
+ friend class SymExprAllocator;
+
+ SymbolOverlyComplex(SymbolID Sym, const SymExpr *OverlyComplicatedSymbol)
+ : SymbolData(ClassKind, Sym),
+ OverlyComplicatedSymbol(OverlyComplicatedSymbol) {
+ assert(OverlyComplicatedSymbol);
+ }
+
+public:
+ QualType getType() const override {
+ return OverlyComplicatedSymbol->getType();
+ }
+
+ StringRef getKindStr() const override;
+
+ void dumpToStream(raw_ostream &os) const override;
+
+ static void Profile(llvm::FoldingSetNodeID &profile,
+ const SymExpr *OverlyComplicatedSymbol) {
+ profile.AddInteger((unsigned)ClassKind);
+ profile.AddPointer(OverlyComplicatedSymbol);
+ }
+
+ void Profile(llvm::FoldingSetNodeID &profile) override {
+ Profile(profile, OverlyComplicatedSymbol);
+ }
+
+ // Implement isa<T> support.
+ static constexpr Kind ClassKind = SymbolOverlyComplexKind;
+ static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
+ static constexpr bool classof(Kind K) { return K == ClassKind; }
+};
+
/// A symbol representing the value of a MemRegion whose parent region has
/// symbolic value.
class SymbolDerived : public SymbolData {
@@ -285,6 +330,7 @@ class SymbolMetadata : public SymbolData {
/// Represents a cast expression.
class SymbolCast : public SymExpr {
+ friend class SymbolManager;
const SymExpr *Operand;
/// Type of the operand.
@@ -295,20 +341,19 @@ class SymbolCast : public SymExpr {
friend class SymExprAllocator;
SymbolCast(SymbolID Sym, const SymExpr *In, QualType From, QualType To)
- : SymExpr(ClassKind, Sym), Operand(In), FromTy(From), ToTy(To) {
+ : SymExpr(ClassKind, Sym, computeComplexity(In, From, To)), Operand(In),
+ FromTy(From), ToTy(To) {
assert(In);
assert(isValidTypeForSymbol(From));
// FIXME: GenericTaintChecker creates symbols of void type.
// Otherwise, 'To' should also be a valid type.
}
-public:
- unsigned computeComplexity() const override {
- if (Complexity == 0)
- Complexity = 1 + Operand->computeComplexity();
- return Complexity;
+ static unsigned computeComplexity(const SymExpr *In, QualType, QualType) {
+ return In->complexity() + 1;
}
+public:
QualType getType() const override { return ToTy; }
LLVM_ATTRIBUTE_RETURNS_NONNULL
@@ -336,6 +381,7 @@ class SymbolCast : public SymExpr {
/// Represents a symbolic expression involving a unary operator.
class UnarySymExpr : public SymExpr {
+ friend class SymbolManager;
const SymExpr *Operand;
UnaryOperator::Opcode Op;
QualType T;
@@ -343,7 +389,8 @@ class UnarySymExpr : public SymExpr {
friend class SymExprAllocator;
UnarySymExpr(SymbolID Sym, const SymExpr *In, UnaryOperator::Opcode Op,
QualType T)
- : SymExpr(ClassKind, Sym), Operand(In), Op(Op), T(T) {
+ : SymExpr(ClassKind, Sym, computeComplexity(In, Op, T)), Operand(In),
+ Op(Op), T(T) {
// Note, some unary operators are modeled as a binary operator. E.g. ++x is
// modeled as x + 1.
assert((Op == UO_Minus || Op == UO_Not) && "non-supported unary
expression");
@@ -354,13 +401,12 @@ class UnarySymExpr : public SymExpr {
assert(!Loc::isLocType(T) && "unary symbol should be nonloc");
}
-public:
- unsigned computeComplexity() const override {
- if (Complexity == 0)
- Complexity = 1 + Operand->computeComplexity();
- return Complexity;
+ static unsigned computeComplexity(const SymExpr *In, UnaryOperator::Opcode,
+ QualType) {
+ return In->complexity() + 1;
}
+public:
const SymExpr *getOperand() const { return Operand; }
UnaryOperator::Opcode getOpcode() const { return Op; }
QualType getType() const override { return T; }
@@ -391,8 +437,9 @@ class BinarySymExpr : public SymExpr {
QualType T;
protected:
- BinarySymExpr(SymbolID Sym, Kind k, BinaryOperator::Opcode op, QualType t)
- : SymExpr(k, Sym), Op(op), T(t) {
+ BinarySymExpr(SymbolID Sym, Kind k, BinaryOperator::Opcode op, QualType t,
+ unsigned Complexity)
+ : SymExpr(k, Sym, Complexity), Op(op), T(t) {
assert(classof(this));
// Binary expressions are results of arithmetic. Pointer arithmetic is not
// handled by binary expressions, but it is instead handled by applying
@@ -415,7 +462,7 @@ class BinarySymExpr : public SymExpr {
protected:
static unsigned computeOperandComplexity(const SymExpr *Value) {
- return Value->computeComplexity();
+ return Value->complexity();
}
static unsigned computeOperandComplexity(const llvm::APSInt &Value) {
return 1;
@@ -432,17 +479,26 @@ class BinarySymExpr : public SymExpr {
/// Template implementation for all binary symbolic expressions
template <class LHSTYPE, class RHSTYPE, SymExpr::Kind ClassK>
class BinarySymExprImpl : public BinarySymExpr {
+ friend class SymbolManager;
LHSTYPE LHS;
RHSTYPE RHS;
friend class SymExprAllocator;
BinarySymExprImpl(SymbolID Sym, LHSTYPE lhs, BinaryOperator::Opcode op,
RHSTYPE rhs, QualType t)
- : BinarySymExpr(Sym, ClassKind, op, t), LHS(lhs), RHS(rhs) {
+ : BinarySymExpr(Sym, ClassKind, op, t,
+ computeComplexity(lhs, op, rhs, t)),
+ LHS(lhs), RHS(rhs) {
assert(getPointer(lhs));
assert(getPointer(rhs));
}
+ static unsigned computeComplexity(LHSTYPE lhs, BinaryOperator::Opcode,
+ RHSTYPE rhs, QualType) {
+ // FIXME: Should we add 1 to complexity?
+ return computeOperandComplexity(lhs) + computeOperandComplexity(rhs);
+ }
+
public:
void dumpToStream(raw_ostream &os) const override {
dumpToStreamImpl(os, LHS);
@@ -453,13 +509,6 @@ class BinarySymExprImpl : public BinarySymExpr {
LHSTYPE getLHS() const { return LHS; }
RHSTYPE getRHS() const { return RHS; }
- unsigned computeComplexity() const override {
- if (Complexity == 0)
- Complexity =
- computeOperandComplexity(RHS) + computeOperandComplexity(LHS);
- return Complexity;
- }
-
static void Profile(llvm::FoldingSetNodeID &ID, LHSTYPE lhs,
BinaryOperator::Opcode op, RHSTYPE rhs, QualType t) {
ID.AddInteger((unsigned)ClassKind);
@@ -520,27 +569,30 @@ class SymbolManager {
SymExprAllocator Alloc;
BasicValueFactory &BV;
ASTContext &Ctx;
+ const unsigned MaxCompComplexity;
public:
SymbolManager(ASTContext &ctx, BasicValueFactory &bv,
- llvm::BumpPtrAllocator &bpalloc)
- : SymbolDependencies(16), Alloc(bpalloc), BV(bv), Ctx(ctx) {}
+ llvm::BumpPtrAllocator &bpalloc, const AnalyzerOptions &Opts)
+ : SymbolDependencies(16), Alloc(bpalloc), BV(bv), Ctx(ctx),
+ MaxCompComplexity(Opts.MaxSymbolComplexity) {
+ assert(MaxCompComplexity > 0 && "Zero max complexity doesn't make sense");
+ }
static bool canSymbolicate(QualType T);
- /// Create or retrieve a SymExpr of type \p SymExprT for the given arguments.
+ /// Create or retrieve a SymExpr of type \p T for the given arguments.
/// Use the arguments to check for an existing SymExpr and return it,
/// otherwise, create a new one and keep a pointer to it to avoid duplicates.
- template <typename SymExprT, typename... Args>
- const SymExprT *acquire(Args &&...args);
+ template <typename T, typename... Args,
+ typename Ret =
std::conditional_t<SymbolData::classof(T::ClassKind),
+ T, SymExpr>>
+ LLVM_ATTRIBUTE_RETURNS_NONNULL const Ret *acquire(Args &&...args);
const SymbolConjured *conjureSymbol(ConstCFGElementRef Elem,
const LocationContext *LCtx, QualType T,
unsigned VisitCount,
- const void *SymbolTag = nullptr) {
-
- return acquire<SymbolConjured>(Elem, LCtx, T, VisitCount, SymbolTag);
- }
+ const void *SymbolTag = nullptr);
QualType getType(const SymExpr *SE) const {
return SE->getType();
@@ -673,8 +725,9 @@ class SymbolVisitor {
virtual bool VisitMemRegion(const MemRegion *) { return true; }
};
-template <typename T, typename... Args>
-const T *SymbolManager::acquire(Args &&...args) {
+// Returns a const pointer to T if T is a SymbolData, otherwise SymExpr.
+template <typename T, typename... Args, typename Ret>
+const Ret *SymbolManager::acquire(Args &&...args) {
llvm::FoldingSetNodeID profile;
T::Profile(profile, args...);
void *InsertPos;
@@ -682,8 +735,12 @@ const T *SymbolManager::acquire(Args &&...args) {
if (!SD) {
SD = Alloc.make<T>(std::forward<Args>(args)...);
DataSet.InsertNode(SD, InsertPos);
+ if (SD->complexity() > MaxCompComplexity) {
+ return cast<Ret>(acquire<SymbolOverlyComplex>(SD));
+ }
}
- return cast<T>(SD);
+
+ return cast<Ret>(SD);
}
} // namespace ento
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
index b93f8e2501559..1c96e05f05b7d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
@@ -45,6 +45,7 @@ SYMBOL(SymbolCast, SymExpr)
ABSTRACT_SYMBOL(SymbolData, SymExpr)
SYMBOL(SymbolConjured, SymbolData)
+ SYMBOL(SymbolOverlyComplex, SymbolData)
SYMBOL(SymbolDerived, SymbolData)
SYMBOL(SymbolExtent, SymbolData)
SYMBOL(SymbolMetadata, SymbolData)
diff --git a/clang/lib/StaticAnalyzer/Checkers/Taint.cpp
b/clang/lib/StaticAnalyzer/Checkers/Taint.cpp
index e55d064253b84..f0dc889f15e7a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/Taint.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/Taint.cpp
@@ -267,7 +267,7 @@ std::vector<SymbolRef>
taint::getTaintedSymbolsImpl(ProgramStateRef State,
//
HACK:https://discourse.llvm.org/t/rfc-make-istainted-and-complex-symbols-friends/79570
if (const auto &Opts = State->getAnalysisManager().getAnalyzerOptions();
- Sym->computeComplexity() > Opts.MaxTaintedSymbolComplexity) {
+ Sym->complexity() > Opts.MaxTaintedSymbolComplexity) {
return {};
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
index e2f8bd541c967..ab0e3d8f56d86 100644
--- a/clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
@@ -66,7 +66,7 @@ class TrustNonnullChecker : public Checker<check::PostCall,
SVal Cond,
bool Assumption) const {
const SymbolRef CondS = Cond.getAsSymbol();
- if (!CondS || CondS->computeComplexity() > ComplexityThreshold)
+ if (!CondS || CondS->complexity() > ComplexityThreshold)
return State;
for (SymbolRef Antecedent : CondS->symbols()) {
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index fa8e669b6bb2f..3486485dcd686 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -67,7 +67,7 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
if (RightV.isUnknown()) {
unsigned Count = currBldrCtx->blockCount();
RightV = svalBuilder.conjureSymbolVal(nullptr, getCFGElementRef(),
LCtx,
- Count);
+ RHS->getType(), Count);
}
// Simulate the effects of a "store": bind the value of the RHS
// to the L-Value represented by the LHS.
diff --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index ab45e678bafd5..0e84a7046ad08 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1264,7 +1264,9 @@ class SymbolicRangeInferrer
private:
SymbolicRangeInferrer(RangeSet::Factory &F, ProgramStateRef S)
- : ValueFactory(F.getValueFactory()), RangeFactory(F), State(S) {}
+ : SVB(S->getStateManager().getSValBuilder()),
+ SymMgr(S->getSymbolManager()), ValueFactory(F.getValueFactory()),
+ RangeFactory(F), State(S) {}
/// Infer range information from the given integer constant.
///
@@ -1525,8 +1527,6 @@ class SymbolicRangeInferrer
const SymExpr *RHS = SSE->getRHS();
QualType T = SSE->getType();
- SymbolManager &SymMgr = State->getSymbolManager();
-
// We use this variable to store the last queried operator (`QueriedOP`)
// for which the `getCmpOpState` returned with `Unknown`. If there are two
// different OPs that returned `Unknown` then we have to query the special
@@ -1540,8 +1540,7 @@ class SymbolicRangeInferrer
// Let's find an expression e.g. (x < y).
BinaryOperatorKind QueriedOP = OperatorRelationsTable::getOpFromIndex(i);
- const SymSymExpr *SymSym =
- SymMgr.acquire<SymSymExpr>(LHS, QueriedOP, RHS, T);
+ SymbolRef SymSym = SymMgr.acquire<SymSymExpr>(LHS, QueriedOP, RHS, T);
const RangeSet *QueriedRangeSet = getConstraint(State, SymSym);
// If ranges were not previously found,
@@ -1622,6 +1621,8 @@ class SymbolicRangeInferrer
return RangeSet(RangeFactory, Zero);
}
+ SValBuilder &SVB;
+ SymbolManager &SymMgr;
BasicValueFactory &ValueFactory;
RangeSet::Factory &RangeFactory;
ProgramStateRef State;
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index 2276c452cce76..a0f959d8adc1a 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -51,11 +51,11 @@ void SValBuilder::anchor() {}
SValBuilder::SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
ProgramStateManager &stateMgr)
- : Context(context), BasicVals(context, alloc),
- SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
- StateMgr(stateMgr),
+ : Context(context),
AnOpts(
stateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions()),
+ BasicVals(context, alloc), SymMgr(context, BasicVals, alloc, AnOpts),
+ MemMgr(context, alloc), StateMgr(stateMgr),
ArrayIndexTy(context.LongLongTy),
ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
@@ -77,6 +77,9 @@ DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) {
nonloc::SymbolVal SValBuilder::makeNonLoc(const SymExpr *lhs,
BinaryOperator::Opcode op,
APSIntPtr rhs, QualType type) {
+ // The Environment ensures we always get a persistent APSInt in
+ // BasicValueFactory, so we don't need to get the APSInt from
+ // BasicValueFactory again.
assert(lhs);
assert(!Loc::isLocType(type));
return nonloc::SymbolVal(SymMgr.acquire<SymIntExpr>(lhs, op, rhs, type));
@@ -98,8 +101,9 @@ nonloc::SymbolVal SValBuilder::makeNonLoc(const SymExpr *lhs,
return nonloc::SymbolVal(SymMgr.acquire<SymSymExpr>(lhs, op, rhs, type));
}
-NonLoc SValBuilder::makeNonLoc(const SymExpr *operand, UnaryOperator::Opcode
op,
- QualType type) {
+nonloc::SymbolVal SValBuilder::makeNonLoc(const SymExpr *operand,
+ UnaryOperator::Opcode op,
+ QualType type) {
assert(operand);
assert(!Loc::isLocType(type));
return nonloc::SymbolVal(SymMgr.acquire<UnarySymExpr>(operand, op, type));
@@ -432,24 +436,17 @@ SVal SValBuilder::makeSymExprValNN(BinaryOperator::Opcode
Op,
QualType ResultTy) {
SymbolRef symLHS = LHS.getAsSymbol();
SymbolRef symRHS = RHS.getAsSymbol();
+ auto lInt = LHS.getAs<nonloc::ConcreteInt>();
+ auto rInt = RHS.getAs<nonloc::ConcreteInt>();
- // TODO: When the Max Complexity is reached, we should conjure a symbol
- // instead of generating an Unknown value and propagate the taint info to it.
- const unsigned MaxComp = AnOpts.MaxSymbolComplexity;
-
- if (symLHS && symRHS &&
- (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
+ if (symLHS && symRHS)
return makeNonLoc(symLHS, Op, symRHS, ResultTy);
- if (symLHS && symLHS->computeComplexity() < MaxComp)
- if (std::optional<nonloc::ConcreteInt> rInt =
- RHS.getAs<nonloc::ConcreteInt>())
- return makeNonLoc(symLHS, Op, rInt->getValue(), ResultTy);
+ if (symLHS && rInt)
+ return makeNonLoc(symLHS, Op, rInt->getValue(), ResultTy);
- if (symRHS && symRHS->computeComplexity() < MaxComp)
- if (std::optional<nonloc::ConcreteInt> lInt =
- LHS.getAs<nonloc::ConcreteInt>())
- return makeNonLoc(lInt->getValue(), Op, symRHS, ResultTy);
+ if (lInt && symRHS)
+ return makeNonLoc(lInt->getValue(), Op, symRHS, ResultTy);
return UnknownVal();
}
@@ -614,10 +611,12 @@ SVal SValBuilder::evalIntegralCast(ProgramStateRef state,
SVal val,
// Check the range of the symbol being casted against the maximum value of
the
// target type.
QualType CmpTy = getConditionType();
- NonLoc CompVal = evalBinOpNN(state, BO_LE, *AsNonLoc, ToTypeMaxVal, CmpTy)
- .castAs<NonLoc>();
+ auto CompVal =
+ evalBinOpNN(state, BO_LE, *AsNonLoc, ToTypeMaxVal,
CmpTy).getAs<NonLoc>();
+ if (!CompVal)
+ return UnknownVal();
ProgramStateRef IsNotTruncated, IsTruncated;
- std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
+ std::tie(IsNotTruncated, IsTruncated) = state->assume(*CompVal);
if (!IsNotTruncated && IsTruncated) {
// Symbol is truncated so we evaluate it as a cast.
return makeNonLoc(AsSymbol, originalTy, castTy);
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 84a9c43d3572e..ef8c3b6b4bdac 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -291,9 +291,9 @@ static std::pair<SymbolRef, APSIntPtr>
decomposeSymbol(SymbolRef Sym,
// same signed integral type and no overflows occur (which should be checked
// by the caller).
static NonLoc doRearrangeUnchecked(ProgramStateRef State,
- BinaryOperator::Opcode Op,
- SymbolRef LSym, llvm::APSInt LInt,
- SymbolRef RSym, llvm::APSInt RInt) {
+ BinaryOperator::Opcode Op, SymbolRef LSym,
+ llvm::APSInt LInt, SymbolRef RSym,
+ llvm::APSInt RInt) {
SValBuilder &SVB = State->getStateManager().getSValBuilder();
BasicValueFactory &BV = SVB.getBasicValueFactory();
SymbolManager &SymMgr = SVB.getSymbolManager();
diff --git a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
index d03f47fa301e6..2149413189dcd 100644
--- a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -32,6 +32,7 @@ using namespace ento;
void SymExpr::anchor() {}
StringRef SymbolConjured::getKindStr() const { return "conj_$"; }
+StringRef SymbolOverlyComplex::getKindStr() const { return "complex_$"; }
StringRef SymbolDerived::getKindStr() const { return "derived_$"; }
StringRef SymbolExtent::getKindStr() const { return "extent_$"; }
StringRef SymbolMetadata::getKindStr() const { return "meta_$"; }
@@ -128,6 +129,10 @@ void SymbolConjured::dumpToStream(raw_ostream &os) const {
os << ", #" << Count << '}';
}
+void SymbolOverlyComplex::dumpToStream(raw_ostream &os) const {
+ os << getKindStr() << getSymbolID();
+}
+
void SymbolDerived::dumpToStream(raw_ostream &os) const {
os << getKindStr() << getSymbolID() << '{' << getParentSymbol() << ','
<< getRegion() << '}';
@@ -176,6 +181,7 @@ void SymExpr::symbol_iterator::expand() {
switch (SE->getKind()) {
case SymExpr::SymbolRegionValueKind:
case SymExpr::SymbolConjuredKind:
+ case SymExpr::SymbolOverlyComplexKind:
case SymExpr::SymbolDerivedKind:
case SymExpr::SymbolExtentKind:
case SymExpr::SymbolMetadataKind:
@@ -238,6 +244,12 @@ bool SymbolManager::canSymbolicate(QualType T) {
return false;
}
+const SymbolConjured *SymbolManager::conjureSymbol(
+ ConstCFGElementRef Elem, const LocationContext *LCtx, QualType T,
+ unsigned VisitCount, const void *SymbolTag /*=nullptr*/) {
+ return acquire<SymbolConjured>(Elem, LCtx, T, VisitCount, SymbolTag);
+}
+
void SymbolManager::addSymbolDependency(const SymbolRef Primary,
const SymbolRef Dependent) {
auto &dependencies = SymbolDependencies[Primary];
@@ -346,6 +358,7 @@ bool SymbolReaper::isLive(SymbolRef sym) {
KnownLive = isReadableRegion(cast<SymbolRegionValue>(sym)->getRegion());
break;
case SymExpr::SymbolConjuredKind:
+ case SymExpr::SymbolOverlyComplexKind:
KnownLive = false;
break;
case SymExpr::SymbolDerivedKind:
diff --git a/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
b/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
new file mode 100644
index 0000000000000..bd076f2709422
--- /dev/null
+++ b/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection %s -verify
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ConfigDumper 2>&1 |
FileCheck %s --match-full-lines
+// CHECK: max-symbol-complexity = 35
+
+void clang_analyzer_dump(int v);
+
+void pumpSymbolComplexity() {
+ extern int *p;
+ *p = (*p + 1) & 1023; // 2
+ *p = (*p + 1) & 1023; // 4
+ *p = (*p + 1) & 1023; // 6
+ *p = (*p + 1) & 1023; // 8
+ *p = (*p + 1) & 1023; // 10
+ *p = (*p + 1) & 1023; // 12
+ *p = (*p + 1) & 1023; // 14
+ *p = (*p + 1) & 1023; // 16
+ *p = (*p + 1) & 1023; // 18
+ *p = (*p + 1) & 1023; // 20
+ *p = (*p + 1) & 1023; // 22
+ *p = (*p + 1) & 1023; // 24
+ *p = (*p + 1) & 1023; // 26
+ *p = (*p + 1) & 1023; // 28
+ *p = (*p + 1) & 1023; // 30
+ *p = (*p + 1) & 1023; // 32
+ *p = (*p + 1) & 1023; // 34
+
+ // The complexity of "*p" is below 35, so it's accurate.
+ clang_analyzer_dump(*p);
+ // expected-warning-re@-1 {{{{^\({34}reg}}}}
+
+ // We would increase the complexity over the threshold, thus it'll get
simplified.
+ *p = (*p + 1) & 1023; // Would be 36, which is over 35.
+
+ // This dump used to print a hugely complicated symbol, over 800 complexity,
taking really long to simplify.
+ clang_analyzer_dump(*p);
+ // expected-warning-re@-1 {{{{^}}(complex_${{[0-9]+}}) & 1023}}
[debug.ExprInspection]{{$}}}}
+}
+
+void hugelyOverComplicatedSymbol() {
+#define TEN_TIMES(x) x x x x x x x x x x
+#define HUNDRED_TIMES(x) TEN_TIMES(TEN_TIMES(x))
+ extern int *p;
+ HUNDRED_TIMES(*p = (*p + 1) & 1023;)
+ HUNDRED_TIMES(*p = (*p + 1) & 1023;)
+ HUNDRED_TIMES(*p = (*p + 1) & 1023;)
+ HUNDRED_TIMES(*p = (*p + 1) & 1023;)
+ *p = (*p + 1) & 1023;
+ *p = (*p + 1) & 1023;
+ *p = (*p + 1) & 1023;
+ *p = (*p + 1) & 1023;
+
+ // This dump used to print a hugely complicated symbol, over 800 complexity,
taking really long to simplify.
+ clang_analyzer_dump(*p);
+ // expected-warning-re@-1 {{{{^}}(((complex_${{[0-9]+}}) & 1023) + 1) & 1023
[debug.ExprInspection]{{$}}}}
+#undef HUNDRED_TIMES
+#undef TEN_TIMES
+}
From 8c29bbc6c309efe0a1d6af948f95dbb9654971b8 Mon Sep 17 00:00:00 2001
From: Balazs Benics <balazs.ben...@sonarsource.com>
Date: Fri, 27 Jun 2025 08:06:16 +0200
Subject: [PATCH 2/5] Fix regression (mostly)
---
.../Core/PathSensitive/SymbolManager.h | 47 +++++++++++++++----
.../ensure-capped-symbol-complexity.cpp | 35 ++++++++++++++
2 files changed, 73 insertions(+), 9 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 9c27669a61ba9..6a0f4ac528a5e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -49,6 +49,7 @@ class SymbolRegionValue : public SymbolData {
const TypedValueRegion *R;
friend class SymExprAllocator;
+ friend class SymbolManager;
SymbolRegionValue(SymbolID sym, const TypedValueRegion *r)
: SymbolData(ClassKind, sym), R(r) {
assert(r);
@@ -91,6 +92,7 @@ class SymbolConjured : public SymbolData {
const void *SymbolTag;
friend class SymExprAllocator;
+ friend class SymbolManager;
SymbolConjured(SymbolID sym, ConstCFGElementRef elem,
const LocationContext *lctx, QualType t, unsigned count,
const void *symbolTag)
@@ -146,6 +148,7 @@ class SymbolOverlyComplex final : public SymbolData {
const SymExpr *OverlyComplicatedSymbol;
friend class SymExprAllocator;
+ friend class SymbolManager;
SymbolOverlyComplex(SymbolID Sym, const SymExpr *OverlyComplicatedSymbol)
: SymbolData(ClassKind, Sym),
@@ -185,6 +188,7 @@ class SymbolDerived : public SymbolData {
const TypedValueRegion *R;
friend class SymExprAllocator;
+ friend class SymbolManager;
SymbolDerived(SymbolID sym, SymbolRef parent, const TypedValueRegion *r)
: SymbolData(ClassKind, sym), parentSymbol(parent), R(r) {
assert(parent);
@@ -229,6 +233,7 @@ class SymbolExtent : public SymbolData {
const SubRegion *R;
friend class SymExprAllocator;
+ friend class SymbolManager;
SymbolExtent(SymbolID sym, const SubRegion *r)
: SymbolData(ClassKind, sym), R(r) {
assert(r);
@@ -274,6 +279,7 @@ class SymbolMetadata : public SymbolData {
const void *Tag;
friend class SymExprAllocator;
+ friend class SymbolManager;
SymbolMetadata(SymbolID sym, const MemRegion *r, const Stmt *s, QualType t,
const LocationContext *LCtx, unsigned count, const void *tag)
: SymbolData(ClassKind, sym), R(r), S(s), T(t), LCtx(LCtx), Count(count),
@@ -340,6 +346,7 @@ class SymbolCast : public SymExpr {
QualType ToTy;
friend class SymExprAllocator;
+ friend class SymbolManager;
SymbolCast(SymbolID Sym, const SymExpr *In, QualType From, QualType To)
: SymExpr(ClassKind, Sym, computeComplexity(In, From, To)), Operand(In),
FromTy(From), ToTy(To) {
@@ -387,6 +394,7 @@ class UnarySymExpr : public SymExpr {
QualType T;
friend class SymExprAllocator;
+ friend class SymbolManager;
UnarySymExpr(SymbolID Sym, const SymExpr *In, UnaryOperator::Opcode Op,
QualType T)
: SymExpr(ClassKind, Sym, computeComplexity(In, Op, T)), Operand(In),
@@ -484,6 +492,7 @@ class BinarySymExprImpl : public BinarySymExpr {
RHSTYPE RHS;
friend class SymExprAllocator;
+ friend class SymbolManager;
BinarySymExprImpl(SymbolID Sym, LHSTYPE lhs, BinaryOperator::Opcode op,
RHSTYPE rhs, QualType t)
: BinarySymExpr(Sym, ClassKind, op, t,
@@ -728,18 +737,38 @@ class SymbolVisitor {
// Returns a const pointer to T if T is a SymbolData, otherwise SymExpr.
template <typename T, typename... Args, typename Ret>
const Ret *SymbolManager::acquire(Args &&...args) {
- llvm::FoldingSetNodeID profile;
- T::Profile(profile, args...);
- void *InsertPos;
- SymExpr *SD = DataSet.FindNodeOrInsertPos(profile, InsertPos);
- if (!SD) {
- SD = Alloc.make<T>(std::forward<Args>(args)...);
- DataSet.InsertNode(SD, InsertPos);
- if (SD->complexity() > MaxCompComplexity) {
- return cast<Ret>(acquire<SymbolOverlyComplex>(SD));
+ T Dummy(/*SymbolID=*/0, args...);
+ llvm::FoldingSetNodeID DummyProfile;
+ Dummy.Profile(DummyProfile);
+
+ if (Dummy.complexity() <= MaxCompComplexity) {
+ void *InsertPos;
+ SymExpr *SD = DataSet.FindNodeOrInsertPos(DummyProfile, InsertPos);
+ if (!SD) {
+ SD = Alloc.make<T>(args...);
+ DataSet.InsertNode(SD, InsertPos);
}
+ return cast<Ret>(SD);
+ }
+ void *WrappedSymInsertPos;
+ SymExpr *WrappedSym =
+ DataSet.FindNodeOrInsertPos(DummyProfile, WrappedSymInsertPos);
+ if (!WrappedSym) {
+ WrappedSym = Alloc.make<T>(args...);
+ DataSet.InsertNode(WrappedSym, WrappedSymInsertPos);
}
+ SymbolOverlyComplex OverlyComplexSym(/*SymbolID=*/0, WrappedSym);
+ llvm::FoldingSetNodeID OverlyComplexSymProfile;
+ OverlyComplexSym.Profile(OverlyComplexSymProfile);
+
+ void *OverlyComplexSymInsertPos;
+ SymExpr *SD = DataSet.FindNodeOrInsertPos(OverlyComplexSymProfile,
+ OverlyComplexSymInsertPos);
+ if (!SD) {
+ SD = Alloc.make<SymbolOverlyComplex>(WrappedSym);
+ DataSet.InsertNode(SD, OverlyComplexSymInsertPos);
+ }
return cast<Ret>(SD);
}
diff --git a/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
b/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
index bd076f2709422..2e716900c3847 100644
--- a/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
+++ b/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
@@ -56,3 +56,38 @@ void hugelyOverComplicatedSymbol() {
#undef HUNDRED_TIMES
#undef TEN_TIMES
}
+
+typedef unsigned long long __attribute__((aligned((8)))) u64a;
+u64a compress64(u64a x, u64a m) {
+ if ((x & m) == 0)
+ return 0;
+ x &= m;
+ u64a mk = ~m << 1;
+ for (unsigned i = 0; i < 6; i++) {
+ u64a mp = mk ^ (mk << 1);
+ mp ^= mp << 2;
+ mp ^= mp << 4;
+ mp ^= mp << 8;
+ mp ^= mp << 16;
+ mp ^= mp << 32;
+ u64a mv = mp & m;
+ m = (m ^ mv) | (mv >> (1 << i));
+ u64a t = x & mv;
+ x = (x ^ t) | (t >> (1 << i));
+ mk = mk & ~mp;
+ }
+ return x;
+}
+void storecompressed512_64bit(u64a *m, u64a *x) {
+ u64a v[8] = {
+ compress64(x[0], m[0]),
+ compress64(x[1], m[1]),
+ compress64(x[2], m[2]),
+ compress64(x[3], m[3]),
+ compress64(x[4], m[4]),
+ compress64(x[5], m[5]),
+ compress64(x[6], m[6]),
+ compress64(x[7], m[7]),
+ };
+ (void)v;
+}
From fd4f0379cedef45207a400660b05f89c5c0bc174 Mon Sep 17 00:00:00 2001
From: Balazs Benics <balazs.ben...@sonarsource.com>
Date: Fri, 27 Jun 2025 12:43:57 +0200
Subject: [PATCH 3/5] Just store the hash of the overly complicated wrapped
symbol instead of allocating it
This would bring 3 benefits:
- Now a single `acquire<T>` call only inserts a single item into the
`DataSet`.
- Simplifies the logic slightly.
- We have the invariant that the cache (`DataSet`) only ever contains
symbols that obey the max complexity threshold.
Unfortunately, this performs just as bad like before this commit.
In other words, this did't improve anything but I'll leave it here for
history.
---
.../Core/PathSensitive/SymbolManager.h | 38 ++++++++-----------
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 6a0f4ac528a5e..62a2e68b7ed01 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -145,34 +145,33 @@ class SymbolConjured : public SymbolData {
/// TODO: When the MaxSymbolComplexity is reached, we should propagate the
taint
/// info to it.
class SymbolOverlyComplex final : public SymbolData {
- const SymExpr *OverlyComplicatedSymbol;
+ QualType Ty;
+ unsigned OverlyComplicatedSymbolHash;
friend class SymExprAllocator;
friend class SymbolManager;
- SymbolOverlyComplex(SymbolID Sym, const SymExpr *OverlyComplicatedSymbol)
- : SymbolData(ClassKind, Sym),
- OverlyComplicatedSymbol(OverlyComplicatedSymbol) {
- assert(OverlyComplicatedSymbol);
- }
+ SymbolOverlyComplex(SymbolID Sym, QualType Ty,
+ unsigned OverlyComplicatedSymbolHash)
+ : SymbolData(ClassKind, Sym), Ty(Ty),
+ OverlyComplicatedSymbolHash(OverlyComplicatedSymbolHash) {}
public:
- QualType getType() const override {
- return OverlyComplicatedSymbol->getType();
- }
+ QualType getType() const override { return Ty; }
StringRef getKindStr() const override;
void dumpToStream(raw_ostream &os) const override;
- static void Profile(llvm::FoldingSetNodeID &profile,
- const SymExpr *OverlyComplicatedSymbol) {
+ static void Profile(llvm::FoldingSetNodeID &profile, QualType Ty,
+ unsigned OverlyComplicatedSymbolHash) {
profile.AddInteger((unsigned)ClassKind);
- profile.AddPointer(OverlyComplicatedSymbol);
+ profile.Add(Ty);
+ profile.AddInteger(OverlyComplicatedSymbolHash);
}
void Profile(llvm::FoldingSetNodeID &profile) override {
- Profile(profile, OverlyComplicatedSymbol);
+ Profile(profile, Ty, OverlyComplicatedSymbolHash);
}
// Implement isa<T> support.
@@ -750,15 +749,9 @@ const Ret *SymbolManager::acquire(Args &&...args) {
}
return cast<Ret>(SD);
}
- void *WrappedSymInsertPos;
- SymExpr *WrappedSym =
- DataSet.FindNodeOrInsertPos(DummyProfile, WrappedSymInsertPos);
- if (!WrappedSym) {
- WrappedSym = Alloc.make<T>(args...);
- DataSet.InsertNode(WrappedSym, WrappedSymInsertPos);
- }
- SymbolOverlyComplex OverlyComplexSym(/*SymbolID=*/0, WrappedSym);
+ SymbolOverlyComplex OverlyComplexSym(/*SymbolID=*/0, Dummy.getType(),
+ DummyProfile.ComputeHash());
llvm::FoldingSetNodeID OverlyComplexSymProfile;
OverlyComplexSym.Profile(OverlyComplexSymProfile);
@@ -766,7 +759,8 @@ const Ret *SymbolManager::acquire(Args &&...args) {
SymExpr *SD = DataSet.FindNodeOrInsertPos(OverlyComplexSymProfile,
OverlyComplexSymInsertPos);
if (!SD) {
- SD = Alloc.make<SymbolOverlyComplex>(WrappedSym);
+ SD = Alloc.make<SymbolOverlyComplex>(Dummy.getType(),
+ DummyProfile.ComputeHash());
DataSet.InsertNode(SD, OverlyComplexSymInsertPos);
}
return cast<Ret>(SD);
From f997868752ba275ffe2f664538c90268a942492b Mon Sep 17 00:00:00 2001
From: Balazs Benics <balazs.ben...@sonarsource.com>
Date: Thu, 3 Jul 2025 11:55:07 +0200
Subject: [PATCH 4/5] Only ever use the top-level evalBinOp API
---
.../Core/PathSensitive/SValBuilder.h | 33 +++----
.../Checkers/ArrayBoundChecker.cpp | 14 +--
.../Checkers/CStringChecker.cpp | 96 +++++++++----------
.../Checkers/ContainerModeling.cpp | 12 ++-
.../lib/StaticAnalyzer/Checkers/Iterator.cpp | 12 +--
.../StaticAnalyzer/Checkers/MallocChecker.cpp | 7 +-
.../Checkers/SetgidSetuidOrderChecker.cpp | 6 +-
.../StaticAnalyzer/Checkers/StreamChecker.cpp | 23 +++--
.../Checkers/UnixAPIChecker.cpp | 5 +-
.../Checkers/VLASizeChecker.cpp | 2 +-
.../lib/StaticAnalyzer/Core/ProgramState.cpp | 13 ++-
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp | 2 +-
.../StaticAnalyzer/Core/SimpleSValBuilder.cpp | 29 +++---
clang/test/Analysis/ArrayBound/brief-tests.c | 2 +-
.../Analysis/bitwise-shift-sanity-checks.c | 2 +-
15 files changed, 127 insertions(+), 131 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index fae78c564e0ab..86875a698f43d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -91,22 +91,6 @@ class SValBuilder {
SVal evalMinus(NonLoc val);
SVal evalComplement(NonLoc val);
- /// Create a new value which represents a binary expression with two non-
- /// location operands.
- virtual SVal evalBinOpNN(ProgramStateRef state, BinaryOperator::Opcode op,
- NonLoc lhs, NonLoc rhs, QualType resultTy) = 0;
-
- /// Create a new value which represents a binary expression with two memory
- /// location operands.
- virtual SVal evalBinOpLL(ProgramStateRef state, BinaryOperator::Opcode op,
- Loc lhs, Loc rhs, QualType resultTy) = 0;
-
- /// Create a new value which represents a binary expression with a memory
- /// location and non-location operands. For example, this would be used to
- /// evaluate a pointer arithmetic operation.
- virtual SVal evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op,
- Loc lhs, NonLoc rhs, QualType resultTy) = 0;
-
/// Evaluates a given SVal. If the SVal has only one possible (integer)
value,
/// that value is returned. Otherwise, returns NULL.
virtual const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal val) =
0;
@@ -396,6 +380,23 @@ class SValBuilder {
/// Return a memory region for the 'this' object reference.
loc::MemRegionVal getCXXThis(const CXXRecordDecl *D,
const StackFrameContext *SFC);
+
+protected:
+ /// Create a new value which represents a binary expression with two non-
+ /// location operands.
+ virtual SVal evalBinOpNN(ProgramStateRef state, BinaryOperator::Opcode op,
+ NonLoc lhs, NonLoc rhs, QualType resultTy) = 0;
+
+ /// Create a new value which represents a binary expression with two memory
+ /// location operands.
+ virtual SVal evalBinOpLL(ProgramStateRef state, BinaryOperator::Opcode op,
+ Loc lhs, Loc rhs, QualType resultTy) = 0;
+
+ /// Create a new value which represents a binary expression with a memory
+ /// location and non-location operands. For example, this would be used to
+ /// evaluate a pointer arithmetic operation.
+ virtual SVal evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op,
+ Loc lhs, NonLoc rhs, QualType resultTy) = 0;
};
SValBuilder* createSimpleSValBuilder(llvm::BumpPtrAllocator &alloc,
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index d35031b5c22df..6272bf89bf437 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -188,7 +188,7 @@ computeOffset(ProgramStateRef State, SValBuilder &SVB, SVal
Location) {
QualType T = SVB.getArrayIndexType();
auto EvalBinOp = [&SVB, State, T](BinaryOperatorKind Op, NonLoc L, NonLoc R)
{
// We will use this utility to add and multiply values.
- return SVB.evalBinOpNN(State, Op, L, R, T).getAs<NonLoc>();
+ return SVB.evalBinOp(State, Op, L, R, T).getAs<NonLoc>();
};
const SubRegion *OwnerRegion = nullptr;
@@ -311,12 +311,12 @@ compareValueToThreshold(ProgramStateRef State, NonLoc
Value, NonLoc Threshold,
}
// We want to perform a _mathematical_ comparison between the numbers `Value`
- // and `Threshold`; but `evalBinOpNN` evaluates a C/C++ operator that may
+ // and `Threshold`; but `evalBinOp` evaluates a C/C++ operator that may
// perform automatic conversions. For example the number -1 is less than the
// number 1000, but -1 < `1000ull` will evaluate to `false` because the `int`
// -1 is converted to ULONGLONG_MAX.
// To avoid automatic conversions, we evaluate the "obvious" cases without
- // calling `evalBinOpNN`:
+ // calling `evalBinOp`:
if (isNegative(SVB, State, Value) && isUnsigned(SVB, Threshold)) {
if (CheckEquality) {
// negative_value == unsigned_threshold is always false
@@ -334,7 +334,7 @@ compareValueToThreshold(ProgramStateRef State, NonLoc
Value, NonLoc Threshold,
// comparisons, but in theory there could be contrived situations where
// automatic conversion of a symbolic value (which can be negative and can be
// positive) leads to incorrect results.
- // NOTE: We NEED to use the `evalBinOpNN` call in the "common" case, because
+ // NOTE: We NEED to use the `evalBinOp` call in the "common" case, because
// we want to ensure that assumptions coming from this precondition and
// assumptions coming from regular C/C++ operator calls are represented by
// constraints on the same symbolic expression. A solution that would
@@ -343,7 +343,7 @@ compareValueToThreshold(ProgramStateRef State, NonLoc
Value, NonLoc Threshold,
const BinaryOperatorKind OpKind = CheckEquality ? BO_EQ : BO_LT;
auto BelowThreshold =
- SVB.evalBinOpNN(State, OpKind, Value, Threshold, SVB.getConditionType())
+ SVB.evalBinOp(State, OpKind, Value, Threshold, SVB.getConditionType())
.getAs<NonLoc>();
if (BelowThreshold)
@@ -647,7 +647,7 @@ void ArrayBoundChecker::performCheck(const Expr *E,
CheckerContext &C) const {
// Actually update the state. The "if" only fails in the extremely unlikely
// case when compareValueToThreshold returns {nullptr, nullptr} because
- // evalBinOpNN fails to evaluate the less-than operator.
+ // evalBinOp fails to evaluate the less-than operator.
if (WithinLowerBound)
State = WithinLowerBound;
}
@@ -708,7 +708,7 @@ void ArrayBoundChecker::performCheck(const Expr *E,
CheckerContext &C) const {
// Actually update the state. The "if" only fails in the extremely unlikely
// case when compareValueToThreshold returns {nullptr, nullptr} because
- // evalBinOpNN fails to evaluate the less-than operator.
+ // evalBinOp fails to evaluate the less-than operator.
if (WithinUpperBound)
State = WithinUpperBound;
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 4d12fdcec1f1a..db47290df5d23 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -421,8 +421,7 @@ static std::optional<NonLoc> getIndex(ProgramStateRef State,
SVB.makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(),
SizeTy)
.castAs<NonLoc>();
- SVal Offset =
- SVB.evalBinOpNN(State, BO_Mul, ER->getIndex(), WideSize, SizeTy);
+ SVal Offset = SVB.evalBinOp(State, BO_Mul, ER->getIndex(), WideSize, SizeTy);
if (Offset.isUnknown())
return {};
return Offset.castAs<NonLoc>();
@@ -508,13 +507,13 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext
&C,
// type. This value will be size of the array, or the index to the
// past-the-end element.
std::optional<NonLoc> Offset =
- SVB.evalBinOpNN(State, clang::BO_Div, Size.castAs<NonLoc>(), ElemSize,
- IdxTy)
+ SVB.evalBinOp(State, clang::BO_Div, Size.castAs<NonLoc>(), ElemSize,
+ IdxTy)
.getAs<NonLoc>();
// Retrieve the index of the last element.
const NonLoc One = SVB.makeIntVal(1, IdxTy).castAs<NonLoc>();
- SVal LastIdx = SVB.evalBinOpNN(State, BO_Sub, *Offset, One, IdxTy);
+ SVal LastIdx = SVB.evalBinOp(State, BO_Sub, *Offset, One, IdxTy);
if (!Offset)
return State;
@@ -642,7 +641,7 @@ CStringChecker::CheckBufferAccess(CheckerContext &C,
ProgramStateRef State,
// Compute the offset of the last element to be accessed: size-1.
NonLoc One = svalBuilder.makeIntVal(1, SizeTy).castAs<NonLoc>();
- SVal Offset = svalBuilder.evalBinOpNN(State, BO_Sub, *Length, One, SizeTy);
+ SVal Offset = svalBuilder.evalBinOp(State, BO_Sub, *Length, One, SizeTy);
if (Offset.isUnknown())
return nullptr;
NonLoc LastOffset = Offset.castAs<NonLoc>();
@@ -651,7 +650,7 @@ CStringChecker::CheckBufferAccess(CheckerContext &C,
ProgramStateRef State,
if (std::optional<Loc> BufLoc = BufStart.getAs<Loc>()) {
SVal BufEnd =
- svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
+ svalBuilder.evalBinOp(State, BO_Add, *BufLoc, LastOffset, PtrTy);
State = CheckLocation(C, State, Buffer, BufEnd, Access, CK);
if (Access == AccessKind::read)
State = checkInit(C, State, Buffer, BufEnd, *Length);
@@ -719,7 +718,7 @@ ProgramStateRef CStringChecker::CheckOverlap(CheckerContext
&C,
// Which value comes first?
QualType cmpTy = svalBuilder.getConditionType();
SVal reverse =
- svalBuilder.evalBinOpLL(state, BO_GT, *firstLoc, *secondLoc, cmpTy);
+ svalBuilder.evalBinOp(state, BO_GT, *firstLoc, *secondLoc, cmpTy);
std::optional<DefinedOrUnknownSVal> reverseTest =
reverse.getAs<DefinedOrUnknownSVal>();
if (!reverseTest)
@@ -756,15 +755,15 @@ ProgramStateRef
CStringChecker::CheckOverlap(CheckerContext &C,
return state;
// Compute the end of the first buffer. Bail out if THAT fails.
- SVal FirstEnd = svalBuilder.evalBinOpLN(state, BO_Add, *FirstStartLoc,
- *Length, CharPtrTy);
+ SVal FirstEnd =
+ svalBuilder.evalBinOp(state, BO_Add, *FirstStartLoc, *Length, CharPtrTy);
std::optional<Loc> FirstEndLoc = FirstEnd.getAs<Loc>();
if (!FirstEndLoc)
return state;
// Is the end of the first buffer past the start of the second buffer?
SVal Overlap =
- svalBuilder.evalBinOpLL(state, BO_GT, *FirstEndLoc, *secondLoc, cmpTy);
+ svalBuilder.evalBinOp(state, BO_GT, *FirstEndLoc, *secondLoc, cmpTy);
std::optional<DefinedOrUnknownSVal> OverlapTest =
Overlap.getAs<DefinedOrUnknownSVal>();
if (!OverlapTest)
@@ -925,21 +924,19 @@ ProgramStateRef
CStringChecker::checkAdditionOverflow(CheckerContext &C,
SVal maxMinusRight;
if (isa<nonloc::ConcreteInt>(right)) {
- maxMinusRight = svalBuilder.evalBinOpNN(state, BO_Sub, maxVal, right,
- sizeTy);
+ maxMinusRight = svalBuilder.evalBinOp(state, BO_Sub, maxVal, right,
sizeTy);
} else {
// Try switching the operands. (The order of these two assignments is
// important!)
- maxMinusRight = svalBuilder.evalBinOpNN(state, BO_Sub, maxVal, left,
- sizeTy);
+ maxMinusRight = svalBuilder.evalBinOp(state, BO_Sub, maxVal, left, sizeTy);
left = right;
}
if (std::optional<NonLoc> maxMinusRightNL = maxMinusRight.getAs<NonLoc>()) {
QualType cmpTy = svalBuilder.getConditionType();
// If left > max - right, we have an overflow.
- SVal willOverflow = svalBuilder.evalBinOpNN(state, BO_GT, left,
- *maxMinusRightNL, cmpTy);
+ SVal willOverflow =
+ svalBuilder.evalBinOp(state, BO_GT, left, *maxMinusRightNL, cmpTy);
ProgramStateRef stateOverflow, stateOkay;
std::tie(stateOverflow, stateOkay) =
@@ -1029,8 +1026,8 @@ SVal
CStringChecker::getCStringLengthForRegion(CheckerContext &C,
std::optional<APSIntPtr> maxLengthInt =
BVF.evalAPSInt(BO_Div, maxValInt, fourInt);
NonLoc maxLength = svalBuilder.makeIntVal(*maxLengthInt);
- SVal evalLength = svalBuilder.evalBinOpNN(state, BO_LE, *strLn,
maxLength,
-
svalBuilder.getConditionType());
+ SVal evalLength = svalBuilder.evalBinOp(state, BO_LE, *strLn, maxLength,
+ svalBuilder.getConditionType());
state = state->assume(evalLength.castAs<DefinedOrUnknownSVal>(), true);
}
state = state->set<CStringLength>(MR, strLength);
@@ -1170,7 +1167,7 @@ bool CStringChecker::isFirstBufInBound(CheckerContext &C,
ProgramStateRef State,
// Compute the offset of the last element to be accessed: size-1.
NonLoc One = SB.makeIntVal(1, LengthTy).castAs<NonLoc>();
- SVal Offset = SB.evalBinOpNN(State, BO_Sub, *Length, One, LengthTy);
+ SVal Offset = SB.evalBinOp(State, BO_Sub, *Length, One, LengthTy);
if (Offset.isUnknown())
return true; // cf top comment
NonLoc LastOffset = Offset.castAs<NonLoc>();
@@ -1181,7 +1178,7 @@ bool CStringChecker::isFirstBufInBound(CheckerContext &C,
ProgramStateRef State,
if (!BufLoc)
return true; // cf top comment.
- SVal BufEnd = SB.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
+ SVal BufEnd = SB.evalBinOp(State, BO_Add, *BufLoc, LastOffset, PtrTy);
// Check for out of bound array element access.
const MemRegion *R = BufEnd.getAsRegion();
@@ -1749,7 +1746,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext
&C,
// Check if the strLength is greater than the maxlen.
std::tie(stateStringTooLong, stateStringNotTooLong) = state->assume(
C.getSValBuilder()
- .evalBinOpNN(state, BO_GT, *strLengthNL, *maxlenValNL, cmpTy)
+ .evalBinOp(state, BO_GT, *strLengthNL, *maxlenValNL, cmpTy)
.castAs<DefinedOrUnknownSVal>());
if (stateStringTooLong && !stateStringNotTooLong) {
@@ -1770,15 +1767,19 @@ void CStringChecker::evalstrLengthCommon(CheckerContext
&C,
NonLoc resultNL = result.castAs<NonLoc>();
if (strLengthNL) {
- state = state->assume(C.getSValBuilder().evalBinOpNN(
- state, BO_LE, resultNL, *strLengthNL, cmpTy)
- .castAs<DefinedOrUnknownSVal>(), true);
+ state = state->assume(
+ C.getSValBuilder()
+ .evalBinOp(state, BO_LE, resultNL, *strLengthNL, cmpTy)
+ .castAs<DefinedOrUnknownSVal>(),
+ true);
}
if (maxlenValNL) {
- state = state->assume(C.getSValBuilder().evalBinOpNN(
- state, BO_LE, resultNL, *maxlenValNL, cmpTy)
- .castAs<DefinedOrUnknownSVal>(), true);
+ state = state->assume(
+ C.getSValBuilder()
+ .evalBinOp(state, BO_LE, resultNL, *maxlenValNL, cmpTy)
+ .castAs<DefinedOrUnknownSVal>(),
+ true);
}
}
@@ -1950,8 +1951,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
// If the bound is equal to the source length, strncpy won't null-
// terminate the result!
std::tie(stateSourceTooLong, stateSourceNotTooLong) = state->assume(
- svalBuilder
- .evalBinOpNN(state, BO_GE, *strLengthNL, *lenValNL, cmpTy)
+ svalBuilder.evalBinOp(state, BO_GE, *strLengthNL, *lenValNL, cmpTy)
.castAs<DefinedOrUnknownSVal>());
if (stateSourceTooLong && !stateSourceNotTooLong) {
@@ -1972,8 +1972,8 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
return;
// amountCopied = min (size - dstLen - 1 , srcLen)
- SVal freeSpace = svalBuilder.evalBinOpNN(state, BO_Sub, *lenValNL,
- *dstStrLengthNL, sizeTy);
+ SVal freeSpace = svalBuilder.evalBinOp(state, BO_Sub, *lenValNL,
+ *dstStrLengthNL, sizeTy);
if (!isa<NonLoc>(freeSpace))
return;
freeSpace =
@@ -1985,8 +1985,8 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
// too complex to compute, let's check whether it succeeded.
if (!freeSpaceNL)
return;
- SVal hasEnoughSpace = svalBuilder.evalBinOpNN(
- state, BO_LE, *strLengthNL, *freeSpaceNL, cmpTy);
+ SVal hasEnoughSpace = svalBuilder.evalBinOp(state, BO_LE, *strLengthNL,
+ *freeSpaceNL, cmpTy);
ProgramStateRef TrueState, FalseState;
std::tie(TrueState, FalseState) =
@@ -2020,8 +2020,8 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
return;
if (dstStrLengthNL) {
- maxLastElementIndex = svalBuilder.evalBinOpNN(
- state, BO_Add, *lenValNL, *dstStrLengthNL, sizeTy);
+ maxLastElementIndex = svalBuilder.evalBinOp(state, BO_Add, *lenValNL,
+ *dstStrLengthNL, sizeTy);
boundWarning = "Size argument is greater than the free space in the "
"destination buffer";
@@ -2068,7 +2068,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
// be sure. We won't warn on a possible zero.
NonLoc one = svalBuilder.makeIntVal(1, sizeTy).castAs<NonLoc>();
maxLastElementIndex =
- svalBuilder.evalBinOpNN(state, BO_Sub, *lenValNL, one, sizeTy);
+ svalBuilder.evalBinOp(state, BO_Sub, *lenValNL, one, sizeTy);
boundWarning = "Size argument is greater than the length of the "
"destination buffer";
break;
@@ -2103,8 +2103,8 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
return;
if (appendK == ConcatFnKind::strlcat && dstStrLengthNL && strLengthNL) {
- strlRetVal = svalBuilder.evalBinOpNN(state, BO_Add, *strLengthNL,
- *dstStrLengthNL, sizeTy);
+ strlRetVal = svalBuilder.evalBinOp(state, BO_Add, *strLengthNL,
+ *dstStrLengthNL, sizeTy);
}
std::optional<NonLoc> amountCopiedNL = amountCopied.getAs<NonLoc>();
@@ -2116,8 +2116,8 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
if (!state)
return;
- finalStrLength = svalBuilder.evalBinOpNN(state, BO_Add, *amountCopiedNL,
- *dstStrLengthNL, sizeTy);
+ finalStrLength = svalBuilder.evalBinOp(state, BO_Add, *amountCopiedNL,
+ *dstStrLengthNL, sizeTy);
}
// If we couldn't get a single value for the final string length,
@@ -2134,7 +2134,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
if (amountCopiedNL && appendK == ConcatFnKind::none) {
// we overwrite dst string with the src
// finalStrLength >= srcStrLength
- SVal sourceInResult = svalBuilder.evalBinOpNN(
+ SVal sourceInResult = svalBuilder.evalBinOp(
state, BO_GE, *finalStrLengthNL, *amountCopiedNL, cmpTy);
state = state->assume(sourceInResult.castAs<DefinedOrUnknownSVal>(),
true);
@@ -2145,10 +2145,8 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
if (dstStrLengthNL && appendK != ConcatFnKind::none) {
// we extend the dst string with the src
// finalStrLength >= dstStrLength
- SVal destInResult = svalBuilder.evalBinOpNN(state, BO_GE,
- *finalStrLengthNL,
- *dstStrLengthNL,
- cmpTy);
+ SVal destInResult = svalBuilder.evalBinOp(
+ state, BO_GE, *finalStrLengthNL, *dstStrLengthNL, cmpTy);
state =
state->assume(destInResult.castAs<DefinedOrUnknownSVal>(), true);
if (!state)
@@ -2189,7 +2187,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
// overflows, rather than our estimate about how much is actually copied.
if (std::optional<NonLoc> maxLastNL = maxLastElementIndex.getAs<NonLoc>())
{
SVal maxLastElement =
- svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal, *maxLastNL,
ptrTy);
+ svalBuilder.evalBinOp(state, BO_Add, *dstRegVal, *maxLastNL, ptrTy);
// Check if the first byte of the destination is writable.
state = CheckLocation(C, state, Dst, DstVal, AccessKind::write);
@@ -2203,8 +2201,8 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C,
const CallEvent &Call,
// Then, if the final length is known...
if (std::optional<NonLoc> knownStrLength = finalStrLength.getAs<NonLoc>())
{
- SVal lastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal,
- *knownStrLength, ptrTy);
+ SVal lastElement = svalBuilder.evalBinOp(state, BO_Add, *dstRegVal,
+ *knownStrLength, ptrTy);
// ...and we haven't checked the bound, we'll check the actual copy.
if (!boundWarning) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index e59a0efcf8692..86aafeb108544 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -1034,16 +1034,18 @@ SymbolRef rebaseSymbol(ProgramStateRef State,
SValBuilder &SVB,
SymbolRef OrigExpr, SymbolRef OldExpr,
SymbolRef NewSym) {
auto &SymMgr = SVB.getSymbolManager();
- auto Diff = SVB.evalBinOpNN(State, BO_Sub, nonloc::SymbolVal(OrigExpr),
- nonloc::SymbolVal(OldExpr),
- SymMgr.getType(OrigExpr));
+ auto Diff =
+ SVB.evalBinOp(State, BO_Sub, nonloc::SymbolVal(OrigExpr),
+ nonloc::SymbolVal(OldExpr), SymMgr.getType(OrigExpr));
const auto DiffInt = Diff.getAs<nonloc::ConcreteInt>();
if (!DiffInt)
return OrigExpr;
- return SVB.evalBinOpNN(State, BO_Add, *DiffInt, nonloc::SymbolVal(NewSym),
- SymMgr.getType(OrigExpr)).getAsSymbol();
+ return SVB
+ .evalBinOp(State, BO_Add, *DiffInt, nonloc::SymbolVal(NewSym),
+ SymMgr.getType(OrigExpr))
+ .getAsSymbol();
}
bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
b/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
index e9825b7077c7d..f5aad4c300af5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
@@ -273,9 +273,9 @@ ProgramStateRef assumeNoOverflow(ProgramStateRef State,
SymbolRef Sym,
ProgramStateRef NewState = State;
llvm::APSInt Max = AT.getMaxValue() / AT.getValue(Scale);
- SVal IsCappedFromAbove = SVB.evalBinOpNN(
- State, BO_LE, nonloc::SymbolVal(Sym),
- nonloc::ConcreteInt(BV.getValue(Max)), SVB.getConditionType());
+ SVal IsCappedFromAbove = SVB.evalBinOp(State, BO_LE, nonloc::SymbolVal(Sym),
+ nonloc::ConcreteInt(BV.getValue(Max)),
+ SVB.getConditionType());
if (auto DV = IsCappedFromAbove.getAs<DefinedSVal>()) {
NewState = NewState->assume(*DV, true);
if (!NewState)
@@ -283,9 +283,9 @@ ProgramStateRef assumeNoOverflow(ProgramStateRef State,
SymbolRef Sym,
}
llvm::APSInt Min = -Max;
- SVal IsCappedFromBelow = SVB.evalBinOpNN(
- State, BO_GE, nonloc::SymbolVal(Sym),
- nonloc::ConcreteInt(BV.getValue(Min)), SVB.getConditionType());
+ SVal IsCappedFromBelow = SVB.evalBinOp(State, BO_GE, nonloc::SymbolVal(Sym),
+ nonloc::ConcreteInt(BV.getValue(Min)),
+ SVB.getConditionType());
if (auto DV = IsCappedFromBelow.getAs<DefinedSVal>()) {
NewState = NewState->assume(*DV, true);
if (!NewState)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fa6e8e4146804..9799b31ff567a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1222,9 +1222,8 @@ MallocChecker::performKernelMalloc(const CallEvent &Call,
CheckerContext &C,
NonLoc ZeroFlag = C.getSValBuilder()
.makeIntVal(*KernelZeroFlagVal, FlagsEx->getType())
.castAs<NonLoc>();
- SVal MaskedFlagsUC = C.getSValBuilder().evalBinOpNN(State, BO_And,
- Flags, ZeroFlag,
- FlagsEx->getType());
+ SVal MaskedFlagsUC = C.getSValBuilder().evalBinOp(
+ State, BO_And, Flags, ZeroFlag, FlagsEx->getType());
if (MaskedFlagsUC.isUnknownOrUndef())
return std::nullopt;
DefinedSVal MaskedFlags = MaskedFlagsUC.castAs<DefinedSVal>();
@@ -1917,7 +1916,7 @@ void MallocChecker::checkTaintedness(CheckerContext &C,
const CallEvent &Call,
NonLoc MaxLength =
SVB.makeIntVal(MaxValInt / APSIntType(MaxValInt).getValue(4));
std::optional<NonLoc> SizeNL = SizeSVal.getAs<NonLoc>();
- auto Cmp = SVB.evalBinOpNN(State, BO_GE, *SizeNL, MaxLength, CmpTy)
+ auto Cmp = SVB.evalBinOp(State, BO_GE, *SizeNL, MaxLength, CmpTy)
.getAs<DefinedOrUnknownSVal>();
if (!Cmp)
return;
diff --git a/clang/lib/StaticAnalyzer/Checkers/SetgidSetuidOrderChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/SetgidSetuidOrderChecker.cpp
index dbe3fd33a6b43..081ff525914b4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SetgidSetuidOrderChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SetgidSetuidOrderChecker.cpp
@@ -101,9 +101,9 @@ ProgramStateRef
SetgidSetuidOrderChecker::evalAssume(ProgramStateRef State,
// too. The "invalid failure check" is a different bug that is not the scope
// of this checker.)
auto FailComparison =
- SVB.evalBinOpNN(State, BO_NE, nonloc::SymbolVal(LastSetuidSym),
- SVB.makeIntVal(0, /*isUnsigned=*/false),
- SVB.getConditionType())
+ SVB.evalBinOp(State, BO_NE, nonloc::SymbolVal(LastSetuidSym),
+ SVB.makeIntVal(0, /*isUnsigned=*/false),
+ SVB.getConditionType())
.getAs<DefinedOrUnknownSVal>();
if (!FailComparison)
return State;
diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 6481b76e69171..a75fbd4b039ea 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -704,10 +704,9 @@ struct StreamOperationEvaluator {
C.getSValBuilder().makeNullWithType(CE->getType()));
}
- ProgramStateRef assumeBinOpNN(ProgramStateRef State,
- BinaryOperator::Opcode Op, NonLoc LHS,
- NonLoc RHS) {
- auto Cond = SVB.evalBinOpNN(State, Op, LHS, RHS, SVB.getConditionType())
+ ProgramStateRef assumeBinOp(ProgramStateRef State, BinaryOperator::Opcode Op,
+ NonLoc LHS, NonLoc RHS) {
+ auto Cond = SVB.evalBinOp(State, Op, LHS, RHS, SVB.getConditionType())
.getAs<DefinedOrUnknownSVal>();
if (!Cond)
return nullptr;
@@ -1203,7 +1202,7 @@ void StreamChecker::evalFreadFwrite(const FnDescription
*Desc,
NonLoc RetVal = makeRetVal(C, E.Elem.value()).castAs<NonLoc>();
ProgramStateRef StateFailed =
State->BindExpr(E.CE, C.getLocationContext(), RetVal);
- StateFailed = E.assumeBinOpNN(StateFailed, BO_LT, RetVal, *NMembVal);
+ StateFailed = E.assumeBinOp(StateFailed, BO_LT, RetVal, *NMembVal);
if (!StateFailed)
return;
@@ -1304,7 +1303,7 @@ void StreamChecker::evalFputx(const FnDescription *Desc,
const CallEvent &Call,
ProgramStateRef StateNotFailed =
State->BindExpr(E.CE, C.getLocationContext(), RetVal);
StateNotFailed =
- E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+ E.assumeBinOp(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
if (!StateNotFailed)
return;
StateNotFailed =
@@ -1383,7 +1382,7 @@ void StreamChecker::evalFscanf(const FnDescription *Desc,
const CallEvent &Call,
ProgramStateRef StateNotFailed =
State->BindExpr(E.CE, C.getLocationContext(), RetVal);
StateNotFailed =
- E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+ E.assumeBinOp(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
if (!StateNotFailed)
return;
@@ -1463,7 +1462,7 @@ void StreamChecker::evalGetdelim(const FnDescription
*Desc,
NonLoc RetVal = makeRetVal(C, E.Elem.value()).castAs<NonLoc>();
ProgramStateRef StateNotFailed = E.bindReturnValue(State, C, RetVal);
StateNotFailed =
- E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+ E.assumeBinOp(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
// On success, a buffer is allocated.
auto NewLinePtr = getPointeeVal(Call.getArgSVal(0), State);
@@ -1476,8 +1475,8 @@ void StreamChecker::evalGetdelim(const FnDescription
*Desc,
SVal SizePtrSval = Call.getArgSVal(1);
auto NVal = getPointeeVal(SizePtrSval, State);
if (NVal && isa<NonLoc>(*NVal)) {
- StateNotFailed = E.assumeBinOpNN(StateNotFailed, BO_GT,
- NVal->castAs<NonLoc>(), RetVal);
+ StateNotFailed =
+ E.assumeBinOp(StateNotFailed, BO_GT, NVal->castAs<NonLoc>(), RetVal);
StateNotFailed = E.bindReturnValue(StateNotFailed, C, RetVal);
}
if (!StateNotFailed)
@@ -1605,7 +1604,7 @@ void StreamChecker::evalFtell(const FnDescription *Desc,
const CallEvent &Call,
ProgramStateRef StateNotFailed =
State->BindExpr(E.CE, C.getLocationContext(), RetVal);
StateNotFailed =
- E.assumeBinOpNN(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
+ E.assumeBinOp(StateNotFailed, BO_GE, RetVal, E.getZeroVal(Call));
if (!StateNotFailed)
return;
@@ -1772,7 +1771,7 @@ void StreamChecker::evalFileno(const FnDescription *Desc,
const CallEvent &Call,
NonLoc RetVal = makeRetVal(C, E.Elem.value()).castAs<NonLoc>();
State = State->BindExpr(E.CE, C.getLocationContext(), RetVal);
- State = E.assumeBinOpNN(State, BO_GE, RetVal, E.getZeroVal(Call));
+ State = E.assumeBinOp(State, BO_GE, RetVal, E.getZeroVal(Call));
if (!State)
return;
diff --git a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index ce5887d56b181..aed392261aad9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -277,9 +277,8 @@ void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext
&C,
NonLoc ocreateFlag = C.getSValBuilder()
.makeIntVal(Val_O_CREAT.value(),
oflagsEx->getType())
.castAs<NonLoc>();
- SVal maskedFlagsUC = C.getSValBuilder().evalBinOpNN(state, BO_And,
- oflags, ocreateFlag,
- oflagsEx->getType());
+ SVal maskedFlagsUC = C.getSValBuilder().evalBinOp(
+ state, BO_And, oflags, ocreateFlag, oflagsEx->getType());
if (maskedFlagsUC.isUnknownOrUndef())
return;
DefinedSVal maskedFlags = maskedFlagsUC.castAs<DefinedSVal>();
diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 1042b43680fd2..5329fb9eb1077 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -112,7 +112,7 @@ ProgramStateRef VLASizeChecker::checkVLA(CheckerContext &C,
NonLoc IndexLength =
SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs<NonLoc>();
// Multiply the array length by the element size.
- SVal Mul = SVB.evalBinOpNN(State, BO_Mul, ArrSize, IndexLength, SizeTy);
+ SVal Mul = SVB.evalBinOp(State, BO_Mul, ArrSize, IndexLength, SizeTy);
if (auto MulNonLoc = Mul.getAs<NonLoc>())
ArrSize = *MulNonLoc;
else
diff --git a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
index c4790b0284281..3f8fa87d59505 100644
--- a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -333,22 +333,21 @@ ProgramState::assumeInBoundDual(DefinedOrUnknownSVal Idx,
nonloc::ConcreteInt Min(BVF.getMinValue(indexTy));
// Adjust the index.
- SVal newIdx = svalBuilder.evalBinOpNN(this, BO_Add,
- Idx.castAs<NonLoc>(), Min, indexTy);
+ SVal newIdx =
+ svalBuilder.evalBinOp(this, BO_Add, Idx.castAs<NonLoc>(), Min, indexTy);
if (newIdx.isUnknownOrUndef())
return {this, this};
// Adjust the upper bound.
- SVal newBound =
- svalBuilder.evalBinOpNN(this, BO_Add, UpperBound.castAs<NonLoc>(),
- Min, indexTy);
+ SVal newBound = svalBuilder.evalBinOp(
+ this, BO_Add, UpperBound.castAs<NonLoc>(), Min, indexTy);
if (newBound.isUnknownOrUndef())
return {this, this};
// Build the actual comparison.
- SVal inBound = svalBuilder.evalBinOpNN(this, BO_LT, newIdx.castAs<NonLoc>(),
- newBound.castAs<NonLoc>(), Ctx.IntTy);
+ SVal inBound = svalBuilder.evalBinOp(this, BO_LT, newIdx.castAs<NonLoc>(),
+ newBound.castAs<NonLoc>(), Ctx.IntTy);
if (inBound.isUnknownOrUndef())
return {this, this};
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index a0f959d8adc1a..ffe2dbb58fee5 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -612,7 +612,7 @@ SVal SValBuilder::evalIntegralCast(ProgramStateRef state,
SVal val,
// target type.
QualType CmpTy = getConditionType();
auto CompVal =
- evalBinOpNN(state, BO_LE, *AsNonLoc, ToTypeMaxVal,
CmpTy).getAs<NonLoc>();
+ evalBinOp(state, BO_LE, *AsNonLoc, ToTypeMaxVal, CmpTy).getAs<NonLoc>();
if (!CompVal)
return UnknownVal();
ProgramStateRef IsNotTruncated, IsTruncated;
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index ef8c3b6b4bdac..5be9fba1d72a0 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -235,9 +235,9 @@ static bool isInRelation(BinaryOperator::Opcode Rel,
SymbolRef Sym,
llvm::APSInt Bound, ProgramStateRef State) {
SValBuilder &SVB = State->getStateManager().getSValBuilder();
BasicValueFactory &BV = SVB.getBasicValueFactory();
- SVal Result = SVB.evalBinOpNN(State, Rel, nonloc::SymbolVal(Sym),
- nonloc::ConcreteInt(BV.getValue(Bound)),
- SVB.getConditionType());
+ SVal Result = SVB.evalBinOp(State, Rel, nonloc::SymbolVal(Sym),
+ nonloc::ConcreteInt(BV.getValue(Bound)),
+ SVB.getConditionType());
if (auto DV = Result.getAs<DefinedSVal>()) {
return !State->assume(*DV, false);
}
@@ -316,8 +316,8 @@ static NonLoc doRearrangeUnchecked(ProgramStateRef State,
if (LSym == RSym)
return SVB
- .evalBinOpNN(State, Op, nonloc::ConcreteInt(BV.getValue(LInt)),
- nonloc::ConcreteInt(BV.getValue(RInt)), ResultTy)
+ .evalBinOp(State, Op, nonloc::ConcreteInt(BV.getValue(LInt)),
+ nonloc::ConcreteInt(BV.getValue(RInt)), ResultTy)
.castAs<NonLoc>();
SymbolRef ResultSym = nullptr;
@@ -487,9 +487,8 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
// of modeling "pointers as integers" is not complete.
if (!BinaryOperator::isComparisonOp(op))
return UnknownVal();
- return evalBinOpLL(state, op, lhsL,
- rhs.castAs<nonloc::LocAsInteger>().getLoc(),
- resultTy);
+ return evalBinOp(state, op, lhsL,
+ rhs.castAs<nonloc::LocAsInteger>().getLoc(),
resultTy);
case nonloc::ConcreteIntKind: {
// FIXME: at the moment the implementation
// of modeling "pointers as integers" is not complete.
@@ -509,7 +508,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
BasicVals.getAPSIntType(lSym->getType()).apply(i);
else
BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
- return evalBinOpLL(state, op, lhsL, makeLoc(i), resultTy);
+ return evalBinOp(state, op, lhsL, makeLoc(i), resultTy);
}
default:
switch (op) {
@@ -802,7 +801,7 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
// Only comparisons and subtractions are valid operations on two pointers.
// See [C99 6.5.5 through 6.5.14] or [C++0x 5.6 through 5.15].
- // However, if a pointer is casted to an integer, evalBinOpNN may end up
+ // However, if a pointer is casted to an integer, evalBinOp may end up
// calling this function with another operation (PR7527). We don't attempt to
// model this for now, but it could be useful, particularly when the
// "location" is actually an integer value that's been passed through a
void*.
@@ -927,7 +926,7 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
QualType boolType = getContext().BoolTy;
NonLoc l = evalCast(lhs, boolType, QualType{}).castAs<NonLoc>();
NonLoc r = makeTruthVal(false, boolType).castAs<NonLoc>();
- return evalBinOpNN(state, op, l, r, resultTy);
+ return evalBinOp(state, op, l, r, resultTy);
}
}
@@ -1023,8 +1022,8 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
return UnknownVal();
// Actually perform the operation.
- // evalBinOpNN expects the two indexes to already be the right type.
- return evalBinOpNN(state, op, *LeftIndex, *RightIndex, resultTy);
+ // evalBinOp expects the two indexes to already be the right type.
+ return evalBinOp(state, op, *LeftIndex, *RightIndex, resultTy);
}
}
@@ -1166,8 +1165,8 @@ SVal SimpleSValBuilder::evalBinOpLN(ProgramStateRef state,
if (const ElementRegion *elemReg = dyn_cast<ElementRegion>(region)) {
assert(op == BO_Add || op == BO_Sub);
- index = evalBinOpNN(state, op, elemReg->getIndex(), rhs,
- getArrayIndexType());
+ index =
+ evalBinOp(state, op, elemReg->getIndex(), rhs, getArrayIndexType());
superR = cast<SubRegion>(elemReg->getSuperRegion());
elementType = elemReg->getElementType();
}
diff --git a/clang/test/Analysis/ArrayBound/brief-tests.c
b/clang/test/Analysis/ArrayBound/brief-tests.c
index f4811efd8d8b6..e6a208c4117c3 100644
--- a/clang/test/Analysis/ArrayBound/brief-tests.c
+++ b/clang/test/Analysis/ArrayBound/brief-tests.c
@@ -183,7 +183,7 @@ struct incomplete;
char test_comparison_with_extent_symbol(struct incomplete *p) {
// Previously this was reported as a (false positive) overflow error because
// the extent symbol of the area pointed by `p` was an unsigned and the '-1'
- // was converted to its type by `evalBinOpNN`.
+ // was converted to its type by `evalBinOp`.
return ((char *)p)[-1]; // no-warning
}
diff --git a/clang/test/Analysis/bitwise-shift-sanity-checks.c
b/clang/test/Analysis/bitwise-shift-sanity-checks.c
index a31424f18818c..1eb1cf8cee361 100644
--- a/clang/test/Analysis/bitwise-shift-sanity-checks.c
+++ b/clang/test/Analysis/bitwise-shift-sanity-checks.c
@@ -124,7 +124,7 @@ void doubles_cast_to_integer(int *c) {
unsigned int strange_cast(unsigned short sh) {
// This testcase triggers a bug in the constant folding (it "forgets" the
- // cast), which is silenced in SimpleSValBuilder::evalBinOpNN() with an ugly
+ // cast), which is silenced in SimpleSValBuilder::evalBinOp() with an ugly
// workaround, because otherwise it would lead to a false positive from
// core.UndefinedBinaryOperatorResult.
unsigned int i;
From f70986e25cfc905f9a316c278593d91b0e74203b Mon Sep 17 00:00:00 2001
From: Balazs Benics <balazs.ben...@sonarsource.com>
Date: Mon, 16 Jun 2025 12:14:06 +0200
Subject: [PATCH 5/5] Check the MaxSymbolComplexity inside evalBinOp
---
.../Core/PathSensitive/SymbolManager.h | 53 ++++++-------------
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp | 14 +++++
.../StaticAnalyzer/Core/SimpleSValBuilder.cpp | 17 +++---
.../lib/StaticAnalyzer/Core/SymbolManager.cpp | 9 ++++
.../ensure-capped-symbol-complexity.cpp | 8 +--
5 files changed, 48 insertions(+), 53 deletions(-)
diff --git
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 62a2e68b7ed01..f8893db5df4f8 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -36,6 +36,7 @@
namespace clang {
+class AnalyzerOptions;
class ASTContext;
class Stmt;
@@ -581,21 +582,15 @@ class SymbolManager {
public:
SymbolManager(ASTContext &ctx, BasicValueFactory &bv,
- llvm::BumpPtrAllocator &bpalloc, const AnalyzerOptions &Opts)
- : SymbolDependencies(16), Alloc(bpalloc), BV(bv), Ctx(ctx),
- MaxCompComplexity(Opts.MaxSymbolComplexity) {
- assert(MaxCompComplexity > 0 && "Zero max complexity doesn't make sense");
- }
+ llvm::BumpPtrAllocator &bpalloc, const AnalyzerOptions &Opts);
static bool canSymbolicate(QualType T);
/// Create or retrieve a SymExpr of type \p T for the given arguments.
/// Use the arguments to check for an existing SymExpr and return it,
/// otherwise, create a new one and keep a pointer to it to avoid duplicates.
- template <typename T, typename... Args,
- typename Ret =
std::conditional_t<SymbolData::classof(T::ClassKind),
- T, SymExpr>>
- LLVM_ATTRIBUTE_RETURNS_NONNULL const Ret *acquire(Args &&...args);
+ template <typename T, typename... Args>
+ LLVM_ATTRIBUTE_RETURNS_NONNULL const T *acquire(Args &&...args);
const SymbolConjured *conjureSymbol(ConstCFGElementRef Elem,
const LocationContext *LCtx, QualType T,
@@ -733,37 +728,19 @@ class SymbolVisitor {
virtual bool VisitMemRegion(const MemRegion *) { return true; }
};
-// Returns a const pointer to T if T is a SymbolData, otherwise SymExpr.
-template <typename T, typename... Args, typename Ret>
-const Ret *SymbolManager::acquire(Args &&...args) {
- T Dummy(/*SymbolID=*/0, args...);
- llvm::FoldingSetNodeID DummyProfile;
- Dummy.Profile(DummyProfile);
-
- if (Dummy.complexity() <= MaxCompComplexity) {
- void *InsertPos;
- SymExpr *SD = DataSet.FindNodeOrInsertPos(DummyProfile, InsertPos);
- if (!SD) {
- SD = Alloc.make<T>(args...);
- DataSet.InsertNode(SD, InsertPos);
- }
- return cast<Ret>(SD);
- }
-
- SymbolOverlyComplex OverlyComplexSym(/*SymbolID=*/0, Dummy.getType(),
- DummyProfile.ComputeHash());
- llvm::FoldingSetNodeID OverlyComplexSymProfile;
- OverlyComplexSym.Profile(OverlyComplexSymProfile);
-
- void *OverlyComplexSymInsertPos;
- SymExpr *SD = DataSet.FindNodeOrInsertPos(OverlyComplexSymProfile,
- OverlyComplexSymInsertPos);
+template <typename T, typename... Args>
+const T *SymbolManager::acquire(Args &&...args) {
+ llvm::FoldingSetNodeID profile;
+ T::Profile(profile, args...);
+ void *InsertPos;
+ SymExpr *SD = DataSet.FindNodeOrInsertPos(profile, InsertPos);
if (!SD) {
- SD = Alloc.make<SymbolOverlyComplex>(Dummy.getType(),
- DummyProfile.ComputeHash());
- DataSet.InsertNode(SD, OverlyComplexSymInsertPos);
+ SD = Alloc.make<T>(std::forward<Args>(args)...);
+ DataSet.InsertNode(SD, InsertPos);
+ assert(SD->complexity() <= MaxCompComplexity);
}
- return cast<Ret>(SD);
+
+ return cast<T>(SD);
}
} // namespace ento
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index ffe2dbb58fee5..cdafc12070842 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -488,6 +488,10 @@ SVal SValBuilder::evalUnaryOp(ProgramStateRef state,
UnaryOperator::Opcode opc,
llvm_unreachable("Unexpected unary operator");
}
+static unsigned getComplexity(SymbolRef Sym) {
+ return Sym ? Sym->complexity() : 1;
+}
+
SVal SValBuilder::evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
SVal lhs, SVal rhs, QualType type) {
if (lhs.isUndef() || rhs.isUndef())
@@ -500,6 +504,14 @@ SVal SValBuilder::evalBinOp(ProgramStateRef state,
BinaryOperator::Opcode op,
return UnknownVal();
}
+ unsigned LeftComplexity = getComplexity(lhs.getAsSymbol());
+ unsigned RightComplexity = getComplexity(rhs.getAsSymbol());
+ // TODO: When the Max Complexity is reached, we should conjure a symbol
+ // instead of generating an Unknown value and propagate the taint info to it.
+ if (LeftComplexity + RightComplexity > AnOpts.MaxSymbolComplexity) {
+ return UnknownVal();
+ }
+
if (op == BinaryOperatorKind::BO_Cmp) {
// We can't reason about C++20 spaceship operator yet.
//
@@ -1102,6 +1114,8 @@ class EvalCastVisitor : public
SValVisitor<EvalCastVisitor, SVal> {
/// FIXME: If `OriginalTy.isNull()` is true, then cast performs based on CastTy
/// only. This behavior is uncertain and should be improved.
SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) {
+ // TODO: Check if we are potentially about to exceed the MaxSymbolComplexity.
+ // Return Unknown in that case.
EvalCastVisitor TRV{*this, CastTy, OriginalTy};
return TRV.Visit(V);
}
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 5be9fba1d72a0..b0d1eb5346cf0 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -290,10 +290,10 @@ static std::pair<SymbolRef, APSIntPtr>
decomposeSymbol(SymbolRef Sym,
// Simplify "(LSym + LInt) Op (RSym + RInt)" assuming all values are of the
// same signed integral type and no overflows occur (which should be checked
// by the caller).
-static NonLoc doRearrangeUnchecked(ProgramStateRef State,
- BinaryOperator::Opcode Op, SymbolRef LSym,
- llvm::APSInt LInt, SymbolRef RSym,
- llvm::APSInt RInt) {
+static SVal doRearrangeUnchecked(ProgramStateRef State,
+ BinaryOperator::Opcode Op, SymbolRef LSym,
+ llvm::APSInt LInt, SymbolRef RSym,
+ llvm::APSInt RInt) {
SValBuilder &SVB = State->getStateManager().getSValBuilder();
BasicValueFactory &BV = SVB.getBasicValueFactory();
SymbolManager &SymMgr = SVB.getSymbolManager();
@@ -315,10 +315,8 @@ static NonLoc doRearrangeUnchecked(ProgramStateRef State,
llvm_unreachable("Operation not suitable for unchecked rearrangement!");
if (LSym == RSym)
- return SVB
- .evalBinOp(State, Op, nonloc::ConcreteInt(BV.getValue(LInt)),
- nonloc::ConcreteInt(BV.getValue(RInt)), ResultTy)
- .castAs<NonLoc>();
+ return SVB.evalBinOp(State, Op, nonloc::ConcreteInt(BV.getValue(LInt)),
+ nonloc::ConcreteInt(BV.getValue(RInt)), ResultTy);
SymbolRef ResultSym = nullptr;
BinaryOperator::Opcode ResultOp;
@@ -417,7 +415,8 @@ static std::optional<NonLoc> tryRearrange(ProgramStateRef
State,
return std::nullopt;
// We know that no overflows can occur anymore.
- return doRearrangeUnchecked(State, Op, LSym, LInt, RSym, RInt);
+ return doRearrangeUnchecked(State, Op, LSym, LInt, RSym, RInt)
+ .getAs<NonLoc>();
}
SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
diff --git a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
index 2149413189dcd..37d42f6c32fc6 100644
--- a/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -17,6 +17,7 @@
#include "clang/Analysis/Analyses/LiveVariables.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/LLVM.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
@@ -152,6 +153,14 @@ void SymbolRegionValue::dumpToStream(raw_ostream &os)
const {
os << getKindStr() << getSymbolID() << '<' << getType() << ' ' << R << '>';
}
+SymbolManager::SymbolManager(ASTContext &ctx, BasicValueFactory &bv,
+ llvm::BumpPtrAllocator &bpalloc,
+ const AnalyzerOptions &Opts)
+ : SymbolDependencies(16), Alloc(bpalloc), BV(bv), Ctx(ctx),
+ MaxCompComplexity(Opts.MaxSymbolComplexity) {
+ assert(MaxCompComplexity > 0 && "Zero max complexity doesn't make sense");
+}
+
bool SymExpr::symbol_iterator::operator==(const symbol_iterator &X) const {
return itr == X.itr;
}
diff --git a/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
b/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
index 2e716900c3847..bd31764367b16 100644
--- a/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
+++ b/clang/test/Analysis/ensure-capped-symbol-complexity.cpp
@@ -34,7 +34,7 @@ void pumpSymbolComplexity() {
// This dump used to print a hugely complicated symbol, over 800 complexity,
taking really long to simplify.
clang_analyzer_dump(*p);
- // expected-warning-re@-1 {{{{^}}(complex_${{[0-9]+}}) & 1023}}
[debug.ExprInspection]{{$}}}}
+ // expected-warning-re@-1 {{{{^}}conj_${{[0-9]+}}{int, LC{{[0-9]+}},
S{{[0-9]+}}, #{{[0-9]+}}} [debug.ExprInspection]{{$}}}}
}
void hugelyOverComplicatedSymbol() {
@@ -45,14 +45,10 @@ void hugelyOverComplicatedSymbol() {
HUNDRED_TIMES(*p = (*p + 1) & 1023;)
HUNDRED_TIMES(*p = (*p + 1) & 1023;)
HUNDRED_TIMES(*p = (*p + 1) & 1023;)
- *p = (*p + 1) & 1023;
- *p = (*p + 1) & 1023;
- *p = (*p + 1) & 1023;
- *p = (*p + 1) & 1023;
// This dump used to print a hugely complicated symbol, over 800 complexity,
taking really long to simplify.
clang_analyzer_dump(*p);
- // expected-warning-re@-1 {{{{^}}(((complex_${{[0-9]+}}) & 1023) + 1) & 1023
[debug.ExprInspection]{{$}}}}
+ // expected-warning-re@-1 {{{{^}}((((((((conj_${{[0-9]+}}{int, LC{{[0-9]+}},
S{{[0-9]+}}, #{{[0-9]+}}}) + 1) & 1023) + 1) & 1023) + 1) & 1023) + 1) & 1023
[debug.ExprInspection]{{$}}}}
#undef HUNDRED_TIMES
#undef TEN_TIMES
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
