[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-analysis

Author: Malavika Samak (malavikasamak)

<details>
<summary>Changes</summary>

Do not warn when a constant sized array is indexed with an expression that contains bitwise and operation
involving constants and it always results in a bound safe access.

(rdar://136684050)

---
Full diff: https://github.com/llvm/llvm-project/pull/112284.diff


2 Files Affected:

- (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+45) 
- (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp (+14) 


``````````diff
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..cdfdcc17536391 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -427,6 +427,45 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
   //    -  e. g. "Try harder to find a NamedDecl to point at in the note."
   //    already duplicated
   //  - call both from Sema and from here
+  std::function<bool(const Expr *exp, const ConstantArrayType *CATy,
+                     unsigned int limit)>
+      SafeMaskedAccess;
+  unsigned int RecLimit = 5;
+  llvm::APInt Max;
+  bool Initialized = false;
+
+  SafeMaskedAccess = [&](const Expr *exp, const ConstantArrayType *CATy,
+                         unsigned int RecLimit) -> bool {
+    if (RecLimit == 0)
+      return false;
+
+    RecLimit--;
+
+    if (const auto *IntLit = dyn_cast<IntegerLiteral>(exp)) {
+      const APInt ArrIdx = IntLit->getValue();
+      if (ArrIdx.isNonNegative() &&
+          ArrIdx.getLimitedValue() < CATy->getLimitedSize())
+        return true;
+      if (!Initialized) {
+        Max = ArrIdx;
+        Initialized = true;
+      } else {
+        Max = Max & ArrIdx.getLimitedValue();
+      }
+      if (Max.getLimitedValue() < CATy->getLimitedSize())
+        return true;
+    }
+
+    if (const auto *BinEx = dyn_cast<BinaryOperator>(exp)) {
+      if (SafeMaskedAccess(BinEx->getLHS()->IgnoreParenCasts(), CATy, RecLimit))
+        return true;
+      else if (SafeMaskedAccess(BinEx->getRHS()->IgnoreParenCasts(), CATy,
+                                RecLimit))
+        return true;
+    }
+
+    return false;
+  };
 
   const auto *BaseDRE =
       dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts());
@@ -446,6 +485,12 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
     if (ArrIdx.isNonNegative() &&
         ArrIdx.getLimitedValue() < CATy->getLimitedSize())
       return true;
+  } else if (const auto *BinEx = dyn_cast<BinaryOperator>(Node.getIdx())) {
+    if (BinEx->getOpcode() != BO_And)
+      return false;
+
+    Max.setAllBits();
+    return SafeMaskedAccess(Node.getIdx(), CATy, RecLimit);
   }
 
   return false;
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..e5a5ca57f66883 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -33,6 +33,20 @@ void constant_idx_safe0(unsigned idx) {
   buffer[0] = 0;
 }
 
+int array[10]; // expected-warning{{'array' is an unsafe buffer that does not perform bounds checks}}
+
+void masked_idx(unsigned long long idx) {
+  array[idx & 5] = 10; // no warning
+  array[idx & 11 & 5] = 3; // no warning
+  array[idx & 11] = 20; // expected-note{{used in buffer access here}} 
+}
+
+int array2[5];
+
+void mased_idx_false() {
+  array2[6 & 5];
+}
+
 void constant_idx_unsafe(unsigned idx) {
   int buffer[10];       // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds checks}}
                         // expected-note at -1{{change type of 'buffer' to 'std::array' to label it for hardening}}

``````````

</details>


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