[clang] [analyzer][NFC] Simplifications in ArrayBoundV2 (PR #67572)

[via cfe-commits]

=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>,
=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>,
=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/67572/clang at github.com>


================
@@ -32,42 +32,72 @@ using namespace taint;
 namespace {
 class ArrayBoundCheckerV2 :
     public Checker<check::Location> {
-  mutable std::unique_ptr<BugType> BT;
-  mutable std::unique_ptr<BugType> TaintBT;
+  BugType BT{this, "Out-of-bound access"};
+  BugType TaintBT{this, "Out-of-bound access", categories::TaintedData};
 
-  enum OOB_Kind { OOB_Precedes, OOB_Excedes };
+  enum OOB_Kind { OOB_Precedes, OOB_Exceeds, OOB_Taint };
 
-  void reportOOB(CheckerContext &C, ProgramStateRef errorState,
-                 OOB_Kind kind) const;
-  void reportTaintOOB(CheckerContext &C, ProgramStateRef errorState,
-                      SVal TaintedSVal) const;
+  void reportOOB(CheckerContext &C, ProgramStateRef ErrorState, OOB_Kind Kind,
+                 SVal TaintedSVal = UnknownVal()) const;
 
   static bool isFromCtypeMacro(const Stmt *S, ASTContext &AC);
 
 public:
   void checkLocation(SVal l, bool isLoad, const Stmt *S,
                      CheckerContext &C) const;
 };
+} // anonymous namespace
 
-// FIXME: Eventually replace RegionRawOffset with this class.
-class RegionRawOffsetV2 {
-private:
-  const SubRegion *baseRegion;
-  NonLoc byteOffset;
+/// For a given Location 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.
+std::optional<std::pair<const SubRegion *, NonLoc>>
+computeOffset(ProgramStateRef State, SValBuilder &SVB, SVal Location) {
+  QualType T = SVB.getArrayIndexType();
+  auto Calc = [&SVB, State, T](BinaryOperatorKind Op, NonLoc LHS, NonLoc RHS) {
+    // We will use this utility to add and multiply values.
+    return SVB.evalBinOpNN(State, Op, LHS, RHS, T).getAs<NonLoc>();
+  };
 
-public:
-  RegionRawOffsetV2(const SubRegion *base, NonLoc offset)
-      : baseRegion(base), byteOffset(offset) { assert(base); }
+  const auto *Region = dyn_cast_or_null<SubRegion>(Location.getAsRegion());
+  std::optional<NonLoc> Offset = std::nullopt;
+
+  while (const auto *ERegion = dyn_cast_or_null<ElementRegion>(Region)) {
+    const auto Index = ERegion->getIndex().getAs<NonLoc>();
+    if (!Index)
+      return std::nullopt;
+
+    QualType ElemType = ERegion->getElementType();
+    // If the element is an incomplete type, go no further.
+    if (ElemType->isIncompleteType())
+      return std::nullopt;
+
+    // Calculate Delta = Index * sizeof(ElemType).
+    NonLoc Size = SVB.makeArrayIndex(
+        SVB.getContext().getTypeSizeInChars(ElemType).getQuantity());
+    auto Delta = Calc(BO_Mul, *Index, Size);
+    if (!Delta)
+      return std::nullopt;
+
+    // Perform Offset += Delta, handling the initial nullopt as 0.
+    if (!Offset) {
+      Offset = Delta;
+    } else {
+      Offset = Calc(BO_Add, *Offset, *Delta);
+      if (!Offset)
+        return std::nullopt;
+    }
----------------
DonatNagyE wrote:

After some thinking I've chosen to use a different logic to filter out the trivial "no ElementRegion layer" case -- in the freshly uploaded revision it's signified by the value of the region pointer.

https://github.com/llvm/llvm-project/pull/67572