[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
Malavika Samak via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 24 09:43:59 PST 2025
https://github.com/malavikasamak updated https://github.com/llvm/llvm-project/pull/112284
>From bf1f0b88c26cef3fd7eab40341558e1742c62321 Mon Sep 17 00:00:00 2001
From: MalavikaSamak <malavika2 at apple.com>
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH 1/2] [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 | 21 +++++++++++--
.../warn-unsafe-buffer-usage-array.cpp | 31 +++++++++++++++++++
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index c064aa30e8aed..3f2e66c03ebd7 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -462,8 +462,25 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
// bug
if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
return true;
- }
- return false;
+ } else if (const auto *BE = dyn_cast<BinaryOperator>(IndexExpr)) {
+ if (BE->getOpcode() != BO_And)
+ return false;
+
+ const Expr *LHS = BE->getLHS();
+ const Expr *RHS = BE->getRHS();
+
+ if ((!LHS->isValueDependent() &&
+ LHS->EvaluateAsInt(EVResult, Finder->getASTContext())) ||
+ (!RHS->isValueDependent() &&
+ RHS->EvaluateAsInt(EVResult, Finder->getASTContext()))) {
+ llvm::APSInt result = EVResult.Val.getInt();
+ if (result.isNonNegative() && result.getLimitedValue() < limit)
+ return true;
+ }
+ return false;
+
+ } else
+ 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 e80b54b7c6967..c1f9bf4c05418 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -18,7 +18,9 @@ void foo2(unsigned idx) {
struct Foo {
int member_buffer[10];
+ int x;
};
+
void foo2(Foo& f, unsigned idx) {
f.member_buffer[idx] = 0; // expected-warning{{unsafe buffer access}}
}
@@ -33,6 +35,35 @@ 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_idx1(unsigned long long idx, Foo f) {
+ // Bitwise and operation
+ array[idx & 5] = 10; // no warning
+ array[idx & 11 & 5] = 3; // no warning
+ array[idx & 11] = 20; // expected-note{{used in buffer access here}}
+ array[idx &=5]; // expected-note{{used in buffer access here}}
+ array[f.x & 5];
+}
+
+typedef unsigned long long uint64_t;
+typedef unsigned int uint32_t;
+typedef unsigned char uint8_t;
+
+void type_conversions(uint64_t idx1, uint32_t idx2, uint8_t idx3) {
+ array[(uint32_t)idx1 & 3];
+ array[idx2 & 3];
+ array[idx3 & 3];
+}
+
+int array2[5]; // expected-warning {{'array2' is an unsafe buffer that does not perform bounds checks}}
+
+void masked_idx_safe(unsigned long long idx) {
+ array2[6 & 5]; // no warning
+ array2[6 & idx & (idx + 1) & 5]; // expected-note{{used in buffer access here}}
+}
+
+
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}}
>From da96efb8092ab6f3213c6f42e9b25ed9f4600afa Mon Sep 17 00:00:00 2001
From: MalavikaSamak <malavika2 at apple.com>
Date: Fri, 21 Feb 2025 13:07:01 -0800
Subject: [PATCH 2/2] Add more tests and a comment explaining the idea behind
the change.
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 12 +++++++-----
.../test/SemaCXX/warn-unsafe-buffer-usage-array.cpp | 8 +++++---
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 3f2e66c03ebd7..5652f4bab2c83 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -463,6 +463,8 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
return true;
} 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)
return false;
@@ -470,17 +472,17 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
const Expr *RHS = BE->getRHS();
if ((!LHS->isValueDependent() &&
- LHS->EvaluateAsInt(EVResult, Finder->getASTContext())) ||
+ LHS->EvaluateAsInt(EVResult,
+ Finder->getASTContext())) || // case: `n & e`
(!RHS->isValueDependent() &&
- RHS->EvaluateAsInt(EVResult, Finder->getASTContext()))) {
+ RHS->EvaluateAsInt(EVResult, Finder->getASTContext()))) { // `e & n`
llvm::APSInt result = EVResult.Val.getInt();
if (result.isNonNegative() && result.getLimitedValue() < limit)
return true;
}
return false;
-
- } else
- 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 c1f9bf4c05418..b340393270f7a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -39,11 +39,14 @@ int array[10]; // expected-warning {{'array' is an unsafe buffer that does not p
void masked_idx1(unsigned long long idx, Foo f) {
// Bitwise and operation
- array[idx & 5] = 10; // no warning
+ array[idx & 5] = 10; // no-warning
+ array[5 &idx] = 12; // no-warning
array[idx & 11 & 5] = 3; // no warning
array[idx & 11] = 20; // expected-note{{used in buffer access here}}
array[idx &=5]; // expected-note{{used in buffer access here}}
- array[f.x & 5];
+ array[f.x & 5]; // no-warning
+ array[5 & f.x]; // no-warning
+ array[f.x & (-5)]; // expected-note{{used in buffer access here}}
}
typedef unsigned long long uint64_t;
@@ -63,7 +66,6 @@ void masked_idx_safe(unsigned long long idx) {
array2[6 & idx & (idx + 1) & 5]; // expected-note{{used in buffer access here}}
}
-
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}}
More information about the cfe-commits
mailing list