https://github.com/jkorous-apple updated https://github.com/llvm/llvm-project/pull/80358
>From 245ea0a82fdd1b24a5c7175fc90379a86df77fe8 Mon Sep 17 00:00:00 2001 From: Jan Korous <jkor...@apple.com> Date: Wed, 24 Jan 2024 17:35:47 -0800 Subject: [PATCH 1/5] [-Wunsafe-buffer-usage] Emit fixits for array used as a pointer Covers cases where DeclRefExpr referring to a const-size array decays to a pointer and is used "as a pointer" (e. g. passed to a pointer type parameter). Since std::array<T, N> doesn't implicitly convert to pointer to its element type T* the cast needs to be done explicitly as part of the fixit when we retrofit std::array to code that previously worked with constant size array. std::array::data() method is used for the explicit cast. In terms of the fixit machine this covers the UPC(DRE) case for Array fixit strategy. The emitted fixit inserts call to std::array::data() method similarly to analogous fixit for Span strategy. --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +- ...afe-buffer-usage-fixits-pointer-access.cpp | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 3c2a6fd81b1d8f..d00c598c4b9de3 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1876,6 +1876,7 @@ std::optional<FixItList> UPCStandalonePointerGadget::getFixits(const FixitStrategy &S) const { const auto VD = cast<VarDecl>(Node->getDecl()); switch (S.lookup(VD)) { + case FixitStrategy::Kind::Array: case FixitStrategy::Kind::Span: { ASTContext &Ctx = VD->getASTContext(); SourceManager &SM = Ctx.getSourceManager(); @@ -1890,7 +1891,6 @@ UPCStandalonePointerGadget::getFixits(const FixitStrategy &S) const { } case FixitStrategy::Kind::Wontfix: case FixitStrategy::Kind::Iterator: - case FixitStrategy::Kind::Array: return std::nullopt; case FixitStrategy::Kind::Vector: llvm_unreachable("unsupported strategies for FixableGadgets"); diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp index ca19702c7ec300..f8caff5b1a3ef7 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp @@ -83,12 +83,27 @@ void unsafe_method_invocation_single_param() { } +void unsafe_method_invocation_single_param_array() { + int p[32]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array<int, 32> p" + + int tmp = p[5]; + foo(p); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:".data()" +} + void safe_method_invocation_single_param() { int* p = new int[10]; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:{{.*}}-[[@LINE-1]]:{{.*}}} foo(p); } +void safe_method_invocation_single_param_array() { + int p[10]; + foo(p); + // CHECK-NO: fix-it:"{{.*}}":{[[@LINE-1]]:{{.*}}-[[@LINE-1]]:{{.*}}}:".data()" +} + void unsafe_method_invocation_double_param() { int* p = new int[10]; // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p" @@ -111,6 +126,20 @@ void unsafe_method_invocation_double_param() { m1(q, q, 8); } +void unsafe_method_invocation_double_param_array() { + int p[14]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array<int, 14> p" + + int q[40]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array<int, 40> q" + + q[5] = p[5]; + + m1(p, p, 10); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:".data()" +} + void unsafe_access_in_lamda() { int* p = new int[10]; // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span<int> p" >From 39a2d21e874756aa914e1461abff93bfda6c8e66 Mon Sep 17 00:00:00 2001 From: Jan Korous <jkor...@apple.com> Date: Tue, 6 Feb 2024 14:51:17 -0800 Subject: [PATCH 2/5] [-Wunsafe-buffer-usage][NFC] Test more fixits of array decayed to pointer --- ...afe-buffer-usage-fixits-pointer-access.cpp | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp index f8caff5b1a3ef7..f94072015ff87d 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp @@ -206,4 +206,23 @@ void fixits_in_lambda_capture_rename() { }; p[5] = 10; -} +} + +bool ptr_comparison(int* ptr, unsigned idx) { + int arr[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:14}:"std::array<int, 10> arr" + arr[idx] = idx; + + return arr > ptr; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:".data()" +} + +int long long ptr_distance(int* ptr, unsigned idx) { + int arr[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:14}:"std::array<int, 10> arr" + arr[idx] = idx; + + int long long dist = arr - ptr; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:27-[[@LINE-1]]:27}:".data()" + return dist; +} >From ecb4aa163c992f00001b2fc8284e40de2dc02157 Mon Sep 17 00:00:00 2001 From: Jan Korous <jkor...@apple.com> Date: Thu, 25 Jan 2024 13:52:12 -0800 Subject: [PATCH 3/5] [-Wunsafe-buffer-usage] Emit fixits for arguments of function pointers calls Currently we ignore calls on function pointers (unlike direct calls of functions and class methods). This patch adds support for function pointers as well. The change is to simply replace use of forEachArgumentWithParam matcher in UPC gadget with forEachArgumentWithParamType. from the documentation of forEachArgumentWithParamType: /// Matches all arguments and their respective types for a \c CallExpr or /// \c CXXConstructExpr. It is very similar to \c forEachArgumentWithParam but /// it works on calls through function pointers as well. --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 4 ++-- ...fer-usage-fixits-pointer-arg-to-func-ptr-call.cpp | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index d00c598c4b9de3..c5a87f14bc8880 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -282,8 +282,8 @@ isInUnspecifiedPointerContext(internal::Matcher<Stmt> InnerMatcher) { // (i.e., computing the distance between two pointers); or ... auto CallArgMatcher = - callExpr(forEachArgumentWithParam(InnerMatcher, - hasPointerType() /* array also decays to pointer type*/), + callExpr(forEachArgumentWithParamType(InnerMatcher, + isAnyPointer() /* array also decays to pointer type*/), unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage))))); auto CastOperandMatcher = diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp new file mode 100644 index 00000000000000..ae761e46a98191 --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \ +// RUN: -fsafe-buffer-usage-suggestions \ +// RUN: -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +void unsafe_array_func_ptr_call(void (*fn_ptr)(int *param)) { + int p[32]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array<int, 32> p" + + int tmp = p[5]; + fn_ptr(p); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()" +} >From e42e0844fba979cd854b6014e2ca8f0803c35938 Mon Sep 17 00:00:00 2001 From: Jan Korous <jkor...@apple.com> Date: Thu, 1 Feb 2024 14:44:01 -0800 Subject: [PATCH 4/5] [-Wunsafe-buffer-usage][NFC] Add tests for function pointer call fixits --- ...ge-fixits-pointer-arg-to-func-ptr-call.cpp | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp index ae761e46a98191..0459d6549fd86f 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-arg-to-func-ptr-call.cpp @@ -6,7 +6,43 @@ void unsafe_array_func_ptr_call(void (*fn_ptr)(int *param)) { int p[32]; // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::array<int, 32> p" - int tmp = p[5]; + p[5] = 10; fn_ptr(p); // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()" } + +void unsafe_ptr_func_ptr_call(void (*fn_ptr)(int *param)) { + int *p; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span<int> p" + + p[5] = 10; + fn_ptr(p); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:11}:".data()" +} + +void addr_of_unsafe_ptr_func_ptr_call(void (*fn_ptr)(int *param)) { + int *p; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span<int> p" + + p[5] = 10; + fn_ptr(&p[0]); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:15}:"p.data()" +} + +void addr_of_unsafe_ptr_w_offset_func_ptr_call(void (*fn_ptr)(int *param)) { + int *p; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span<int> p" + + p[5] = 10; + fn_ptr(&p[3]); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:15}:"&p.data()[3]" +} + +void preincrement_unsafe_ptr_func_ptr_call(void (*fn_ptr)(int *param)) { + int *p; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:9}:"std::span<int> p" + + p[5] = 10; + fn_ptr(++p); + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:13}:"(p = p.subspan(1)).data()" +} >From 6ef8c72a29d54ca01e23830058daca6e726eb736 Mon Sep 17 00:00:00 2001 From: Jan Korous <jkor...@apple.com> Date: Tue, 13 Feb 2024 14:24:45 -0800 Subject: [PATCH 5/5] [-Wunsafe-buffer-usage][NFC] Format AST matcher in isInUnspecifiedPointerContext ...and turn off clang-format for the block of AST matchers. --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index c5a87f14bc8880..c07517190bbcec 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -281,10 +281,14 @@ isInUnspecifiedPointerContext(internal::Matcher<Stmt> InnerMatcher) { // 4. the operand of a pointer subtraction operation // (i.e., computing the distance between two pointers); or ... + // clang-format off auto CallArgMatcher = - callExpr(forEachArgumentWithParamType(InnerMatcher, - isAnyPointer() /* array also decays to pointer type*/), - unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage))))); + callExpr( + forEachArgumentWithParam( + InnerMatcher, + hasPointerType() /* array also decays to pointer type*/), + unless(callee( + functionDecl(hasAttr(attr::UnsafeBufferUsage))))); auto CastOperandMatcher = castExpr(anyOf(hasCastKind(CastKind::CK_PointerToIntegral), @@ -306,6 +310,7 @@ isInUnspecifiedPointerContext(internal::Matcher<Stmt> InnerMatcher) { hasRHS(hasPointerType())), eachOf(hasLHS(InnerMatcher), hasRHS(InnerMatcher))); + // clang-format on return stmt(anyOf(CallArgMatcher, CastOperandMatcher, CompOperandMatcher, PtrSubtractionMatcher)); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits