[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added inline comments.


================
Comment at: clang/include/clang/AST/Expr.h:4961
+  /// - `nullptr` for invalid index (`i < 0` or `i >= array_size`).
+  const Expr *getExprForConstArrayByRawIndex(int64_t Idx) const;
+
----------------
I think in most (all?) other methods in this class, array indices are unsigned in the API.  If the array index itself comes from an expression that is negative (i.e., a literal negative integer, or an constant expression that evaluates to a negative number), that has to be handled correctly, but I'm not sure this is the right place to do it.  As this code stands, if an integer literal used used, which is greater than LONG_MAX, but less than ULONG_MAX, it will be end up being treated as invalid in this method, won't it?


================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1671
           if (auto CI = R->getIndex().getAs<nonloc::ConcreteInt>()) {
             int64_t i = CI->getValue().getSExtValue();
+            const Expr *E = InitList->getExprForConstArrayByRawIndex(i);
----------------
I see where you got the int64_t from -- that's what getSExtValue() returns.  So, if the literal const index value in the expr is greater than LONG_MAX (but less than ULONG_MAX, of course), this would assert.  That seems undesirable....


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104285