[clang] [-Wunsafe-buffer-usage] Fix false warnings when const sized array is safely accessed (array [idx %const]) (PR #140113)
Malavika Samak via cfe-commits
cfe-commits at lists.llvm.org
Thu May 15 12:03:41 PDT 2025
https://github.com/malavikasamak updated https://github.com/llvm/llvm-project/pull/140113
>From c6d8a58a8468fb3e6bcbe7d5935e2aa033745d99 Mon Sep 17 00:00:00 2001
From: MalavikaSamak <malavika2 at apple.com>
Date: Thu, 15 May 2025 10:46:01 -0700
Subject: [PATCH 1/2] [-Wunsafe-buffer-usage] Fix false warnings when const
array is safely accessed array [idx %const]
The -Wunsafe-buffer-usage analysis currently warns when a const sized array is safely accessed,
with an index that is bound by the remainder operator. The false alarm is now suppressed.
Example:
int circular_buffer(int array[10], uint idx) {
return array[idx %10]; // Safe operation
}
rdar://148443453
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 17 ++++++++++++++++-
.../SemaCXX/warn-unsafe-buffer-usage-array.cpp | 10 +++++++++-
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5b72382ca9772..2f669637a2731 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -600,12 +600,27 @@ static bool isSafeArraySubscript(const ArraySubscriptExpr &Node,
} 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)
+ if (BE->getOpcode() != BO_And && BE->getOpcode() != BO_Rem)
return false;
const Expr *LHS = BE->getLHS();
const Expr *RHS = BE->getRHS();
+ if(BE->getOpcode() == BO_Rem) {
+ // If n is a negative number, then n % const can be greater than const
+ if(!LHS->getType()->isUnsignedIntegerType()) {
+ return false;
+ }
+
+ if(!RHS->isValueDependent() && RHS->EvaluateAsInt(EVResult, Ctx)) {
+ llvm::APSInt result = EVResult.Val.getInt();
+ if (result.isNonNegative() && result.getLimitedValue() <= limit)
+ return true;
+ }
+
+ return false;
+ }
+
if ((!LHS->isValueDependent() &&
LHS->EvaluateAsInt(EVResult, Ctx)) || // case: `n & e`
(!RHS->isValueDependent() &&
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 3233999ea8ea2..b2358bb9f6be3 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -35,7 +35,15 @@ 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}}
+int array[10]; // expected-warning 2{{'array' is an unsafe buffer that does not perform bounds checks}}
+
+void circular_access_safe(unsigned idx) {
+ array[idx % 10];
+}
+
+void circular_access_unsafe(int idx) {
+ array[idx % 10]; // expected-note {{used in buffer access here}}
+}
void masked_idx1(unsigned long long idx, Foo f) {
// Bitwise and operation
>From 334b062ccf7e93f93653a6ef989e36b3524efdc8 Mon Sep 17 00:00:00 2001
From: MalavikaSamak <malavika2 at apple.com>
Date: Thu, 15 May 2025 11:57:51 -0700
Subject: [PATCH 2/2] Fixing the formatting issue and added tests.
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 6 +++---
.../test/SemaCXX/warn-unsafe-buffer-usage-array.cpp | 12 +++++++++---
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 2f669637a2731..ec648e1a17af9 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -606,13 +606,13 @@ static bool isSafeArraySubscript(const ArraySubscriptExpr &Node,
const Expr *LHS = BE->getLHS();
const Expr *RHS = BE->getRHS();
- if(BE->getOpcode() == BO_Rem) {
+ if (BE->getOpcode() == BO_Rem) {
// If n is a negative number, then n % const can be greater than const
- if(!LHS->getType()->isUnsignedIntegerType()) {
+ if (!LHS->getType()->isUnsignedIntegerType()) {
return false;
}
- if(!RHS->isValueDependent() && RHS->EvaluateAsInt(EVResult, Ctx)) {
+ if (!RHS->isValueDependent() && RHS->EvaluateAsInt(EVResult, Ctx)) {
llvm::APSInt result = EVResult.Val.getInt();
if (result.isNonNegative() && result.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 b2358bb9f6be3..00daa28b433eb 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -35,13 +35,19 @@ void constant_idx_safe0(unsigned idx) {
buffer[0] = 0;
}
-int array[10]; // expected-warning 2{{'array' is an unsafe buffer that does not perform bounds checks}}
+int array[10]; // expected-warning 3{{'array' is an unsafe buffer that does not perform bounds checks}}
-void circular_access_safe(unsigned idx) {
+void circular_access_unsigned(unsigned idx) {
array[idx % 10];
+ array[idx % 11]; // expected-note {{used in buffer access here}}
+ array[(idx + 3) % 10];
+ array[(--idx) % 8];
+ array[idx & 9 % 10];
+ array[9 & idx % 11];
+ array [12 % 10];
}
-void circular_access_unsafe(int idx) {
+void circular_access_signed(int idx) {
array[idx % 10]; // expected-note {{used in buffer access here}}
}
More information about the cfe-commits
mailing list