ziqingluo-90 updated this revision to Diff 547422.
ziqingluo-90 marked an inline comment as done.
ziqingluo-90 added a comment.
Address comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153059/new/
https://reviews.llvm.org/D153059
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -36,7 +36,7 @@
void * voidPtrCall(void);
char * charPtrCall(void);
-void testArraySubscripts(int *p, int **pp) { // expected-note{{change type of 'pp' to 'std::span' to preserve bounds information}}
+void testArraySubscripts(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}}
@@ -109,7 +109,6 @@
sizeof(decltype(p[1]))); // no-warning
}
-// expected-note@+1{{change type of 'a' to 'std::span' to preserve bounds information}}
void testQualifiedParameters(const int * p, const int * const q, const int a[10], const int b[10][10]) {
// expected-warning@-1{{'p' is an unsafe pointer used for buffer access}}
// expected-warning@-2{{'q' is an unsafe pointer used for buffer access}}
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-span.cpp
@@ -29,8 +29,7 @@
int tmp;
tmp = p[5] + q[5];
}
-// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void twoParms(int *p, int * q) {return twoParms(std::span<int>(p, <# size #>), q);}\n"
-// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:2-[[@LINE-2]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void twoParms(int *p, int * q) {return twoParms(p, std::span<int>(q, <# size #>));}\n"
+// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void twoParms(int *p, int * q) {return twoParms(std::span<int>(p, <# size #>), std::span<int>(q, <# size #>));}\n"
void ptrToConst(const int * x) {
// CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:30}:"std::span<int const> x"
@@ -100,22 +99,30 @@
// namned
} NAMED_S;
+
// FIXME: `decltype(ANON_S)` represents an unnamed type but it can
// be referred as "`decltype(ANON_S)`", so the analysis should
// fix-it.
- void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,
- decltype(NAMED_S) ** rr) {
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:26-[[@LINE-2]]:41}:"std::span<decltype(C)> p"
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:65-[[@LINE-3]]:86}:"std::span<decltype(NAMED_S)> r"
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:26-[[@LINE-3]]:49}:"std::span<decltype(NAMED_S) *> rr"
+ // As parameter `q` cannot be fixed, fixes to parameters are all being given up.
+ void decltypeSpecifierAnon(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,
+ decltype(NAMED_S) ** rr) {
+ // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
if (++p) {}
if (++q) {}
if (++r) {}
if (++rr) {}
}
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(std::span<decltype(C)>(p, <# size #>), q, r, rr);}\n
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:4-[[@LINE-2]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(p, q, std::span<decltype(NAMED_S)>(r, <# size #>), rr);}\n"
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:4-[[@LINE-3]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifier(decltype(C) * p, decltype(ANON_S) * q, decltype(NAMED_S) * r,\n{{.*}}decltype(NAMED_S) ** rr) {return decltypeSpecifier(p, q, r, std::span<decltype(NAMED_S) *>(rr, <# size #>));}\n"
+ // CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+
+ void decltypeSpecifier(decltype(C) * p, decltype(NAMED_S) * r, decltype(NAMED_S) ** rr) {
+ // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:26-[[@LINE-1]]:41}:"std::span<decltype(C)> p"
+ // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:43-[[@LINE-2]]:64}:"std::span<decltype(NAMED_S)> r"
+ // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-3]]:66-[[@LINE-3]]:89}:"std::span<decltype(NAMED_S) *> rr"
+ if (++p) {}
+ if (++r) {}
+ if (++rr) {}
+ }
+ // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void decltypeSpecifier(decltype(C) * p, decltype(NAMED_S) * r, decltype(NAMED_S) ** rr) {return decltypeSpecifier(std::span<decltype(C)>(p, <# size #>), std::span<decltype(NAMED_S)>(r, <# size #>), std::span<decltype(NAMED_S) *>(rr, <# size #>));}\n
#define MACRO_TYPE(T) long T
@@ -125,8 +132,7 @@
int tmp = p[5];
tmp = q[5];
}
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(std::span<unsigned MACRO_TYPE(int)>(p, <# size #>), q);}\n"
- // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-2]]:4-[[@LINE-2]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(p, std::span<unsigned MACRO_TYPE(long)>(q, <# size #>));}\n"
+ // CHECK-DAG: fix-it:{{.*}}:{[[@LINE-1]]:4-[[@LINE-1]]:4}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void macroType(unsigned MACRO_TYPE(int) * p, unsigned MACRO_TYPE(long) * q) {return macroType(std::span<unsigned MACRO_TYPE(int)>(p, <# size #>), std::span<unsigned MACRO_TYPE(long)>(q, <# size #>));}\n"
}
// The followings test various declarators:
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp
@@ -0,0 +1,317 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fblocks -fsafe-buffer-usage-suggestions -verify %s
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fblocks -fsafe-buffer-usage-suggestions \
+// RUN: %s 2>&1 | FileCheck %s
+
+// FIXME: what about possible diagnostic message non-determinism?
+
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]:
+void parmsNoFix(int *p, int *q) {
+ int * a = new int[10];
+ int * b = new int[10]; //expected-warning{{'b' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'b' to 'std::span' to preserve bounds information}}
+
+ a = p;
+ a = q;
+ b[5] = 5; // expected-note{{used in buffer access here}}
+}
+
+// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:21-[[@LINE+2]]:27}:"std::span<int> p"
+// CHECK: fix-it:{{.*}}:{[[@LINE+14]]:2-[[@LINE+14]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void parmsSingleton(int *p) {return parmsSingleton(std::span<int>(p, <# size #>));}\n"
+void parmsSingleton(int *p) { //expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'p' to 'std::span' to preserve bounds information}}
+ // CHECK: fix-it:{{.*}}:{[[@LINE+3]]:3-[[@LINE+3]]:12}:"std::span<int> a"
+ // CHECK: fix-it:{{.*}}:{[[@LINE+2]]:13-[[@LINE+2]]:13}:"{"
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:24-[[@LINE+1]]:24}:", 10}"
+ int * a = new int[10];
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> b"
+ int * b; //expected-warning{{'b' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'b' to 'std::span' to preserve bounds information, and change 'a' to 'std::span' to propagate bounds information between them}}
+
+ b = a;
+ b[5] = 5; // expected-note{{used in buffer access here}}
+ p[5] = 5; // expected-note{{used in buffer access here}}
+}
+
+
+// Parameters other than `r` all will be fixed
+// CHECK: fix-it:{{.*}}:{[[@LINE+15]]:24-[[@LINE+15]]:30}:"std::span<int> p"
+// CHECK fix-it:{{.*}}:{[[@LINE+14]]:32-[[@LINE+14]]:39}:"std::span<int *> q"
+// CHECK: fix-it:{{.*}}:{[[@LINE+13]]:41-[[@LINE+13]]:48}:"std::span<int> a"
+// CHECK: fix-it:{{.*}}:{[[@LINE+23]]:2-[[@LINE+23]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void * multiParmAllFix(int *p, int **q, int a[], int * r) {return multiParmAllFix(std::span<int>(p, <# size #>), std::span<int *>(q, <# size #>), std::span<int>(a, <# size #>), r);}\n"
+
+// repeat 2 more times as each of the 3 fixing parameters generates the set of fix-its above.
+
+// CHECK: fix-it:{{.*}}:{[[@LINE+8]]:24-[[@LINE+8]]:30}:"std::span<int> p"
+// CHECK fix-it:{{.*}}:{[[@LINE+7]]:32-[[@LINE+7]]:39}:"std::span<int *> q"
+// CHECK: fix-it:{{.*}}:{[[@LINE+6]]:41-[[@LINE+6]]:48}:"std::span<int> a"
+// CHECK: fix-it:{{.*}}:{[[@LINE+16]]:2-[[@LINE+16]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void * multiParmAllFix(int *p, int **q, int a[], int * r) {return multiParmAllFix(std::span<int>(p, <# size #>), std::span<int *>(q, <# size #>), std::span<int>(a, <# size #>), r);}\n"
+// CHECK: fix-it:{{.*}}:{[[@LINE+4]]:24-[[@LINE+4]]:30}:"std::span<int> p"
+// CHECK fix-it:{{.*}}:{[[@LINE+3]]:32-[[@LINE+3]]:39}:"std::span<int *> q"
+// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:41-[[@LINE+2]]:48}:"std::span<int> a"
+// CHECK: fix-it:{{.*}}:{[[@LINE+12]]:2-[[@LINE+12]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void * multiParmAllFix(int *p, int **q, int a[], int * r) {return multiParmAllFix(std::span<int>(p, <# size #>), std::span<int *>(q, <# size #>), std::span<int>(a, <# size #>), r);}\n"
+void * multiParmAllFix(int *p, int **q, int a[], int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'a' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' and 'a' to safe types to make function 'multiParmAllFix' bounds-safe}} \
+ expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p' and 'a' to safe types to make function 'multiParmAllFix' bounds-safe}} \
+ expected-note{{change type of 'a' to 'std::span' to preserve bounds information, and change 'p' and 'q' to safe types to make function 'multiParmAllFix' bounds-safe}}
+ int tmp;
+
+ tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = a[5]; // expected-note{{used in buffer access here}}
+ if (++q) {} // expected-note{{used in pointer arithmetic here}}
+ return r;
+}
+
+void * multiParmAllFix(int *p, int **q, int a[], int * r);
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} "
+// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:58-[[@LINE-2]]:58}:";\nvoid * multiParmAllFix(std::span<int> p, std::span<int *> q, std::span<int> a, int * r)"
+
+// Fixing local variables implicates fixing parameters
+void multiParmLocalAllFix(int *p, int * r) {
+ // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:28-[[@LINE-1]]:34}:"std::span<int> p"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:36-[[@LINE-2]]:43}:"std::span<int> r"
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> x"
+ int * x; // expected-warning{{'x' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'x' to 'std::span' to preserve bounds information, and change 'p', 'z', and 'r' to safe types to make function 'multiParmLocalAllFix' bounds-safe}}
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> z"
+ int * z; // expected-warning{{'z' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'z' to 'std::span' to preserve bounds information, and change 'x', 'p', and 'r' to safe types to make function 'multiParmLocalAllFix' bounds-safe}}
+ int * y;
+
+ x = p;
+ y = x;
+ // `x` needs to be fixed so does the pointer assigned to `x`, i.e.,`p`
+ x[5] = 5; // expected-note{{used in buffer access here}}
+ z = r;
+ // `z` needs to be fixed so does the pointer assigned to `z`, i.e.,`r`
+ z[5] = 5; // expected-note{{used in buffer access here}}
+ // Since `p` and `r` are parameters need to be fixed together,
+ // fixing `x` involves fixing all `p`, `r` and `z`. Similar for
+ // fixing `z`.
+}
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void multiParmLocalAllFix(int *p, int * r) {return multiParmLocalAllFix(std::span<int>(p, <# size #>), std::span<int>(r, <# size #>));}\n"
+
+
+// Fixing parameters implicates fixing local variables
+// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:29-[[@LINE+2]]:35}:"std::span<int> p"
+// CHECK: fix-it:{{.*}}:{[[@LINE+1]]:37-[[@LINE+1]]:44}:"std::span<int> r"
+void multiParmLocalAllFix2(int *p, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'x', 'r', and 'z' to safe types to make function 'multiParmLocalAllFix2' bounds-safe}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'x', and 'z' to safe types to make function 'multiParmLocalAllFix2' bounds-safe}}
+ int * x = new int[10];
+ // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> x"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+ int * z = new int[10];
+ // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> z"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+ int * y;
+
+ p = x;
+ y = x;
+ p[5] = 5; // expected-note{{used in buffer access here}}
+ r = z;
+ r[5] = 5; // expected-note{{used in buffer access here}}
+}
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void multiParmLocalAllFix2(int *p, int * r) {return multiParmLocalAllFix2(std::span<int>(p, <# size #>), std::span<int>(r, <# size #>));}\n"
+
+
+// No fix emitted for any of the parameter since parameter `r` cannot be fixed
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]
+void noneParmFix(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ r++; // expected-note{{used in pointer arithmetic here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+}
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+
+// To show what if the `r` in `noneParmFix` can be fixed:
+void noneParmFix_control(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q' and 'r' to safe types to make function 'noneParmFix_control' bounds-safe}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p' and 'r' to safe types to make function 'noneParmFix_control' bounds-safe}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p' and 'q' to safe types to make function 'noneParmFix_control' bounds-safe}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ if (++r) {} // expected-note{{used in pointer arithmetic here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+}
+
+
+// No fix emitted for any of the parameter since local variable `l` cannot be fixed.
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]
+void noneParmOrLocalFix(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+ // `l` and `r` must be fixed together while all parameters must be fixed together as well:
+ int * l; l = r; // expected-warning{{'l' is an unsafe pointer used for buffer access}}
+
+ l++; // expected-note{{used in pointer arithmetic here}}
+}
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+
+// To show what if the `l` can be fixed in `noneParmOrLocalFix`:
+void noneParmOrLocalFix_control(int * p, int * q, int * r) {// \
+ expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q', 'r', and 'l' to safe types to make function 'noneParmOrLocalFix_control' bounds-safe}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p', 'r', and 'l' to safe types to make function 'noneParmOrLocalFix_control' bounds-safe}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'q', and 'l' to safe types to make function 'noneParmOrLocalFix_control' bounds-safe}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+ int * l; // expected-warning{{'l' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'l' to 'std::span' to preserve bounds information, and change 'p', 'q', and 'r' to safe types to make function 'noneParmOrLocalFix_control' bounds-safe}}
+ l = r;
+ if (++l){}; // expected-note{{used in pointer arithmetic here}}
+}
+
+
+// No fix emitted for any of the parameter since local variable `l` cannot be fixed.
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]
+void noneParmOrLocalFix2(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+
+ int * a; a = r;
+ int * b; b = a;
+ int * l; l = b; // expected-warning{{'l' is an unsafe pointer used for buffer access}}
+
+ // `a, b, l` and parameters must be fixed together but `l` can't be fixed:
+ l++; // expected-note{{used in pointer arithmetic here}}
+}
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+
+// To show what if the `l` can be fixed in `noneParmOrLocalFix2`:
+void noneParmOrLocalFix2_control(int * p, int * q, int * r) { // \
+ expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'q', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix2_control' bounds-safe}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix2_control' bounds-safe}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'q', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix2_control' bounds-safe}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+
+ int * a; a = r;
+ int * b; b = a;
+ int * l; // expected-warning{{'l' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'l' to 'std::span' to preserve bounds information, and change 'p', 'q', 'r', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix2_control' bounds-safe}}
+
+ l = b;
+ if(++l){} // expected-note{{used in pointer arithmetic here}}
+}
+
+// No fix emitted for any of the parameter since local variable `l` cannot be fixed
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]
+void noneParmOrLocalFix3(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+
+ int * a; a = r;
+ int * b; b = a;
+ int * l; l = b; // expected-warning{{'l' is an unsafe pointer used for buffer access}}
+
+ l++; // expected-note{{used in pointer arithmetic here}}
+
+ int * x; x = p; // expected-warning{{'x' is an unsafe pointer used for buffer access}}
+ tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+
+void noneParmOrLocalFix3_control(int * p, int * q, int * r) { // \
+ expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'p' to 'std::span' to preserve bounds information, and change 'x', 'q', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'q' to 'std::span' to preserve bounds information, and change 'p', 'x', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'r' to 'std::span' to preserve bounds information, and change 'p', 'x', 'q', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+
+ int * a; a = r;
+ int * b; b = a;
+ int * l; // expected-warning{{'l' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'l' to 'std::span' to preserve bounds information, and change 'p', 'x', 'q', 'r', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}}
+
+ l = b;
+ if (++l){}; // expected-note{{used in pointer arithmetic here}}
+
+ int * x; // expected-warning{{'x' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'x' to 'std::span' to preserve bounds information, and change 'p', 'q', 'r', 'l', 'b', and 'a' to safe types to make function 'noneParmOrLocalFix3_control' bounds-safe}}
+ x = p;
+ tmp = x[5]; // expected-note{{used in buffer access here}}
+}
+
+
+// No fix emitted for any of the parameter but some local variables will get fixed
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE+1]]
+void noneParmSomeLocalFix(int * p, int * q, int * r) { // expected-warning{{'p' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'q' is an unsafe pointer used for buffer access}} \
+ expected-warning{{'r' is an unsafe pointer used for buffer access}}
+ int tmp = p[5]; // expected-note{{used in buffer access here}}
+ tmp = q[5]; // expected-note{{used in buffer access here}}
+ tmp = r[5]; // expected-note{{used in buffer access here}}
+
+ int * a; a = r;
+ int * b; b = a;
+ int * l; l = b; // expected-warning{{'l' is an unsafe pointer used for buffer access}}
+
+ l++; // expected-note{{used in pointer arithmetic here}}
+
+ //`x` and `y` can be fixed
+ int * x = new int[10];
+ // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:12}:"std::span<int> x"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-2]]:13-[[@LINE-2]]:13}:"{"
+ // CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:", 10}"
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> y"
+ int * y; // expected-warning{{'y' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'y' to 'std::span' to preserve bounds information, and change 'x' to 'std::span' to propagate bounds information between them}}
+ y = x;
+ tmp = y[5]; // expected-note{{used in buffer access here}}
+}
+// CHECK-NOT: fix-it:{{.*}}:{[[@LINE-1]]
+
+// Test that other parameters of (lambdas and blocks) do not interfere
+// the grouping of variables of the function.
+// CHECK: fix-it:{{.*}}:{[[@LINE+3]]:30-[[@LINE+3]]:37}:"std::span<int> p"
+// CHECK: fix-it:{{.*}}:{[[@LINE+2]]:39-[[@LINE+2]]:46}:"std::span<int> q"
+// CHECK: fix-it:{{.*}}:{[[@LINE+20]]:2-[[@LINE+20]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void parmsFromLambdaAndBlock(int * p, int * q) {return parmsFromLambdaAndBlock(std::span<int>(p, <# size #>), std::span<int>(q, <# size #>));}\n"
+void parmsFromLambdaAndBlock(int * p, int * q) {
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> a"
+ int * a; // expected-warning{{'a' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'a' to 'std::span' to preserve bounds information, and change 'p', 'b', and 'q' to safe types to make function 'parmsFromLambdaAndBlock' bounds-safe}}
+ // CHECK: fix-it:{{.*}}:{[[@LINE+1]]:3-[[@LINE+1]]:10}:"std::span<int> b"
+ int * b; // expected-warning{{'b' is an unsafe pointer used for buffer access}} \
+ expected-note{{change type of 'b' to 'std::span' to preserve bounds information, and change 'a', 'p', and 'q' to safe types to make function 'parmsFromLambdaAndBlock' bounds-safe}}
+ auto Lamb = [](int * x) -> void { // expected-warning{{'x' is an unsafe pointer used for buffer access}}
+ x[5] = 5; // expected-note{{used in buffer access here}}
+ };
+
+ void (^Blk)(int*) = ^(int * y) { // expected-warning{{'y' is an unsafe pointer used for buffer access}}
+ y[5] = 5; // expected-note{{used in buffer access here}}
+ };
+
+ a = p;
+ b = q;
+ a[5] = 5; // expected-note{{used in buffer access here}}
+ b[5] = 5; // expected-note{{used in buffer access here}}
+}
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2255,21 +2255,30 @@
void handleUnsafeVariableGroup(const VarDecl *Variable,
const VariableGroupsManager &VarGrpMgr,
- FixItList &&Fixes) override {
+ FixItList &&Fixes, const Decl *D) override {
assert(!SuggestSuggestions &&
"Unsafe buffer usage fixits displayed without suggestions!");
S.Diag(Variable->getLocation(), diag::warn_unsafe_buffer_variable)
<< Variable << (Variable->getType()->isPointerType() ? 0 : 1)
<< Variable->getSourceRange();
if (!Fixes.empty()) {
- const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Variable);
+ assert(isa<NamedDecl>(D) &&
+ "Fix-its are generated only for `NamedDecl`s");
+ const NamedDecl *ND = cast<NamedDecl>(D);
+ bool BriefMsg = false;
+ // If the variable group involves parameters, the diagnostic message will
+ // NOT explain how the variables are grouped as the reason is non-trivial
+ // and irrelavant to users' experience:
+ const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Variable, &BriefMsg);
unsigned FixItStrategy = 0; // For now we only have 'std::span' strategy
- const auto &FD = S.Diag(Variable->getLocation(),
- diag::note_unsafe_buffer_variable_fixit_group);
+ const auto &FD =
+ S.Diag(Variable->getLocation(),
+ BriefMsg ? diag::note_unsafe_buffer_variable_fixit_together
+ : diag::note_unsafe_buffer_variable_fixit_group);
FD << Variable << FixItStrategy;
FD << listVariableGroupAsString(Variable, VarGroupForVD)
- << (VarGroupForVD.size() > 1);
+ << (VarGroupForVD.size() > 1) << ND->getNameAsString();
for (const auto &F : Fixes) {
FD << F;
}
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1383,6 +1383,22 @@
return std::nullopt;
}
+// Returns the `SourceRange` of `D`. The reason why this function exists is
+// that `D->getSourceRange()` may return a range where the end location is the
+// starting location of the last token. The end location of the source range
+// returned by this function is the last location of the last token.
+static SourceRange getSourceRangeToTokenEnd(const Decl *D,
+ const SourceManager &SM,
+ LangOptions LangOpts) {
+ SourceLocation Begin = D->getBeginLoc();
+ SourceLocation
+ End = // `D->getEndLoc` should always return the starting location of the
+ // last token, so we should get the end of the token
+ Lexer::getLocForEndOfToken(D->getEndLoc(), 0, SM, LangOpts);
+
+ return SourceRange(Begin, End);
+}
+
// Returns the text of the pointee type of `T` from a `VarDecl` of a pointer
// type. The text is obtained through from `TypeLoc`s. Since `TypeLoc` does not
// have source ranges of qualifiers ( The `QualifiedTypeLoc` looks hacky too me
@@ -1452,6 +1468,21 @@
return getRangeText(NameRange, SM, LangOpts);
}
+// Returns the text representing a `std::span` type where the element type is
+// represented by `EltTyText`.
+//
+// Note the optional parameter `Qualifiers`: one needs to pass qualifiers
+// explicitly if the element type needs to be qualified.
+static std::string
+getSpanTypeText(StringRef EltTyText,
+ std::optional<Qualifiers> Quals = std::nullopt) {
+ const char *const SpanOpen = "std::span<";
+
+ if (Quals)
+ return SpanOpen + EltTyText.str() + ' ' + Quals->getAsString() + '>';
+ return SpanOpen + EltTyText.str() + '>';
+}
+
std::optional<FixItList>
DerefSimplePtrArithFixableGadget::getFixits(const Strategy &s) const {
const VarDecl *VD = dyn_cast<VarDecl>(BaseDeclRefExpr->getDecl());
@@ -1787,7 +1818,7 @@
SmallString<32> Replacement;
raw_svector_ostream OS(Replacement);
- OS << "std::span<" << SpanEltT.getAsString() << "> " << D->getName();
+ OS << getSpanTypeText(SpanEltT.getAsString()) << ' ' << D->getName();
if (!ReplacementLastLoc) {
DEBUG_NOTE_DECL_FAIL(D, " : failed to get end char loc (macro)");
@@ -1803,11 +1834,11 @@
return !FD->getDeclContext()->lookup(FD->getDeclName()).isSingleResult();
}
-// For a `FunDecl`, one of whose `ParmVarDecl`s is being changed to have a new
-// type, this function produces fix-its to make the change self-contained. Let
-// 'F' be the entity defined by the original `FunDecl` and "NewF" be the entity
-// defined by the `FunDecl` after the change to the parameter. Fix-its produced
-// by this function are
+// For a `FunctionDecl`, whose `ParmVarDecl`s are being changed to have new
+// types, this function produces fix-its to make the change self-contained. Let
+// 'F' be the entity defined by the original `FunctionDecl` and "NewF" be the
+// entity defined by the `FunctionDecl` after the change to the parameters.
+// Fix-its produced by this function are
// 1. Add the `[[clang::unsafe_buffer_usage]]` attribute to each declaration
// of 'F';
// 2. Create a declaration of "NewF" next to each declaration of `F`;
@@ -1840,10 +1871,11 @@
// [[clang::unsafe_buffer_usage]]
// #endif
//
-// `NewTypeText` is the string representation of the new type, to which the
-// parameter indexed by `ParmIdx` is being changed.
+// `ParmsMask` is an array of size of `FD->getNumParams()` such
+// that `ParmsMask[i]` is true iff the `i`-th parameter will be fixed with some
+// strategy.
static std::optional<FixItList>
-createOverloadsForFixedParams(unsigned ParmIdx, StringRef NewTyText,
+createOverloadsForFixedParams(const std::vector<bool> &ParmsMask, const Strategy &S,
const FunctionDecl *FD, const ASTContext &Ctx,
UnsafeBufferUsageHandler &Handler) {
// FIXME: need to make this conflict checking better:
@@ -1852,13 +1884,30 @@
const SourceManager &SM = Ctx.getSourceManager();
const LangOptions &LangOpts = Ctx.getLangOpts();
+ const unsigned NumParms = FD->getNumParams();
+ std::vector<std::string> NewTysTexts(NumParms);
+
+ for (unsigned i = 0; i < NumParms; i++) {
+ if (!ParmsMask[i])
+ continue;
+
+ std::optional<Qualifiers> PteTyQuals = std::nullopt;
+ std::optional<std::string> PteTyText =
+ getPointeeTypeText(FD->getParamDecl(i), SM, LangOpts, &PteTyQuals);
+
+ if (!PteTyText)
+ // something wrong in obtaining the text of the pointee type, give up
+ return std::nullopt;
+ // FIXME: whether we should create std::span type depends on the Strategy.
+ NewTysTexts[i] = getSpanTypeText(*PteTyText, PteTyQuals);
+ }
// FIXME Respect indentation of the original code.
// A lambda that creates the text representation of a function declaration
- // with the new type signature:
+ // with the new type signatures:
const auto NewOverloadSignatureCreator =
- [&SM, &LangOpts](const FunctionDecl *FD, unsigned ParmIdx,
- StringRef NewTypeText) -> std::optional<std::string> {
+ [&SM, &LangOpts, &NewTysTexts,
+ &ParmsMask](const FunctionDecl *FD) -> std::optional<std::string> {
std::stringstream SS;
SS << ";";
@@ -1878,13 +1927,16 @@
if (Parm->isImplicit())
continue;
- if (i == ParmIdx) {
- SS << NewTypeText.str();
+ if (ParmsMask[i]) {
+ // This `i`-th parameter will be fixed with `NewTysTexts[i]` being its
+ // new type:
+ SS << NewTysTexts[i];
// print parameter name if provided:
- if (IdentifierInfo * II = Parm->getIdentifier())
- SS << " " << II->getName().str();
- } else if (auto ParmTypeText =
- getRangeText(Parm->getSourceRange(), SM, LangOpts)) {
+ if (IdentifierInfo *II = Parm->getIdentifier())
+ SS << ' ' << II->getName().str();
+ } else if (auto ParmTypeText = getRangeText(
+ getSourceRangeToTokenEnd(Parm, SM, LangOpts),
+ SM, LangOpts)) {
// print the whole `Parm` without modification:
SS << ParmTypeText->str();
} else
@@ -1899,9 +1951,8 @@
// A lambda that creates the text representation of a function definition with
// the original signature:
const auto OldOverloadDefCreator =
- [&SM, &Handler,
- &LangOpts](const FunctionDecl *FD, unsigned ParmIdx,
- StringRef NewTypeText) -> std::optional<std::string> {
+ [&Handler, &SM, &LangOpts, &NewTysTexts,
+ &ParmsMask](const FunctionDecl *FD) -> std::optional<std::string> {
std::stringstream SS;
SS << getEndOfLine().str();
@@ -1931,10 +1982,10 @@
if (!Parm->getIdentifier())
// If a parameter of a function definition has no name:
return std::nullopt;
- if (i == ParmIdx)
+ if (ParmsMask[i])
// This is our spanified paramter!
- SS << NewTypeText.str() << "(" << Parm->getIdentifier()->getName().str() << ", "
- << getUserFillPlaceHolder("size") << ")";
+ SS << NewTysTexts[i] << "(" << Parm->getIdentifier()->getName().str()
+ << ", " << getUserFillPlaceHolder("size") << ")";
else
SS << Parm->getIdentifier()->getName().str();
if (i != NumParms - 1)
@@ -1956,8 +2007,7 @@
assert(FReDecl == FD && "inconsistent function definition");
// Inserts a definition with the old signature to the end of
// `FReDecl`:
- if (auto OldOverloadDef =
- OldOverloadDefCreator(FReDecl, ParmIdx, NewTyText))
+ if (auto OldOverloadDef = OldOverloadDefCreator(FReDecl))
FixIts.emplace_back(FixItHint::CreateInsertion(*Loc, *OldOverloadDef));
else
return {}; // give up
@@ -1969,8 +2019,7 @@
FReDecl->getBeginLoc(), " ")));
}
// Inserts a declaration with the new signature to the end of `FReDecl`:
- if (auto NewOverloadDecl =
- NewOverloadSignatureCreator(FReDecl, ParmIdx, NewTyText))
+ if (auto NewOverloadDecl = NewOverloadSignatureCreator(FReDecl))
FixIts.emplace_back(FixItHint::CreateInsertion(*Loc, *NewOverloadDecl));
else
return {};
@@ -1979,24 +2028,16 @@
return FixIts;
}
-// To fix a `ParmVarDecl` to be of `std::span` type. In addition, creating a
-// new overload of the function so that the change is self-contained (see
-// `createOverloadsForFixedParams`).
+// To fix a `ParmVarDecl` to be of `std::span` type.
static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx,
UnsafeBufferUsageHandler &Handler) {
+ assert(PVD->getType()->isPointerType() &&
+ "Unexpected non-pointer type parameter for fix");
if (PVD->hasDefaultArg()) {
// FIXME: generate fix-its for default values:
DEBUG_NOTE_DECL_FAIL(PVD, " : has default arg");
return {};
}
-
- assert(PVD->getType()->isPointerType());
- auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext());
-
- if (!FD) {
- DEBUG_NOTE_DECL_FAIL(PVD, " : invalid func decl");
- return {};
- }
std::optional<Qualifiers> PteTyQualifiers = std::nullopt;
std::optional<std::string> PteTyText = getPointeeTypeText(
@@ -2014,42 +2055,20 @@
return {};
}
- std::string SpanOpen = "std::span<";
- std::string SpanClose = ">";
- std::string SpanTyText;
std::stringstream SS;
- SS << SpanOpen << *PteTyText;
- // Append qualifiers to span element type:
if (PteTyQualifiers)
- SS << " " << PteTyQualifiers->getAsString();
- SS << SpanClose;
- // Save the Span type text:
- SpanTyText = SS.str();
+ // Append qualifiers if they exist:
+ SS << getSpanTypeText(*PteTyText, PteTyQualifiers);
+ else
+ SS << getSpanTypeText(*PteTyText);
// Append qualifiers to the type of the parameter:
if (PVD->getType().hasQualifiers())
- SS << " " << PVD->getType().getQualifiers().getAsString();
+ SS << ' ' << PVD->getType().getQualifiers().getAsString();
// Append parameter's name:
- SS << " " << PVDNameText->str();
-
- FixItList Fixes;
- unsigned ParmIdx = 0;
-
- Fixes.push_back(
- FixItHint::CreateReplacement(PVD->getSourceRange(), SS.str()));
- for (auto *ParmIter : FD->parameters()) {
- if (PVD == ParmIter)
- break;
- ParmIdx++;
- }
- if (ParmIdx < FD->getNumParams())
- if (auto OverloadFix = createOverloadsForFixedParams(ParmIdx, SpanTyText,
- FD, Ctx, Handler)) {
- Fixes.append(*OverloadFix);
- return Fixes;
- }
- DEBUG_NOTE_DECL_FAIL(PVD, " : invalid number of parameters");
- return {};
+ SS << ' ' << PVDNameText->str();
+ // Add replacement fix-it:
+ return {FixItHint::CreateReplacement(PVD->getSourceRange(), SS.str())};
}
static FixItList fixVariableWithSpan(const VarDecl *VD,
@@ -2142,6 +2161,12 @@
});
}
+// Returns true iff `VD` is a parameter of the declaration `D`:
+static bool isParameterOf(const VarDecl *VD, const Decl *D) {
+ return isa<ParmVarDecl>(VD) &&
+ VD->getDeclContext() == dyn_cast<DeclContext>(D);
+}
+
// Erases variables in `FixItsForVariable`, if such a variable has an unfixable
// group mate. A variable `v` is unfixable iff `FixItsForVariable` does not
// contain `v`.
@@ -2155,8 +2180,7 @@
VarGrpRef Grp = VarGrpMgr.getGroupOfVar(VD);
if (llvm::any_of(Grp,
[&FixItsForVariable](const VarDecl *GrpMember) -> bool {
- return FixItsForVariable.find(GrpMember) ==
- FixItsForVariable.end();
+ return !FixItsForVariable.count(GrpMember);
})) {
// At least one group member cannot be fixed, so we have to erase the
// whole group:
@@ -2168,6 +2192,45 @@
FixItsForVariable.erase(VarToErase);
}
+// Returns the fix-its that create bounds-safe function overloads for the
+// function `D`, if `D`'s parameters will be changed to safe-types through
+// fix-its in `FixItsForVariable`.
+//
+// NOTE: In case `D`'s parameters will be changed but bounds-safe function
+// overloads cannot created, the whole group that contains the parameters will
+// be erased from `FixItsForVariable`.
+static FixItList createFunctionOverloadsForParms(
+ std::map<const VarDecl *, FixItList> &FixItsForVariable /* mutable */,
+ const VariableGroupsManager &VarGrpMgr, const FunctionDecl *FD,
+ const Strategy &S, ASTContext &Ctx, UnsafeBufferUsageHandler &Handler) {
+ FixItList FixItsSharedByParms{};
+ std::vector<bool> ParmsNeedFixMask(FD->getNumParams(), false);
+ const VarDecl *FirstParmNeedsFix = nullptr;
+
+ for (unsigned i = 0; i < FD->getNumParams(); i++)
+ if (FixItsForVariable.count(FD->getParamDecl(i))) {
+ ParmsNeedFixMask[i] = true;
+ FirstParmNeedsFix = FD->getParamDecl(i);
+ }
+ if (FirstParmNeedsFix) {
+ // In case at least one parameter needs to be fixed:
+ std::optional<FixItList> OverloadFixes =
+ createOverloadsForFixedParams(ParmsNeedFixMask, S, FD, Ctx, Handler);
+
+ if (OverloadFixes) {
+ FixItsSharedByParms.append(*OverloadFixes);
+ } else {
+ // Something wrong in generating `OverloadFixes`, need to remove the
+ // whole group, where parameters are in, from `FixItsForVariable` (Note
+ // that all parameters should be in the same group):
+ for (auto *Member : VarGrpMgr.getGroupOfVar(FirstParmNeedsFix))
+ FixItsForVariable.erase(Member);
+ }
+ }
+ return FixItsSharedByParms;
+}
+
+// Constructs self-contained fix-its for each variable in `FixablesForAllVars`.
static std::map<const VarDecl *, FixItList>
getFixIts(FixableGadgetSets &FixablesForAllVars, const Strategy &S,
ASTContext &Ctx,
@@ -2221,21 +2284,36 @@
// `FixItsForVariable` iff it can be fixed and all its group mates can be
// fixed.
+ // Fix-its of bounds-safe overloads of `D` are shared by parameters of `D`.
+ // That is, when fixing multiple parameters in one step, these fix-its will
+ // be applied only once (instead of being applied per parameter).
+ FixItList FixItsSharedByParms{};
+
+ if (auto *FD = dyn_cast<FunctionDecl>(D))
+ FixItsSharedByParms = createFunctionOverloadsForParms(
+ FixItsForVariable, VarGrpMgr, FD, S, Ctx, Handler);
+
// The map that maps each variable `v` to fix-its for the whole group where
// `v` is in:
std::map<const VarDecl *, FixItList> FinalFixItsForVariable{
FixItsForVariable};
for (auto &[Var, Ignore] : FixItsForVariable) {
- const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Var);
+ bool AnyParm = false;
+ const auto VarGroupForVD = VarGrpMgr.getGroupOfVar(Var, &AnyParm);
for (const VarDecl *GrpMate : VarGroupForVD) {
if (Var == GrpMate)
continue;
if (FixItsForVariable.count(GrpMate))
- FinalFixItsForVariable[Var].insert(FinalFixItsForVariable[Var].end(),
- FixItsForVariable[GrpMate].begin(),
- FixItsForVariable[GrpMate].end());
+ FinalFixItsForVariable[Var].append(FixItsForVariable[GrpMate]);
+ }
+ if (AnyParm) {
+ // This assertion should never fail. Otherwise we have a bug.
+ assert(!FixItsSharedByParms.empty() &&
+ "Should not try to fix a parameter that does not belong to a "
+ "FunctionDecl");
+ FinalFixItsForVariable[Var].append(FixItsSharedByParms);
}
}
// Fix-its that will be applied in one step shall NOT:
@@ -2265,17 +2343,27 @@
class VariableGroupsManagerImpl : public VariableGroupsManager {
const std::vector<VarGrpTy> Groups;
const std::map<const VarDecl *, unsigned> &VarGrpMap;
+ const llvm::SetVector<const VarDecl *> &GrpsUnionForParms;
public:
VariableGroupsManagerImpl(
const std::vector<VarGrpTy> &Groups,
- const std::map<const VarDecl *, unsigned> &VarGrpMap)
- : Groups(Groups), VarGrpMap(VarGrpMap) {}
-
- VarGrpRef getGroupOfVar(const VarDecl *Var) const override {
- auto I = VarGrpMap.find(Var);
- assert(I != VarGrpMap.end());
- return Groups[I->second];
+ const std::map<const VarDecl *, unsigned> &VarGrpMap,
+ const llvm::SetVector<const VarDecl *> &GrpsUnionForParms)
+ : Groups(Groups), VarGrpMap(VarGrpMap),
+ GrpsUnionForParms(GrpsUnionForParms) {}
+
+ VarGrpRef getGroupOfVar(const VarDecl *Var, bool *HasParm) const override {
+ if (GrpsUnionForParms.contains(Var)) {
+ if (HasParm)
+ *HasParm = true;
+ return GrpsUnionForParms.getArrayRef();
+ }
+ if (HasParm)
+ *HasParm = false;
+ if (!VarGrpMap.count(Var))
+ return std::nullopt;
+ return Groups[VarGrpMap.at(Var)];
}
};
@@ -2462,6 +2550,9 @@
// variables belong to. Group indexes refer to the elements in `Groups`.
// `VarGrpMap` is complete in that every variable that needs fix is in it.
std::map<const VarDecl *, unsigned> VarGrpMap;
+ // The union group over the ones in "Groups" that contain parameters of `D`:
+ llvm::SetVector<const VarDecl *>
+ GrpsUnionForParms; // these variables need to be fixed in one step
// Group Connected Components for Unsafe Vars
// (Dependencies based on pointer assignments)
@@ -2484,11 +2575,17 @@
}
}
}
+
+ bool HasParm = false;
unsigned GrpIdx = Groups.size() - 1;
for (const VarDecl *V : VarGroup) {
VarGrpMap[V] = GrpIdx;
+ if (!HasParm && isParameterOf(V, D))
+ HasParm = true;
}
+ if (HasParm)
+ GrpsUnionForParms.insert(VarGroup.begin(), VarGroup.end());
}
}
@@ -2513,7 +2610,7 @@
for (auto I = FixablesForAllVars.byVar.begin();
I != FixablesForAllVars.byVar.end();) {
// Note `VisitedVars` contain all the variables in the graph:
- if (VisitedVars.find((*I).first) == VisitedVars.end()) {
+ if (!VisitedVars.count((*I).first)) {
// no such var in graph:
I = FixablesForAllVars.byVar.erase(I);
} else
@@ -2521,11 +2618,12 @@
}
Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars);
- VariableGroupsManagerImpl VarGrpMgr(Groups, VarGrpMap);
+ VariableGroupsManagerImpl VarGrpMgr(Groups, VarGrpMap, GrpsUnionForParms);
- FixItsForVariableGroup =
- getFixIts(FixablesForAllVars, NaiveStrategy, D->getASTContext(), D,
- Tracker, Handler, VarGrpMgr);
+ if (isa<NamedDecl>(D))
+ FixItsForVariableGroup =
+ getFixIts(FixablesForAllVars, NaiveStrategy, D->getASTContext(), D,
+ Tracker, Handler, VarGrpMgr);
for (const auto &G : UnsafeOps.noVar) {
Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false);
@@ -2536,7 +2634,8 @@
Handler.handleUnsafeVariableGroup(VD, VarGrpMgr,
FixItsIt != FixItsForVariableGroup.end()
? std::move(FixItsIt->second)
- : FixItList{});
+ : FixItList{},
+ D);
for (const auto &G : WarningGadgets) {
Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true);
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11870,6 +11870,9 @@
"used%select{| in pointer arithmetic| in buffer access}0 here">;
def note_unsafe_buffer_variable_fixit_group : Note<
"change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information%select{|, and change %2 to '%select{std::span|std::array|std::span::iterator}1' to propagate bounds information between them}3">;
+def note_unsafe_buffer_variable_fixit_together : Note<
+ "change type of %0 to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds information"
+ "%select{|, and change %2 to safe types to make function '%4' bounds-safe}3">;
def note_safe_buffer_usage_suggestions_disabled : Note<
"pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions">;
#ifndef NDEBUG
Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
===================================================================
--- clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -31,7 +31,10 @@
/// together in one step.
///
/// `Var` must be a variable that needs fix (so it must be in a group).
- virtual VarGrpRef getGroupOfVar(const VarDecl *Var) const;
+ /// `HasParm` is an optional argument that will be set to true if the set of
+ /// variables, where `Var` is in, contains parameters.
+ virtual VarGrpRef getGroupOfVar(const VarDecl *Var,
+ bool *HasParm = nullptr) const;
};
/// The interface that lets the caller handle unsafe buffer usage analysis
@@ -62,11 +65,14 @@
bool IsRelatedToDecl) = 0;
/// Invoked when a fix is suggested against a variable. This function groups
- /// all variables that must be fixed together (i.e their types must be changed to the
- /// same target type to prevent type mismatches) into a single fixit.
+ /// all variables that must be fixed together (i.e their types must be changed
+ /// to the same target type to prevent type mismatches) into a single fixit.
+ ///
+ /// `D` is the declaration of the callable under analysis that owns `Variable`
+ /// and all of its group mates.
virtual void handleUnsafeVariableGroup(const VarDecl *Variable,
const VariableGroupsManager &VarGrpMgr,
- FixItList &&Fixes) = 0;
+ FixItList &&Fixes, const Decl *D) = 0;
#ifndef NDEBUG
public:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
