Skip to content

Commit

Permalink
[-Wunsafe-buffer-usage] Emit fixits for array decayed to pointer (#80347
Browse files Browse the repository at this point in the history
)

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.
  • Loading branch information
jkorous-apple committed Feb 13, 2024
1 parent 137bd78 commit e06f352
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 2 deletions.
2 changes: 1 addition & 1 deletion clang/lib/Analysis/UnsafeBufferUsage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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;
}

0 comments on commit e06f352

Please sign in to comment.