[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:13966
   if (index.isUnsigned() || !index.isNegative()) {
-    // It is possible that the type of the base expression after
-    // IgnoreParenCasts is incomplete, even though the type of the base
-    // expression before IgnoreParenCasts is complete (see PR39746 for an
-    // example). In this case we have no information about whether the array
-    // access exceeds the array bounds. However we can still diagnose an array
-    // access which precedes the array bounds.
-    if (BaseType->isIncompleteType())
-      return;
+    if (isUnboundedArray) {
+      const auto &ASTC = getASTContext();
----------------
It might simplify the patch to move this condition out of the tree and just early return for the other case. That is:

```
if (isUnboundedArray) {
  if (!(index.isUnsigned() || !index.isNegative()))
    return;

  ...
  return;
}

if (index.isUnsigned() ...
```


================
Comment at: clang/lib/Sema/SemaChecking.cpp:13989
+      MaxElems <<= AddrBits;
+      MaxElems /= ElemBytes;
+
----------------
The size calculations here could probably be simplified by doing something like this:

* If getActiveBits of the index is greater than AddrBits, it's indexing outside
* Construct an AddrBits-wide APInt containing the index value
* Use umul_ovf with getTypeSizeInChars(ElementType); if that overflows, it's indexing outside



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86796