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

[Malavika Samak via cfe-commits]

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

>From 81af812176768eb663a09b5ccabe3c7211119b76 Mon Sep 17 00:00:00 2001
From: MalavikaSamak <malavika2 at apple.com>
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH] [Wunsafe-buffer-usage] False positives for & expression
 indexing constant size array (arr[anything & 0])

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)
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 48 ++++++++++++++++++-
 .../warn-unsafe-buffer-usage-array.cpp        | 16 +++++++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..f80076ee90978b 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -427,6 +427,48 @@ 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<llvm::APInt(const Expr *exp, unsigned int limit)>
+      SafeMaskedAccess;
+  unsigned int RecLimit = 5;
+
+  SafeMaskedAccess = [&](const Expr *exp, unsigned int RecLimit) -> llvm::APInt {
+    llvm::APInt Max = llvm::APInt::getMaxValue(Finder->getASTContext().getIntWidth(exp->getType()));
+    Max.setAllBits();
+
+    if (RecLimit == 0)
+      return Max;
+
+    //RecLimit--;
+
+    if (const auto *IntLit = dyn_cast<IntegerLiteral>(exp)) {
+      const APInt ArrIdx = IntLit->getValue();
+      return ArrIdx;
+    }
+
+    if (const auto *BinEx = dyn_cast<BinaryOperator>(exp)) {
+      llvm::APInt LHS = SafeMaskedAccess(BinEx->getLHS()->IgnoreParenCasts(), RecLimit);
+      llvm::APInt RHS = SafeMaskedAccess(BinEx->getRHS()->IgnoreParenCasts(), RecLimit);
+      
+      switch(BinEx->getOpcode()) {
+        case BO_And:
+          LHS = LHS & RHS.getLimitedValue();
+          return LHS;
+        case BO_Or:
+          LHS = LHS | RHS.getLimitedValue();
+          return LHS;
+        case BO_Shl:
+          LHS = LHS << RHS.getLimitedValue();
+          return LHS;
+        case BO_Xor:
+          LHS = LHS ^ RHS.getLimitedValue();
+          return LHS;
+        default:
+          return Max;
+      }
+    }
+
+    return Max;
+  };
 
   const auto *BaseDRE =
       dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts());
@@ -446,6 +488,10 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
     if (ArrIdx.isNonNegative() &&
         ArrIdx.getLimitedValue() < CATy->getLimitedSize())
       return true;
+  } else if (const auto *BinEx = dyn_cast<BinaryOperator>(Node.getIdx())) {
+    APInt result = SafeMaskedAccess(Node.getIdx(), RecLimit);
+    if (result.isNonNegative() && result.getLimitedValue() < CATy->getLimitedSize())
+      return true;
   }
 
   return false;
@@ -1152,7 +1198,7 @@ class ArraySubscriptGadget : public WarningGadget {
     Handler.handleUnsafeOperation(ASE, IsRelatedToDecl, Ctx);
   }
   SourceLocation getSourceLoc() const override { return ASE->getBeginLoc(); }
-
+  
   DeclUseList getClaimedVarUseSites() const override {
     if (const auto *DRE =
             dyn_cast<DeclRefExpr>(ASE->getBase()->IgnoreParenImpCasts())) {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..2b3f1388c3f28b 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -33,6 +33,22 @@ 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(unsigned long long idx) {
+  array2[6 & 5]; // no warning
+  array2[6 & idx & (idx + 1) & 5]; // no warning
+  array2[6 & idx & 5 & (idx + 1) | 4]; 
+}
+
 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}}