-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[-Wunsafe-buffer-usage] Fixits for array decayed to pointer #80347
[-Wunsafe-buffer-usage] Fixits for array decayed to pointer #80347
Conversation
41b1c38
to
ff51c1a
Compare
7ff3a18
to
2d0f4cb
Compare
✅ With the latest revision this PR passed the C/C++ code formatter. |
b7471ab
to
2d0f4cb
Compare
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.
2d0f4cb
to
39a2d21
Compare
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: None (jkorous-apple) Changesdepends on Full diff: https://github.com/llvm/llvm-project/pull/80347.diff 2 Files Affected:
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..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
@@ -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"
@@ -177,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;
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
(We probably do not need to use CHECK-DAG
at most places in our tests since we have fixed the non-deterministic issue, but it's irrelevant to this PR.)
…m#80347) 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. (cherry picked from commit e06f352)
depends on
#80084