ziqingluo-90 updated this revision to Diff 541269.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155524/new/
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
@@ -1092,6 +1092,18 @@
M.match(*D->getBody(), D->getASTContext());
+ // If no `WarningGadget`s ever matched, there is no unsafe operations in the
+ // function under the analysis. Thus, there is nothing needs to be fixed.
+ //
+ // Note this claim is based on the assumption that there is no unsafe
+ // variable whose declaration is invisible from the analyzing function.
+ // Otherwise, we need to consider if the uses of those unsafe varuables needs
+ // fix.
+ // So far, we are not fixing any global variables or class members. And,
+ // lambdas will be analyzed along with the enclosing function. So the claim is
+ // true for now.
+ if (CB.WarningGadgets.empty())
+ CB.FixableGadgets.clear();
// Gadgets "claim" variables they're responsible for. Once this loop finishes,
// the tracker will only track DREs that weren't claimed by any gadgets,
// i.e. not understood by the analysis.
@@ -2341,6 +2353,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
