https://github.com/malavikasamak updated https://github.com/llvm/llvm-project/pull/140113
>From c6d8a58a8468fb3e6bcbe7d5935e2aa033745d99 Mon Sep 17 00:00:00 2001 From: MalavikaSamak <malavi...@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 <malavi...@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}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits