[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: clang/include/clang/AST/Type.h:2953
   const llvm::APInt &getSize() const { return Size; }
+  SmallVector<uint64_t, 2> getAllExtents() const;
   const Expr *getSizeExpr() const {
----------------
Could this be a free function (not a member function)? That way we could put it as an implementation detail of `RegionStore.cpp` and we could spare the timely review from the AST code owners.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:444-446
+  getSValFromInitListExpr(const InitListExpr *ILE,
+                          const SmallVector<uint64_t, 2> &ConcreteOffsets,
+                          QualType ElemT);
----------------
Could you please add docs to this function? Especially to `ConcreteOffsets` param.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1641-1654
+  // Treat an n-dimensional array. Get offsets from the expression,
+  // like `arr[4][2][1];`, `SValOffsets` should be {1, 2, 4};
+  const VarRegion *VR = dyn_cast<VarRegion>(R->getSuperRegion());
+  SmallVector<SVal, 2> SValOffsets;
+  SValOffsets.push_back(R->getIndex());
+  if (const ElementRegion *ER = dyn_cast<ElementRegion>(R->getSuperRegion())) {
+    const ElementRegion *LastER = nullptr;
----------------
I think this could be implemented in it's own separate function, couldn't it?


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1656
+
+  // TODO: Add a test case for this check.
+  if (!VR)
----------------
Please address this TODO.


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1706-1743
+  // Check offsets for being out of bounds.
   // C++20 [expr.add] 7.6.6.4 (excerpt):
   //   If P points to an array element i of an array object x with n
   //   elements, where i < 0 or i > n, the behavior is undefined.
   //   Dereferencing is not allowed on the "one past the last
   //   element", when i == n.
   // Example:
----------------
Perhaps this hunk could be in an individual implementation function?


================
Comment at: clang/test/Analysis/initialization.c:76
   int x = 2, y = -2;
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(glob_arr2[x][y] == 5); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr2[x][y] == 5); // expected-warning{{UNDEFINED}}
   x = 3;
----------------
Very good!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111654/new/

https://reviews.llvm.org/D111654