================
@@ -57,92 +57,147 @@ 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());
+ }
-class StateUpdateReporter {
- const MemSpaceRegion *Space;
- const SubRegion *Reg;
- const NonLoc ByteOffsetVal;
- const std::optional<QualType> ElementType;
- const std::optional<int64_t> ElementSize;
- bool AssumedNonNegative = false;
- std::optional<NonLoc> AssumedUpperBound = std::nullopt;
+ static SizeUnit bytes() { return SizeUnit(); }
-public:
- StateUpdateReporter(const SubRegion *R, NonLoc ByteOffsVal, const Expr *E,
- CheckerContext &C)
- : Space(R->getMemorySpace(C.getState())), Reg(R),
- ByteOffsetVal(ByteOffsVal), ElementType(determineElementType(E, C)),
- ElementSize(determineElementSize(ElementType, C)) {}
+ 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();
+ }
- void recordNonNegativeAssumption() { AssumedNonNegative = true; }
- void recordUpperBoundAssumption(NonLoc UpperBoundVal) {
- AssumedUpperBound = UpperBoundVal;
+ /// 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.
+ /// FIXME: It is unfortunate that this heuristic differs from the heuristic
+ /// used for reporting assumption; but this difference is currently needed
+ /// due to the unfortunate phrasing of the assumption messages.
+ /// Get rid of this when the assumption note is rephrased and improved.
+ 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());
}
- bool assumedNonNegative() { return AssumedNonNegative; }
+ int64_t asCharUnits() const { return AsCharUnits; }
- const NoteTag *createNoteTag(CheckerContext &C) const;
+ bool canExpress(std::optional<int64_t> Val) const {
+ return asCharUnits() && (!Val || !(*Val % asCharUnits()));
+ }
-private:
- std::string getMessage(PathSensitiveBugReport &BR) const;
-
- /// 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);
+ std::string asExtentDesc() const {
+ if (isBytes())
+ return "the extent of";
+ return formatv("the number of '{0}' elements in", AsType.getAsString());
+ }
+
+ std::string asElementName() const {
+ if (isBytes())
+ return "byte";
+ return formatv("'{0}' element", AsType.getAsString());
}
};
-struct Messages {
- std::string Short, Full;
+} // anonymous namespace
+
+namespace clang::ento::bounds {
+
+struct CheckFlags {
+ unsigned CheckUnderflow : 1;
+ unsigned OffsetObviouslyNonnegative : 1;
+ unsigned AcceptPastTheEnd : 1;
};
-enum class BadOffsetKind { Negative, Overflowing, Indeterminate };
+class CheckResult;
-constexpr llvm::StringLiteral Adjectives[] = {"a negative", "an overflowing",
- "a negative or overflowing"};
-static StringRef asAdjective(BadOffsetKind Problem) {
- return Adjectives[static_cast<int>(Problem)];
-}
+CheckResult checkBounds(ProgramStateRef State, SValBuilder &SVB, NonLoc Offset,
+ std::optional<NonLoc> Extent, CheckFlags Flags);
-constexpr llvm::StringLiteral Prepositions[] = {"preceding", "after the end
of",
- "around"};
-static StringRef asPreposition(BadOffsetKind Problem) {
- return Prepositions[static_cast<int>(Problem)];
-}
+class CheckResult {
+public:
+ /// When true, the bounds check noticed that the value of an unsigned
+ /// expression is constrained to negative values (because the analyzer
+ /// skipped the modeling of a cast expression). This execution path must be
+ /// discarded because it does not represent a real possibility.
+ /// FIXME: This hack is currently needed to filter out many ugly false
+ /// positives; but it should be removed when we fix cast modeling.
+ 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 mayBeValid() const { return static_cast<bool>(ValidState); }
+
+ /// 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 MayOverflowExtent.has_value(); }
+ /// When true, the checked offset may be out of bounds.
+ bool mayBeInvalid() const { return MayUnderflow || MayOverflowExtent; }
+
+ /// Returns the offset of the accessed location from the beginning of the
+ /// accessd region.
+ NonLoc getOffset() const { return Offset; }
+
+ /// Returns the extent of the accessed region if it is relevant (because the
----------------
Xazax-hun wrote:
I think it would be clearer if we used more direct names rather than comments
to explain what these terms mean. E.g., you could just say
"getOverflownExtent". Actually, LLMs can sometimes be surprisingly good at
brainstorming some names. Might be worth giving it a try when it is hard to
come up with a good one.
https://github.com/llvm/llvm-project/pull/210774
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
