https://github.com/malavikasamak updated https://github.com/llvm/llvm-project/pull/118249
>From 6281b990096f058cfb6ed862631b8d6436db23f7 Mon Sep 17 00:00:00 2001 From: MalavikaSamak <malavi...@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 | 51 +++++++++---------- .../warn-unsafe-buffer-usage-array.cpp | 34 +++++++++++++ .../warn-unsafe-buffer-usage-field-attr.cpp | 1 - .../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 26 +++++----- 4 files changed, 71 insertions(+), 41 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 5f36ffa926b269..e7ba2c324247d7 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -433,37 +433,36 @@ 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) { + std::function<bool(const ArraySubscriptExpr *)> CheckBounds = + [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool { + uint64_t limit; + if (const auto *CATy = + dyn_cast<ConstantArrayType>(ASE->getBase() + ->IgnoreParenImpCasts() + ->getType() + ->getUnqualifiedDesugaredType())) { + limit = CATy->getLimitedSize(); + } else if (const auto *SLiteral = dyn_cast<StringLiteral>( + ASE->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>(ASE->getIdx())) { + const APInt ArrIdx = IdxLit->getValue(); + if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit) + return false; + if (const auto *BaseASE = dyn_cast<ArraySubscriptExpr>( + ASE->getBase()->IgnoreParenImpCasts())) { + return CheckBounds(BaseASE); + } return true; - } + } + return false; + }; - return false; + return CheckBounds(&Node); } 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..2d143a94bf86f4 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp @@ -52,3 +52,37 @@ 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@+1 {{'matrix' is an unsafe buffer that does not perform bounds checks}} +float two_dimension_array(Float4x4& matrix) { + // expected-warning@+1{{unsafe buffer access}} + float a = matrix[0][4]; + + a = matrix[0][3]; + + // expected-note@+1{{used in buffer access here}} + a = matrix[4][0]; + + 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@+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.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp index 642db0e9d3c632..3e5a0578603207 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp @@ -128,25 +128,25 @@ 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}} ); } @@ -213,7 +213,6 @@ void testTypedefs(T_ptr_t p) { // expected-warning@-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@-1{{unsafe buffer access}} p[1].b[1] // expected-note{{used in buffer access here}} // expected-warning@-1{{unsafe buffer access}} ); @@ -223,10 +222,9 @@ template<typename T, int N> T f(T t, T * pt, T a[N], T (&b)[N]) { // expected-warning@-1{{'t' is an unsafe pointer used for buffer access}} // expected-warning@-2{{'pt' is an unsafe pointer used for buffer access}} // expected-warning@-3{{'a' is an unsafe pointer used for buffer access}} - // expected-warning@-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 +374,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}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits