llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Donát Nagy (NagyDonat) <details> <summary>Changes</summary> The `security.ArrayBound` checker does not do anything when the value of the expression that activated its callbacks isn't expressed as an `ElementRegion`. Previously this was a sneaky consequence of the logic in `computeOffset`, but this commit highlights it with a straightforward early return statement to ensure that the accessed element region is available within the body of `ArrayBoundChecker::handleAccessExpr`. (I intend to use this in a follow-up commit.) --- Full diff: https://github.com/llvm/llvm-project/pull/216049.diff 1 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp (+13-13) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index 9e4e8e83749ad..5201eeddca16f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -75,11 +75,10 @@ class SizeUnit { bool isBytes() const { return AsType.isNull(); } /// Return the element type that is "natural" for reporting out-of-bounds - /// memory access to 'Location'. - static SizeUnit forSVal(SVal Location, const ASTContext &ACtx) { - if (const auto *R = Location.getAsRegion()->getAs<TypedValueRegion>()) - return SizeUnit(R->getValueType(), ACtx); - return bytes(); + /// memory access to \p ER. + static SizeUnit forElementRegion(const ElementRegion *ER, + const ASTContext &ACtx) { + return SizeUnit(ER->getElementType(), ACtx); } /// If `E` is a "clean" array subscript expression, return the type of the @@ -201,13 +200,14 @@ static bool isDeterminedByInterestingSymbol(SVal SV, return false; } -/// For a given Location that can be represented as a symbolic expression +/// For a given \p CurRegion 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 /// NonLoc that specifies the number of CharUnits). Returns nullopt when these /// cannot be determined. static std::optional<std::pair<const SubRegion *, NonLoc>> -computeOffset(ProgramStateRef State, SValBuilder &SVB, SVal Location) { +computeOffset(ProgramStateRef State, SValBuilder &SVB, + const ElementRegion *CurRegion) { QualType T = SVB.getArrayIndexType(); auto EvalBinOp = [&SVB, State, T](BinaryOperatorKind Op, NonLoc L, NonLoc R) { // We will use this utility to add and multiply values. @@ -217,9 +217,6 @@ computeOffset(ProgramStateRef State, SValBuilder &SVB, SVal Location) { const SubRegion *OwnerRegion = nullptr; std::optional<NonLoc> Offset = SVB.makeZeroArrayIndex(); - const ElementRegion *CurRegion = - dyn_cast_or_null<ElementRegion>(Location.getAsRegion()); - while (CurRegion) { const auto Index = CurRegion->getIndex().getAs<NonLoc>(); if (!Index) @@ -412,7 +409,10 @@ 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); + const ElementRegion *AccessedER = + dyn_cast_or_null<ElementRegion>(C.getSVal(E).getAsRegion()); + if (!AccessedER) + return; // The header ctype.h (from e.g. glibc) implements the isXXXXX() macros as // #define isXXXXX(arg) (LOOKUP_TABLE[arg] & BITMASK_FOR_XXXXX) @@ -426,7 +426,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E, SValBuilder &SVB = C.getSValBuilder(); const std::optional<std::pair<const SubRegion *, NonLoc>> &RawOffset = - computeOffset(State, SVB, Location); + computeOffset(State, SVB, AccessedER); if (!RawOffset) return; @@ -474,7 +474,7 @@ void ArrayBoundChecker::handleAccessExpr(const Expr *E, } } - SizeUnit SU = SizeUnit::forSVal(Location, ACtx); + SizeUnit SU = SizeUnit::forElementRegion(AccessedER, ACtx); BugDescription Desc = describeInvalidAccess(Res, RegName, SU); reportOOB(C, State, Desc, ByteOffset, Res.getExtentIfMayOverflow()); return; `````````` </details> https://github.com/llvm/llvm-project/pull/216049 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
