[clang] [analyzer] Improve handling of &array[size] in ArrayBound (PR #214540)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 6 10:55:25 PDT 2026
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/214540
>From 148535d6617f71b5034d97cd9b2d6860aa1541a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 6 Aug 2026 19:26:03 +0200
Subject: [PATCH 1/2] [analyzer] Improve handling of &array[size] in ArrayBound
The checker `security.ArrayBound` had a special case for not reporting
the `&array[size]` expressions where `size` is equal to the the element
count of `array`.
(Note that `array[size]` is reported as out-of-bounds access, but
`&array[size]` does not actually access the past-the end location and it
is a somewhat idiomatic way of expressing the past-the-end pointer.)
The primary goal of this change is that it simplifies the contract of
`bounds::checkBounds` which previously had a flag to act as if forming this
past-the-end pointer was valid in-bounds access.
With the new implementation, `checkBounds()` diagnoses the `&array[size]`
expression as out-of-bounds access, then `ArrayBoundChecker.cpp`
postprocesses this result and suppresses the report when it detects that
it was an idiomatic `&array[size]` expression.
This also removes the ugly but practically irrelevant corner case: the
old code for recognizing an idiomatic past-the-end pointer expression
also accepted it when the index (`size`) was constrained as "either
negative, or the element count of the array" (becasue the "assume lower
bound" step happens before checking the upper bound).
(The new code only accepts `&array[size]` as an idiomatic past-the
end pointer if `size` is constrained to be equal to the element count.)
---
.../StaticAnalyzer/Checkers/BoundsChecking.h | 21 +++++++------
.../Checkers/ArrayBoundChecker.cpp | 22 +++++++++----
.../Checkers/BoundsChecking.cpp | 31 +++++--------------
3 files changed, 34 insertions(+), 40 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
index 3e9a639e36cef..2c8469694b661 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
@@ -29,15 +29,12 @@ namespace clang::ento::bounds {
struct CheckFlags {
unsigned CheckUnderflow : 1;
unsigned OffsetObviouslyNonnegative : 1;
- unsigned AcceptPastTheEnd : 1;
};
class CheckResult;
/// Checks the validity of accessing a memory region with extent \p Extent at
-/// offset \p Offset. The \p Flags influence the semantics of the check, in
-/// particular if `AcceptPastTheEnd` is true, then Offset == Extent is also
-/// accepted as valid.
+/// offset \p Offset. The \p Flags influence the semantics of the check.
CheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB, NonLoc Offset,
std::optional<NonLoc> Extent, CheckFlags Flags);
@@ -52,15 +49,11 @@ class CheckResult {
bool isCorruptedState() const { return IsCorruptedState; }
/// When true, the checked offset may be in bounds.
- /// As an exceptional case, this is also true for idiomatic expressions that
- /// define a past-the-end pointer (and do not dereference it).
bool mayBeInBounds() const { return static_cast<bool>(InBoundsState); }
/// When true, the checked offset may be negative.
bool mayUnderflow() const { return MayUnderflow; }
/// When true, the checked offset may be >= the extent of the region.
- /// As an exceptional case, this is also false for idiomatic expressions that
- /// define a past-the-end pointer (and do not dereference it).
bool mayOverflow() const { return ExtentIfMayOverflow.has_value(); }
/// When true, the checked offset may be out of bounds.
bool mayBeInvalid() const { return MayUnderflow || ExtentIfMayOverflow; }
@@ -78,8 +71,6 @@ class CheckResult {
/// Returns the program state that should be used for continuing the analysis
/// after this bounds check. This returns null if mayBeInBounds() is false, in
/// that case the state before the check should be used in the error node.
- /// Note that we also have a valid state in the exception case when the
- /// 'access' calculates the past-the-end pointer without dereferencing it.
ProgramStateRef getInBoundsState() const { return InBoundsState; }
friend CheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB,
@@ -100,6 +91,16 @@ class CheckResult {
ProgramStateRef InBoundsState = nullptr;
};
+// Evaluate the comparison Value < Threshold with the help of the custom
+// simplification algorithm. 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.
+std::pair<ProgramStateRef, ProgramStateRef>
+compareValueToThreshold(ProgramStateRef State, SValBuilder &SVB, NonLoc Value,
+ NonLoc Threshold, bool CheckEquality = false);
} // 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 897b3b22c5bb8..2afa9e7122480 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -411,6 +411,7 @@ static std::string getAssumptionNote(bounds::CheckResult Res,
void ArrayBoundChecker::handleAccessExpr(const Expr *E,
CheckerContext &C) const {
+ ASTContext &ACtx = C.getASTContext();
const SVal Location = C.getSVal(E);
// The header ctype.h (from e.g. glibc) implements the isXXXXX() macros as
@@ -418,7 +419,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
// and incomplete analysis of these leads to false positives. As even
// accurate reports would be confusing for the users, just disable reports
// from these macros:
- if (isFromCtypeMacro(E, C.getASTContext()))
+ if (isFromCtypeMacro(E, ACtx))
return;
ProgramStateRef State = C.getState();
@@ -446,10 +447,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
bounds::CheckFlags Flags = {
/*CheckUnderflow=*/!(isa<SymbolicRegion>(Reg) &&
isa<UnknownSpaceRegion>(Space)),
- /*OffsetObviouslyNonnegative=*/isOffsetObviouslyNonnegative(E, C),
- /*AcceptPastTheEnd=*/isa<ArraySubscriptExpr>(E) &&
- isInAddressOf(E, C.getASTContext()),
- };
+ /*OffsetObviouslyNonnegative=*/isOffsetObviouslyNonnegative(E, C)};
bounds::CheckResult Res = checkBounds(State, SVB, ByteOffset, Extent, Flags);
@@ -464,7 +462,19 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
const NoteTag *T = nullptr;
if (Res.mayBeInvalid()) {
if (!Res.mayBeInBounds()) {
- SizeUnit SU = SizeUnit::forSVal(Location, C.getASTContext());
+ if (isa<ArraySubscriptExpr>(E) && isInAddressOf(E, ACtx) && Extent) {
+ // Recognize and accept the idiomatic `&array[size]` expression that
+ // forms the past-the-end pointer without actually dereferencing it.
+ auto [EqualsToThreshold, NotEqualToThreshold] =
+ bounds::compareValueToThreshold(State, SVB, ByteOffset, *Extent,
+ /*CheckEquality=*/true);
+ if (EqualsToThreshold && !NotEqualToThreshold) {
+ C.addTransition(EqualsToThreshold, T);
+ return;
+ }
+ }
+
+ SizeUnit SU = SizeUnit::forSVal(Location, ACtx);
BugDescription Desc = describeInvalidAccess(Res, RegName, SU);
reportOOB(C, State, Desc, ByteOffset, Res.getExtentIfMayOverflow());
return;
diff --git a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
index 52086254fff3a..e368931d9df51 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -76,16 +76,10 @@ static bool isUnsigned(SValBuilder &SVB, NonLoc Value) {
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) {
+std::pair<ProgramStateRef, ProgramStateRef>
+bounds::compareValueToThreshold(ProgramStateRef State, SValBuilder &SVB,
+ NonLoc Value, NonLoc Threshold,
+ bool CheckEquality) {
if (auto ConcreteThreshold = Threshold.getAs<nonloc::ConcreteInt>()) {
std::tie(Value, Threshold) =
getSimplifiedOffsets(Value, *ConcreteThreshold, SVB);
@@ -143,7 +137,7 @@ bounds::CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
// CHECK LOWER BOUND
if (Flags.CheckUnderflow) {
auto [PrecedesLowerBound, WithinLowerBound] =
- compareValueToThreshold(State, Offset, SVB.makeZeroArrayIndex(), SVB);
+ compareValueToThreshold(State, SVB, Offset, SVB.makeZeroArrayIndex());
if (PrecedesLowerBound) {
// The analyzer thinks that the offset may be invalid (negative)...
@@ -198,25 +192,14 @@ bounds::CheckResult bounds::checkBounds(ProgramStateRef State, SValBuilder &SVB,
// In this situation the warning message should mention both possibilities.
auto [WithinUpperBound, ExceedsUpperBound] =
- compareValueToThreshold(State, Offset, *Extent, SVB);
+ compareValueToThreshold(State, SVB, Offset, *Extent);
if (ExceedsUpperBound) {
// The offset may be invalid (>= Size)...
Res.ExtentIfMayOverflow = Extent;
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.ExtentIfMayOverflow = std::nullopt;
- Res.InBoundsState = EqualsToThreshold;
- }
- }
+ // ...and it cannot be within bounds.
return Res;
}
}
>From 06c5026a090a31ba00dca9d7e41fa6273f76dd9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Thu, 6 Aug 2026 19:54:28 +0200
Subject: [PATCH 2/2] Don't pass a NoteTag* which is always nullptr here
Its presence had no effect (the default is nullptr anyway) but was
a bit confusing.
---
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 2afa9e7122480..9e4e8e83749ad 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -469,7 +469,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E,
bounds::compareValueToThreshold(State, SVB, ByteOffset, *Extent,
/*CheckEquality=*/true);
if (EqualsToThreshold && !NotEqualToThreshold) {
- C.addTransition(EqualsToThreshold, T);
+ C.addTransition(EqualsToThreshold);
return;
}
}
More information about the cfe-commits
mailing list