================
@@ -595,139 +148,74 @@ void ArrayBoundChecker::performCheck(const Expr *E,
CheckerContext &C) const {
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(Reg, ByteOffset, E, C);
-
- // 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 (PrecedesLowerBound) {
- // The analyzer thinks that the offset may be invalid (negative)...
-
- if (isOffsetObviouslyNonnegative(E, C)) {
- // ...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!
- C.addSink();
- return;
- }
- // 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.
- Messages Msgs = getNonTaintMsgs(C.getASTContext(), Space, Reg,
- ByteOffset, /*Extent=*/std::nullopt,
- Location, BadOffsetKind::Negative);
- reportOOB(C, PrecedesLowerBound, Msgs, ByteOffset, std::nullopt);
- return;
- }
- // ...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();
- }
- }
+ 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()),
+ };
- // 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;
- }
+ BoundsCheckResult Res = checkBounds(State, SVB, ByteOffset, Extent, Flags);
+
+ std::string RN = getRegionName(Reg->getMemorySpace(C.getState()), Reg);
----------------
steakhal wrote:
I think `Space` already refers to the correct memory space.
```suggestion
std::string RN = getRegionName(Space, Reg);
```
https://github.com/llvm/llvm-project/pull/202372
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
