llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Malavika Samak (malavikasamak) <details> <summary>Changes</summary> Do not warn about unsafe buffer access, when 2-D constant arrays are accessed and the indices are within the bounds of the buffer. Warning in such cases is a false postive. Such a suppression aleady exists for 1-d arrays and it is now extended to 2-D arrays. (rdar://137926311) --- Full diff: https://github.com/llvm/llvm-project/pull/118249.diff 5 Files Affected: - (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+49-15) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp (+6) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp (+1-2) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp (+1-1) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp (+11-22) ``````````diff diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 5f36ffa926b269..f1e3b28fc03249 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -433,20 +433,56 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) { // already duplicated // - call both from Sema and from here - const auto *BaseDRE = + auto CheckBounds = [](const ArraySubscriptExpr *ASE, uint64_t limit) -> bool { + if (const auto *IdxLit = dyn_cast<IntegerLiteral>(ASE->getIdx())) { + const APInt ArrIdx = IdxLit->getValue(); + if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit) + return true; + } + return false; + }; + + const auto *BaseASE = + dyn_cast<ArraySubscriptExpr>(Node.getBase()->IgnoreParenImpCasts()); + uint64_t size = 0; + + if (BaseASE) { + const auto *DRE = + dyn_cast<DeclRefExpr>(BaseASE->getBase()->IgnoreParenImpCasts()); + + if (!DRE) + return false; + + const auto *CATy = dyn_cast<ConstantArrayType>( + DRE->getType()->getUnqualifiedDesugaredType()); + if (!CATy) { + return false; + } + size = CATy->getLimitedSize(); + + if (!CheckBounds(BaseASE, size)) + return false; + + CATy = Finder->getASTContext().getAsConstantArrayType(BaseASE->getType()); + if (!CATy) { + return false; + } + + size = CATy->getLimitedSize(); + return CheckBounds(&Node, size); + } + + const DeclRefExpr *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()); + const auto *CATy = + Finder->getASTContext().getAsConstantArrayType(BaseDRE->getType()); if (!CATy) { return false; } @@ -455,15 +491,7 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) { 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) - return true; - } - - return false; + return CheckBounds(&Node, size); } AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) { @@ -1172,6 +1200,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-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..5d75aa66a0781f 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 ); } @@ -223,10 +215,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 +357,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}} `````````` </details> https://github.com/llvm/llvm-project/pull/118249 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
