"Reland "[Wunsafe-buffer-usage] Fix false positive when const sized array is indexed by const evaluatable expressions (#119340)""

[malavikasamak]

This reverts commit 7dd34ba.

Fixed the assertion violation reported by 7dd34ba

[llvmbot]

@llvm/pr-subscribers-clang-analysis

Author: Malavika Samak (malavikasamak)

Changes

This reverts commit 7dd34ba.

Fixed the assertion violation reported by 7dd34ba

---

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

2 Files Affected:

• (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+7-2)
• (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp (+32)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a9aff39df64746..c064aa30e8aedc 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -453,8 +453,13 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
     return false;
   }
 
-  if (const auto *IdxLit = dyn_cast<IntegerLiteral>(Node.getIdx())) {
-    const APInt ArrIdx = IdxLit->getValue();
+  Expr::EvalResult EVResult;
+  const Expr *IndexExpr = Node.getIdx();
+  if (!IndexExpr->isValueDependent() &&
+      IndexExpr->EvaluateAsInt(EVResult, Finder->getASTContext())) {
+    llvm::APSInt ArrIdx = EVResult.Val.getInt();
+    // FIXME: ArrIdx.isNegative() we could immediately emit an error as that's a
+    // bug
     if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
       return true;
   }
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 7dd6c83dbba2a8..e80b54b7c69677 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -92,3 +92,35 @@ char access_strings() {
   c = array_string[5];
   return c;
 }
+
+struct T {
+  int array[10];
+};
+
+const int index = 1;
+
+constexpr int get_const(int x) {
+  if(x < 3)
+    return ++x;
+  else
+    return x + 5;
+};
+
+void array_indexed_const_expr(unsigned idx) {
+  // expected-note@+2 {{change type of 'arr' to 'std::array' to label it for hardening}}
+  // expected-warning@+1{{'arr' is an unsafe buffer that does not perform bounds checks}}
+  int arr[10];
+  arr[sizeof(int)] = 5;
+
+  int array[sizeof(T)];
+  array[sizeof(int)] = 5;
+  array[sizeof(T) -1 ] = 3;
+
+  int k = arr[6 & 5];
+  k = arr[2 << index];
+  k = arr[8 << index]; // expected-note {{used in buffer access here}}
+  k = arr[16 >> 1];
+  k = arr[get_const(index)];
+  k = arr[get_const(5)]; // expected-note {{used in buffer access here}}
+  k = arr[get_const(4)];
+}

[llvmbot]

@llvm/pr-subscribers-clang

Author: Malavika Samak (malavikasamak)

Changes

This reverts commit 7dd34ba.

Fixed the assertion violation reported by 7dd34ba

---

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

2 Files Affected:

• (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+7-2)
• (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp (+32)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a9aff39df64746..c064aa30e8aedc 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -453,8 +453,13 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
     return false;
   }
 
-  if (const auto *IdxLit = dyn_cast<IntegerLiteral>(Node.getIdx())) {
-    const APInt ArrIdx = IdxLit->getValue();
+  Expr::EvalResult EVResult;
+  const Expr *IndexExpr = Node.getIdx();
+  if (!IndexExpr->isValueDependent() &&
+      IndexExpr->EvaluateAsInt(EVResult, Finder->getASTContext())) {
+    llvm::APSInt ArrIdx = EVResult.Val.getInt();
+    // FIXME: ArrIdx.isNegative() we could immediately emit an error as that's a
+    // bug
     if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
       return true;
   }
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 7dd6c83dbba2a8..e80b54b7c69677 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -92,3 +92,35 @@ char access_strings() {
   c = array_string[5];
   return c;
 }
+
+struct T {
+  int array[10];
+};
+
+const int index = 1;
+
+constexpr int get_const(int x) {
+  if(x < 3)
+    return ++x;
+  else
+    return x + 5;
+};
+
+void array_indexed_const_expr(unsigned idx) {
+  // expected-note@+2 {{change type of 'arr' to 'std::array' to label it for hardening}}
+  // expected-warning@+1{{'arr' is an unsafe buffer that does not perform bounds checks}}
+  int arr[10];
+  arr[sizeof(int)] = 5;
+
+  int array[sizeof(T)];
+  array[sizeof(int)] = 5;
+  array[sizeof(T) -1 ] = 3;
+
+  int k = arr[6 & 5];
+  k = arr[2 << index];
+  k = arr[8 << index]; // expected-note {{used in buffer access here}}
+  k = arr[16 >> 1];
+  k = arr[get_const(index)];
+  k = arr[get_const(5)]; // expected-note {{used in buffer access here}}
+  k = arr[get_const(4)];
+}