[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)

Malavika Samak via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 6 21:35:09 PST 2024


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

>From e1eadf464509ab74ff4f097f92e7140f43c61262 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 for
 multi-dimensional constant arrays

Do not warn about unsafe buffer access, when multi-dimensional constant arrays
are accessed and their indices are within the bounds of the buffer. Warning in
such cases would be a false positive. Such a suppression already exists for 1-d
arrays and it is now extended to multi-dimensional arrays.

(rdar://137926311)
(rdar://140320139)
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 44 +++++++------------
 .../warn-unsafe-buffer-usage-array.cpp        | 40 +++++++++++++++++
 .../warn-unsafe-buffer-usage-field-attr.cpp   |  1 -
 ...e-buffer-usage-fixits-parm-unsupported.cpp |  2 +-
 .../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 39 +++++++++-------
 5 files changed, 82 insertions(+), 44 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..c3f580ec42eba3 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,37 +433,27 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
   //    already duplicated
   //  - call both from Sema and from here
 
-  const auto *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());
-    if (!CATy) {
+    uint64_t limit;
+    if (const auto *CATy =
+            dyn_cast<ConstantArrayType>(Node.getBase()
+                                            ->IgnoreParenImpCasts()
+                                            ->getType()
+                                            ->getUnqualifiedDesugaredType())) {
+      limit = CATy->getLimitedSize();
+    } else if (const auto *SLiteral = dyn_cast<StringLiteral>(
+                   Node.getBase()->IgnoreParenImpCasts())) {
+      limit = SLiteral->getLength() + 1;
+    } else {
       return false;
     }
-    size = CATy->getLimitedSize();
-  } else if (SLiteral) {
-    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)
+    if (const auto *IdxLit = dyn_cast<IntegerLiteral>(Node.getIdx())) {
+      const APInt ArrIdx = IdxLit->getValue();
+      if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+        return false;
       return true;
-  }
-
-  return false;
+    }
+    return false;
 }
 
 AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..7dd6c83dbba2a8 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,43 @@ 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];
+
+// expected-warning at +1 {{'matrix' is an unsafe buffer that does not perform bounds checks}}
+float two_dimension_array(Float4x4& matrix, unsigned idx) {
+  // expected-warning at +1{{unsafe buffer access}}
+  float a = matrix[0][4];
+
+  a = matrix[0][3];
+
+  // expected-note at +1{{used in buffer access here}}
+  a = matrix[4][0];
+
+  a = matrix[idx][0]; // expected-note{{used in buffer access here}}
+
+  a = matrix[0][idx]; //expected-warning{{unsafe buffer access}}
+
+  a = matrix[idx][idx]; //expected-warning{{unsafe buffer access}} // expected-note{{used in buffer access here}}
+
+  return matrix[1][1];
+}
+
+typedef float Float2x3x4[2][3][4];
+float multi_dimension_array(Float2x3x4& matrix) {
+  float *f = matrix[0][2];
+  return matrix[1][2][3];
+}
+
+char array_strings[][11] = {
+  "Apple", "Banana", "Cherry", "Date", "Elderberry"
+};
+
+char array_string[] = "123456";
+
+char access_strings() {
+  char c = array_strings[0][4];
+  c = array_strings[3][10];
+  c = array_string[5];
+  return c;
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
index 0ba605475925b9..1636c948da075a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
@@ -96,7 +96,6 @@ void test_attribute_multiple_fields (D d) {
 
    int v = d.buf[0]; //expected-warning{{field 'buf' prone to unsafe buffer manipulation}}
 
-   //expected-warning at +1{{unsafe buffer access}}
    v = d.buf[5]; //expected-warning{{field 'buf' prone to unsafe buffer manipulation}}
 }
 
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..0c80da63f82912 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-note2{{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..41d38ada48788c 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -109,7 +109,6 @@ void testQualifiedParameters(const int * p, const int * const q, const int a[10]
       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}}
       );
 }
 
@@ -128,29 +127,41 @@ T_t funRetT();
 T_t * funRetTStar();
 
 void testStructMembers(struct T * sp, struct T s, T_t * sp2, T_t s2) {
-  foo(sp->a[1],     // expected-warning{{unsafe buffer access}}
+  foo(sp->a[1],
       sp->b[1],     // expected-warning{{unsafe buffer access}}
-      sp->c.a[1],   // expected-warning{{unsafe buffer access}}
+      sp->c.a[1],
       sp->c.b[1],   // expected-warning{{unsafe buffer access}}
-      s.a[1],       // expected-warning{{unsafe buffer access}}
+      s.a[1],
       s.b[1],       // expected-warning{{unsafe buffer access}}
-      s.c.a[1],     // expected-warning{{unsafe buffer access}}
+      s.c.a[1],
       s.c.b[1],     // expected-warning{{unsafe buffer access}}
-      sp2->a[1],    // expected-warning{{unsafe buffer access}}
+      sp2->a[1],
       sp2->b[1],    // expected-warning{{unsafe buffer access}}
-      sp2->c.a[1],  // expected-warning{{unsafe buffer access}}
+      sp2->c.a[1],
       sp2->c.b[1],  // expected-warning{{unsafe buffer access}}
-      s2.a[1],      // expected-warning{{unsafe buffer access}}
+      s2.a[1],
       s2.b[1],      // expected-warning{{unsafe buffer access}}
-      s2.c.a[1],           // expected-warning{{unsafe buffer access}}
+      s2.c.a[1],
       s2.c.b[1],           // expected-warning{{unsafe buffer access}}
-      funRetT().a[1],      // expected-warning{{unsafe buffer access}}
+      funRetT().a[1],
       funRetT().b[1],      // expected-warning{{unsafe buffer access}}
-      funRetTStar()->a[1], // expected-warning{{unsafe buffer access}}
+      funRetTStar()->a[1],
       funRetTStar()->b[1]  // expected-warning{{unsafe buffer access}}
       );
 }
 
+union Foo {
+   bool b;
+   int arr[10];
+};
+
+int testUnionMembers(Foo f) {
+  int a = f.arr[0];
+  a = f.arr[5];
+  a = f.arr[10]; // expected-warning{{unsafe buffer access}}
+  return a;
+}
+
 int garray[10];     // expected-warning{{'garray' is an unsafe buffer that does not perform bounds checks}}
 int * gp = garray;  // expected-warning{{'gp' is an unsafe pointer used for buffer access}}
 int gvar = gp[1];   // FIXME: file scope unsafe buffer access is not warned
@@ -213,7 +224,6 @@ void testTypedefs(T_ptr_t p) {
   // expected-warning at -1{{'p' is an unsafe pointer used for buffer access}}
   foo(p[1],       // expected-note{{used in buffer access here}}
       p[1].a[1],  // expected-note{{used in buffer access here}}
-                  // expected-warning at -1{{unsafe buffer access}}
       p[1].b[1]   // expected-note{{used in buffer access here}}
                   // expected-warning at -1{{unsafe buffer access}}
       );
@@ -223,10 +233,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}}
 }
 
@@ -376,7 +385,7 @@ int testArrayAccesses(int n, int idx) {
     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