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

[via cfe-commits]

Author: Malavika Samak
Date: 2025-02-25T00:31:52+05:30
New Revision: ab9cd53b86e84cc2db47d312232de4789c15adc4

URL: https://github.com/llvm/llvm-project/commit/ab9cd53b86e84cc2db47d312232de4789c15adc4
DIFF: https://github.com/llvm/llvm-project/commit/ab9cd53b86e84cc2db47d312232de4789c15adc4.diff

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

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)

---------

Co-authored-by: MalavikaSamak <malavika2 at apple.com>

Added: 
    

Modified: 
    clang/lib/Analysis/UnsafeBufferUsage.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index c51398698922b..ff4f940a596e3 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -462,6 +462,25 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
     // bug
     if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
       return true;
+  } else if (const auto *BE = dyn_cast<BinaryOperator>(IndexExpr)) {
+    // For an integer expression `e` and an integer constant `n`, `e & n` and
+    // `n & e` are bounded by `n`:
+    if (BE->getOpcode() != BO_And)
+      return false;
+
+    const Expr *LHS = BE->getLHS();
+    const Expr *RHS = BE->getRHS();
+
+    if ((!LHS->isValueDependent() &&
+         LHS->EvaluateAsInt(EVResult,
+                            Finder->getASTContext())) || // case: `n & e`
+        (!RHS->isValueDependent() &&
+         RHS->EvaluateAsInt(EVResult, Finder->getASTContext()))) { // `e & n`
+      llvm::APSInt result = EVResult.Val.getInt();
+      if (result.isNonNegative() && result.getLimitedValue() < limit)
+        return true;
+    }
+    return false;
   }
   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 9bfc31bd07b0e..3233999ea8ea2 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -18,7 +18,9 @@ void foo2(unsigned idx) {
 
 struct Foo {
   int member_buffer[10];
+  int x;
 };
+
 void foo2(Foo& f, unsigned idx) {
   f.member_buffer[idx] = 0; // expected-warning{{unsafe buffer access}}
 }
@@ -33,6 +35,37 @@ 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_idx1(unsigned long long idx, Foo f) {
+  // Bitwise and operation
+  array[idx & 5] = 10; // no-warning
+  array[5 &idx] = 12; // no-warning
+  array[idx & 11 & 5] = 3; // no warning
+  array[idx & 11] = 20; // expected-note{{used in buffer access here}}
+  array[idx &=5]; // expected-note{{used in buffer access here}}
+  array[f.x & 5]; // no-warning
+  array[5 & f.x]; // no-warning
+  array[f.x & (-5)]; // expected-note{{used in buffer access here}}
+}
+
+typedef unsigned long long uint64_t;
+typedef unsigned int uint32_t;
+typedef unsigned char uint8_t;
+
+void type_conversions(uint64_t idx1, uint32_t idx2, uint8_t idx3) {
+  array[(uint32_t)idx1 & 3];
+  array[idx2 & 3];
+  array[idx3 & 3];
+}
+
+int array2[5]; // expected-warning {{'array2' is an unsafe buffer that does not perform bounds checks}}
+
+void masked_idx_safe(unsigned long long idx) {
+  array2[6 & 5]; // no warning
+  array2[6 & idx & (idx + 1) & 5]; // expected-note{{used in buffer access here}}
+}
+
 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}}