ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, jkorous, t-rasmud, malavikasamak.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.
When some unsafe operations are suppressed, there will be (non-strictly)
fewer variables being warned about. `FixableGadget`s associated with the
suppressed variables will not be fixed.
Removing these `FixableGadget` as early as possible could help improve the
performance and stability of the analysis.
The variable grouping algorithm introduced in D145739
<https://reviews.llvm.org/D145739> nearly completes this task.
For a variable that is neither warned about nor reachable from a warned
variable, it does not exist in the graph computed by the algorithm.
So this patch simply removes `FixableGadget`s whose variables are not present
in the graph computed for variable grouping.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D155524
Files:
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-fixit.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-fixit.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-fixit.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-pragma-fixit.cpp
@@ -107,3 +107,128 @@
p[5]; // not to note since the associated warning is suppressed
}
+
+
+// Test suppressing interacts with variable grouping:
+
+// The implication edges are: `a` -> `b` -> `c`.
+// If the unsafe operation on `a` is supressed, none of the variables
+// will be fixed.
+void suppressedVarInGroup() {
+ int * a;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int * b;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int * c;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+
+#pragma clang unsafe_buffer_usage begin
+ a[5] = 5;
+#pragma clang unsafe_buffer_usage end
+ a = b;
+ b = c;
+}
+
+// To show that if `a[5]` is not suppressed in the
+// `suppressedVarInGroup` function above, all variables will be fixed.
+void suppressedVarInGroup_control() {
+ int * a;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> a"
+ int * b;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> b"
+ int * c;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> c"
+
+ a[5] = 5;
+ a = b;
+ b = c;
+}
+
+// The implication edges are: `a` -> `b` -> `c`.
+// The unsafe operation on `b` is supressed, while the unsafe
+// operation on `a` is not suppressed. Variable `b` will still be
+// fixed when fixing `a`.
+void suppressedVarInGroup2() {
+ int * a;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> a"
+ int * b;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> b"
+ int * c;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> c"
+
+ a[5] = 5;
+#pragma clang unsafe_buffer_usage begin
+ b[5] = 5;
+#pragma clang unsafe_buffer_usage end
+ a = b;
+ b = c;
+}
+
+// The implication edges are: `a` -> `b` -> `c`.
+// The unsafe operation on `b` is supressed, while the unsafe
+// operation on `c` is not suppressed. Only variable `c` will be fixed
+// then.
+void suppressedVarInGroup3() {
+ int * a;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int * b;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int * c;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> c"
+
+ c[5] = 5;
+#pragma clang unsafe_buffer_usage begin
+ b[5] = 5;
+#pragma clang unsafe_buffer_usage end
+ a = b;
+ b = c;
+}
+
+// The implication edges are: `a` -> `b` -> `c` -> `a`.
+// The unsafe operation on `b` is supressed, while the unsafe
+// operation on `c` is not suppressed. Since the implication graph
+// forms a cycle, all variables will be fixed.
+void suppressedVarInGroup4() {
+ int * a;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> a"
+ int * b;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> b"
+ int * c;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> c"
+
+ c[5] = 5;
+#pragma clang unsafe_buffer_usage begin
+ b[5] = 5;
+#pragma clang unsafe_buffer_usage end
+ a = b;
+ b = c;
+ c = a;
+}
+
+// There are two groups: `a` -> `b` -> `c` and `d` -> `e` -> `f`.
+// Suppressing unsafe operations on variables in one group does not
+// affect other groups.
+void suppressedVarInGroup5() {
+ int * a;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int * b;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int * c;
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+ int * d;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> d"
+ int * e;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> e"
+ int * f;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span<int> f"
+
+#pragma clang unsafe_buffer_usage begin
+ a[5] = 5;
+#pragma clang unsafe_buffer_usage end
+ a = b;
+ b = c;
+
+ d[5] = 5;
+ d = e;
+ e = f;
+}
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -2341,6 +2341,19 @@
}
}
+ // Remove a `FixableGadget` if the associated variable is not in the graph
+ // computed above. We do not want to generate fix-its for such variables,
+ // since they are neither warned nor reachable from a warned one.
+ 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()) {
+ // no such var in graph:
+ I = FixablesForAllVars.byVar.erase(I);
+ } else
+ ++I;
+ }
+
Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars);
FixItsForVariableGroup =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
