[clang] [-Wunsafe-buffer-usage] Suppress warning when 2-D constant arrays are indexed (PR #118249)

Malavika Samak via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 1 20:58:12 PST 2024


https://github.com/malavikasamak created https://github.com/llvm/llvm-project/pull/118249

Do not warn about unsafe buffer access, when 2-D constant arrays are accessed and the indices are within the bounds of the buffer. Warning in such cases is a false postive. Such a suppression aleady exists for 1-d arrays and it is now extended to 2-D arrays.

(rdar://137926311)

>From 1b21a2eab2fabbb9bdc2e540e48a38aab90fd028 Mon Sep 17 00:00:00 2001
From: MalavikaSamak <malavika2 at apple.com>
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning when 2-D constant
 arrays are indexed

Do not warn about unsafe buffer access, when 2-D constant arrays are accessed
and the indices are within the bounds of the buffer. Warning in such cases is
a false postive. Such a suppression aleady exists for 1-d arrays and it is now
extended to 2-D arrays.

(rdar://137926311)
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 64 ++++++++++++++-----
 .../warn-unsafe-buffer-usage-array.cpp        |  6 ++
 ...n-unsafe-buffer-usage-fixits-parm-main.cpp |  3 +-
 ...e-buffer-usage-fixits-parm-unsupported.cpp |  2 +-
 .../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 33 ++++------
 5 files changed, 68 insertions(+), 40 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..f1e3b28fc03249 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,20 +433,56 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
   //    already duplicated
   //  - call both from Sema and from here
 
-  const auto *BaseDRE =
+  auto CheckBounds = [](const ArraySubscriptExpr *ASE, uint64_t limit) -> bool {
+    if (const auto *IdxLit = dyn_cast<IntegerLiteral>(ASE->getIdx())) {
+      const APInt ArrIdx = IdxLit->getValue();
+      if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
+        return true;
+    }
+    return false;
+  };
+
+  const auto *BaseASE =
+      dyn_cast<ArraySubscriptExpr>(Node.getBase()->IgnoreParenImpCasts());
+  uint64_t size = 0;
+
+  if (BaseASE) {
+    const auto *DRE =
+        dyn_cast<DeclRefExpr>(BaseASE->getBase()->IgnoreParenImpCasts());
+
+    if (!DRE)
+      return false;
+
+    const auto *CATy = dyn_cast<ConstantArrayType>(
+        DRE->getType()->getUnqualifiedDesugaredType());
+    if (!CATy) {
+      return false;
+    }
+    size = CATy->getLimitedSize();
+
+    if (!CheckBounds(BaseASE, size))
+      return false;
+
+    CATy = Finder->getASTContext().getAsConstantArrayType(BaseASE->getType());
+    if (!CATy) {
+      return false;
+    }
+
+    size = CATy->getLimitedSize();
+    return CheckBounds(&Node, size);
+  }
+
+  const DeclRefExpr *BaseDRE =
       dyn_cast<DeclRefExpr>(Node.getBase()->IgnoreParenImpCasts());
   const auto *SLiteral =
       dyn_cast<StringLiteral>(Node.getBase()->IgnoreParenImpCasts());
-  uint64_t size;
 
   if (!BaseDRE && !SLiteral)
     return false;
 
   if (BaseDRE) {
-    if (!BaseDRE->getDecl())
-      return false;
-    const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-        BaseDRE->getDecl()->getType());
+    const auto *CATy =
+        Finder->getASTContext().getAsConstantArrayType(BaseDRE->getType());
     if (!CATy) {
       return false;
     }
@@ -455,15 +491,7 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
     size = SLiteral->getLength() + 1;
   }
 
-  if (const auto *IdxLit = dyn_cast<IntegerLiteral>(Node.getIdx())) {
-    const APInt ArrIdx = IdxLit->getValue();
-    // FIXME: ArrIdx.isNegative() we could immediately emit an error as that's a
-    // bug
-    if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
-      return true;
-  }
-
-  return false;
+  return CheckBounds(&Node, size);
 }
 
 AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
@@ -1172,6 +1200,12 @@ class ArraySubscriptGadget : public WarningGadget {
     if (const auto *DRE =
             dyn_cast<DeclRefExpr>(ASE->getBase()->IgnoreParenImpCasts())) {
       return {DRE};
+    } else if (const auto *BaseASE = dyn_cast<ArraySubscriptExpr>(
+                   ASE->getBase()->IgnoreParenImpCasts())) {
+      if (const auto *DRE = dyn_cast<DeclRefExpr>(
+              BaseASE->getBase()->IgnoreParenImpCasts())) {
+        return {DRE};
+      }
     }
 
     return {};
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..41cdc122b7d2fa 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,9 @@ void constant_id_string(unsigned idx) {
   unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}} 
   unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
 }
+
+typedef float Float4x4[4][4];
+
+float multi_dimension_array(Float4x4& matrix) {
+  return matrix[1][1];
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp
index c6b095031e0e32..d1fd1c00a9ea34 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp
@@ -8,6 +8,5 @@
 // main function
 int main(int argc, char *argv[]) { // expected-warning{{'argv' is an unsafe pointer used for buffer access}}
   char tmp;
-  tmp = argv[5][5];                // expected-note{{used in buffer access here}} \
-				      expected-warning{{unsafe buffer access}}
+  tmp = argv[5][5];                // expected-note2{{used in buffer access here}}
 }
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp
index 71350098613d19..5b22a5c5f8acfb 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp
@@ -118,7 +118,7 @@ void isArrayDecayToPointerUPC(int a[][10], int (*b)[10]) {
 // expected-warning at -2{{'b' is an unsafe pointer used for buffer access}}
   int tmp;
 
-  tmp = a[5][5] + b[5][5];  // expected-warning2{{unsafe buffer access}}  expected-note2{{used in buffer access here}}
+  tmp = a[5][5] + b[5][5];  // expected-note4{{used in buffer access here}}
 }
 
 // parameter having default values:
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
index 642db0e9d3c632..5d75aa66a0781f 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -40,12 +40,9 @@ void testArraySubscripts(int idx, int *p, int **pp) {
 // expected-warning at -1{{'p' is an unsafe pointer used for buffer access}}
 // expected-warning at -2{{'pp' is an unsafe pointer used for buffer access}}
   foo(p[1],             // expected-note{{used in buffer access here}}
-      pp[1][1],         // expected-note{{used in buffer access here}}
-                        // expected-warning at -1{{unsafe buffer access}}
-      1[1[pp]],         // expected-note{{used in buffer access here}}
-                        // expected-warning at -1{{unsafe buffer access}}
-      1[pp][1]          // expected-note{{used in buffer access here}}
-                        // expected-warning at -1{{unsafe buffer access}}
+      pp[1][1],         // expected-note2{{used in buffer access here}}
+      1[1[pp]],         // expected-note2{{used in buffer access here}}
+      1[pp][1]          // expected-note2{{used in buffer access here}}
       );
 
   if (p[3]) {           // expected-note{{used in buffer access here}}
@@ -65,13 +62,9 @@ void testArraySubscripts(int idx, int *p, int **pp) {
     int b[10][10];      // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}}
 
   foo(a[idx], idx[a],   // expected-note2{{used in buffer access here}}
-      b[idx][idx + 1],  // expected-warning{{unsafe buffer access}}
-                        // expected-note at -1{{used in buffer access here}}
-      (idx + 1)[b][idx],// expected-warning{{unsafe buffer access}}
-                        // expected-note at -1{{used in buffer access here}}
-      (idx + 1)[idx[b]]);
-                        // expected-warning at -1{{unsafe buffer access}}
-                        // expected-note at -2{{used in buffer access here}}
+      b[idx][idx + 1],  // expected-note2{{used in buffer access here}}
+      (idx + 1)[b][idx],// expected-note2{{used in buffer access here}}
+      (idx + 1)[idx[b]]); // expected-note2{{used in buffer access here}}
 
   // Not to warn when index is zero
   foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0],
@@ -108,8 +101,7 @@ void testQualifiedParameters(const int * p, const int * const q, const int a[10]
   foo(p[1], 1[p], p[-1],    // expected-note3{{used in buffer access here}}
       q[1], 1[q], q[-1],    // expected-note3{{used in buffer access here}}
       a[1],                 // expected-note{{used in buffer access here}}     `a` is of pointer type
-      b[1][2]               // expected-note{{used in buffer access here}}     `b[1]` is of array type
-                            // expected-warning at -1{{unsafe buffer access}}
+      b[1][2]               // expected-note2{{used in buffer access here}}     `b[1]` is of array type
       );
 }
 
@@ -223,10 +215,9 @@ template<typename T, int N> T f(T t, T * pt, T a[N], T (&b)[N]) {
   // expected-warning at -1{{'t' is an unsafe pointer used for buffer access}}
   // expected-warning at -2{{'pt' is an unsafe pointer used for buffer access}}
   // expected-warning at -3{{'a' is an unsafe pointer used for buffer access}}
-  // expected-warning at -4{{'b' is an unsafe buffer that does not perform bounds checks}}
   foo(pt[1],    // expected-note{{used in buffer access here}}
       a[1],     // expected-note{{used in buffer access here}}
-      b[1]);    // expected-note{{used in buffer access here}}
+      b[1]);    
   return &t[1]; // expected-note{{used in buffer access here}}
 }
 
@@ -366,17 +357,15 @@ int testArrayAccesses(int n, int idx) {
     // expected-warning at -1{{'cArr' is an unsafe buffer that does not perform bounds checks}}
     int d = cArr[0][0];
     foo(cArr[0][0]);
-    foo(cArr[idx][idx + 1]);        // expected-note{{used in buffer access here}}
-                                    // expected-warning at -1{{unsafe buffer access}}
-    auto cPtr = cArr[idx][idx * 2]; // expected-note{{used in buffer access here}}
-                                    // expected-warning at -1{{unsafe buffer access}}
+    foo(cArr[idx][idx + 1]);        // expected-note2{{used in buffer access here}}
+    auto cPtr = cArr[idx][idx * 2]; // expected-note2{{used in buffer access here}}
     foo(cPtr);
 
     // Typdefs
     typedef int A[3];
     const A tArr = {4, 5, 6};
     foo(tArr[0], tArr[1]);
-    return cArr[0][1];      // expected-warning{{unsafe buffer access}}
+    return cArr[0][1];      
 }
 
 void testArrayPtrArithmetic(int x[]) { // expected-warning{{'x' is an unsafe pointer used for buffer access}}



More information about the cfe-commits mailing list