[clang] [llvm] [NFC][analyzer] Extract bounds checking library (PR #202372)
Donát Nagy via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 2 08:21:01 PDT 2026
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/202372
>From 8c23f5f8063c0f49fa3c90f06d6b69f7219f395f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 3 Jun 2026 13:31:30 +0200
Subject: [PATCH 01/29] Only take the name of the region in StateUpdateReporter
constructor
It does not need to know about the region object.
---
.../StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 67110f021bc56..6b12fa6717fbf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -77,8 +77,7 @@ determineElementSize(const std::optional<QualType> T, const CheckerContext &C) {
}
class StateUpdateReporter {
- const MemSpaceRegion *Space;
- const SubRegion *Reg;
+ const std::string RegName;
const NonLoc ByteOffsetVal;
const std::optional<QualType> ElementType;
const std::optional<int64_t> ElementSize;
@@ -86,10 +85,10 @@ class StateUpdateReporter {
std::optional<NonLoc> AssumedUpperBound = std::nullopt;
public:
- StateUpdateReporter(const SubRegion *R, NonLoc ByteOffsVal, const Expr *E,
+ StateUpdateReporter(std::string RN, NonLoc ByteOffsVal, const Expr *E,
CheckerContext &C)
- : Space(R->getMemorySpace(C.getState())), Reg(R),
- ByteOffsetVal(ByteOffsVal), ElementType(determineElementType(E, C)),
+ : RegName(RN), ByteOffsetVal(ByteOffsVal),
+ ElementType(determineElementType(E, C)),
ElementSize(determineElementSize(ElementType, C)) {}
void recordNonNegativeAssumption() { AssumedNonNegative = true; }
@@ -550,7 +549,7 @@ std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR) const {
<< "' elements in ";
else
Out << "the extent of ";
- Out << getRegionName(Space, Reg);
+ Out << RegName;
}
return std::string(Out.str());
}
@@ -597,7 +596,8 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
// The state updates will be reported as a single note tag, which will be
// composed by this helper class.
- StateUpdateReporter SUR(Reg, ByteOffset, E, C);
+ std::string RegName = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
+ StateUpdateReporter SUR(RegName, ByteOffset, E, C);
// CHECK LOWER BOUND
const MemSpaceRegion *Space = Reg->getMemorySpace(State);
>From 2f961f8d3e136dfb8627827dd4aa9030e980ef01 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 3 Jun 2026 14:52:33 +0200
Subject: [PATCH 02/29] Move note creation out of StateUpdateReporter
I'm gradually transforming this class into a generic utility, taking
away some responsibilities and then adding other responsibilities.
---
.../Checkers/ArrayBoundChecker.cpp | 46 +++++++++----------
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 6b12fa6717fbf..750d79993c5b7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -77,7 +77,6 @@ determineElementSize(const std::optional<QualType> T, const CheckerContext &C) {
}
class StateUpdateReporter {
- const std::string RegName;
const NonLoc ByteOffsetVal;
const std::optional<QualType> ElementType;
const std::optional<int64_t> ElementSize;
@@ -85,10 +84,8 @@ class StateUpdateReporter {
std::optional<NonLoc> AssumedUpperBound = std::nullopt;
public:
- StateUpdateReporter(std::string RN, NonLoc ByteOffsVal, const Expr *E,
- CheckerContext &C)
- : RegName(RN), ByteOffsetVal(ByteOffsVal),
- ElementType(determineElementType(E, C)),
+ StateUpdateReporter(NonLoc ByteOffsVal, const Expr *E, CheckerContext &C)
+ : ByteOffsetVal(ByteOffsVal), ElementType(determineElementType(E, C)),
ElementSize(determineElementSize(ElementType, C)) {}
void recordNonNegativeAssumption() { AssumedNonNegative = true; }
@@ -98,11 +95,11 @@ class StateUpdateReporter {
bool assumedNonNegative() { return AssumedNonNegative; }
- const NoteTag *createNoteTag(CheckerContext &C) const;
+ bool hasAssumption() const { return AssumedNonNegative || AssumedUpperBound; }
-private:
- std::string getMessage(PathSensitiveBugReport &BR) const;
+ std::string getMessage(PathSensitiveBugReport &BR, StringRef RegName) const;
+private:
/// Return true if information about the value of `Sym` can put constraints
/// on some symbol which is interesting within the bug report `BR`.
/// In particular, this returns true when `Sym` is interesting within `BR`;
@@ -488,17 +485,8 @@ static Messages getTaintMsgs(const MemSpaceRegion *Space,
AlsoMentionUnderflow ? "negative or " : "")};
}
-const NoteTag *StateUpdateReporter::createNoteTag(CheckerContext &C) const {
- // Don't create a note tag if we didn't assume anything:
- if (!AssumedNonNegative && !AssumedUpperBound)
- return nullptr;
-
- return C.getNoteTag([*this](PathSensitiveBugReport &BR) -> std::string {
- return getMessage(BR);
- });
-}
-
-std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR) const {
+std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
+ StringRef RegName) const {
bool ShouldReportNonNegative = AssumedNonNegative;
if (!providesInformationAboutInteresting(ByteOffsetVal, BR)) {
if (AssumedUpperBound &&
@@ -596,8 +584,7 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
// The state updates will be reported as a single note tag, which will be
// composed by this helper class.
- std::string RegName = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
- StateUpdateReporter SUR(RegName, ByteOffset, E, C);
+ StateUpdateReporter SUR(ByteOffset, E, C);
// CHECK LOWER BOUND
const MemSpaceRegion *Space = Reg->getMemorySpace(State);
@@ -683,8 +670,9 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
// expression that calculates the past-the-end pointer.
if (isIdiomaticPastTheEndPtr(E, ExceedsUpperBound, ByteOffset,
*KnownSize, C)) {
- C.addTransition(ExceedsUpperBound, SUR.createNoteTag(C));
- return;
+ // The use of 'goto' is a temporary solution, will be eliminated in
+ // the next steps of the refactoring.
+ goto NormalTransition;
}
BadOffsetKind Problem = AlsoMentionUnderflow
@@ -726,8 +714,18 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
State = WithinUpperBound;
}
+NormalTransition:
// Add a transition, reporting the state updates that we accumulated.
- C.addTransition(State, SUR.createNoteTag(C));
+ const NoteTag *Tag = nullptr;
+
+ if (SUR.hasAssumption()) {
+ std::string RN = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
+ Tag = C.getNoteTag([SUR, RN](PathSensitiveBugReport &BR) -> std::string {
+ return SUR.getMessage(BR, RN);
+ });
+ }
+
+ C.addTransition(State, Tag);
}
void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
>From 48158251c347a472a484720e3fd693f563e54374 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 3 Jun 2026 14:58:52 +0200
Subject: [PATCH 03/29] Place small helpers before StateUpdateReporter
Because they will be used in the generalized version of
StateUpdateReporter.
---
.../Checkers/ArrayBoundChecker.cpp | 36 +++++++++----------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 750d79993c5b7..9843824bc2ebf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -76,6 +76,24 @@ determineElementSize(const std::optional<QualType> T, const CheckerContext &C) {
return C.getASTContext().getTypeSizeInChars(*T).getQuantity();
}
+struct Messages {
+ std::string Short, Full;
+};
+
+enum class BadOffsetKind { Negative, Overflowing, Indeterminate };
+
+constexpr llvm::StringLiteral Adjectives[] = {"a negative", "an overflowing",
+ "a negative or overflowing"};
+static StringRef asAdjective(BadOffsetKind Problem) {
+ return Adjectives[static_cast<int>(Problem)];
+}
+
+constexpr llvm::StringLiteral Prepositions[] = {"preceding", "after the end of",
+ "around"};
+static StringRef asPreposition(BadOffsetKind Problem) {
+ return Prepositions[static_cast<int>(Problem)];
+}
+
class StateUpdateReporter {
const NonLoc ByteOffsetVal;
const std::optional<QualType> ElementType;
@@ -122,24 +140,6 @@ class StateUpdateReporter {
}
};
-struct Messages {
- std::string Short, Full;
-};
-
-enum class BadOffsetKind { Negative, Overflowing, Indeterminate };
-
-constexpr llvm::StringLiteral Adjectives[] = {"a negative", "an overflowing",
- "a negative or overflowing"};
-static StringRef asAdjective(BadOffsetKind Problem) {
- return Adjectives[static_cast<int>(Problem)];
-}
-
-constexpr llvm::StringLiteral Prepositions[] = {"preceding", "after the end of",
- "around"};
-static StringRef asPreposition(BadOffsetKind Problem) {
- return Prepositions[static_cast<int>(Problem)];
-}
-
// NOTE: The `ArraySubscriptExpr` and `UnaryOperator` callbacks are `PostStmt`
// instead of `PreStmt` because the current implementation passes the whole
// expression to `CheckerContext::getSVal()` which only works after the
>From 6ed1ad457236440ad824beb78e4a1199d9958e02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 3 Jun 2026 15:31:48 +0200
Subject: [PATCH 04/29] Introduce the helper class SizeUnit
This is currently used for the reporting of assumptions, but in
follow-up commits I will use it to unify the reporting of assumptions
and overflow reports.
---
.../Checkers/ArrayBoundChecker.cpp | 81 +++++++++++--------
1 file changed, 46 insertions(+), 35 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 9843824bc2ebf..1c4c86fc8aa92 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -57,24 +57,41 @@ getAsCleanArraySubscriptExpr(const Expr *E, const CheckerContext &C) {
return ASE;
}
-/// If `E` is a "clean" array subscript expression, return the type of the
-/// accessed element; otherwise return std::nullopt because that's the best (or
-/// least bad) option for the diagnostic generation that relies on this.
-static std::optional<QualType> determineElementType(const Expr *E,
- const CheckerContext &C) {
- const auto *ASE = getAsCleanArraySubscriptExpr(E, C);
- if (!ASE)
- return std::nullopt;
+class SizeUnit {
+ QualType AsType;
+ int64_t AsCharUnits;
- return ASE->getType();
-}
+ SizeUnit() : AsType(), AsCharUnits(1) {}
-static std::optional<int64_t>
-determineElementSize(const std::optional<QualType> T, const CheckerContext &C) {
- if (!T)
- return std::nullopt;
- return C.getASTContext().getTypeSizeInChars(*T).getQuantity();
-}
+public:
+ SizeUnit(QualType T, const ASTContext &ACtx)
+ : AsType(T), AsCharUnits(ACtx.getTypeSizeInChars(T).getQuantity()) {
+ assert(!T.isNull());
+ }
+
+ static SizeUnit bytes() { return SizeUnit(); }
+
+ bool isBytes() const { return AsType.isNull(); }
+
+ /// If `E` is a "clean" array subscript expression, return the type of the
+ /// accessed element; otherwise return 'Bytes' because that's the best (or
+ /// least bad) option for the diagnostic generation that relies on this.
+ static SizeUnit forExpr(const Expr *E, const CheckerContext &C) {
+ const auto *ASE = getAsCleanArraySubscriptExpr(E, C);
+ if (!ASE)
+ return bytes();
+
+ return SizeUnit(ASE->getType(), C.getASTContext());
+ }
+
+ int64_t asCharUnits() const { return AsCharUnits; }
+
+ std::string asExtentDesc(bool ForceBytes) const {
+ if (ForceBytes || isBytes())
+ return "the extent of";
+ return formatv("the number of '{0}' elements in", AsType.getAsString());
+ }
+};
struct Messages {
std::string Short, Full;
@@ -96,15 +113,11 @@ static StringRef asPreposition(BadOffsetKind Problem) {
class StateUpdateReporter {
const NonLoc ByteOffsetVal;
- const std::optional<QualType> ElementType;
- const std::optional<int64_t> ElementSize;
bool AssumedNonNegative = false;
std::optional<NonLoc> AssumedUpperBound = std::nullopt;
public:
- StateUpdateReporter(NonLoc ByteOffsVal, const Expr *E, CheckerContext &C)
- : ByteOffsetVal(ByteOffsVal), ElementType(determineElementType(E, C)),
- ElementSize(determineElementSize(ElementType, C)) {}
+ StateUpdateReporter(NonLoc ByteOffsVal) : ByteOffsetVal(ByteOffsVal) {}
void recordNonNegativeAssumption() { AssumedNonNegative = true; }
void recordUpperBoundAssumption(NonLoc UpperBoundVal) {
@@ -115,7 +128,8 @@ class StateUpdateReporter {
bool hasAssumption() const { return AssumedNonNegative || AssumedUpperBound; }
- std::string getMessage(PathSensitiveBugReport &BR, StringRef RegName) const;
+ std::string getMessage(PathSensitiveBugReport &BR, StringRef RegName,
+ SizeUnit SU) const;
private:
/// Return true if information about the value of `Sym` can put constraints
@@ -486,7 +500,8 @@ static Messages getTaintMsgs(const MemSpaceRegion *Space,
}
std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
- StringRef RegName) const {
+ StringRef RegName,
+ SizeUnit SU) const {
bool ShouldReportNonNegative = AssumedNonNegative;
if (!providesInformationAboutInteresting(ByteOffsetVal, BR)) {
if (AssumedUpperBound &&
@@ -505,7 +520,7 @@ std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
std::optional<int64_t> ExtentN = getConcreteValue(AssumedUpperBound);
const bool UseIndex =
- ElementSize && tryDividePair(OffsetN, ExtentN, *ElementSize);
+ !SU.isBytes() && tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
SmallString<256> Buf;
llvm::raw_svector_ostream Out(Buf);
@@ -532,12 +547,7 @@ std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
Out << " less than ";
if (ExtentN)
Out << *ExtentN << ", ";
- if (UseIndex && ElementType)
- Out << "the number of '" << ElementType->getAsString()
- << "' elements in ";
- else
- Out << "the extent of ";
- Out << RegName;
+ Out << SU.asExtentDesc(/*ForceBytes=*/!UseIndex) << ' ' << RegName;
}
return std::string(Out.str());
}
@@ -584,7 +594,7 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
// The state updates will be reported as a single note tag, which will be
// composed by this helper class.
- StateUpdateReporter SUR(ByteOffset, E, C);
+ StateUpdateReporter SUR(ByteOffset);
// CHECK LOWER BOUND
const MemSpaceRegion *Space = Reg->getMemorySpace(State);
@@ -716,16 +726,17 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
NormalTransition:
// Add a transition, reporting the state updates that we accumulated.
- const NoteTag *Tag = nullptr;
+ const NoteTag *T = nullptr;
if (SUR.hasAssumption()) {
std::string RN = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
- Tag = C.getNoteTag([SUR, RN](PathSensitiveBugReport &BR) -> std::string {
- return SUR.getMessage(BR, RN);
+ SizeUnit SU = SizeUnit::forExpr(E, C);
+ T = C.getNoteTag([SUR, RN, SU](PathSensitiveBugReport &BR) -> std::string {
+ return SUR.getMessage(BR, RN, SU);
});
}
- C.addTransition(State, Tag);
+ C.addTransition(State, T);
}
void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
>From 5031aa4ac1a6b298ba55c50e0b596b944fee08ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 4 Jun 2026 11:37:33 +0200
Subject: [PATCH 05/29] Rename 'performCheck' to 'handleAccessExpr'
---
.../lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 1c4c86fc8aa92..be667550aabc5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -167,7 +167,7 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
BugType BT{this, "Out-of-bound access"};
BugType TaintBT{this, "Out-of-bound access", categories::TaintedData};
- void performCheck(const Expr *E, CheckerContext &C) const;
+ void handleAccessExpr(const Expr *E, CheckerContext &C) const;
void reportOOB(CheckerContext &C, ProgramStateRef ErrorState, Messages Msgs,
NonLoc Offset, std::optional<NonLoc> Extent,
@@ -188,15 +188,15 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
public:
void checkPostStmt(const ArraySubscriptExpr *E, CheckerContext &C) const {
- performCheck(E, C);
+ handleAccessExpr(E, C);
}
void checkPostStmt(const UnaryOperator *E, CheckerContext &C) const {
if (E->getOpcode() == UO_Deref)
- performCheck(E, C);
+ handleAccessExpr(E, C);
}
void checkPostStmt(const MemberExpr *E, CheckerContext &C) const {
if (E->isArrow())
- performCheck(E->getBase(), C);
+ handleAccessExpr(E->getBase(), C);
}
};
@@ -570,7 +570,8 @@ bool StateUpdateReporter::providesInformationAboutInteresting(
return false;
}
-void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const {
+void ArrayBoundChecker::handleAccessExpr(const Expr *E,
+ CheckerContext &C) const {
const SVal Location = C.getSVal(E);
// The header ctype.h (from e.g. glibc) implements the isXXXXX() macros as
>From 5fb3516b1d48ca1f369dd1e118bd3c0a17d2d74c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 4 Jun 2026 20:28:26 +0200
Subject: [PATCH 06/29] Separate determining bounds and performing the bounds
check
Follow-up changes will reduce the complexity and move functions to more
natural places.
---
.../Checkers/ArrayBoundChecker.cpp | 268 +++++++++++-------
1 file changed, 158 insertions(+), 110 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index be667550aabc5..9f9fabab815c1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -111,23 +111,51 @@ static StringRef asPreposition(BadOffsetKind Problem) {
return Prepositions[static_cast<int>(Problem)];
}
-class StateUpdateReporter {
- const NonLoc ByteOffsetVal;
+class BoundsCheckResult {
+public:
+ enum class Kind { Underflow, Overflow, TaintBug, Paradox, Valid };
+
+private:
+ Kind K = Kind::Valid;
bool AssumedNonNegative = false;
- std::optional<NonLoc> AssumedUpperBound = std::nullopt;
+ bool AssumedUpperBound = false;
+ const NonLoc Offset;
+ std::optional<NonLoc> Extent;
+ ProgramStateRef State = nullptr;
public:
- StateUpdateReporter(NonLoc ByteOffsVal) : ByteOffsetVal(ByteOffsVal) {}
+ BoundsCheckResult(NonLoc Offs, std::optional<NonLoc> E)
+ : Offset(Offs), Extent(E) {}
void recordNonNegativeAssumption() { AssumedNonNegative = true; }
- void recordUpperBoundAssumption(NonLoc UpperBoundVal) {
- AssumedUpperBound = UpperBoundVal;
+
+ void recordUpperBoundAssumption() { AssumedUpperBound = true; }
+
+ void finalize(Kind K_, ProgramStateRef S) {
+ K = K_;
+ State = S;
}
- bool assumedNonNegative() { return AssumedNonNegative; }
+ bool assumedNonNegative() const { return AssumedNonNegative; }
bool hasAssumption() const { return AssumedNonNegative || AssumedUpperBound; }
+ ProgramStateRef getState() const { return State; }
+
+ Kind getKind() const { return K; }
+
+ std::optional<BadOffsetKind> getBadOffsetKind() const {
+ switch (K) {
+ case Kind::Underflow:
+ return BadOffsetKind::Negative;
+ case Kind::Overflow:
+ return assumedNonNegative() ? BadOffsetKind::Indeterminate
+ : BadOffsetKind::Overflowing;
+ default:
+ return std::nullopt;
+ }
+ }
+
std::string getMessage(PathSensitiveBugReport &BR, StringRef RegName,
SizeUnit SU) const;
@@ -154,6 +182,12 @@ class StateUpdateReporter {
}
};
+struct CheckFlags {
+ bool CheckUnderflow;
+ bool OffsetObviouslyNonnegative;
+ bool AcceptPastTheEnd;
+};
+
// NOTE: The `ArraySubscriptExpr` and `UnaryOperator` callbacks are `PostStmt`
// instead of `PreStmt` because the current implementation passes the whole
// expression to `CheckerContext::getSVal()` which only works after the
@@ -168,6 +202,9 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
BugType TaintBT{this, "Out-of-bound access", categories::TaintedData};
void handleAccessExpr(const Expr *E, CheckerContext &C) const;
+ BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
+ NonLoc Offset, std::optional<NonLoc> Extent,
+ CheckFlags Flags) const;
void reportOOB(CheckerContext &C, ProgramStateRef ErrorState, Messages Msgs,
NonLoc Offset, std::optional<NonLoc> Extent,
@@ -181,9 +218,6 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
static bool isOffsetObviouslyNonnegative(const Expr *E, CheckerContext &C);
- static bool isIdiomaticPastTheEndPtr(const Expr *E, ProgramStateRef State,
- NonLoc Offset, NonLoc Limit,
- CheckerContext &C);
static bool isInAddressOf(const Stmt *S, ASTContext &AC);
public:
@@ -444,6 +478,9 @@ static Messages getNonTaintMsgs(const ASTContext &ACtx,
std::optional<int64_t> OffsetN = getConcreteValue(Offset);
std::optional<int64_t> ExtentN = getConcreteValue(Extent);
+ if (Problem == BadOffsetKind::Negative)
+ ExtentN = std::nullopt;
+
int64_t ElemSize = ACtx.getTypeSizeInChars(ElemType).getQuantity();
bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, ElemSize);
@@ -499,13 +536,12 @@ static Messages getTaintMsgs(const MemSpaceRegion *Space,
AlsoMentionUnderflow ? "negative or " : "")};
}
-std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
- StringRef RegName,
- SizeUnit SU) const {
+std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
+ StringRef RegName,
+ SizeUnit SU) const {
bool ShouldReportNonNegative = AssumedNonNegative;
- if (!providesInformationAboutInteresting(ByteOffsetVal, BR)) {
- if (AssumedUpperBound &&
- providesInformationAboutInteresting(*AssumedUpperBound, BR)) {
+ if (!providesInformationAboutInteresting(Offset, BR)) {
+ if (AssumedUpperBound && providesInformationAboutInteresting(*Extent, BR)) {
// Even if the byte offset isn't interesting (e.g. it's a constant value),
// the assumption can still be interesting if it provides information
// about an interesting symbolic upper bound.
@@ -516,8 +552,8 @@ std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
}
}
- std::optional<int64_t> OffsetN = getConcreteValue(ByteOffsetVal);
- std::optional<int64_t> ExtentN = getConcreteValue(AssumedUpperBound);
+ std::optional<int64_t> OffsetN = getConcreteValue(Offset);
+ std::optional<int64_t> ExtentN = getConcreteValue(Extent);
const bool UseIndex =
!SU.isBytes() && tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
@@ -529,7 +565,7 @@ std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
Out << "index ";
if (OffsetN)
Out << "'" << OffsetN << "' ";
- } else if (AssumedUpperBound) {
+ } else if (Extent) {
Out << "byte offset ";
if (OffsetN)
Out << "'" << OffsetN << "' ";
@@ -541,7 +577,7 @@ std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
if (ShouldReportNonNegative) {
Out << " non-negative";
}
- if (AssumedUpperBound) {
+ if (Extent) {
if (ShouldReportNonNegative)
Out << " and";
Out << " less than ";
@@ -552,7 +588,7 @@ std::string StateUpdateReporter::getMessage(PathSensitiveBugReport &BR,
return std::string(Out.str());
}
-bool StateUpdateReporter::providesInformationAboutInteresting(
+bool BoundsCheckResult::providesInformationAboutInteresting(
SymbolRef Sym, PathSensitiveBugReport &BR) {
if (!Sym)
return false;
@@ -593,27 +629,89 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
auto [Reg, ByteOffset] = *RawOffset;
- // The state updates will be reported as a single note tag, which will be
- // composed by this helper class.
- StateUpdateReporter SUR(ByteOffset);
+ const MemSpaceRegion *Space = Reg->getMemorySpace(State);
+ auto Extent = getDynamicExtent(State, Reg, SVB).getAs<NonLoc>();
+
+ // A symbolic region in unknown space represents an unknown pointer that
+ // may point into the middle of an array, so we don't look for underflows.
+ // Both conditions are significant because we want to check underflows in
+ // symbolic regions on the heap (which may be introduced by checkers like
+ // MallocChecker that call SValBuilder::getConjuredHeapSymbolVal()) and
+ // non-symbolic regions (e.g. a field subregion of a symbolic region) in
+ // unknown space.
+
+ CheckFlags Flags = {
+ /*CheckUnderflow=*/!(isa<SymbolicRegion>(Reg) &&
+ isa<UnknownSpaceRegion>(Space)),
+ /*OffsetObviouslyNonnegative=*/isOffsetObviouslyNonnegative(E, C),
+ /*AcceptPastTheEnd=*/isa<ArraySubscriptExpr>(E) &&
+ isInAddressOf(E, C.getASTContext()),
+ };
+
+ BoundsCheckResult Res = checkBounds(State, SVB, ByteOffset, Extent, Flags);
+
+ switch (Res.getKind()) {
+ case BoundsCheckResult::Kind::Paradox:
+ // The current state is paradoxical (due to bad modeling of casts we
+ // assumed that an unsigned value is negative), so we should sink the
+ // execution path.
+ C.addSink();
+ return;
+
+ case BoundsCheckResult::Kind::Valid: {
+ const NoteTag *Tag = nullptr;
+ if (Res.hasAssumption()) {
+ std::string RN = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
+ SizeUnit SU = SizeUnit::forExpr(E, C);
+ Tag = C.getNoteTag(
+ [Res, RN, SU](PathSensitiveBugReport &BR) -> std::string {
+ return Res.getMessage(BR, RN, SU);
+ });
+ }
+
+ C.addTransition(Res.getState(), Tag);
+ return;
+ }
+ case BoundsCheckResult::Kind::TaintBug: {
+ // Diagnostic detail: saying "tainted offset" is always correct, but
+ // the common case is that 'idx' is tainted in 'arr[idx]' and then it's
+ // nicer to say "tainted index".
+ const char *OffsetName = "offset";
+ if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
+ if (isTainted(State, ASE->getIdx(), C.getStackFrame()))
+ OffsetName = "index";
+
+ Messages Msgs =
+ getTaintMsgs(Space, Reg, OffsetName,
+ /*AlsoMentionUnderflow=*/Res.assumedNonNegative());
+ reportOOB(C, Res.getState(), Msgs, ByteOffset, Extent,
+ /*IsTaintBug=*/true);
+ return;
+ }
+ default: {
+ Messages Msgs = getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset,
+ Extent, Location, *Res.getBadOffsetKind());
+ reportOOB(C, Res.getState(), Msgs, ByteOffset, Extent);
+ return;
+ }
+ }
+}
+
+BoundsCheckResult ArrayBoundChecker::checkBounds(ProgramStateRef State,
+ SValBuilder &SVB,
+ NonLoc Offset,
+ std::optional<NonLoc> Extent,
+ CheckFlags Flags) const {
+ BoundsCheckResult Res(Offset, Extent);
// CHECK LOWER BOUND
- const MemSpaceRegion *Space = Reg->getMemorySpace(State);
- if (!(isa<SymbolicRegion>(Reg) && isa<UnknownSpaceRegion>(Space))) {
- // A symbolic region in unknown space represents an unknown pointer that
- // may point into the middle of an array, so we don't look for underflows.
- // Both conditions are significant because we want to check underflows in
- // symbolic regions on the heap (which may be introduced by checkers like
- // MallocChecker that call SValBuilder::getConjuredHeapSymbolVal()) and
- // non-symbolic regions (e.g. a field subregion of a symbolic region) in
- // unknown space.
- auto [PrecedesLowerBound, WithinLowerBound] = compareValueToThreshold(
- State, ByteOffset, SVB.makeZeroArrayIndex(), SVB);
+ if (Flags.CheckUnderflow) {
+ auto [PrecedesLowerBound, WithinLowerBound] =
+ compareValueToThreshold(State, Offset, SVB.makeZeroArrayIndex(), SVB);
if (PrecedesLowerBound) {
// The analyzer thinks that the offset may be invalid (negative)...
-
- if (isOffsetObviouslyNonnegative(E, C)) {
+ if (Flags.OffsetObviouslyNonnegative) {
// ...but the offset is obviously non-negative (clear array subscript
// with an unsigned index), so we're in a buggy situation.
@@ -632,8 +730,8 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
if (!WithinLowerBound) {
// The state is completely nonsense -- let's just sink it!
- C.addSink();
- return;
+ Res.finalize(BoundsCheckResult::Kind::Paradox, PrecedesLowerBound);
+ return Res;
}
// Otherwise continue on the 'WithinLowerBound' branch where the
// unsigned index _is_ non-negative. Don't mention this assumption as a
@@ -641,15 +739,12 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
} else {
if (!WithinLowerBound) {
// ...and it cannot be valid (>= 0), so report an error.
- Messages Msgs = getNonTaintMsgs(C.getASTContext(), Space, Reg,
- ByteOffset, /*Extent=*/std::nullopt,
- Location, BadOffsetKind::Negative);
- reportOOB(C, PrecedesLowerBound, Msgs, ByteOffset, std::nullopt);
- return;
+ Res.finalize(BoundsCheckResult::Kind::Underflow, PrecedesLowerBound);
+ return Res;
}
// ...but it can be valid as well, so the checker will (optimistically)
// assume that it's valid and mention this in the note tag.
- SUR.recordNonNegativeAssumption();
+ Res.recordNonNegativeAssumption();
}
}
@@ -661,17 +756,9 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
}
// CHECK UPPER BOUND
- DefinedOrUnknownSVal Size = getDynamicExtent(State, Reg, SVB);
- if (auto KnownSize = Size.getAs<NonLoc>()) {
- // In a situation where both underflow and overflow are possible (but the
- // index is either tainted or known to be invalid), the logic of this
- // checker will first assume that the offset is non-negative, and then
- // (with this additional assumption) it will detect an overflow error.
- // In this situation the warning message should mention both possibilities.
- bool AlsoMentionUnderflow = SUR.assumedNonNegative();
-
+ if (Extent) {
auto [WithinUpperBound, ExceedsUpperBound] =
- compareValueToThreshold(State, ByteOffset, *KnownSize, SVB);
+ compareValueToThreshold(State, Offset, *Extent, SVB);
if (ExceedsUpperBound) {
// The offset may be invalid (>= Size)...
@@ -679,43 +766,28 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
// ...and it cannot be within bounds, so report an error, unless we can
// definitely determine that this is an idiomatic `&array[size]`
// expression that calculates the past-the-end pointer.
- if (isIdiomaticPastTheEndPtr(E, ExceedsUpperBound, ByteOffset,
- *KnownSize, C)) {
- // The use of 'goto' is a temporary solution, will be eliminated in
- // the next steps of the refactoring.
- goto NormalTransition;
+ if (Flags.AcceptPastTheEnd) {
+ auto [EqualsToThreshold, NotEqualToThreshold] =
+ compareValueToThreshold(State, Offset, *Extent, SVB,
+ /*CheckEquality=*/true);
+ if (EqualsToThreshold && !NotEqualToThreshold) {
+ Res.finalize(BoundsCheckResult::Kind::Valid, State);
+ return Res;
+ }
}
- BadOffsetKind Problem = AlsoMentionUnderflow
- ? BadOffsetKind::Indeterminate
- : BadOffsetKind::Overflowing;
- Messages Msgs =
- getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset,
- *KnownSize, Location, Problem);
- reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize);
- return;
+ Res.finalize(BoundsCheckResult::Kind::Overflow, ExceedsUpperBound);
+ return Res;
}
// ...and it can be valid as well...
- if (isTainted(State, ByteOffset)) {
+ if (isTainted(State, Offset)) {
// ...but it's tainted, so report an error.
-
- // Diagnostic detail: saying "tainted offset" is always correct, but
- // the common case is that 'idx' is tainted in 'arr[idx]' and then it's
- // nicer to say "tainted index".
- const char *OffsetName = "offset";
- if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
- if (isTainted(State, ASE->getIdx(), C.getStackFrame()))
- OffsetName = "index";
-
- Messages Msgs =
- getTaintMsgs(Space, Reg, OffsetName, AlsoMentionUnderflow);
- reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize,
- /*IsTaintBug=*/true);
- return;
+ Res.finalize(BoundsCheckResult::Kind::TaintBug, State);
+ return Res;
}
// ...and it isn't tainted, so the checker will (optimistically) assume
// that the offset is in bounds and mention this in the note tag.
- SUR.recordUpperBoundAssumption(*KnownSize);
+ Res.recordUpperBoundAssumption();
}
// Actually update the state. The "if" only fails in the extremely unlikely
@@ -724,20 +796,8 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
if (WithinUpperBound)
State = WithinUpperBound;
}
-
-NormalTransition:
- // Add a transition, reporting the state updates that we accumulated.
- const NoteTag *T = nullptr;
-
- if (SUR.hasAssumption()) {
- std::string RN = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
- SizeUnit SU = SizeUnit::forExpr(E, C);
- T = C.getNoteTag([SUR, RN, SU](PathSensitiveBugReport &BR) -> std::string {
- return SUR.getMessage(BR, RN, SU);
- });
- }
-
- C.addTransition(State, T);
+ Res.finalize(BoundsCheckResult::Kind::Valid, State);
+ return Res;
}
void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
@@ -834,18 +894,6 @@ bool ArrayBoundChecker::isInAddressOf(const Stmt *S, ASTContext &ACtx) {
return UnaryOp && UnaryOp->getOpcode() == UO_AddrOf;
}
-bool ArrayBoundChecker::isIdiomaticPastTheEndPtr(const Expr *E,
- ProgramStateRef State,
- NonLoc Offset, NonLoc Limit,
- CheckerContext &C) {
- if (isa<ArraySubscriptExpr>(E) && isInAddressOf(E, C.getASTContext())) {
- auto [EqualsToThreshold, NotEqualToThreshold] = compareValueToThreshold(
- State, Offset, Limit, C.getSValBuilder(), /*CheckEquality=*/true);
- return EqualsToThreshold && !NotEqualToThreshold;
- }
- return false;
-}
-
void ento::registerArrayBoundChecker(CheckerManager &mgr) {
mgr.registerChecker<ArrayBoundChecker>();
}
>From bcbaf55ea162b05c152a5a29dd349f118ed33e92 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 5 Jun 2026 12:11:19 +0200
Subject: [PATCH 07/29] Move 'checkBounds' out of 'ArrayBoundChecker'
..to prepare for moving it to a separate source file.
Additionally, this way I can make `checkBounds` a friend of
`BoundsCheckResult` and hide the methods of `BoundsCheckResult` that
allow creation of "incomplete" (not yet `finalize`d) `BoundsCheckResult`
objects (e.g. the constructor).
---
.../Checkers/ArrayBoundChecker.cpp | 34 ++++++++++++-------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 9f9fabab815c1..9007b1cc12d32 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -111,6 +111,12 @@ static StringRef asPreposition(BadOffsetKind Problem) {
return Prepositions[static_cast<int>(Problem)];
}
+struct CheckFlags {
+ bool CheckUnderflow;
+ bool OffsetObviouslyNonnegative;
+ bool AcceptPastTheEnd;
+};
+
class BoundsCheckResult {
public:
enum class Kind { Underflow, Overflow, TaintBug, Paradox, Valid };
@@ -123,7 +129,6 @@ class BoundsCheckResult {
std::optional<NonLoc> Extent;
ProgramStateRef State = nullptr;
-public:
BoundsCheckResult(NonLoc Offs, std::optional<NonLoc> E)
: Offset(Offs), Extent(E) {}
@@ -136,6 +141,13 @@ class BoundsCheckResult {
State = S;
}
+public:
+ friend BoundsCheckResult checkBounds(ProgramStateRef State,
+ SValBuilder &SVB,
+ NonLoc Offset,
+ std::optional<NonLoc> Extent,
+ CheckFlags Flags);
+
bool assumedNonNegative() const { return AssumedNonNegative; }
bool hasAssumption() const { return AssumedNonNegative || AssumedUpperBound; }
@@ -182,11 +194,11 @@ class BoundsCheckResult {
}
};
-struct CheckFlags {
- bool CheckUnderflow;
- bool OffsetObviouslyNonnegative;
- bool AcceptPastTheEnd;
-};
+BoundsCheckResult checkBounds(ProgramStateRef State,
+ SValBuilder &SVB,
+ NonLoc Offset,
+ std::optional<NonLoc> Extent,
+ CheckFlags Flags);
// NOTE: The `ArraySubscriptExpr` and `UnaryOperator` callbacks are `PostStmt`
// instead of `PreStmt` because the current implementation passes the whole
@@ -202,9 +214,6 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
BugType TaintBT{this, "Out-of-bound access", categories::TaintedData};
void handleAccessExpr(const Expr *E, CheckerContext &C) const;
- BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
- NonLoc Offset, std::optional<NonLoc> Extent,
- CheckFlags Flags) const;
void reportOOB(CheckerContext &C, ProgramStateRef ErrorState, Messages Msgs,
NonLoc Offset, std::optional<NonLoc> Extent,
@@ -235,7 +244,6 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
};
} // anonymous namespace
-
/// For a given Location that can be represented as a symbolic expression
/// Arr[Idx] (or perhaps Arr[Idx1][Idx2] etc.), return the parent memory block
/// Arr and the distance of Location from the beginning of Arr (expressed in a
@@ -697,11 +705,12 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
}
}
-BoundsCheckResult ArrayBoundChecker::checkBounds(ProgramStateRef State,
+namespace {
+BoundsCheckResult checkBounds(ProgramStateRef State,
SValBuilder &SVB,
NonLoc Offset,
std::optional<NonLoc> Extent,
- CheckFlags Flags) const {
+ CheckFlags Flags) {
BoundsCheckResult Res(Offset, Extent);
// CHECK LOWER BOUND
@@ -799,6 +808,7 @@ BoundsCheckResult ArrayBoundChecker::checkBounds(ProgramStateRef State,
Res.finalize(BoundsCheckResult::Kind::Valid, State);
return Res;
}
+} // anonymous namespace
void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
ProgramStateRef ErrorState,
>From 7a077d1f81b0f228e05f65bfad480b8d2fa85040 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 5 Jun 2026 12:45:34 +0200
Subject: [PATCH 08/29] Unify 'getRegionName' calls
---
.../StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 9007b1cc12d32..33875ad5ddeb1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -474,11 +474,9 @@ static bool tryDividePair(std::optional<int64_t> &Val1,
}
static Messages getNonTaintMsgs(const ASTContext &ACtx,
- const MemSpaceRegion *Space,
- const SubRegion *Region, NonLoc Offset,
+ std::string RegName, NonLoc Offset,
std::optional<NonLoc> Extent, SVal Location,
BadOffsetKind Problem) {
- std::string RegName = getRegionName(Space, Region);
const auto *EReg = Location.getAsRegion()->getAs<ElementRegion>();
assert(EReg && "this checker only handles element access");
QualType ElemType = EReg->getElementType();
@@ -533,10 +531,8 @@ static Messages getNonTaintMsgs(const ASTContext &ACtx,
std::string(Buf)};
}
-static Messages getTaintMsgs(const MemSpaceRegion *Space,
- const SubRegion *Region, const char *OffsetName,
+static Messages getTaintMsgs(std::string RegName, const char *OffsetName,
bool AlsoMentionUnderflow) {
- std::string RegName = getRegionName(Space, Region);
return {formatv("Potential out of bound access to {0} with tainted {1}",
RegName, OffsetName),
formatv("Access of {0} with a tainted {1} that may be {2}too large",
@@ -658,6 +654,8 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
BoundsCheckResult Res = checkBounds(State, SVB, ByteOffset, Extent, Flags);
+ std::string RN = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
+
switch (Res.getKind()) {
case BoundsCheckResult::Kind::Paradox:
// The current state is paradoxical (due to bad modeling of casts we
@@ -669,7 +667,6 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
case BoundsCheckResult::Kind::Valid: {
const NoteTag *Tag = nullptr;
if (Res.hasAssumption()) {
- std::string RN = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
SizeUnit SU = SizeUnit::forExpr(E, C);
Tag = C.getNoteTag(
[Res, RN, SU](PathSensitiveBugReport &BR) -> std::string {
@@ -690,14 +687,14 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
OffsetName = "index";
Messages Msgs =
- getTaintMsgs(Space, Reg, OffsetName,
+ getTaintMsgs(RN, OffsetName,
/*AlsoMentionUnderflow=*/Res.assumedNonNegative());
reportOOB(C, Res.getState(), Msgs, ByteOffset, Extent,
/*IsTaintBug=*/true);
return;
}
default: {
- Messages Msgs = getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset,
+ Messages Msgs = getNonTaintMsgs(C.getASTContext(), RN, ByteOffset,
Extent, Location, *Res.getBadOffsetKind());
reportOOB(C, Res.getState(), Msgs, ByteOffset, Extent);
return;
>From 416f31ad0f50800b57024c29201cdd8610382af2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 5 Jun 2026 13:08:07 +0200
Subject: [PATCH 09/29] Use 'SizeUnit' in 'getNonTaintMsgs'
This methods duplicates lots of logic that also appears in the method
'BoundsCheckResult::getMessage' (which composes the assumption
messages), so move towards unifying them.
---
.../Checkers/ArrayBoundChecker.cpp | 43 +++++++++++--------
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 33875ad5ddeb1..622ffafe18e64 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -75,7 +75,7 @@ class SizeUnit {
/// If `E` is a "clean" array subscript expression, return the type of the
/// accessed element; otherwise return 'Bytes' because that's the best (or
- /// least bad) option for the diagnostic generation that relies on this.
+ /// least bad) option for the assumption messages that use this.
static SizeUnit forExpr(const Expr *E, const CheckerContext &C) {
const auto *ASE = getAsCleanArraySubscriptExpr(E, C);
if (!ASE)
@@ -84,6 +84,16 @@ class SizeUnit {
return SizeUnit(ASE->getType(), C.getASTContext());
}
+ /// Return the element type that is "natural" for reporting out-of-bounds
+ /// memory access to 'Location'.
+ /// FIXME: It is unfortunate that this heuristic differs from the heuristic
+ /// used for reporting assumption (`SizeUnit::forExpr`).
+ static SizeUnit forSVal(SVal Location, const ASTContext &ACtx) {
+ const auto *EReg = Location.getAsRegion()->getAs<ElementRegion>();
+ assert(EReg && "this checker only handles element access");
+ return SizeUnit(EReg->getElementType(), ACtx);
+ }
+
int64_t asCharUnits() const { return AsCharUnits; }
std::string asExtentDesc(bool ForceBytes) const {
@@ -91,6 +101,12 @@ class SizeUnit {
return "the extent of";
return formatv("the number of '{0}' elements in", AsType.getAsString());
}
+
+ std::string asElementName(bool ForceBytes) const {
+ if (ForceBytes || isBytes())
+ return "byte";
+ return formatv("'{0}' element", AsType.getAsString());
+ }
};
struct Messages {
@@ -473,13 +489,9 @@ static bool tryDividePair(std::optional<int64_t> &Val1,
return true;
}
-static Messages getNonTaintMsgs(const ASTContext &ACtx,
- std::string RegName, NonLoc Offset,
- std::optional<NonLoc> Extent, SVal Location,
+static Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
+ std::optional<NonLoc> Extent,
BadOffsetKind Problem) {
- const auto *EReg = Location.getAsRegion()->getAs<ElementRegion>();
- assert(EReg && "this checker only handles element access");
- QualType ElemType = EReg->getElementType();
std::optional<int64_t> OffsetN = getConcreteValue(Offset);
std::optional<int64_t> ExtentN = getConcreteValue(Extent);
@@ -487,9 +499,7 @@ static Messages getNonTaintMsgs(const ASTContext &ACtx,
if (Problem == BadOffsetKind::Negative)
ExtentN = std::nullopt;
- int64_t ElemSize = ACtx.getTypeSizeInChars(ElemType).getQuantity();
-
- bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, ElemSize);
+ bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
const char *OffsetOrIndex = UseByteOffsets ? "byte offset" : "index";
SmallString<256> Buf;
@@ -501,7 +511,7 @@ static Messages getNonTaintMsgs(const ASTContext &ACtx,
// natural to mention the element type later where the extent is described,
// but if the extent is unknown/irrelevant, then the element type can be
// inserted into the message at this point.
- Out << "'" << ElemType.getAsString() << "' element in ";
+ Out << SU.asElementName(/*ForceBytes=*/false) << " in ";
}
Out << RegName << " at ";
if (OffsetN) {
@@ -517,10 +527,8 @@ static Messages getNonTaintMsgs(const ASTContext &ACtx,
Out << *ExtentN;
else
Out << "a single";
- if (UseByteOffsets)
- Out << " byte";
- else
- Out << " '" << ElemType.getAsString() << "' element";
+
+ Out << ' ' << SU.asElementName(/*ForceBytes=*/UseByteOffsets);
if (*ExtentN > 1)
Out << "s";
@@ -694,8 +702,9 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
return;
}
default: {
- Messages Msgs = getNonTaintMsgs(C.getASTContext(), RN, ByteOffset,
- Extent, Location, *Res.getBadOffsetKind());
+ SizeUnit SU = SizeUnit::forSVal(Location, C.getASTContext());
+ Messages Msgs =
+ getNonTaintMsgs(RN, SU, ByteOffset, Extent, *Res.getBadOffsetKind());
reportOOB(C, Res.getState(), Msgs, ByteOffset, Extent);
return;
}
>From 92b237d458c3194fbbb87e201caaea6f335e7189 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 5 Jun 2026 14:42:32 +0200
Subject: [PATCH 10/29] Extract BoundsChecking library to separate files
The code is moved to the new location with only very few necessary
changes (removing 'static' from some functions).
It will be improved with further commits.
---
.../StaticAnalyzer/Checkers/BoundsChecking.h | 205 ++++++
.../Checkers/ArrayBoundChecker.cpp | 599 +-----------------
.../Checkers/BoundsChecking.cpp | 447 +++++++++++++
.../StaticAnalyzer/Checkers/CMakeLists.txt | 1 +
.../lib/StaticAnalyzer/Checkers/BUILD.gn | 1 +
5 files changed, 655 insertions(+), 598 deletions(-)
create mode 100644 clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
create mode 100644 clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
new file mode 100644
index 0000000000000..8aebe63556182
--- /dev/null
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -0,0 +1,205 @@
+//===- BoundsChecking.h - Bounds checking related APIs ----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines APIs for performing a bounds check (i.e. comparing a
+// symbolic Offset value to zero and a symbolic Extent value) and composing
+// descriptions that explain its results.
+//
+// This is intended as a replacement for `ProgramState::assumeInBound` to
+// avoid its incorrect logic and compensate for deficiencies of other parts of
+// the analyzer.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BOUNDSCHECKING_H
+#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BOUNDSCHECKING_H
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/Support/FormatVariadic.h"
+#include <optional>
+
+using llvm::formatv;
+
+namespace clang {
+namespace ento {
+
+/// If `E` is an array subscript expression with a base that is "clean" (= not
+/// modified by pointer arithmetic = the beginning of a memory region), return
+/// it as a pointer to ArraySubscriptExpr; otherwise return nullptr.
+/// This helper function is used by two separate heuristics that are only valid
+/// in these "clean" cases.
+const ArraySubscriptExpr *getAsCleanArraySubscriptExpr(const Expr *E,
+ const CheckerContext &C);
+
+class SizeUnit {
+ QualType AsType;
+ int64_t AsCharUnits;
+
+ SizeUnit() : AsType(), AsCharUnits(1) {}
+
+public:
+ SizeUnit(QualType T, const ASTContext &ACtx)
+ : AsType(T), AsCharUnits(ACtx.getTypeSizeInChars(T).getQuantity()) {
+ assert(!T.isNull());
+ }
+
+ static SizeUnit bytes() { return SizeUnit(); }
+
+ bool isBytes() const { return AsType.isNull(); }
+
+ /// If `E` is a "clean" array subscript expression, return the type of the
+ /// accessed element; otherwise return 'Bytes' because that's the best (or
+ /// least bad) option for the assumption messages that use this.
+ static SizeUnit forExpr(const Expr *E, const CheckerContext &C) {
+ const auto *ASE = getAsCleanArraySubscriptExpr(E, C);
+ if (!ASE)
+ return bytes();
+
+ return SizeUnit(ASE->getType(), C.getASTContext());
+ }
+
+ /// Return the element type that is "natural" for reporting out-of-bounds
+ /// memory access to 'Location'.
+ /// FIXME: It is unfortunate that this heuristic differs from the heuristic
+ /// used for reporting assumption (`SizeUnit::forExpr`).
+ static SizeUnit forSVal(SVal Location, const ASTContext &ACtx) {
+ const auto *EReg = Location.getAsRegion()->getAs<ElementRegion>();
+ assert(EReg && "this checker only handles element access");
+ return SizeUnit(EReg->getElementType(), ACtx);
+ }
+
+ int64_t asCharUnits() const { return AsCharUnits; }
+
+ std::string asExtentDesc(bool ForceBytes) const {
+ if (ForceBytes || isBytes())
+ return "the extent of";
+ return formatv("the number of '{0}' elements in", AsType.getAsString());
+ }
+
+ std::string asElementName(bool ForceBytes) const {
+ if (ForceBytes || isBytes())
+ return "byte";
+ return formatv("'{0}' element", AsType.getAsString());
+ }
+};
+
+struct Messages {
+ std::string Short, Full;
+};
+
+enum class BadOffsetKind { Negative, Overflowing, Indeterminate };
+
+constexpr llvm::StringLiteral Adjectives[] = {"a negative", "an overflowing",
+ "a negative or overflowing"};
+inline StringRef asAdjective(BadOffsetKind Problem) {
+ return Adjectives[static_cast<int>(Problem)];
+}
+
+constexpr llvm::StringLiteral Prepositions[] = {"preceding", "after the end of",
+ "around"};
+inline StringRef asPreposition(BadOffsetKind Problem) {
+ return Prepositions[static_cast<int>(Problem)];
+}
+
+struct CheckFlags {
+ bool CheckUnderflow;
+ bool OffsetObviouslyNonnegative;
+ bool AcceptPastTheEnd;
+};
+
+class BoundsCheckResult {
+public:
+ enum class Kind { Underflow, Overflow, TaintBug, Paradox, Valid };
+
+private:
+ Kind K = Kind::Valid;
+ bool AssumedNonNegative = false;
+ bool AssumedUpperBound = false;
+ const NonLoc Offset;
+ std::optional<NonLoc> Extent;
+ ProgramStateRef State = nullptr;
+
+ BoundsCheckResult(NonLoc Offs, std::optional<NonLoc> E)
+ : Offset(Offs), Extent(E) {}
+
+ void recordNonNegativeAssumption() { AssumedNonNegative = true; }
+
+ void recordUpperBoundAssumption() { AssumedUpperBound = true; }
+
+ void finalize(Kind K_, ProgramStateRef S) {
+ K = K_;
+ State = S;
+ }
+
+public:
+ friend BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
+ NonLoc Offset,
+ std::optional<NonLoc> Extent,
+ CheckFlags Flags);
+
+ bool assumedNonNegative() const { return AssumedNonNegative; }
+
+ bool hasAssumption() const { return AssumedNonNegative || AssumedUpperBound; }
+
+ ProgramStateRef getState() const { return State; }
+
+ Kind getKind() const { return K; }
+
+ std::optional<BadOffsetKind> getBadOffsetKind() const {
+ switch (K) {
+ case Kind::Underflow:
+ return BadOffsetKind::Negative;
+ case Kind::Overflow:
+ return assumedNonNegative() ? BadOffsetKind::Indeterminate
+ : BadOffsetKind::Overflowing;
+ default:
+ return std::nullopt;
+ }
+ }
+
+ std::string getMessage(PathSensitiveBugReport &BR, StringRef RegName,
+ SizeUnit SU) const;
+
+private:
+ /// Return true if information about the value of `Sym` can put constraints
+ /// on some symbol which is interesting within the bug report `BR`.
+ /// In particular, this returns true when `Sym` is interesting within `BR`;
+ /// but it also returns true if `Sym` is an expression that contains integer
+ /// constants and a single symbolic operand which is interesting (in `BR`).
+ /// We need to use this instead of plain `BR.isInteresting()` because if we
+ /// are analyzing code like
+ /// int array[10];
+ /// int f(int arg) {
+ /// return array[arg] && array[arg + 10];
+ /// }
+ /// then the byte offsets are `arg * 4` and `(arg + 10) * 4`, which are not
+ /// sub-expressions of each other (but `getSimplifiedOffsets` is smart enough
+ /// to detect this out of bounds access).
+ static bool providesInformationAboutInteresting(SymbolRef Sym,
+ PathSensitiveBugReport &BR);
+ static bool providesInformationAboutInteresting(SVal SV,
+ PathSensitiveBugReport &BR) {
+ return providesInformationAboutInteresting(SV.getAsSymbol(), BR);
+ }
+};
+
+BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
+ NonLoc Offset, std::optional<NonLoc> Extent,
+ CheckFlags Flags);
+
+std::string getRegionName(const MemSpaceRegion *Space, const SubRegion *Region);
+
+Messages getTaintMsgs(std::string RegName, const char *OffsetName,
+ bool AlsoMentionUnderflow);
+
+Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
+ std::optional<NonLoc> Extent, BadOffsetKind Problem);
+
+} // namespace ento
+} // namespace clang
+
+#endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BOUNDSCHECKING_H
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 622ffafe18e64..a3b68854bef5c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -11,211 +11,21 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/AST/CharUnits.h"
#include "clang/AST/ParentMapContext.h"
+#include "clang/StaticAnalyzer/Checkers/BoundsChecking.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Checkers/Taint.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "llvm/ADT/APSInt.h"
-#include "llvm/Support/FormatVariadic.h"
-#include "llvm/Support/raw_ostream.h"
#include <optional>
using namespace clang;
using namespace ento;
using namespace taint;
-using llvm::formatv;
namespace {
-/// If `E` is an array subscript expression with a base that is "clean" (= not
-/// modified by pointer arithmetic = the beginning of a memory region), return
-/// it as a pointer to ArraySubscriptExpr; otherwise return nullptr.
-/// This helper function is used by two separate heuristics that are only valid
-/// in these "clean" cases.
-static const ArraySubscriptExpr *
-getAsCleanArraySubscriptExpr(const Expr *E, const CheckerContext &C) {
- const auto *ASE = dyn_cast<ArraySubscriptExpr>(E);
- if (!ASE)
- return nullptr;
-
- const MemRegion *SubscriptBaseReg = C.getSVal(ASE->getBase()).getAsRegion();
- if (!SubscriptBaseReg)
- return nullptr;
-
- // The base of the subscript expression is affected by pointer arithmetics,
- // so we want to report byte offsets instead of indices and we don't want to
- // activate the "index is unsigned -> cannot be negative" shortcut.
- if (isa<ElementRegion>(SubscriptBaseReg->StripCasts()))
- return nullptr;
-
- return ASE;
-}
-
-class SizeUnit {
- QualType AsType;
- int64_t AsCharUnits;
-
- SizeUnit() : AsType(), AsCharUnits(1) {}
-
-public:
- SizeUnit(QualType T, const ASTContext &ACtx)
- : AsType(T), AsCharUnits(ACtx.getTypeSizeInChars(T).getQuantity()) {
- assert(!T.isNull());
- }
-
- static SizeUnit bytes() { return SizeUnit(); }
-
- bool isBytes() const { return AsType.isNull(); }
-
- /// If `E` is a "clean" array subscript expression, return the type of the
- /// accessed element; otherwise return 'Bytes' because that's the best (or
- /// least bad) option for the assumption messages that use this.
- static SizeUnit forExpr(const Expr *E, const CheckerContext &C) {
- const auto *ASE = getAsCleanArraySubscriptExpr(E, C);
- if (!ASE)
- return bytes();
-
- return SizeUnit(ASE->getType(), C.getASTContext());
- }
-
- /// Return the element type that is "natural" for reporting out-of-bounds
- /// memory access to 'Location'.
- /// FIXME: It is unfortunate that this heuristic differs from the heuristic
- /// used for reporting assumption (`SizeUnit::forExpr`).
- static SizeUnit forSVal(SVal Location, const ASTContext &ACtx) {
- const auto *EReg = Location.getAsRegion()->getAs<ElementRegion>();
- assert(EReg && "this checker only handles element access");
- return SizeUnit(EReg->getElementType(), ACtx);
- }
-
- int64_t asCharUnits() const { return AsCharUnits; }
-
- std::string asExtentDesc(bool ForceBytes) const {
- if (ForceBytes || isBytes())
- return "the extent of";
- return formatv("the number of '{0}' elements in", AsType.getAsString());
- }
-
- std::string asElementName(bool ForceBytes) const {
- if (ForceBytes || isBytes())
- return "byte";
- return formatv("'{0}' element", AsType.getAsString());
- }
-};
-
-struct Messages {
- std::string Short, Full;
-};
-
-enum class BadOffsetKind { Negative, Overflowing, Indeterminate };
-
-constexpr llvm::StringLiteral Adjectives[] = {"a negative", "an overflowing",
- "a negative or overflowing"};
-static StringRef asAdjective(BadOffsetKind Problem) {
- return Adjectives[static_cast<int>(Problem)];
-}
-
-constexpr llvm::StringLiteral Prepositions[] = {"preceding", "after the end of",
- "around"};
-static StringRef asPreposition(BadOffsetKind Problem) {
- return Prepositions[static_cast<int>(Problem)];
-}
-
-struct CheckFlags {
- bool CheckUnderflow;
- bool OffsetObviouslyNonnegative;
- bool AcceptPastTheEnd;
-};
-
-class BoundsCheckResult {
-public:
- enum class Kind { Underflow, Overflow, TaintBug, Paradox, Valid };
-
-private:
- Kind K = Kind::Valid;
- bool AssumedNonNegative = false;
- bool AssumedUpperBound = false;
- const NonLoc Offset;
- std::optional<NonLoc> Extent;
- ProgramStateRef State = nullptr;
-
- BoundsCheckResult(NonLoc Offs, std::optional<NonLoc> E)
- : Offset(Offs), Extent(E) {}
-
- void recordNonNegativeAssumption() { AssumedNonNegative = true; }
-
- void recordUpperBoundAssumption() { AssumedUpperBound = true; }
-
- void finalize(Kind K_, ProgramStateRef S) {
- K = K_;
- State = S;
- }
-
-public:
- friend BoundsCheckResult checkBounds(ProgramStateRef State,
- SValBuilder &SVB,
- NonLoc Offset,
- std::optional<NonLoc> Extent,
- CheckFlags Flags);
-
- bool assumedNonNegative() const { return AssumedNonNegative; }
-
- bool hasAssumption() const { return AssumedNonNegative || AssumedUpperBound; }
-
- ProgramStateRef getState() const { return State; }
-
- Kind getKind() const { return K; }
-
- std::optional<BadOffsetKind> getBadOffsetKind() const {
- switch (K) {
- case Kind::Underflow:
- return BadOffsetKind::Negative;
- case Kind::Overflow:
- return assumedNonNegative() ? BadOffsetKind::Indeterminate
- : BadOffsetKind::Overflowing;
- default:
- return std::nullopt;
- }
- }
-
- std::string getMessage(PathSensitiveBugReport &BR, StringRef RegName,
- SizeUnit SU) const;
-
-private:
- /// Return true if information about the value of `Sym` can put constraints
- /// on some symbol which is interesting within the bug report `BR`.
- /// In particular, this returns true when `Sym` is interesting within `BR`;
- /// but it also returns true if `Sym` is an expression that contains integer
- /// constants and a single symbolic operand which is interesting (in `BR`).
- /// We need to use this instead of plain `BR.isInteresting()` because if we
- /// are analyzing code like
- /// int array[10];
- /// int f(int arg) {
- /// return array[arg] && array[arg + 10];
- /// }
- /// then the byte offsets are `arg * 4` and `(arg + 10) * 4`, which are not
- /// sub-expressions of each other (but `getSimplifiedOffsets` is smart enough
- /// to detect this out of bounds access).
- static bool providesInformationAboutInteresting(SymbolRef Sym,
- PathSensitiveBugReport &BR);
- static bool providesInformationAboutInteresting(SVal SV,
- PathSensitiveBugReport &BR) {
- return providesInformationAboutInteresting(SV.getAsSymbol(), BR);
- }
-};
-
-BoundsCheckResult checkBounds(ProgramStateRef State,
- SValBuilder &SVB,
- NonLoc Offset,
- std::optional<NonLoc> Extent,
- CheckFlags Flags);
-
// NOTE: The `ArraySubscriptExpr` and `UnaryOperator` callbacks are `PostStmt`
// instead of `PreStmt` because the current implementation passes the whole
// expression to `CheckerContext::getSVal()` which only works after the
@@ -316,308 +126,6 @@ computeOffset(ProgramStateRef State, SValBuilder &SVB, SVal Location) {
return std::nullopt;
}
-// NOTE: This function is the "heart" of this checker. It simplifies
-// inequalities with transformations that are valid (and very elementary) in
-// pure mathematics, but become invalid if we use them in C++ number model
-// where the calculations may overflow.
-// Due to the overflow issues I think it's impossible (or at least not
-// practical) to integrate this kind of simplification into the resolution of
-// arbitrary inequalities (i.e. the code of `evalBinOp`); but this function
-// produces valid results when the calculations are handling memory offsets
-// and every value is well below SIZE_MAX.
-// TODO: This algorithm should be moved to a central location where it's
-// available for other checkers that need to compare memory offsets.
-// NOTE: the simplification preserves the order of the two operands in a
-// mathematical sense, but it may change the result produced by a C++
-// comparison operator (and the automatic type conversions).
-// For example, consider a comparison "X+1 < 0", where the LHS is stored as a
-// size_t and the RHS is stored in an int. (As size_t is unsigned, this
-// comparison is false for all values of "X".) However, the simplification may
-// turn it into "X < -1", which is still always false in a mathematical sense,
-// but can produce a true result when evaluated by `evalBinOp` (which follows
-// the rules of C++ and casts -1 to SIZE_MAX).
-static std::pair<NonLoc, nonloc::ConcreteInt>
-getSimplifiedOffsets(NonLoc offset, nonloc::ConcreteInt extent,
- SValBuilder &svalBuilder) {
- const llvm::APSInt &extentVal = extent.getValue();
- std::optional<nonloc::SymbolVal> SymVal = offset.getAs<nonloc::SymbolVal>();
- if (SymVal && SymVal->isExpression()) {
- if (const SymIntExpr *SIE = dyn_cast<SymIntExpr>(SymVal->getSymbol())) {
- llvm::APSInt constant = APSIntType(extentVal).convert(SIE->getRHS());
- switch (SIE->getOpcode()) {
- case BO_Mul:
- // The constant should never be 0 here, becasue multiplication by zero
- // is simplified by the engine.
- if ((extentVal % constant) != 0)
- return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
- else
- return getSimplifiedOffsets(
- nonloc::SymbolVal(SIE->getLHS()),
- svalBuilder.makeIntVal(extentVal / constant), svalBuilder);
- case BO_Add:
- return getSimplifiedOffsets(
- nonloc::SymbolVal(SIE->getLHS()),
- svalBuilder.makeIntVal(extentVal - constant), svalBuilder);
- default:
- break;
- }
- }
- }
-
- return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
-}
-
-static bool isNegative(SValBuilder &SVB, ProgramStateRef State, NonLoc Value) {
- const llvm::APSInt *MaxV = SVB.getMaxValue(State, Value);
- return MaxV && MaxV->isNegative();
-}
-
-static bool isUnsigned(SValBuilder &SVB, NonLoc Value) {
- QualType T = Value.getType(SVB.getContext());
- return T->isUnsignedIntegerType();
-}
-
-// Evaluate the comparison Value < Threshold with the help of the custom
-// simplification algorithm defined for this checker. Return a pair of states,
-// where the first one corresponds to "value below threshold" and the second
-// corresponds to "value at or above threshold". Returns {nullptr, nullptr} in
-// the case when the evaluation fails.
-// If the optional argument CheckEquality is true, then use BO_EQ instead of
-// the default BO_LT after consistently applying the same simplification steps.
-static std::pair<ProgramStateRef, ProgramStateRef>
-compareValueToThreshold(ProgramStateRef State, NonLoc Value, NonLoc Threshold,
- SValBuilder &SVB, bool CheckEquality = false) {
- if (auto ConcreteThreshold = Threshold.getAs<nonloc::ConcreteInt>()) {
- std::tie(Value, Threshold) =
- getSimplifiedOffsets(Value, *ConcreteThreshold, SVB);
- }
-
- // We want to perform a _mathematical_ comparison between the numbers `Value`
- // and `Threshold`; but `evalBinOpNN` 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`:
- if (isNegative(SVB, State, Value) && isUnsigned(SVB, Threshold)) {
- if (CheckEquality) {
- // negative_value == unsigned_threshold is always false
- return {nullptr, State};
- }
- // negative_value < unsigned_threshold is always true
- return {State, nullptr};
- }
- if (isUnsigned(SVB, Value) && isNegative(SVB, State, Threshold)) {
- // unsigned_value == negative_threshold and
- // unsigned_value < negative_threshold are both always false
- return {nullptr, State};
- }
- // FIXME: These special cases are sufficient for handling real-world
- // 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
- // 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
- // evaluate these "mathematical" comparisons through a separate pathway would
- // be a step backwards in this sense.
-
- const BinaryOperatorKind OpKind = CheckEquality ? BO_EQ : BO_LT;
- auto BelowThreshold =
- SVB.evalBinOpNN(State, OpKind, Value, Threshold, SVB.getConditionType())
- .getAs<NonLoc>();
-
- if (BelowThreshold)
- return State->assume(*BelowThreshold);
-
- return {nullptr, nullptr};
-}
-
-static std::string getRegionName(const MemSpaceRegion *Space,
- const SubRegion *Region) {
- if (std::string RegName = Region->getDescriptiveName(); !RegName.empty())
- return RegName;
-
- // Field regions only have descriptive names when their parent has a
- // descriptive name; so we provide a fallback representation for them:
- if (const auto *FR = Region->getAs<FieldRegion>()) {
- if (StringRef Name = FR->getDecl()->getName(); !Name.empty())
- return formatv("the field '{0}'", Name);
- return "the unnamed field";
- }
-
- if (isa<AllocaRegion>(Region))
- return "the memory returned by 'alloca'";
-
- if (isa<SymbolicRegion>(Region) && isa<HeapSpaceRegion>(Space))
- return "the heap area";
-
- if (isa<StringRegion>(Region))
- return "the string literal";
-
- return "the region";
-}
-
-static std::optional<int64_t> getConcreteValue(NonLoc SV) {
- if (auto ConcreteVal = SV.getAs<nonloc::ConcreteInt>()) {
- return ConcreteVal->getValue()->tryExtValue();
- }
- return std::nullopt;
-}
-
-static std::optional<int64_t> getConcreteValue(std::optional<NonLoc> SV) {
- return SV ? getConcreteValue(*SV) : std::nullopt;
-}
-
-/// Try to divide `Val1` and `Val2` (in place) by `Divisor` and return true if
-/// it can be performed (`Divisor` is nonzero and there is no remainder). The
-/// values `Val1` and `Val2` may be nullopt and in that case the corresponding
-/// division is considered to be successful.
-static bool tryDividePair(std::optional<int64_t> &Val1,
- std::optional<int64_t> &Val2, int64_t Divisor) {
- if (!Divisor)
- return false;
- const bool Val1HasRemainder = Val1 && *Val1 % Divisor;
- const bool Val2HasRemainder = Val2 && *Val2 % Divisor;
- if (Val1HasRemainder || Val2HasRemainder)
- return false;
- if (Val1)
- *Val1 /= Divisor;
- if (Val2)
- *Val2 /= Divisor;
- return true;
-}
-
-static Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
- std::optional<NonLoc> Extent,
- BadOffsetKind Problem) {
-
- std::optional<int64_t> OffsetN = getConcreteValue(Offset);
- std::optional<int64_t> ExtentN = getConcreteValue(Extent);
-
- if (Problem == BadOffsetKind::Negative)
- ExtentN = std::nullopt;
-
- bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
- const char *OffsetOrIndex = UseByteOffsets ? "byte offset" : "index";
-
- SmallString<256> Buf;
- llvm::raw_svector_ostream Out(Buf);
- Out << "Access of ";
- if (OffsetN && !ExtentN && !UseByteOffsets) {
- // If the offset is reported as an index, then the report must mention the
- // element type (because it is not always clear from the code). It's more
- // natural to mention the element type later where the extent is described,
- // but if the extent is unknown/irrelevant, then the element type can be
- // inserted into the message at this point.
- Out << SU.asElementName(/*ForceBytes=*/false) << " in ";
- }
- Out << RegName << " at ";
- if (OffsetN) {
- if (Problem == BadOffsetKind::Negative)
- Out << "negative ";
- Out << OffsetOrIndex << " " << *OffsetN;
- } else {
- Out << asAdjective(Problem) << " " << OffsetOrIndex;
- }
- if (ExtentN) {
- Out << ", while it holds only ";
- if (*ExtentN != 1)
- Out << *ExtentN;
- else
- Out << "a single";
-
- Out << ' ' << SU.asElementName(/*ForceBytes=*/UseByteOffsets);
-
- if (*ExtentN > 1)
- Out << "s";
- }
-
- return {formatv("Out of bound access to memory {0} {1}",
- asPreposition(Problem), RegName),
- std::string(Buf)};
-}
-
-static Messages getTaintMsgs(std::string RegName, const char *OffsetName,
- bool AlsoMentionUnderflow) {
- return {formatv("Potential out of bound access to {0} with tainted {1}",
- RegName, OffsetName),
- formatv("Access of {0} with a tainted {1} that may be {2}too large",
- RegName, OffsetName,
- AlsoMentionUnderflow ? "negative or " : "")};
-}
-
-std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
- StringRef RegName,
- SizeUnit SU) const {
- bool ShouldReportNonNegative = AssumedNonNegative;
- if (!providesInformationAboutInteresting(Offset, BR)) {
- if (AssumedUpperBound && providesInformationAboutInteresting(*Extent, BR)) {
- // Even if the byte offset isn't interesting (e.g. it's a constant value),
- // the assumption can still be interesting if it provides information
- // about an interesting symbolic upper bound.
- ShouldReportNonNegative = false;
- } else {
- // We don't have anything interesting, don't report the assumption.
- return "";
- }
- }
-
- std::optional<int64_t> OffsetN = getConcreteValue(Offset);
- std::optional<int64_t> ExtentN = getConcreteValue(Extent);
-
- const bool UseIndex =
- !SU.isBytes() && tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
-
- SmallString<256> Buf;
- llvm::raw_svector_ostream Out(Buf);
- Out << "Assuming ";
- if (UseIndex) {
- Out << "index ";
- if (OffsetN)
- Out << "'" << OffsetN << "' ";
- } else if (Extent) {
- Out << "byte offset ";
- if (OffsetN)
- Out << "'" << OffsetN << "' ";
- } else {
- Out << "offset ";
- }
-
- Out << "is";
- if (ShouldReportNonNegative) {
- Out << " non-negative";
- }
- if (Extent) {
- if (ShouldReportNonNegative)
- Out << " and";
- Out << " less than ";
- if (ExtentN)
- Out << *ExtentN << ", ";
- Out << SU.asExtentDesc(/*ForceBytes=*/!UseIndex) << ' ' << RegName;
- }
- return std::string(Out.str());
-}
-
-bool BoundsCheckResult::providesInformationAboutInteresting(
- SymbolRef Sym, PathSensitiveBugReport &BR) {
- if (!Sym)
- return false;
- for (SymbolRef PartSym : Sym->symbols()) {
- // The interestingess mark may appear on any layer as we're stripping off
- // the SymIntExpr, UnarySymExpr etc. layers...
- if (BR.isInteresting(PartSym))
- return true;
- // ...but if both sides of the expression are symbolic, then there is no
- // practical algorithm to produce separate constraints for the two
- // operands (from the single combined result).
- if (isa<SymSymExpr>(PartSym))
- return false;
- }
- return false;
-}
-
void ArrayBoundChecker::handleAccessExpr(const Expr *E,
CheckerContext &C) const {
const SVal Location = C.getSVal(E);
@@ -711,111 +219,6 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
}
}
-namespace {
-BoundsCheckResult checkBounds(ProgramStateRef State,
- SValBuilder &SVB,
- NonLoc Offset,
- std::optional<NonLoc> Extent,
- CheckFlags Flags) {
- BoundsCheckResult Res(Offset, Extent);
-
- // CHECK LOWER BOUND
- if (Flags.CheckUnderflow) {
- auto [PrecedesLowerBound, WithinLowerBound] =
- compareValueToThreshold(State, Offset, SVB.makeZeroArrayIndex(), SVB);
-
- if (PrecedesLowerBound) {
- // The analyzer thinks that the offset may be invalid (negative)...
- if (Flags.OffsetObviouslyNonnegative) {
- // ...but the offset is obviously non-negative (clear array subscript
- // with an unsigned index), so we're in a buggy situation.
-
- // TODO: Currently the analyzer ignores many casts (e.g. signed ->
- // unsigned casts), so it can easily reach states where it will load a
- // signed (and negative) value from an unsigned variable. This sanity
- // check is a duct tape "solution" that silences most of the ugly false
- // positives that are caused by this buggy behavior. Note that this is
- // not a complete solution: this cannot silence reports where pointer
- // arithmetic complicates the picture and cannot ensure modeling of the
- // "unsigned index is positive with highest bit set" cases which are
- // "usurped" by the nonsense "unsigned index is negative" case.
- // For more information about this topic, see the umbrella ticket
- // https://github.com/llvm/llvm-project/issues/39492
- // TODO: Remove this hack once 'SymbolCast's are modeled properly.
-
- if (!WithinLowerBound) {
- // The state is completely nonsense -- let's just sink it!
- Res.finalize(BoundsCheckResult::Kind::Paradox, PrecedesLowerBound);
- return Res;
- }
- // Otherwise continue on the 'WithinLowerBound' branch where the
- // unsigned index _is_ non-negative. Don't mention this assumption as a
- // note tag, because it would just confuse the users!
- } else {
- if (!WithinLowerBound) {
- // ...and it cannot be valid (>= 0), so report an error.
- Res.finalize(BoundsCheckResult::Kind::Underflow, PrecedesLowerBound);
- return Res;
- }
- // ...but it can be valid as well, so the checker will (optimistically)
- // assume that it's valid and mention this in the note tag.
- Res.recordNonNegativeAssumption();
- }
- }
-
- // 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.
- if (WithinLowerBound)
- State = WithinLowerBound;
- }
-
- // CHECK UPPER BOUND
- if (Extent) {
- auto [WithinUpperBound, ExceedsUpperBound] =
- compareValueToThreshold(State, Offset, *Extent, SVB);
-
- if (ExceedsUpperBound) {
- // The offset may be invalid (>= Size)...
- if (!WithinUpperBound) {
- // ...and it cannot be within bounds, so report an error, unless we can
- // definitely determine that this is an idiomatic `&array[size]`
- // expression that calculates the past-the-end pointer.
- if (Flags.AcceptPastTheEnd) {
- auto [EqualsToThreshold, NotEqualToThreshold] =
- compareValueToThreshold(State, Offset, *Extent, SVB,
- /*CheckEquality=*/true);
- if (EqualsToThreshold && !NotEqualToThreshold) {
- Res.finalize(BoundsCheckResult::Kind::Valid, State);
- return Res;
- }
- }
-
- Res.finalize(BoundsCheckResult::Kind::Overflow, ExceedsUpperBound);
- return Res;
- }
- // ...and it can be valid as well...
- if (isTainted(State, Offset)) {
- // ...but it's tainted, so report an error.
- Res.finalize(BoundsCheckResult::Kind::TaintBug, State);
- return Res;
- }
- // ...and it isn't tainted, so the checker will (optimistically) assume
- // that the offset is in bounds and mention this in the note tag.
- Res.recordUpperBoundAssumption();
- }
-
- // 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.
- if (WithinUpperBound)
- State = WithinUpperBound;
- }
- Res.finalize(BoundsCheckResult::Kind::Valid, State);
- return Res;
-}
-} // anonymous namespace
-
void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
ProgramStateRef ErrorState,
NonLoc Val, bool MarkTaint) {
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
new file mode 100644
index 0000000000000..dbdccf516517a
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -0,0 +1,447 @@
+//===- BoundsChecking.cpp - Bounds checking related APIs --------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines APIs for performing a bounds check (i.e. comparing a
+// symbolic Offset value to zero and a symbolic Extent value) and composing
+// descriptions that explain its results.
+//
+// This is intended as a replacement for `ProgramState::assumeInBound` to
+// avoid its incorrect logic and compensate for deficiencies of other parts of
+// the analyzer.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Checkers/BoundsChecking.h"
+#include "clang/StaticAnalyzer/Checkers/Taint.h"
+
+namespace clang {
+namespace ento {
+
+const ArraySubscriptExpr *
+getAsCleanArraySubscriptExpr(const Expr *E, const CheckerContext &C) {
+ const auto *ASE = dyn_cast<ArraySubscriptExpr>(E);
+ if (!ASE)
+ return nullptr;
+
+ const MemRegion *SubscriptBaseReg = C.getSVal(ASE->getBase()).getAsRegion();
+ if (!SubscriptBaseReg)
+ return nullptr;
+
+ // The base of the subscript expression is affected by pointer arithmetics,
+ // so we want to report byte offsets instead of indices and we don't want to
+ // activate the "index is unsigned -> cannot be negative" shortcut.
+ if (isa<ElementRegion>(SubscriptBaseReg->StripCasts()))
+ return nullptr;
+
+ return ASE;
+}
+
+// NOTE: This function is the "heart" of this checker. It simplifies
+// inequalities with transformations that are valid (and very elementary) in
+// pure mathematics, but become invalid if we use them in C++ number model
+// where the calculations may overflow.
+// Due to the overflow issues I think it's impossible (or at least not
+// practical) to integrate this kind of simplification into the resolution of
+// arbitrary inequalities (i.e. the code of `evalBinOp`); but this function
+// produces valid results when the calculations are handling memory offsets
+// and every value is well below SIZE_MAX.
+// TODO: This algorithm should be moved to a central location where it's
+// available for other checkers that need to compare memory offsets.
+// NOTE: the simplification preserves the order of the two operands in a
+// mathematical sense, but it may change the result produced by a C++
+// comparison operator (and the automatic type conversions).
+// For example, consider a comparison "X+1 < 0", where the LHS is stored as a
+// size_t and the RHS is stored in an int. (As size_t is unsigned, this
+// comparison is false for all values of "X".) However, the simplification may
+// turn it into "X < -1", which is still always false in a mathematical sense,
+// but can produce a true result when evaluated by `evalBinOp` (which follows
+// the rules of C++ and casts -1 to SIZE_MAX).
+static std::pair<NonLoc, nonloc::ConcreteInt>
+getSimplifiedOffsets(NonLoc offset, nonloc::ConcreteInt extent,
+ SValBuilder &svalBuilder) {
+ const llvm::APSInt &extentVal = extent.getValue();
+ std::optional<nonloc::SymbolVal> SymVal = offset.getAs<nonloc::SymbolVal>();
+ if (SymVal && SymVal->isExpression()) {
+ if (const SymIntExpr *SIE = dyn_cast<SymIntExpr>(SymVal->getSymbol())) {
+ llvm::APSInt constant = APSIntType(extentVal).convert(SIE->getRHS());
+ switch (SIE->getOpcode()) {
+ case BO_Mul:
+ // The constant should never be 0 here, becasue multiplication by zero
+ // is simplified by the engine.
+ if ((extentVal % constant) != 0)
+ return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
+ else
+ return getSimplifiedOffsets(
+ nonloc::SymbolVal(SIE->getLHS()),
+ svalBuilder.makeIntVal(extentVal / constant), svalBuilder);
+ case BO_Add:
+ return getSimplifiedOffsets(
+ nonloc::SymbolVal(SIE->getLHS()),
+ svalBuilder.makeIntVal(extentVal - constant), svalBuilder);
+ default:
+ break;
+ }
+ }
+ }
+
+ return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
+}
+
+static bool isNegative(SValBuilder &SVB, ProgramStateRef State, NonLoc Value) {
+ const llvm::APSInt *MaxV = SVB.getMaxValue(State, Value);
+ return MaxV && MaxV->isNegative();
+}
+
+static bool isUnsigned(SValBuilder &SVB, NonLoc Value) {
+ QualType T = Value.getType(SVB.getContext());
+ return T->isUnsignedIntegerType();
+}
+
+// Evaluate the comparison Value < Threshold with the help of the custom
+// simplification algorithm defined for this checker. Return a pair of states,
+// where the first one corresponds to "value below threshold" and the second
+// corresponds to "value at or above threshold". Returns {nullptr, nullptr} in
+// the case when the evaluation fails.
+// If the optional argument CheckEquality is true, then use BO_EQ instead of
+// the default BO_LT after consistently applying the same simplification steps.
+static std::pair<ProgramStateRef, ProgramStateRef>
+compareValueToThreshold(ProgramStateRef State, NonLoc Value, NonLoc Threshold,
+ SValBuilder &SVB, bool CheckEquality = false) {
+ if (auto ConcreteThreshold = Threshold.getAs<nonloc::ConcreteInt>()) {
+ std::tie(Value, Threshold) =
+ getSimplifiedOffsets(Value, *ConcreteThreshold, SVB);
+ }
+
+ // We want to perform a _mathematical_ comparison between the numbers `Value`
+ // and `Threshold`; but `evalBinOpNN` 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`:
+ if (isNegative(SVB, State, Value) && isUnsigned(SVB, Threshold)) {
+ if (CheckEquality) {
+ // negative_value == unsigned_threshold is always false
+ return {nullptr, State};
+ }
+ // negative_value < unsigned_threshold is always true
+ return {State, nullptr};
+ }
+ if (isUnsigned(SVB, Value) && isNegative(SVB, State, Threshold)) {
+ // unsigned_value == negative_threshold and
+ // unsigned_value < negative_threshold are both always false
+ return {nullptr, State};
+ }
+ // FIXME: These special cases are sufficient for handling real-world
+ // 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
+ // 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
+ // evaluate these "mathematical" comparisons through a separate pathway would
+ // be a step backwards in this sense.
+
+ const BinaryOperatorKind OpKind = CheckEquality ? BO_EQ : BO_LT;
+ auto BelowThreshold =
+ SVB.evalBinOpNN(State, OpKind, Value, Threshold, SVB.getConditionType())
+ .getAs<NonLoc>();
+
+ if (BelowThreshold)
+ return State->assume(*BelowThreshold);
+
+ return {nullptr, nullptr};
+}
+
+std::string getRegionName(const MemSpaceRegion *Space,
+ const SubRegion *Region) {
+ if (std::string RegName = Region->getDescriptiveName(); !RegName.empty())
+ return RegName;
+
+ // Field regions only have descriptive names when their parent has a
+ // descriptive name; so we provide a fallback representation for them:
+ if (const auto *FR = Region->getAs<FieldRegion>()) {
+ if (StringRef Name = FR->getDecl()->getName(); !Name.empty())
+ return formatv("the field '{0}'", Name);
+ return "the unnamed field";
+ }
+
+ if (isa<AllocaRegion>(Region))
+ return "the memory returned by 'alloca'";
+
+ if (isa<SymbolicRegion>(Region) && isa<HeapSpaceRegion>(Space))
+ return "the heap area";
+
+ if (isa<StringRegion>(Region))
+ return "the string literal";
+
+ return "the region";
+}
+
+static std::optional<int64_t> getConcreteValue(NonLoc SV) {
+ if (auto ConcreteVal = SV.getAs<nonloc::ConcreteInt>()) {
+ return ConcreteVal->getValue()->tryExtValue();
+ }
+ return std::nullopt;
+}
+
+static std::optional<int64_t> getConcreteValue(std::optional<NonLoc> SV) {
+ return SV ? getConcreteValue(*SV) : std::nullopt;
+}
+
+/// Try to divide `Val1` and `Val2` (in place) by `Divisor` and return true if
+/// it can be performed (`Divisor` is nonzero and there is no remainder). The
+/// values `Val1` and `Val2` may be nullopt and in that case the corresponding
+/// division is considered to be successful.
+static bool tryDividePair(std::optional<int64_t> &Val1,
+ std::optional<int64_t> &Val2, int64_t Divisor) {
+ if (!Divisor)
+ return false;
+ const bool Val1HasRemainder = Val1 && *Val1 % Divisor;
+ const bool Val2HasRemainder = Val2 && *Val2 % Divisor;
+ if (Val1HasRemainder || Val2HasRemainder)
+ return false;
+ if (Val1)
+ *Val1 /= Divisor;
+ if (Val2)
+ *Val2 /= Divisor;
+ return true;
+}
+
+Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
+ std::optional<NonLoc> Extent, BadOffsetKind Problem) {
+
+ std::optional<int64_t> OffsetN = getConcreteValue(Offset);
+ std::optional<int64_t> ExtentN = getConcreteValue(Extent);
+
+ if (Problem == BadOffsetKind::Negative)
+ ExtentN = std::nullopt;
+
+ bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
+ const char *OffsetOrIndex = UseByteOffsets ? "byte offset" : "index";
+
+ SmallString<256> Buf;
+ llvm::raw_svector_ostream Out(Buf);
+ Out << "Access of ";
+ if (OffsetN && !ExtentN && !UseByteOffsets) {
+ // If the offset is reported as an index, then the report must mention the
+ // element type (because it is not always clear from the code). It's more
+ // natural to mention the element type later where the extent is described,
+ // but if the extent is unknown/irrelevant, then the element type can be
+ // inserted into the message at this point.
+ Out << SU.asElementName(/*ForceBytes=*/false) << " in ";
+ }
+ Out << RegName << " at ";
+ if (OffsetN) {
+ if (Problem == BadOffsetKind::Negative)
+ Out << "negative ";
+ Out << OffsetOrIndex << " " << *OffsetN;
+ } else {
+ Out << asAdjective(Problem) << " " << OffsetOrIndex;
+ }
+ if (ExtentN) {
+ Out << ", while it holds only ";
+ if (*ExtentN != 1)
+ Out << *ExtentN;
+ else
+ Out << "a single";
+
+ Out << ' ' << SU.asElementName(/*ForceBytes=*/UseByteOffsets);
+
+ if (*ExtentN > 1)
+ Out << "s";
+ }
+
+ return {formatv("Out of bound access to memory {0} {1}",
+ asPreposition(Problem), RegName),
+ std::string(Buf)};
+}
+
+Messages getTaintMsgs(std::string RegName, const char *OffsetName,
+ bool AlsoMentionUnderflow) {
+ return {formatv("Potential out of bound access to {0} with tainted {1}",
+ RegName, OffsetName),
+ formatv("Access of {0} with a tainted {1} that may be {2}too large",
+ RegName, OffsetName,
+ AlsoMentionUnderflow ? "negative or " : "")};
+}
+
+std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
+ StringRef RegName,
+ SizeUnit SU) const {
+ bool ShouldReportNonNegative = AssumedNonNegative;
+ if (!providesInformationAboutInteresting(Offset, BR)) {
+ if (AssumedUpperBound && providesInformationAboutInteresting(*Extent, BR)) {
+ // Even if the byte offset isn't interesting (e.g. it's a constant value),
+ // the assumption can still be interesting if it provides information
+ // about an interesting symbolic upper bound.
+ ShouldReportNonNegative = false;
+ } else {
+ // We don't have anything interesting, don't report the assumption.
+ return "";
+ }
+ }
+
+ std::optional<int64_t> OffsetN = getConcreteValue(Offset);
+ std::optional<int64_t> ExtentN = getConcreteValue(Extent);
+
+ const bool UseIndex =
+ !SU.isBytes() && tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
+
+ SmallString<256> Buf;
+ llvm::raw_svector_ostream Out(Buf);
+ Out << "Assuming ";
+ if (UseIndex) {
+ Out << "index ";
+ if (OffsetN)
+ Out << "'" << OffsetN << "' ";
+ } else if (Extent) {
+ Out << "byte offset ";
+ if (OffsetN)
+ Out << "'" << OffsetN << "' ";
+ } else {
+ Out << "offset ";
+ }
+
+ Out << "is";
+ if (ShouldReportNonNegative) {
+ Out << " non-negative";
+ }
+ if (Extent) {
+ if (ShouldReportNonNegative)
+ Out << " and";
+ Out << " less than ";
+ if (ExtentN)
+ Out << *ExtentN << ", ";
+ Out << SU.asExtentDesc(/*ForceBytes=*/!UseIndex) << ' ' << RegName;
+ }
+ return std::string(Out.str());
+}
+
+bool BoundsCheckResult::providesInformationAboutInteresting(
+ SymbolRef Sym, PathSensitiveBugReport &BR) {
+ if (!Sym)
+ return false;
+ for (SymbolRef PartSym : Sym->symbols()) {
+ // The interestingess mark may appear on any layer as we're stripping off
+ // the SymIntExpr, UnarySymExpr etc. layers...
+ if (BR.isInteresting(PartSym))
+ return true;
+ // ...but if both sides of the expression are symbolic, then there is no
+ // practical algorithm to produce separate constraints for the two
+ // operands (from the single combined result).
+ if (isa<SymSymExpr>(PartSym))
+ return false;
+ }
+ return false;
+}
+
+BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
+ NonLoc Offset, std::optional<NonLoc> Extent,
+ CheckFlags Flags) {
+ BoundsCheckResult Res(Offset, Extent);
+
+ // CHECK LOWER BOUND
+ if (Flags.CheckUnderflow) {
+ auto [PrecedesLowerBound, WithinLowerBound] =
+ compareValueToThreshold(State, Offset, SVB.makeZeroArrayIndex(), SVB);
+
+ if (PrecedesLowerBound) {
+ // The analyzer thinks that the offset may be invalid (negative)...
+ if (Flags.OffsetObviouslyNonnegative) {
+ // ...but the offset is obviously non-negative (clear array subscript
+ // with an unsigned index), so we're in a buggy situation.
+
+ // TODO: Currently the analyzer ignores many casts (e.g. signed ->
+ // unsigned casts), so it can easily reach states where it will load a
+ // signed (and negative) value from an unsigned variable. This sanity
+ // check is a duct tape "solution" that silences most of the ugly false
+ // positives that are caused by this buggy behavior. Note that this is
+ // not a complete solution: this cannot silence reports where pointer
+ // arithmetic complicates the picture and cannot ensure modeling of the
+ // "unsigned index is positive with highest bit set" cases which are
+ // "usurped" by the nonsense "unsigned index is negative" case.
+ // For more information about this topic, see the umbrella ticket
+ // https://github.com/llvm/llvm-project/issues/39492
+ // TODO: Remove this hack once 'SymbolCast's are modeled properly.
+
+ if (!WithinLowerBound) {
+ // The state is completely nonsense -- let's just sink it!
+ Res.finalize(BoundsCheckResult::Kind::Paradox, PrecedesLowerBound);
+ return Res;
+ }
+ // Otherwise continue on the 'WithinLowerBound' branch where the
+ // unsigned index _is_ non-negative. Don't mention this assumption as a
+ // note tag, because it would just confuse the users!
+ } else {
+ if (!WithinLowerBound) {
+ // ...and it cannot be valid (>= 0), so report an error.
+ Res.finalize(BoundsCheckResult::Kind::Underflow, PrecedesLowerBound);
+ return Res;
+ }
+ // ...but it can be valid as well, so the checker will (optimistically)
+ // assume that it's valid and mention this in the note tag.
+ Res.recordNonNegativeAssumption();
+ }
+ }
+
+ // 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.
+ if (WithinLowerBound)
+ State = WithinLowerBound;
+ }
+
+ // CHECK UPPER BOUND
+ if (Extent) {
+ auto [WithinUpperBound, ExceedsUpperBound] =
+ compareValueToThreshold(State, Offset, *Extent, SVB);
+
+ if (ExceedsUpperBound) {
+ // The offset may be invalid (>= Size)...
+ if (!WithinUpperBound) {
+ // ...and it cannot be within bounds, so report an error, unless we can
+ // definitely determine that this is an idiomatic `&array[size]`
+ // expression that calculates the past-the-end pointer.
+ if (Flags.AcceptPastTheEnd) {
+ auto [EqualsToThreshold, NotEqualToThreshold] =
+ compareValueToThreshold(State, Offset, *Extent, SVB,
+ /*CheckEquality=*/true);
+ if (EqualsToThreshold && !NotEqualToThreshold) {
+ Res.finalize(BoundsCheckResult::Kind::Valid, State);
+ return Res;
+ }
+ }
+
+ Res.finalize(BoundsCheckResult::Kind::Overflow, ExceedsUpperBound);
+ return Res;
+ }
+ // ...and it can be valid as well...
+ if (taint::isTainted(State, Offset)) {
+ // ...but it's tainted, so report an error.
+ Res.finalize(BoundsCheckResult::Kind::TaintBug, State);
+ return Res;
+ }
+ // ...and it isn't tainted, so the checker will (optimistically) assume
+ // that the offset is in bounds and mention this in the note tag.
+ Res.recordUpperBoundAssumption();
+ }
+
+ // 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.
+ if (WithinUpperBound)
+ State = WithinUpperBound;
+ }
+ Res.finalize(BoundsCheckResult::Kind::Valid, State);
+ return Res;
+}
+
+} // namespace ento
+} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 8a0621077b977..d2874f1758e14 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -13,6 +13,7 @@ add_clang_library(clangStaticAnalyzerCheckers
BitwiseShiftChecker.cpp
BlockInCriticalSectionChecker.cpp
BoolAssignmentChecker.cpp
+ BoundsChecking.cpp
BuiltinFunctionChecker.cpp
CStringChecker.cpp
CStringSyntaxChecker.cpp
diff --git a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
index b3ac48975ee85..aa0dfc042e022 100644
--- a/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Checkers/BUILD.gn
@@ -22,6 +22,7 @@ static_library("Checkers") {
"BitwiseShiftChecker.cpp",
"BlockInCriticalSectionChecker.cpp",
"BoolAssignmentChecker.cpp",
+ "BoundsChecking.cpp",
"BuiltinFunctionChecker.cpp",
"CStringChecker.cpp",
"CStringSyntaxChecker.cpp",
>From f92d798102934dc827c8ad23ec767002f5c1bbfd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 8 Jun 2026 16:42:27 +0200
Subject: [PATCH 11/29] Remove using declaration from header
---
.../include/clang/StaticAnalyzer/Checkers/BoundsChecking.h | 7 +++----
clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp | 2 ++
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index 8aebe63556182..e873210cdaca2 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -22,8 +22,6 @@
#include "llvm/Support/FormatVariadic.h"
#include <optional>
-using llvm::formatv;
-
namespace clang {
namespace ento {
@@ -77,13 +75,14 @@ class SizeUnit {
std::string asExtentDesc(bool ForceBytes) const {
if (ForceBytes || isBytes())
return "the extent of";
- return formatv("the number of '{0}' elements in", AsType.getAsString());
+ return llvm::formatv("the number of '{0}' elements in",
+ AsType.getAsString());
}
std::string asElementName(bool ForceBytes) const {
if (ForceBytes || isBytes())
return "byte";
- return formatv("'{0}' element", AsType.getAsString());
+ return llvm::formatv("'{0}' element", AsType.getAsString());
}
};
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index dbdccf516517a..df42581683b16 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -19,6 +19,8 @@
#include "clang/StaticAnalyzer/Checkers/BoundsChecking.h"
#include "clang/StaticAnalyzer/Checkers/Taint.h"
+using llvm::formatv;
+
namespace clang {
namespace ento {
>From 132e445bb17ca8fe0f2244f0c572c5e4bd9890ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 8 Jun 2026 16:43:21 +0200
Subject: [PATCH 12/29] Remove barely used 'using taint;'
---
clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index a3b68854bef5c..bd6936599b0ce 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -23,7 +23,6 @@
using namespace clang;
using namespace ento;
-using namespace taint;
namespace {
// NOTE: The `ArraySubscriptExpr` and `UnaryOperator` callbacks are `PostStmt`
@@ -199,7 +198,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
// nicer to say "tainted index".
const char *OffsetName = "offset";
if (const auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
- if (isTainted(State, ASE->getIdx(), C.getStackFrame()))
+ if (taint::isTainted(State, ASE->getIdx(), C.getStackFrame()))
OffsetName = "index";
Messages Msgs =
@@ -237,7 +236,7 @@ void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
// offset, then put interestingness onto symbols that could be the origin
// of the taint. Note that this may find symbols that did not appear in
// `Sym->symbols()` (because they're only loosely connected to `Val`).
- for (SymbolRef Sym : getTaintedSymbols(ErrorState, Val))
+ for (SymbolRef Sym : taint::getTaintedSymbols(ErrorState, Val))
BR.markInteresting(Sym);
}
}
>From 563e954657baf79d1a7431478a7a375b735c4281 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 8 Jun 2026 16:49:53 +0200
Subject: [PATCH 13/29] Capitalize variable names in moved code
---
.../Checkers/BoundsChecking.cpp | 29 +++++++++----------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index df42581683b16..64b558e7157c2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -64,34 +64,33 @@ getAsCleanArraySubscriptExpr(const Expr *E, const CheckerContext &C) {
// but can produce a true result when evaluated by `evalBinOp` (which follows
// the rules of C++ and casts -1 to SIZE_MAX).
static std::pair<NonLoc, nonloc::ConcreteInt>
-getSimplifiedOffsets(NonLoc offset, nonloc::ConcreteInt extent,
- SValBuilder &svalBuilder) {
- const llvm::APSInt &extentVal = extent.getValue();
- std::optional<nonloc::SymbolVal> SymVal = offset.getAs<nonloc::SymbolVal>();
+getSimplifiedOffsets(NonLoc Offset, nonloc::ConcreteInt Extent,
+ SValBuilder &SVB) {
+ const llvm::APSInt &ExtentVal = Extent.getValue();
+ std::optional<nonloc::SymbolVal> SymVal = Offset.getAs<nonloc::SymbolVal>();
if (SymVal && SymVal->isExpression()) {
if (const SymIntExpr *SIE = dyn_cast<SymIntExpr>(SymVal->getSymbol())) {
- llvm::APSInt constant = APSIntType(extentVal).convert(SIE->getRHS());
+ llvm::APSInt Constant = APSIntType(ExtentVal).convert(SIE->getRHS());
switch (SIE->getOpcode()) {
case BO_Mul:
- // The constant should never be 0 here, becasue multiplication by zero
+ // The Constant should never be 0 here, becasue multiplication by zero
// is simplified by the engine.
- if ((extentVal % constant) != 0)
- return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
+ if ((ExtentVal % Constant) != 0)
+ return std::pair<NonLoc, nonloc::ConcreteInt>(Offset, Extent);
else
- return getSimplifiedOffsets(
- nonloc::SymbolVal(SIE->getLHS()),
- svalBuilder.makeIntVal(extentVal / constant), svalBuilder);
+ return getSimplifiedOffsets(nonloc::SymbolVal(SIE->getLHS()),
+ SVB.makeIntVal(ExtentVal / Constant),
+ SVB);
case BO_Add:
- return getSimplifiedOffsets(
- nonloc::SymbolVal(SIE->getLHS()),
- svalBuilder.makeIntVal(extentVal - constant), svalBuilder);
+ return getSimplifiedOffsets(nonloc::SymbolVal(SIE->getLHS()),
+ SVB.makeIntVal(ExtentVal - Constant), SVB);
default:
break;
}
}
}
- return std::pair<NonLoc, nonloc::ConcreteInt>(offset, extent);
+ return std::pair<NonLoc, nonloc::ConcreteInt>(Offset, Extent);
}
static bool isNegative(SValBuilder &SVB, ProgramStateRef State, NonLoc Value) {
>From 4103833e30b847513aa181972ce5d7ee09319c0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 8 Jun 2026 17:14:42 +0200
Subject: [PATCH 14/29] Remove the ForceBytes hack
---
.../StaticAnalyzer/Checkers/BoundsChecking.h | 19 +++++++---
.../Checkers/BoundsChecking.cpp | 36 +++++++++----------
2 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index e873210cdaca2..9d86f7314ca63 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -72,18 +72,29 @@ class SizeUnit {
int64_t asCharUnits() const { return AsCharUnits; }
- std::string asExtentDesc(bool ForceBytes) const {
- if (ForceBytes || isBytes())
+ std::string asExtentDesc() const {
+ if (isBytes())
return "the extent of";
return llvm::formatv("the number of '{0}' elements in",
AsType.getAsString());
}
- std::string asElementName(bool ForceBytes) const {
- if (ForceBytes || isBytes())
+ std::string asElementName() const {
+ if (isBytes())
return "byte";
return llvm::formatv("'{0}' element", AsType.getAsString());
}
+
+ std::string getOffsetName() const {
+ return isBytes() ? "byte offset" : "index";
+ }
+
+ /// Try to divide `Val1` and `Val2` (in place) by `this->asCharUnits()` and
+ /// return true if it can be performed without remainder. The values `Val1`
+ /// and `Val2` may be nullopt and in that case the corresponding division is
+ /// considered to be successful.
+ bool tryConvertValuesFromBytes(std::optional<int64_t> &Val1,
+ std::optional<int64_t> &Val2) const;
};
struct Messages {
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index 64b558e7157c2..9739c6ffa6e65 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -200,18 +200,18 @@ static std::optional<int64_t> getConcreteValue(std::optional<NonLoc> SV) {
/// it can be performed (`Divisor` is nonzero and there is no remainder). The
/// values `Val1` and `Val2` may be nullopt and in that case the corresponding
/// division is considered to be successful.
-static bool tryDividePair(std::optional<int64_t> &Val1,
- std::optional<int64_t> &Val2, int64_t Divisor) {
- if (!Divisor)
+bool SizeUnit::tryConvertValuesFromBytes(std::optional<int64_t> &Val1,
+ std::optional<int64_t> &Val2) const {
+ if (!AsCharUnits)
return false;
- const bool Val1HasRemainder = Val1 && *Val1 % Divisor;
- const bool Val2HasRemainder = Val2 && *Val2 % Divisor;
+ const bool Val1HasRemainder = Val1 && *Val1 % AsCharUnits;
+ const bool Val2HasRemainder = Val2 && *Val2 % AsCharUnits;
if (Val1HasRemainder || Val2HasRemainder)
return false;
if (Val1)
- *Val1 /= Divisor;
+ *Val1 /= AsCharUnits;
if (Val2)
- *Val2 /= Divisor;
+ *Val2 /= AsCharUnits;
return true;
}
@@ -224,27 +224,27 @@ Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
if (Problem == BadOffsetKind::Negative)
ExtentN = std::nullopt;
- bool UseByteOffsets = !tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
- const char *OffsetOrIndex = UseByteOffsets ? "byte offset" : "index";
+ if (!SU.tryConvertValuesFromBytes(OffsetN, ExtentN))
+ SU = SizeUnit::bytes();
SmallString<256> Buf;
llvm::raw_svector_ostream Out(Buf);
Out << "Access of ";
- if (OffsetN && !ExtentN && !UseByteOffsets) {
+ if (OffsetN && !ExtentN && !SU.isBytes()) {
// If the offset is reported as an index, then the report must mention the
// element type (because it is not always clear from the code). It's more
// natural to mention the element type later where the extent is described,
// but if the extent is unknown/irrelevant, then the element type can be
// inserted into the message at this point.
- Out << SU.asElementName(/*ForceBytes=*/false) << " in ";
+ Out << SU.asElementName() << " in ";
}
Out << RegName << " at ";
if (OffsetN) {
if (Problem == BadOffsetKind::Negative)
Out << "negative ";
- Out << OffsetOrIndex << " " << *OffsetN;
+ Out << SU.getOffsetName() << " " << *OffsetN;
} else {
- Out << asAdjective(Problem) << " " << OffsetOrIndex;
+ Out << asAdjective(Problem) << " " << SU.getOffsetName();
}
if (ExtentN) {
Out << ", while it holds only ";
@@ -253,7 +253,7 @@ Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
else
Out << "a single";
- Out << ' ' << SU.asElementName(/*ForceBytes=*/UseByteOffsets);
+ Out << ' ' << SU.asElementName();
if (*ExtentN > 1)
Out << "s";
@@ -292,13 +292,13 @@ std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
std::optional<int64_t> OffsetN = getConcreteValue(Offset);
std::optional<int64_t> ExtentN = getConcreteValue(Extent);
- const bool UseIndex =
- !SU.isBytes() && tryDividePair(OffsetN, ExtentN, SU.asCharUnits());
+ if (!SU.tryConvertValuesFromBytes(OffsetN, ExtentN))
+ SU = SizeUnit::bytes();
SmallString<256> Buf;
llvm::raw_svector_ostream Out(Buf);
Out << "Assuming ";
- if (UseIndex) {
+ if (!SU.isBytes()) {
Out << "index ";
if (OffsetN)
Out << "'" << OffsetN << "' ";
@@ -320,7 +320,7 @@ std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
Out << " less than ";
if (ExtentN)
Out << *ExtentN << ", ";
- Out << SU.asExtentDesc(/*ForceBytes=*/!UseIndex) << ' ' << RegName;
+ Out << SU.asExtentDesc() << ' ' << RegName;
}
return std::string(Out.str());
}
>From 68286cba24b150c6b64527c756836dde8c5dc783 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 8 Jun 2026 17:22:01 +0200
Subject: [PATCH 15/29] Simplify providesInformationAboutInteresting
---
.../clang/StaticAnalyzer/Checkers/BoundsChecking.h | 14 +++++---------
.../lib/StaticAnalyzer/Checkers/BoundsChecking.cpp | 3 ++-
2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index 9d86f7314ca63..e6c07c06fc010 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -175,10 +175,10 @@ class BoundsCheckResult {
SizeUnit SU) const;
private:
- /// Return true if information about the value of `Sym` can put constraints
- /// on some symbol which is interesting within the bug report `BR`.
- /// In particular, this returns true when `Sym` is interesting within `BR`;
- /// but it also returns true if `Sym` is an expression that contains integer
+ /// Return true if information about the symbol behind `SV` can constrain
+ /// some symbol which is interesting within the bug report `BR`.
+ /// In particular, this returns true when `SV` is interesting within `BR`;
+ /// but it also returns true if `SV` is an expression that contains integer
/// constants and a single symbolic operand which is interesting (in `BR`).
/// We need to use this instead of plain `BR.isInteresting()` because if we
/// are analyzing code like
@@ -189,12 +189,8 @@ class BoundsCheckResult {
/// then the byte offsets are `arg * 4` and `(arg + 10) * 4`, which are not
/// sub-expressions of each other (but `getSimplifiedOffsets` is smart enough
/// to detect this out of bounds access).
- static bool providesInformationAboutInteresting(SymbolRef Sym,
- PathSensitiveBugReport &BR);
static bool providesInformationAboutInteresting(SVal SV,
- PathSensitiveBugReport &BR) {
- return providesInformationAboutInteresting(SV.getAsSymbol(), BR);
- }
+ PathSensitiveBugReport &BR);
};
BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index 9739c6ffa6e65..49e8ee9e6be16 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -326,7 +326,8 @@ std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
}
bool BoundsCheckResult::providesInformationAboutInteresting(
- SymbolRef Sym, PathSensitiveBugReport &BR) {
+ SVal SV, PathSensitiveBugReport &BR) {
+ SymbolRef Sym = SV.getAsSymbol();
if (!Sym)
return false;
for (SymbolRef PartSym : Sym->symbols()) {
>From 2466bc63a5631fb1f6d8b81ea45f6a7dc0bd6ea6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 29 Jun 2026 13:32:31 +0200
Subject: [PATCH 16/29] Ensure that the extent is not interesting for underflow
reports
... and refactor to clarify that `reportOOB` handled the offset and
extent in the same way, they were just symbolic values that needed to be
marked as interesting.
---
.../Checkers/ArrayBoundChecker.cpp | 26 ++++++++++---------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index bd6936599b0ce..ca9907485a95a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -41,8 +41,7 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
void handleAccessExpr(const Expr *E, CheckerContext &C) const;
void reportOOB(CheckerContext &C, ProgramStateRef ErrorState, Messages Msgs,
- NonLoc Offset, std::optional<NonLoc> Extent,
- bool IsTaintBug = false) const;
+ ArrayRef<NonLoc> Extent, bool IsTaintBug = false) const;
static void markPartsInteresting(PathSensitiveBugReport &BR,
ProgramStateRef ErrorState, NonLoc Val,
@@ -204,15 +203,20 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
Messages Msgs =
getTaintMsgs(RN, OffsetName,
/*AlsoMentionUnderflow=*/Res.assumedNonNegative());
- reportOOB(C, Res.getState(), Msgs, ByteOffset, Extent,
- /*IsTaintBug=*/true);
+ SmallVector<NonLoc, 2> Interesting = {ByteOffset};
+ if (Extent)
+ Interesting.push_back(*Extent);
+ reportOOB(C, Res.getState(), Msgs, Interesting, /*IsTaintBug=*/true);
return;
}
default: {
SizeUnit SU = SizeUnit::forSVal(Location, C.getASTContext());
- Messages Msgs =
- getNonTaintMsgs(RN, SU, ByteOffset, Extent, *Res.getBadOffsetKind());
- reportOOB(C, Res.getState(), Msgs, ByteOffset, Extent);
+ BadOffsetKind BOK = *Res.getBadOffsetKind();
+ Messages Msgs = getNonTaintMsgs(RN, SU, ByteOffset, Extent, BOK);
+ SmallVector<NonLoc, 2> Interesting = {ByteOffset};
+ if (Extent && BOK != BadOffsetKind::Negative)
+ Interesting.push_back(*Extent);
+ reportOOB(C, Res.getState(), Msgs, Interesting);
return;
}
}
@@ -242,8 +246,7 @@ void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
}
void ArrayBoundChecker::reportOOB(CheckerContext &C, ProgramStateRef ErrorState,
- Messages Msgs, NonLoc Offset,
- std::optional<NonLoc> Extent,
+ Messages Msgs, ArrayRef<NonLoc> Interesting,
bool IsTaintBug /*=false*/) const {
ExplodedNode *ErrorNode = C.generateErrorNode(ErrorState);
@@ -266,9 +269,8 @@ void ArrayBoundChecker::reportOOB(CheckerContext &C, ProgramStateRef ErrorState,
// (which is technically true, but irrelevant).
// If trackExpressionValue() becomes reliable, it should be applied instead
// of this custom markPartsInteresting().
- markPartsInteresting(*BR, ErrorState, Offset, IsTaintBug);
- if (Extent)
- markPartsInteresting(*BR, ErrorState, *Extent, IsTaintBug);
+ for (NonLoc Val : Interesting)
+ markPartsInteresting(*BR, ErrorState, Val, IsTaintBug);
C.emitReport(std::move(BR));
}
>From c824b92fb9d0760617bfccc98ce1c5f40165bd3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 29 Jun 2026 13:56:02 +0200
Subject: [PATCH 17/29] Test that extent is not interesting in underflows
---
.../Analysis/ArrayBound/assumption-reporting.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/clang/test/Analysis/ArrayBound/assumption-reporting.c b/clang/test/Analysis/ArrayBound/assumption-reporting.c
index bffd5d9bc35b5..0409b1da19b85 100644
--- a/clang/test/Analysis/ArrayBound/assumption-reporting.c
+++ b/clang/test/Analysis/ArrayBound/assumption-reporting.c
@@ -191,8 +191,8 @@ int assumingExtent(int arg) {
}
int *extentInterestingness(int arg) {
- // Verify that in an out-of-bounds access issue the extent is marked as
- // interesting (so assumptions about its value are printed).
+ // Verify that in a buffer overflow issue the extent is marked as interesting
+ // (so assumptions about its value are printed).
int *mem = (int*)malloc(arg);
TenElements[arg] = 123;
@@ -203,6 +203,18 @@ int *extentInterestingness(int arg) {
// expected-note at -2 {{Access of 'int' element in the heap area at index 12}}
}
+int *extentNonInterestingInUnderflow(int arg) {
+ // Verify that in a buffer underflow issue the extent is _not_ marked as
+ // interesting (because it does not influence anything).
+ int *mem = (int*)malloc(arg);
+
+ TenElements[arg] = 123; // no-note: arg is not interesting
+
+ return &mem[-2];
+ // expected-warning at -1 {{Out of bound access to memory preceding the heap area}}
+ // expected-note at -2 {{Access of 'int' element in the heap area at negative index -2}}
+}
+
int triggeredByAnyReport(int arg) {
// Verify that note tags explaining the assumptions made by ArrayBound are
// not limited to ArrayBound reports but will appear on any bug report (that
>From 4738affc6650108a405eb1ee735f6794a945f41e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 29 Jun 2026 14:34:20 +0200
Subject: [PATCH 18/29] [side] Fix an obsolete comment in a test
---
clang/test/Analysis/ArrayBound/verbose-tests.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/test/Analysis/ArrayBound/verbose-tests.c b/clang/test/Analysis/ArrayBound/verbose-tests.c
index c0da93ea48591..772ef3d8868a5 100644
--- a/clang/test/Analysis/ArrayBound/verbose-tests.c
+++ b/clang/test/Analysis/ArrayBound/verbose-tests.c
@@ -44,8 +44,7 @@ struct TwoInts underflowReportedAsStruct(void) {
struct TwoInts underflowOnlyByteOffset(void) {
// In this case the negative byte offset is not a multiple of the size of the
- // accessed element, so the part "= -... * sizeof(type)" is omitted at the
- // end of the message.
+ // accessed element, so we use a byte offset instead of an index.
return *(struct TwoInts*)(TenElements - 3);
// expected-warning at -1 {{Out of bound access to memory preceding 'TenElements'}}
// expected-note at -2 {{Access of 'TenElements' at negative byte offset -12}}
>From dd99b9bfe5d3f4babb4c7779e1e9a46af70a542b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 29 Jun 2026 15:22:01 +0200
Subject: [PATCH 19/29] Fix assumption message when only lower bound is assumed
The extent value must be completely ignored when there is no upper bound
assumption. Also add tests that try to validate this behavior (and some
of them ends up highlighting unrelated limitations).
---
.../Checkers/BoundsChecking.cpp | 8 ++--
.../ArrayBound/assumption-reporting.c | 43 ++++++++++++++++++-
2 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index 49e8ee9e6be16..23fb4a0a22e96 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -290,7 +290,9 @@ std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
}
std::optional<int64_t> OffsetN = getConcreteValue(Offset);
- std::optional<int64_t> ExtentN = getConcreteValue(Extent);
+ // The extent must be ignored if we did not assume it as an upper bound:
+ std::optional<int64_t> ExtentN =
+ AssumedUpperBound ? getConcreteValue(Extent) : std::nullopt;
if (!SU.tryConvertValuesFromBytes(OffsetN, ExtentN))
SU = SizeUnit::bytes();
@@ -302,7 +304,7 @@ std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
Out << "index ";
if (OffsetN)
Out << "'" << OffsetN << "' ";
- } else if (Extent) {
+ } else if (AssumedUpperBound) {
Out << "byte offset ";
if (OffsetN)
Out << "'" << OffsetN << "' ";
@@ -314,7 +316,7 @@ std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
if (ShouldReportNonNegative) {
Out << " non-negative";
}
- if (Extent) {
+ if (AssumedUpperBound) {
if (ShouldReportNonNegative)
Out << " and";
Out << " less than ";
diff --git a/clang/test/Analysis/ArrayBound/assumption-reporting.c b/clang/test/Analysis/ArrayBound/assumption-reporting.c
index 0409b1da19b85..5ba62a1522d67 100644
--- a/clang/test/Analysis/ArrayBound/assumption-reporting.c
+++ b/clang/test/Analysis/ArrayBound/assumption-reporting.c
@@ -54,7 +54,32 @@ int assumingLower(int arg) {
if (arg >= 10)
return 0;
int a = TenElements[arg];
- // expected-note at -1 {{Assuming index is non-negative}}
+ // expected-note-re at -1 {{Assuming index is non-negative{{$}}}}
+ int b = TenElements[arg + 10];
+ // expected-warning at -1 {{Out of bound access to memory after the end of 'TenElements'}}
+ // expected-note at -2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}}
+ return a + b;
+}
+
+int assumingLowerOnlyUseIndex(int arg) {
+ // This testcase validates that the note tag says that the _index_ is
+ // non-negative when there is no upper bound assumption -- even in the case
+ // when the extent (which is totally irrelevant) is not an integer multiple
+ // of the element size.
+
+ char TwoAndHalfInts[10] = {0};
+ // expected-note at +2 {{Assuming 'arg' is < 2}}
+ // expected-note at +1 {{Taking false branch}}
+ if (arg >= 2)
+ return 0;
+
+ int a = ((int*)TwoAndHalfInts)[arg];
+ // expected-note at -1 {{Assuming byte offset is non-negative and less than 10, the extent of 'TwoAndHalfInts'}}
+ // FIXME: This assumption note should say
+ // {{Assuming index is non-negative{{$}}}}
+ // but the comparison logic is not smart enough to deduce that if arg < 2,
+ // then 4*arg < 10.
+
int b = TenElements[arg + 10];
// expected-warning at -1 {{Out of bound access to memory after the end of 'TenElements'}}
// expected-note at -2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}}
@@ -94,6 +119,22 @@ int assumingUpperIrrelevant(int arg) {
return a + b;
}
+int assumingLowerIrrelevant(int arg) {
+ // FIXME: Analogously to `assumingUpperIrrelevant` here the assumption
+ // "assuming index is non-negative" is irrelevant, but printed.
+ //
+ // expected-note at +2 {{Assuming 'arg' is < 10}}
+ // expected-note at +1 {{Taking false branch}}
+ if (arg >= 10)
+ return 0;
+ int a = TenElements[arg];
+ // expected-note-re at -1 {{Assuming index is non-negative{{$}}}}
+ int b = TenElements[arg - 10];
+ // expected-warning at -1 {{Out of bound access to memory preceding 'TenElements'}}
+ // expected-note at -2 {{Access of 'TenElements' at a negative index}}
+ return a + b;
+}
+
int assumingUpperUnsigned(unsigned arg) {
int a = TenElements[arg];
// expected-note at -1 {{Assuming index is less than 10, the number of 'int' elements in 'TenElements'}}
>From ec73601409f073e1a368d6fc196872b7a3a9fa53 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 29 Jun 2026 18:58:34 +0200
Subject: [PATCH 20/29] Fix header guard
---
.../include/clang/StaticAnalyzer/Checkers/BoundsChecking.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index e6c07c06fc010..cbb41cdbcfbf8 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -16,8 +16,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BOUNDSCHECKING_H
-#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BOUNDSCHECKING_H
+#ifndef LLVM_CLANG_STATICANALYZER_CHECKERS_BOUNDSCHECKING_H
+#define LLVM_CLANG_STATICANALYZER_CHECKERS_BOUNDSCHECKING_H
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "llvm/Support/FormatVariadic.h"
#include <optional>
@@ -208,4 +208,4 @@ Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
} // namespace ento
} // namespace clang
-#endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BOUNDSCHECKING_H
+#endif // LLVM_CLANG_STATICANALYZER_CHECKERS_BOUNDSCHECKING_H
>From 50b2997b27cbd53194b20def14c8a538e65f4f46 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 1 Jul 2026 13:13:35 +0200
Subject: [PATCH 21/29] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Balázs Benics <benicsbalazs at gmail.com>
---
.../StaticAnalyzer/Checkers/BoundsChecking.h | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index cbb41cdbcfbf8..1c71661c38444 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -54,10 +54,7 @@ class SizeUnit {
/// least bad) option for the assumption messages that use this.
static SizeUnit forExpr(const Expr *E, const CheckerContext &C) {
const auto *ASE = getAsCleanArraySubscriptExpr(E, C);
- if (!ASE)
- return bytes();
-
- return SizeUnit(ASE->getType(), C.getASTContext());
+ return ASE ? SizeUnit(ASE->getType(), C.getASTContext()) : bytes();
}
/// Return the element type that is "natural" for reporting out-of-bounds
@@ -90,15 +87,16 @@ class SizeUnit {
}
/// Try to divide `Val1` and `Val2` (in place) by `this->asCharUnits()` and
- /// return true if it can be performed without remainder. The values `Val1`
- /// and `Val2` may be nullopt and in that case the corresponding division is
+ /// return true if it can be performed without remainder. The values \p Val1
+ /// and \p Val2 may be nullopt and in that case the corresponding division is
/// considered to be successful.
bool tryConvertValuesFromBytes(std::optional<int64_t> &Val1,
std::optional<int64_t> &Val2) const;
};
struct Messages {
- std::string Short, Full;
+ std::string Short;
+ std::string Full;
};
enum class BadOffsetKind { Negative, Overflowing, Indeterminate };
@@ -116,9 +114,9 @@ inline StringRef asPreposition(BadOffsetKind Problem) {
}
struct CheckFlags {
- bool CheckUnderflow;
- bool OffsetObviouslyNonnegative;
- bool AcceptPastTheEnd;
+ unsigned CheckUnderflow : 1;
+ unsigned OffsetObviouslyNonnegative : 1;
+ unsigned AcceptPastTheEnd : 1;
};
class BoundsCheckResult {
>From c9b1b47cfd7576639372571aba62b96782f58fd8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 1 Jul 2026 13:24:15 +0200
Subject: [PATCH 22/29] Use variable 'Space' instead of calculating its value
again
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Balázs Benics <benicsbalazs at gmail.com>
---
clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index ca9907485a95a..9cc6fee2abbb5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -168,7 +168,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
BoundsCheckResult Res = checkBounds(State, SVB, ByteOffset, Extent, Flags);
- std::string RN = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
+ std::string RN = getRegionName(Space, Reg);
switch (Res.getKind()) {
case BoundsCheckResult::Kind::Paradox:
>From d165fb2f92f8217a2012234d30eb76ba51af2248 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 1 Jul 2026 14:13:09 +0200
Subject: [PATCH 23/29] Place bounds library in clang::ento::bounds
...and improve the use of namespaces.
`BoundsCheckResult` is renamed to `CheckResult` to avoid the awkward
qualified name `bounds::BoundsCheckResult`.
---
.../StaticAnalyzer/Checkers/BoundsChecking.h | 24 ++++----
.../Checkers/ArrayBoundChecker.cpp | 39 +++++++------
.../Checkers/BoundsChecking.cpp | 55 +++++++++----------
3 files changed, 58 insertions(+), 60 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index 1c71661c38444..8acb4c4454228 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -22,8 +22,7 @@
#include "llvm/Support/FormatVariadic.h"
#include <optional>
-namespace clang {
-namespace ento {
+namespace clang::ento::bounds {
/// If `E` is an array subscript expression with a base that is "clean" (= not
/// modified by pointer arithmetic = the beginning of a memory region), return
@@ -119,7 +118,7 @@ struct CheckFlags {
unsigned AcceptPastTheEnd : 1;
};
-class BoundsCheckResult {
+class CheckResult {
public:
enum class Kind { Underflow, Overflow, TaintBug, Paradox, Valid };
@@ -131,8 +130,7 @@ class BoundsCheckResult {
std::optional<NonLoc> Extent;
ProgramStateRef State = nullptr;
- BoundsCheckResult(NonLoc Offs, std::optional<NonLoc> E)
- : Offset(Offs), Extent(E) {}
+ CheckResult(NonLoc Offs, std::optional<NonLoc> E) : Offset(Offs), Extent(E) {}
void recordNonNegativeAssumption() { AssumedNonNegative = true; }
@@ -144,10 +142,9 @@ class BoundsCheckResult {
}
public:
- friend BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
- NonLoc Offset,
- std::optional<NonLoc> Extent,
- CheckFlags Flags);
+ friend CheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
+ NonLoc Offset, std::optional<NonLoc> Extent,
+ CheckFlags Flags);
bool assumedNonNegative() const { return AssumedNonNegative; }
@@ -191,10 +188,10 @@ class BoundsCheckResult {
PathSensitiveBugReport &BR);
};
-BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
- NonLoc Offset, std::optional<NonLoc> Extent,
- CheckFlags Flags);
+CheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB, NonLoc Offset,
+ std::optional<NonLoc> Extent, CheckFlags Flags);
+// FIXME: This utility probably should become a method of `MemRegion`.
std::string getRegionName(const MemSpaceRegion *Space, const SubRegion *Region);
Messages getTaintMsgs(std::string RegName, const char *OffsetName,
@@ -203,7 +200,6 @@ Messages getTaintMsgs(std::string RegName, const char *OffsetName,
Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
std::optional<NonLoc> Extent, BadOffsetKind Problem);
-} // namespace ento
-} // namespace clang
+} // namespace clang::ento::bounds
#endif // LLVM_CLANG_STATICANALYZER_CHECKERS_BOUNDSCHECKING_H
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 9cc6fee2abbb5..99860f882bec9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -40,8 +40,9 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
void handleAccessExpr(const Expr *E, CheckerContext &C) const;
- void reportOOB(CheckerContext &C, ProgramStateRef ErrorState, Messages Msgs,
- ArrayRef<NonLoc> Extent, bool IsTaintBug = false) const;
+ void reportOOB(CheckerContext &C, ProgramStateRef ErrorState,
+ bounds::Messages Msgs, ArrayRef<NonLoc> Extent,
+ bool IsTaintBug = false) const;
static void markPartsInteresting(PathSensitiveBugReport &BR,
ProgramStateRef ErrorState, NonLoc Val,
@@ -158,7 +159,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
// non-symbolic regions (e.g. a field subregion of a symbolic region) in
// unknown space.
- CheckFlags Flags = {
+ bounds::CheckFlags Flags = {
/*CheckUnderflow=*/!(isa<SymbolicRegion>(Reg) &&
isa<UnknownSpaceRegion>(Space)),
/*OffsetObviouslyNonnegative=*/isOffsetObviouslyNonnegative(E, C),
@@ -166,22 +167,22 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
isInAddressOf(E, C.getASTContext()),
};
- BoundsCheckResult Res = checkBounds(State, SVB, ByteOffset, Extent, Flags);
+ bounds::CheckResult Res = checkBounds(State, SVB, ByteOffset, Extent, Flags);
- std::string RN = getRegionName(Space, Reg);
+ std::string RN = bounds::getRegionName(Space, Reg);
switch (Res.getKind()) {
- case BoundsCheckResult::Kind::Paradox:
+ case bounds::CheckResult::Kind::Paradox:
// The current state is paradoxical (due to bad modeling of casts we
// assumed that an unsigned value is negative), so we should sink the
// execution path.
C.addSink();
return;
- case BoundsCheckResult::Kind::Valid: {
+ case bounds::CheckResult::Kind::Valid: {
const NoteTag *Tag = nullptr;
if (Res.hasAssumption()) {
- SizeUnit SU = SizeUnit::forExpr(E, C);
+ bounds::SizeUnit SU = bounds::SizeUnit::forExpr(E, C);
Tag = C.getNoteTag(
[Res, RN, SU](PathSensitiveBugReport &BR) -> std::string {
return Res.getMessage(BR, RN, SU);
@@ -191,7 +192,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
C.addTransition(Res.getState(), Tag);
return;
}
- case BoundsCheckResult::Kind::TaintBug: {
+ case bounds::CheckResult::Kind::TaintBug: {
// Diagnostic detail: saying "tainted offset" is always correct, but
// the common case is that 'idx' is tainted in 'arr[idx]' and then it's
// nicer to say "tainted index".
@@ -200,9 +201,9 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
if (taint::isTainted(State, ASE->getIdx(), C.getStackFrame()))
OffsetName = "index";
- Messages Msgs =
- getTaintMsgs(RN, OffsetName,
- /*AlsoMentionUnderflow=*/Res.assumedNonNegative());
+ bounds::Messages Msgs =
+ bounds::getTaintMsgs(RN, OffsetName,
+ /*AlsoMentionUnderflow=*/Res.assumedNonNegative());
SmallVector<NonLoc, 2> Interesting = {ByteOffset};
if (Extent)
Interesting.push_back(*Extent);
@@ -210,11 +211,12 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
return;
}
default: {
- SizeUnit SU = SizeUnit::forSVal(Location, C.getASTContext());
- BadOffsetKind BOK = *Res.getBadOffsetKind();
- Messages Msgs = getNonTaintMsgs(RN, SU, ByteOffset, Extent, BOK);
+ bounds::SizeUnit SU =
+ bounds::SizeUnit::forSVal(Location, C.getASTContext());
+ bounds::BadOffsetKind BOK = *Res.getBadOffsetKind();
+ bounds::Messages Msgs = getNonTaintMsgs(RN, SU, ByteOffset, Extent, BOK);
SmallVector<NonLoc, 2> Interesting = {ByteOffset};
- if (Extent && BOK != BadOffsetKind::Negative)
+ if (Extent && BOK != bounds::BadOffsetKind::Negative)
Interesting.push_back(*Extent);
reportOOB(C, Res.getState(), Msgs, Interesting);
return;
@@ -246,7 +248,8 @@ void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
}
void ArrayBoundChecker::reportOOB(CheckerContext &C, ProgramStateRef ErrorState,
- Messages Msgs, ArrayRef<NonLoc> Interesting,
+ bounds::Messages Msgs,
+ ArrayRef<NonLoc> Interesting,
bool IsTaintBug /*=false*/) const {
ExplodedNode *ErrorNode = C.generateErrorNode(ErrorState);
@@ -296,7 +299,7 @@ bool ArrayBoundChecker::isFromCtypeMacro(const Expr *E, ASTContext &ACtx) {
bool ArrayBoundChecker::isOffsetObviouslyNonnegative(const Expr *E,
CheckerContext &C) {
- const ArraySubscriptExpr *ASE = getAsCleanArraySubscriptExpr(E, C);
+ const ArraySubscriptExpr *ASE = bounds::getAsCleanArraySubscriptExpr(E, C);
if (!ASE)
return false;
return ASE->getIdx()->getType()->isUnsignedIntegerOrEnumerationType();
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index 23fb4a0a22e96..b5756cdac76cf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -21,11 +21,12 @@
using llvm::formatv;
-namespace clang {
-namespace ento {
+using namespace clang;
+using namespace ento;
+using namespace bounds;
const ArraySubscriptExpr *
-getAsCleanArraySubscriptExpr(const Expr *E, const CheckerContext &C) {
+bounds::getAsCleanArraySubscriptExpr(const Expr *E, const CheckerContext &C) {
const auto *ASE = dyn_cast<ArraySubscriptExpr>(E);
if (!ASE)
return nullptr;
@@ -160,8 +161,8 @@ compareValueToThreshold(ProgramStateRef State, NonLoc Value, NonLoc Threshold,
return {nullptr, nullptr};
}
-std::string getRegionName(const MemSpaceRegion *Space,
- const SubRegion *Region) {
+std::string bounds::getRegionName(const MemSpaceRegion *Space,
+ const SubRegion *Region) {
if (std::string RegName = Region->getDescriptiveName(); !RegName.empty())
return RegName;
@@ -200,8 +201,8 @@ static std::optional<int64_t> getConcreteValue(std::optional<NonLoc> SV) {
/// it can be performed (`Divisor` is nonzero and there is no remainder). The
/// values `Val1` and `Val2` may be nullopt and in that case the corresponding
/// division is considered to be successful.
-bool SizeUnit::tryConvertValuesFromBytes(std::optional<int64_t> &Val1,
- std::optional<int64_t> &Val2) const {
+bool bounds::SizeUnit::tryConvertValuesFromBytes(
+ std::optional<int64_t> &Val1, std::optional<int64_t> &Val2) const {
if (!AsCharUnits)
return false;
const bool Val1HasRemainder = Val1 && *Val1 % AsCharUnits;
@@ -215,8 +216,9 @@ bool SizeUnit::tryConvertValuesFromBytes(std::optional<int64_t> &Val1,
return true;
}
-Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
- std::optional<NonLoc> Extent, BadOffsetKind Problem) {
+Messages bounds::getNonTaintMsgs(std::string RegName, SizeUnit SU,
+ NonLoc Offset, std::optional<NonLoc> Extent,
+ BadOffsetKind Problem) {
std::optional<int64_t> OffsetN = getConcreteValue(Offset);
std::optional<int64_t> ExtentN = getConcreteValue(Extent);
@@ -264,8 +266,8 @@ Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
std::string(Buf)};
}
-Messages getTaintMsgs(std::string RegName, const char *OffsetName,
- bool AlsoMentionUnderflow) {
+Messages bounds::getTaintMsgs(std::string RegName, const char *OffsetName,
+ bool AlsoMentionUnderflow) {
return {formatv("Potential out of bound access to {0} with tainted {1}",
RegName, OffsetName),
formatv("Access of {0} with a tainted {1} that may be {2}too large",
@@ -273,9 +275,9 @@ Messages getTaintMsgs(std::string RegName, const char *OffsetName,
AlsoMentionUnderflow ? "negative or " : "")};
}
-std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
- StringRef RegName,
- SizeUnit SU) const {
+std::string bounds::CheckResult::getMessage(PathSensitiveBugReport &BR,
+ StringRef RegName,
+ SizeUnit SU) const {
bool ShouldReportNonNegative = AssumedNonNegative;
if (!providesInformationAboutInteresting(Offset, BR)) {
if (AssumedUpperBound && providesInformationAboutInteresting(*Extent, BR)) {
@@ -327,7 +329,7 @@ std::string BoundsCheckResult::getMessage(PathSensitiveBugReport &BR,
return std::string(Out.str());
}
-bool BoundsCheckResult::providesInformationAboutInteresting(
+bool bounds::CheckResult::providesInformationAboutInteresting(
SVal SV, PathSensitiveBugReport &BR) {
SymbolRef Sym = SV.getAsSymbol();
if (!Sym)
@@ -346,10 +348,10 @@ bool BoundsCheckResult::providesInformationAboutInteresting(
return false;
}
-BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
- NonLoc Offset, std::optional<NonLoc> Extent,
- CheckFlags Flags) {
- BoundsCheckResult Res(Offset, Extent);
+CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
+ NonLoc Offset, std::optional<NonLoc> Extent,
+ CheckFlags Flags) {
+ CheckResult Res(Offset, Extent);
// CHECK LOWER BOUND
if (Flags.CheckUnderflow) {
@@ -377,7 +379,7 @@ BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
if (!WithinLowerBound) {
// The state is completely nonsense -- let's just sink it!
- Res.finalize(BoundsCheckResult::Kind::Paradox, PrecedesLowerBound);
+ Res.finalize(CheckResult::Kind::Paradox, PrecedesLowerBound);
return Res;
}
// Otherwise continue on the 'WithinLowerBound' branch where the
@@ -386,7 +388,7 @@ BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
} else {
if (!WithinLowerBound) {
// ...and it cannot be valid (>= 0), so report an error.
- Res.finalize(BoundsCheckResult::Kind::Underflow, PrecedesLowerBound);
+ Res.finalize(CheckResult::Kind::Underflow, PrecedesLowerBound);
return Res;
}
// ...but it can be valid as well, so the checker will (optimistically)
@@ -418,18 +420,18 @@ BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
compareValueToThreshold(State, Offset, *Extent, SVB,
/*CheckEquality=*/true);
if (EqualsToThreshold && !NotEqualToThreshold) {
- Res.finalize(BoundsCheckResult::Kind::Valid, State);
+ Res.finalize(CheckResult::Kind::Valid, State);
return Res;
}
}
- Res.finalize(BoundsCheckResult::Kind::Overflow, ExceedsUpperBound);
+ Res.finalize(CheckResult::Kind::Overflow, ExceedsUpperBound);
return Res;
}
// ...and it can be valid as well...
if (taint::isTainted(State, Offset)) {
// ...but it's tainted, so report an error.
- Res.finalize(BoundsCheckResult::Kind::TaintBug, State);
+ Res.finalize(CheckResult::Kind::TaintBug, State);
return Res;
}
// ...and it isn't tainted, so the checker will (optimistically) assume
@@ -443,9 +445,6 @@ BoundsCheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
if (WithinUpperBound)
State = WithinUpperBound;
}
- Res.finalize(BoundsCheckResult::Kind::Valid, State);
+ Res.finalize(CheckResult::Kind::Valid, State);
return Res;
}
-
-} // namespace ento
-} // namespace clang
>From 922e7b3a78107c5b212825ea5b0ab2ab66a09e42 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 1 Jul 2026 14:32:02 +0200
Subject: [PATCH 24/29] Generalize contract of forSVal
Note that for `ElementRegion`s `getElementType()` is identical to
`getValueType()` (`getElementType()` is the more idiomatic name, while
`getValueType()` is the generic method declared in the base class).
---
clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index 8acb4c4454228..6df337a728529 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -61,9 +61,8 @@ class SizeUnit {
/// FIXME: It is unfortunate that this heuristic differs from the heuristic
/// used for reporting assumption (`SizeUnit::forExpr`).
static SizeUnit forSVal(SVal Location, const ASTContext &ACtx) {
- const auto *EReg = Location.getAsRegion()->getAs<ElementRegion>();
- assert(EReg && "this checker only handles element access");
- return SizeUnit(EReg->getElementType(), ACtx);
+ const auto *TVR = Location.getAsRegion()->getAs<TypedValueRegion>();
+ return TVR ? SizeUnit(TVR->getValueType(), ACtx) : bytes();
}
int64_t asCharUnits() const { return AsCharUnits; }
>From 95f609faf093fb141f2b7afae764c367b691d3e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 2 Jul 2026 13:36:53 +0200
Subject: [PATCH 25/29] Rename 'Paradox' to 'CorruptedState'
---
.../clang/StaticAnalyzer/Checkers/BoundsChecking.h | 2 +-
clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 8 ++++----
clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index 6df337a728529..f7adc9e719965 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -119,7 +119,7 @@ struct CheckFlags {
class CheckResult {
public:
- enum class Kind { Underflow, Overflow, TaintBug, Paradox, Valid };
+ enum class Kind { Underflow, Overflow, TaintBug, CorruptedState, Valid };
private:
Kind K = Kind::Valid;
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 99860f882bec9..86248a5e9b912 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -172,10 +172,10 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
std::string RN = bounds::getRegionName(Space, Reg);
switch (Res.getKind()) {
- case bounds::CheckResult::Kind::Paradox:
- // The current state is paradoxical (due to bad modeling of casts we
- // assumed that an unsigned value is negative), so we should sink the
- // execution path.
+ case bounds::CheckResult::Kind::CorruptedState:
+ // The current state is corrupted (due to bad modeling of casts we assumed
+ // that an unsigned value is negative), so we should sink the execution
+ // path.
C.addSink();
return;
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index b5756cdac76cf..d2a7b65b6c2e0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -378,8 +378,8 @@ CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
// TODO: Remove this hack once 'SymbolCast's are modeled properly.
if (!WithinLowerBound) {
- // The state is completely nonsense -- let's just sink it!
- Res.finalize(CheckResult::Kind::Paradox, PrecedesLowerBound);
+ // The state is corrupted -- let's just sink it!
+ Res.finalize(CheckResult::Kind::CorruptedState, PrecedesLowerBound);
return Res;
}
// Otherwise continue on the 'WithinLowerBound' branch where the
>From 8260f7af34109dd4ecc1b85a97f7e03a59cf1087 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 2 Jul 2026 14:02:54 +0200
Subject: [PATCH 26/29] Turn 'getTaintMsgs'/'getNonTaintMsgs' into methods
... of `bounds::CheckResult` because they were tightly connected to that
class and were receiving its should-be-internal data as arguments.
Also rename `bounds::CheckResults::getMessage` to `getAssumptionMsg` now
that this class has other methods that return different messages.
The code of these methods will be refactored in a follow-up commit to
make it more idiomatic.
---
.../StaticAnalyzer/Checkers/BoundsChecking.h | 14 ++++++--------
.../Checkers/ArrayBoundChecker.cpp | 8 +++-----
.../Checkers/BoundsChecking.cpp | 19 ++++++++++---------
3 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index f7adc9e719965..ecfd23a32982b 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -165,8 +165,12 @@ class CheckResult {
}
}
- std::string getMessage(PathSensitiveBugReport &BR, StringRef RegName,
- SizeUnit SU) const;
+ Messages getTaintMsgs(std::string RegName, const char *OffsetName);
+
+ Messages getNonTaintMsgs(std::string RegName, SizeUnit SU);
+
+ std::string getAssumptionMsg(PathSensitiveBugReport &BR, StringRef RegName,
+ SizeUnit SU) const;
private:
/// Return true if information about the symbol behind `SV` can constrain
@@ -193,12 +197,6 @@ CheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB, NonLoc Offset,
// FIXME: This utility probably should become a method of `MemRegion`.
std::string getRegionName(const MemSpaceRegion *Space, const SubRegion *Region);
-Messages getTaintMsgs(std::string RegName, const char *OffsetName,
- bool AlsoMentionUnderflow);
-
-Messages getNonTaintMsgs(std::string RegName, SizeUnit SU, NonLoc Offset,
- std::optional<NonLoc> Extent, BadOffsetKind Problem);
-
} // namespace clang::ento::bounds
#endif // LLVM_CLANG_STATICANALYZER_CHECKERS_BOUNDSCHECKING_H
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 86248a5e9b912..95a9993f8d47e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -185,7 +185,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
bounds::SizeUnit SU = bounds::SizeUnit::forExpr(E, C);
Tag = C.getNoteTag(
[Res, RN, SU](PathSensitiveBugReport &BR) -> std::string {
- return Res.getMessage(BR, RN, SU);
+ return Res.getAssumptionMsg(BR, RN, SU);
});
}
@@ -201,9 +201,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
if (taint::isTainted(State, ASE->getIdx(), C.getStackFrame()))
OffsetName = "index";
- bounds::Messages Msgs =
- bounds::getTaintMsgs(RN, OffsetName,
- /*AlsoMentionUnderflow=*/Res.assumedNonNegative());
+ bounds::Messages Msgs = Res.getTaintMsgs(RN, OffsetName);
SmallVector<NonLoc, 2> Interesting = {ByteOffset};
if (Extent)
Interesting.push_back(*Extent);
@@ -214,7 +212,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
bounds::SizeUnit SU =
bounds::SizeUnit::forSVal(Location, C.getASTContext());
bounds::BadOffsetKind BOK = *Res.getBadOffsetKind();
- bounds::Messages Msgs = getNonTaintMsgs(RN, SU, ByteOffset, Extent, BOK);
+ bounds::Messages Msgs = Res.getNonTaintMsgs(RN, SU);
SmallVector<NonLoc, 2> Interesting = {ByteOffset};
if (Extent && BOK != bounds::BadOffsetKind::Negative)
Interesting.push_back(*Extent);
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index d2a7b65b6c2e0..bc90bcf88a6e5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -216,9 +216,10 @@ bool bounds::SizeUnit::tryConvertValuesFromBytes(
return true;
}
-Messages bounds::getNonTaintMsgs(std::string RegName, SizeUnit SU,
- NonLoc Offset, std::optional<NonLoc> Extent,
- BadOffsetKind Problem) {
+Messages bounds::CheckResult::getNonTaintMsgs(std::string RegName,
+ SizeUnit SU) {
+ // BadOffsetKind will be removed in a follow-up change.
+ BadOffsetKind Problem = *getBadOffsetKind();
std::optional<int64_t> OffsetN = getConcreteValue(Offset);
std::optional<int64_t> ExtentN = getConcreteValue(Extent);
@@ -266,18 +267,18 @@ Messages bounds::getNonTaintMsgs(std::string RegName, SizeUnit SU,
std::string(Buf)};
}
-Messages bounds::getTaintMsgs(std::string RegName, const char *OffsetName,
- bool AlsoMentionUnderflow) {
+Messages bounds::CheckResult::getTaintMsgs(std::string RegName,
+ const char *OffsetName) {
return {formatv("Potential out of bound access to {0} with tainted {1}",
RegName, OffsetName),
formatv("Access of {0} with a tainted {1} that may be {2}too large",
RegName, OffsetName,
- AlsoMentionUnderflow ? "negative or " : "")};
+ assumedNonNegative() ? "negative or " : "")};
}
-std::string bounds::CheckResult::getMessage(PathSensitiveBugReport &BR,
- StringRef RegName,
- SizeUnit SU) const {
+std::string bounds::CheckResult::getAssumptionMsg(PathSensitiveBugReport &BR,
+ StringRef RegName,
+ SizeUnit SU) const {
bool ShouldReportNonNegative = AssumedNonNegative;
if (!providesInformationAboutInteresting(Offset, BR)) {
if (AssumedUpperBound && providesInformationAboutInteresting(*Extent, BR)) {
>From c168873510c1a2c19573841296bf99af6ecda06c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 2 Jul 2026 16:46:21 +0200
Subject: [PATCH 27/29] Refactor CheckResult, remove BadOffsetKind
Now that `getNotTaintMsgs` and `getTaintMsgs` became methods of
`CheckResult`, they can directly access the data members directly
instead of using an intermediate `BadOffsetKind` value.
Another significant change is that commit reunifies the data members
`AssumedUpperBound` and `Extent`: now `Extent != nullopt` expresses that
the overflow was feasible based on previous knowledge.
Moreover, this commit renames `AssumedNonNegative` to
`UnderflowFeasible` (to reflect that it is used in reporting pure
underflow errors) and introduces `getInteresting()` to de-duplicate the
"which symbolic values are interesting" calculations and eliminate
another reference to the `BadOffsetKing`.
---
.../StaticAnalyzer/Checkers/BoundsChecking.h | 79 +++++++++----------
.../Checkers/ArrayBoundChecker.cpp | 12 +--
.../Checkers/BoundsChecking.cpp | 47 ++++++-----
3 files changed, 63 insertions(+), 75 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index ecfd23a32982b..0b48d107b6b1d 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -97,20 +97,6 @@ struct Messages {
std::string Full;
};
-enum class BadOffsetKind { Negative, Overflowing, Indeterminate };
-
-constexpr llvm::StringLiteral Adjectives[] = {"a negative", "an overflowing",
- "a negative or overflowing"};
-inline StringRef asAdjective(BadOffsetKind Problem) {
- return Adjectives[static_cast<int>(Problem)];
-}
-
-constexpr llvm::StringLiteral Prepositions[] = {"preceding", "after the end of",
- "around"};
-inline StringRef asPreposition(BadOffsetKind Problem) {
- return Prepositions[static_cast<int>(Problem)];
-}
-
struct CheckFlags {
unsigned CheckUnderflow : 1;
unsigned OffsetObviouslyNonnegative : 1;
@@ -123,48 +109,30 @@ class CheckResult {
private:
Kind K = Kind::Valid;
- bool AssumedNonNegative = false;
- bool AssumedUpperBound = false;
+ // Changed to true if we see that underflow was not ruled out by the previous
+ // knowledge about the offset.
+ bool UnderflowFeasible = false;
+ // The offset from the beginning of the accessed region in CharUnits.
const NonLoc Offset;
- std::optional<NonLoc> Extent;
+ // The extent of the accessed region in CharUnits; or `nullopt` if the extent
+ // is irrelevant because overflow was ruled out by previous knowledge about
+ // the offset and extent.
+ std::optional<NonLoc> Extent = std::nullopt;
ProgramStateRef State = nullptr;
- CheckResult(NonLoc Offs, std::optional<NonLoc> E) : Offset(Offs), Extent(E) {}
-
- void recordNonNegativeAssumption() { AssumedNonNegative = true; }
-
- void recordUpperBoundAssumption() { AssumedUpperBound = true; }
-
- void finalize(Kind K_, ProgramStateRef S) {
- K = K_;
- State = S;
- }
+ CheckResult(NonLoc Offs) : Offset(Offs) {}
public:
friend CheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
NonLoc Offset, std::optional<NonLoc> Extent,
CheckFlags Flags);
- bool assumedNonNegative() const { return AssumedNonNegative; }
-
- bool hasAssumption() const { return AssumedNonNegative || AssumedUpperBound; }
+ bool hasAssumption() const { return UnderflowFeasible || Extent; }
ProgramStateRef getState() const { return State; }
Kind getKind() const { return K; }
- std::optional<BadOffsetKind> getBadOffsetKind() const {
- switch (K) {
- case Kind::Underflow:
- return BadOffsetKind::Negative;
- case Kind::Overflow:
- return assumedNonNegative() ? BadOffsetKind::Indeterminate
- : BadOffsetKind::Overflowing;
- default:
- return std::nullopt;
- }
- }
-
Messages getTaintMsgs(std::string RegName, const char *OffsetName);
Messages getNonTaintMsgs(std::string RegName, SizeUnit SU);
@@ -172,7 +140,34 @@ class CheckResult {
std::string getAssumptionMsg(PathSensitiveBugReport &BR, StringRef RegName,
SizeUnit SU) const;
+ using InterestingVec = SmallVector<NonLoc, 2>;
+ InterestingVec getInteresting() const {
+ InterestingVec Res = {Offset};
+ if (Extent)
+ Res.push_back(*Extent);
+ return Res;
+ }
+
private:
+ void recordUnderflowFeasible() { UnderflowFeasible = true; }
+ void recordRelevantExtent(NonLoc E) { Extent = E; }
+ void discardExtentInformation() { Extent = std::nullopt; }
+
+ void finalize(Kind K_, ProgramStateRef S) {
+ K = K_;
+ State = S;
+ }
+
+ const char *offsetAdjective() const {
+ return UnderflowFeasible
+ ? (Extent ? "a negative or overflowing" : "a negative")
+ : (Extent ? "an overflowing" : "a valid");
+ }
+ const char *offsetPreposition() const {
+ return UnderflowFeasible ? (Extent ? "around" : "preceding")
+ : (Extent ? "after the end of" : "in");
+ }
+
/// Return true if information about the symbol behind `SV` can constrain
/// some symbol which is interesting within the bug report `BR`.
/// In particular, this returns true when `SV` is interesting within `BR`;
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 95a9993f8d47e..20e166db0ef04 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -202,21 +202,15 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
OffsetName = "index";
bounds::Messages Msgs = Res.getTaintMsgs(RN, OffsetName);
- SmallVector<NonLoc, 2> Interesting = {ByteOffset};
- if (Extent)
- Interesting.push_back(*Extent);
- reportOOB(C, Res.getState(), Msgs, Interesting, /*IsTaintBug=*/true);
+ reportOOB(C, Res.getState(), Msgs, Res.getInteresting(),
+ /*IsTaintBug=*/true);
return;
}
default: {
bounds::SizeUnit SU =
bounds::SizeUnit::forSVal(Location, C.getASTContext());
- bounds::BadOffsetKind BOK = *Res.getBadOffsetKind();
bounds::Messages Msgs = Res.getNonTaintMsgs(RN, SU);
- SmallVector<NonLoc, 2> Interesting = {ByteOffset};
- if (Extent && BOK != bounds::BadOffsetKind::Negative)
- Interesting.push_back(*Extent);
- reportOOB(C, Res.getState(), Msgs, Interesting);
+ reportOOB(C, Res.getState(), Msgs, Res.getInteresting());
return;
}
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index bc90bcf88a6e5..bb7d1c2e50c79 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -218,15 +218,9 @@ bool bounds::SizeUnit::tryConvertValuesFromBytes(
Messages bounds::CheckResult::getNonTaintMsgs(std::string RegName,
SizeUnit SU) {
- // BadOffsetKind will be removed in a follow-up change.
- BadOffsetKind Problem = *getBadOffsetKind();
-
std::optional<int64_t> OffsetN = getConcreteValue(Offset);
std::optional<int64_t> ExtentN = getConcreteValue(Extent);
- if (Problem == BadOffsetKind::Negative)
- ExtentN = std::nullopt;
-
if (!SU.tryConvertValuesFromBytes(OffsetN, ExtentN))
SU = SizeUnit::bytes();
@@ -243,11 +237,11 @@ Messages bounds::CheckResult::getNonTaintMsgs(std::string RegName,
}
Out << RegName << " at ";
if (OffsetN) {
- if (Problem == BadOffsetKind::Negative)
+ if (UnderflowFeasible && !Extent)
Out << "negative ";
Out << SU.getOffsetName() << " " << *OffsetN;
} else {
- Out << asAdjective(Problem) << " " << SU.getOffsetName();
+ Out << offsetAdjective() << " " << SU.getOffsetName();
}
if (ExtentN) {
Out << ", while it holds only ";
@@ -262,8 +256,8 @@ Messages bounds::CheckResult::getNonTaintMsgs(std::string RegName,
Out << "s";
}
- return {formatv("Out of bound access to memory {0} {1}",
- asPreposition(Problem), RegName),
+ return {formatv("Out of bound access to memory {0} {1}", offsetPreposition(),
+ RegName),
std::string(Buf)};
}
@@ -273,15 +267,15 @@ Messages bounds::CheckResult::getTaintMsgs(std::string RegName,
RegName, OffsetName),
formatv("Access of {0} with a tainted {1} that may be {2}too large",
RegName, OffsetName,
- assumedNonNegative() ? "negative or " : "")};
+ UnderflowFeasible ? "negative or " : "")};
}
std::string bounds::CheckResult::getAssumptionMsg(PathSensitiveBugReport &BR,
StringRef RegName,
SizeUnit SU) const {
- bool ShouldReportNonNegative = AssumedNonNegative;
+ bool ShouldReportNonNegative = UnderflowFeasible;
if (!providesInformationAboutInteresting(Offset, BR)) {
- if (AssumedUpperBound && providesInformationAboutInteresting(*Extent, BR)) {
+ if (Extent && providesInformationAboutInteresting(*Extent, BR)) {
// Even if the byte offset isn't interesting (e.g. it's a constant value),
// the assumption can still be interesting if it provides information
// about an interesting symbolic upper bound.
@@ -293,9 +287,7 @@ std::string bounds::CheckResult::getAssumptionMsg(PathSensitiveBugReport &BR,
}
std::optional<int64_t> OffsetN = getConcreteValue(Offset);
- // The extent must be ignored if we did not assume it as an upper bound:
- std::optional<int64_t> ExtentN =
- AssumedUpperBound ? getConcreteValue(Extent) : std::nullopt;
+ std::optional<int64_t> ExtentN = getConcreteValue(Extent);
if (!SU.tryConvertValuesFromBytes(OffsetN, ExtentN))
SU = SizeUnit::bytes();
@@ -307,7 +299,7 @@ std::string bounds::CheckResult::getAssumptionMsg(PathSensitiveBugReport &BR,
Out << "index ";
if (OffsetN)
Out << "'" << OffsetN << "' ";
- } else if (AssumedUpperBound) {
+ } else if (Extent) {
Out << "byte offset ";
if (OffsetN)
Out << "'" << OffsetN << "' ";
@@ -319,7 +311,7 @@ std::string bounds::CheckResult::getAssumptionMsg(PathSensitiveBugReport &BR,
if (ShouldReportNonNegative) {
Out << " non-negative";
}
- if (AssumedUpperBound) {
+ if (Extent) {
if (ShouldReportNonNegative)
Out << " and";
Out << " less than ";
@@ -352,7 +344,7 @@ bool bounds::CheckResult::providesInformationAboutInteresting(
CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
NonLoc Offset, std::optional<NonLoc> Extent,
CheckFlags Flags) {
- CheckResult Res(Offset, Extent);
+ CheckResult Res(Offset);
// CHECK LOWER BOUND
if (Flags.CheckUnderflow) {
@@ -360,6 +352,7 @@ CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
compareValueToThreshold(State, Offset, SVB.makeZeroArrayIndex(), SVB);
if (PrecedesLowerBound) {
+ Res.recordUnderflowFeasible();
// The analyzer thinks that the offset may be invalid (negative)...
if (Flags.OffsetObviouslyNonnegative) {
// ...but the offset is obviously non-negative (clear array subscript
@@ -394,7 +387,6 @@ CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
}
// ...but it can be valid as well, so the checker will (optimistically)
// assume that it's valid and mention this in the note tag.
- Res.recordNonNegativeAssumption();
}
}
@@ -411,24 +403,32 @@ CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
compareValueToThreshold(State, Offset, *Extent, SVB);
if (ExceedsUpperBound) {
+ Res.recordRelevantExtent(*Extent);
+
// The offset may be invalid (>= Size)...
if (!WithinUpperBound) {
- // ...and it cannot be within bounds, so report an error, unless we can
- // definitely determine that this is an idiomatic `&array[size]`
- // expression that calculates the past-the-end pointer.
+ // ...and it cannot be within bounds.
+
if (Flags.AcceptPastTheEnd) {
auto [EqualsToThreshold, NotEqualToThreshold] =
compareValueToThreshold(State, Offset, *Extent, SVB,
/*CheckEquality=*/true);
if (EqualsToThreshold && !NotEqualToThreshold) {
+ // This is an idiomatic `&array[size]` past-the-end pointer
+ // expression, which is valid and well-defined. We discard the
+ // extent information because otherwise we would get an
+ // inappropriate note tag about "assuming offset < extent".
+ Res.discardExtentInformation();
Res.finalize(CheckResult::Kind::Valid, State);
return Res;
}
}
+ // Straightforward overflow, report an error.
Res.finalize(CheckResult::Kind::Overflow, ExceedsUpperBound);
return Res;
}
+
// ...and it can be valid as well...
if (taint::isTainted(State, Offset)) {
// ...but it's tainted, so report an error.
@@ -437,7 +437,6 @@ CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
}
// ...and it isn't tainted, so the checker will (optimistically) assume
// that the offset is in bounds and mention this in the note tag.
- Res.recordUpperBoundAssumption();
}
// Actually update the state. The "if" only fails in the extremely unlikely
>From 07426529998d6313ca881befc62ce06f3df03add Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 2 Jul 2026 17:09:39 +0200
Subject: [PATCH 28/29] Unify Underflow and Overflow results into Invalid
Underflow and overflow reports are handled by the same logic (they both
lead to standard non-taint bug reports), so there was no reason to
represent them with two separate enumerators in `CheckResult::Kind`.
Among these `Invalid` reports the possibility of underflow is
represented by `UnderflowFeasible == true`, while the possibility of
overflow is represented by `Extent != nullopt`. Note that these two are
not mutually exclusive -- there is an "either underflow or overflow, but
not correct" case which was previously bundled into `Overflow`.
---
clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h | 2 +-
clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 4 +++-
clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index 0b48d107b6b1d..22f5de7d79032 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -105,7 +105,7 @@ struct CheckFlags {
class CheckResult {
public:
- enum class Kind { Underflow, Overflow, TaintBug, CorruptedState, Valid };
+ enum class Kind { Invalid, TaintBug, CorruptedState, Valid };
private:
Kind K = Kind::Valid;
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 20e166db0ef04..33b4696d89533 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -206,7 +206,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
/*IsTaintBug=*/true);
return;
}
- default: {
+ case bounds::CheckResult::Kind::Invalid: {
bounds::SizeUnit SU =
bounds::SizeUnit::forSVal(Location, C.getASTContext());
bounds::Messages Msgs = Res.getNonTaintMsgs(RN, SU);
@@ -214,6 +214,8 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
return;
}
}
+
+ llvm_unreachable("all CheckResult values are covered in switch");
}
void ArrayBoundChecker::markPartsInteresting(PathSensitiveBugReport &BR,
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index bb7d1c2e50c79..5d85a18bb0901 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -382,7 +382,7 @@ CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
} else {
if (!WithinLowerBound) {
// ...and it cannot be valid (>= 0), so report an error.
- Res.finalize(CheckResult::Kind::Underflow, PrecedesLowerBound);
+ Res.finalize(CheckResult::Kind::Invalid, PrecedesLowerBound);
return Res;
}
// ...but it can be valid as well, so the checker will (optimistically)
@@ -425,7 +425,7 @@ CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
}
// Straightforward overflow, report an error.
- Res.finalize(CheckResult::Kind::Overflow, ExceedsUpperBound);
+ Res.finalize(CheckResult::Kind::Invalid, ExceedsUpperBound);
return Res;
}
>From e8d9c9da8392fcbe10f24763caf8953f01552bfa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 2 Jul 2026 17:19:09 +0200
Subject: [PATCH 29/29] Reorder enum CheckResult::Kind
... to follow a more natural order. Also, adjust the order of the
`case:`s of the `switch` in `ArrayBoundChecker.cpp` to match this order.
---
.../StaticAnalyzer/Checkers/BoundsChecking.h | 2 +-
.../Checkers/ArrayBoundChecker.cpp | 23 +++++++++----------
2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index 22f5de7d79032..c1ab2b6b97a96 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -105,7 +105,7 @@ struct CheckFlags {
class CheckResult {
public:
- enum class Kind { Invalid, TaintBug, CorruptedState, Valid };
+ enum class Kind { Valid, Invalid, TaintBug, CorruptedState };
private:
Kind K = Kind::Valid;
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 33b4696d89533..54912480d7740 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -172,13 +172,13 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
std::string RN = bounds::getRegionName(Space, Reg);
switch (Res.getKind()) {
- case bounds::CheckResult::Kind::CorruptedState:
- // The current state is corrupted (due to bad modeling of casts we assumed
- // that an unsigned value is negative), so we should sink the execution
- // path.
- C.addSink();
+ case bounds::CheckResult::Kind::Invalid: {
+ bounds::SizeUnit SU =
+ bounds::SizeUnit::forSVal(Location, C.getASTContext());
+ bounds::Messages Msgs = Res.getNonTaintMsgs(RN, SU);
+ reportOOB(C, Res.getState(), Msgs, Res.getInteresting());
return;
-
+ }
case bounds::CheckResult::Kind::Valid: {
const NoteTag *Tag = nullptr;
if (Res.hasAssumption()) {
@@ -206,14 +206,13 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
/*IsTaintBug=*/true);
return;
}
- case bounds::CheckResult::Kind::Invalid: {
- bounds::SizeUnit SU =
- bounds::SizeUnit::forSVal(Location, C.getASTContext());
- bounds::Messages Msgs = Res.getNonTaintMsgs(RN, SU);
- reportOOB(C, Res.getState(), Msgs, Res.getInteresting());
+ case bounds::CheckResult::Kind::CorruptedState:
+ // The current state is corrupted (due to bad modeling of casts we assumed
+ // that an unsigned value is negative), so we should sink the execution
+ // path.
+ C.addSink();
return;
}
- }
llvm_unreachable("all CheckResult values are covered in switch");
}
More information about the llvm-commits
mailing list