[clang] 9ff674e - [analyzer] Improve handling of &array[size] in ArrayBound (#214540)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 7 06:38:49 PDT 2026
Author: Donát Nagy
Date: 2026-08-07T15:38:45+02:00
New Revision: 9ff674efdb2be183f8a873f70726107b1399d708
URL: https://github.com/llvm/llvm-project/commit/9ff674efdb2be183f8a873f70726107b1399d708
DIFF: https://github.com/llvm/llvm-project/commit/9ff674efdb2be183f8a873f70726107b1399d708.diff
LOG: [analyzer] Improve handling of &array[size] in ArrayBound (#214540)
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 an 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 an 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.)
Added:
Modified:
clang/include/clang/StaticAnalyzer/Checkers/BoundsChecking.h
clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
Removed:
################################################################################
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..9e4e8e83749ad 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);
+ 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 b204155c87b86..9c11e9e2bd69b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BoundsChecking.cpp
@@ -74,16 +74,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);
@@ -141,7 +135,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)...
@@ -196,25 +190,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;
}
}
More information about the cfe-commits
mailing list