https://github.com/malavikasamak updated https://github.com/llvm/llvm-project/pull/118249
>From a3bc09e9ad3e12a2041eb41671872781638b7aa9 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) --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 58 ++++++++++--------- .../warn-unsafe-buffer-usage-array.cpp | 6 ++ .../warn-unsafe-buffer-usage-field-attr.cpp | 1 - ...n-unsafe-buffer-usage-fixits-parm-main.cpp | 3 +- ...e-buffer-usage-fixits-parm-unsupported.cpp | 2 +- .../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 54 +++++++---------- 6 files changed, 60 insertions(+), 64 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 5f36ffa926b269..d3aab0ccd589d6 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -433,37 +433,35 @@ 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) { @@ -1172,6 +1170,12 @@ class ArraySubscriptGadget : public WarningGadget { if (const auto *DRE = dyn_cast<DeclRefExpr>(ASE->getBase()->IgnoreParenImpCasts())) { return {DRE}; + } else if (const auto *BaseASE = dyn_cast<ArraySubscriptExpr>( + ASE->getBase()->IgnoreParenImpCasts())) { + if (const auto *DRE = dyn_cast<DeclRefExpr>( + BaseASE->getBase()->IgnoreParenImpCasts())) { + return {DRE}; + } } return {}; diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp index c6c93a27e4b969..41cdc122b7d2fa 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp @@ -52,3 +52,9 @@ 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]; + +float multi_dimension_array(Float4x4& matrix) { + return matrix[1][1]; +} 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-fixits-parm-main.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp index c6b095031e0e32..d1fd1c00a9ea34 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp @@ -8,6 +8,5 @@ // main function int main(int argc, char *argv[]) { // expected-warning{{'argv' is an unsafe pointer used for buffer access}} char tmp; - tmp = argv[5][5]; // expected-note{{used in buffer access here}} \ - expected-warning{{unsafe buffer access}} + tmp = argv[5][5]; // expected-note2{{used in buffer access here}} } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp index 71350098613d19..5b22a5c5f8acfb 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp @@ -118,7 +118,7 @@ void isArrayDecayToPointerUPC(int a[][10], int (*b)[10]) { // expected-warning@-2{{'b' is an unsafe pointer used for buffer access}} int tmp; - tmp = a[5][5] + b[5][5]; // expected-warning2{{unsafe buffer access}} expected-note2{{used in buffer access here}} + tmp = a[5][5] + b[5][5]; // expected-note4{{used in buffer access here}} } // parameter having default values: diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp index 642db0e9d3c632..d2bdd3762a65ad 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp @@ -40,12 +40,9 @@ void testArraySubscripts(int idx, int *p, int **pp) { // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}} // expected-warning@-2{{'pp' is an unsafe pointer used for buffer access}} foo(p[1], // expected-note{{used in buffer access here}} - pp[1][1], // expected-note{{used in buffer access here}} - // expected-warning@-1{{unsafe buffer access}} - 1[1[pp]], // expected-note{{used in buffer access here}} - // expected-warning@-1{{unsafe buffer access}} - 1[pp][1] // expected-note{{used in buffer access here}} - // expected-warning@-1{{unsafe buffer access}} + pp[1][1], // expected-note2{{used in buffer access here}} + 1[1[pp]], // expected-note2{{used in buffer access here}} + 1[pp][1] // expected-note2{{used in buffer access here}} ); if (p[3]) { // expected-note{{used in buffer access here}} @@ -65,13 +62,9 @@ void testArraySubscripts(int idx, int *p, int **pp) { int b[10][10]; // expected-warning{{'b' is an unsafe buffer that does not perform bounds checks}} foo(a[idx], idx[a], // expected-note2{{used in buffer access here}} - b[idx][idx + 1], // expected-warning{{unsafe buffer access}} - // expected-note@-1{{used in buffer access here}} - (idx + 1)[b][idx],// expected-warning{{unsafe buffer access}} - // expected-note@-1{{used in buffer access here}} - (idx + 1)[idx[b]]); - // expected-warning@-1{{unsafe buffer access}} - // expected-note@-2{{used in buffer access here}} + b[idx][idx + 1], // expected-note2{{used in buffer access here}} + (idx + 1)[b][idx],// expected-note2{{used in buffer access here}} + (idx + 1)[idx[b]]); // expected-note2{{used in buffer access here}} // Not to warn when index is zero foo(p[0], pp[0][0], 0[0[pp]], 0[pp][0], @@ -108,8 +101,7 @@ void testQualifiedParameters(const int * p, const int * const q, const int a[10] foo(p[1], 1[p], p[-1], // expected-note3{{used in buffer access here}} q[1], 1[q], q[-1], // expected-note3{{used in buffer access here}} a[1], // expected-note{{used in buffer access here}} `a` is of pointer type - b[1][2] // expected-note{{used in buffer access here}} `b[1]` is of array type - // expected-warning@-1{{unsafe buffer access}} + b[1][2] // expected-note2{{used in buffer access here}} `b[1]` is of array type ); } @@ -128,25 +120,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 +205,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 +214,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}} } @@ -366,17 +356,15 @@ int testArrayAccesses(int n, int idx) { // expected-warning@-1{{'cArr' is an unsafe buffer that does not perform bounds checks}} int d = cArr[0][0]; foo(cArr[0][0]); - foo(cArr[idx][idx + 1]); // expected-note{{used in buffer access here}} - // expected-warning@-1{{unsafe buffer access}} - auto cPtr = cArr[idx][idx * 2]; // expected-note{{used in buffer access here}} - // expected-warning@-1{{unsafe buffer access}} + foo(cArr[idx][idx + 1]); // expected-note2{{used in buffer access here}} + auto cPtr = cArr[idx][idx * 2]; // expected-note2{{used in buffer access here}} foo(cPtr); // Typdefs 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