[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1095-1102
   // 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.
   for (const auto  : CB.FixableGadgets) {
 for (const auto *DRE : G->getClaimedVarUseSites()) {
   CB.Tracker.claimUse(DRE);
 }

NoQ wrote:
> ziqingluo-90 wrote:
> > NoQ wrote:
> > > Let's also skip this part when there are no warning gadgets? Your initial 
> > > patch was clearing the `FixableGadgets` list so it was naturally skipped, 
> > > but now it's active again.
> > Oh, I intentionally chose not to skip it:
> >  - 1. To keep the `Tracker` consistent with `CB.FixableGadgets` in case in 
> > the future we want to use these two objects even if there is no 
> > `WarningGadget`; 
> >  - 2. The `findGadgets` function can stay as straightforward as its name 
> > suggests.  It doesn't have to know the specific optimization for empty 
> > `WarningGadget`s.
> > 
> > But the two thoughts above probably aren't important enough.  I will change 
> > it back to skipping the loop when there is no warnings.
> > 
> > 
> Hmm, I agree that this turns into a confusing contract. Maybe move this loop 
> out of the function, to the call site, so that it was more obvious that we 
> skip it? This would leave the tracker in a mildly inconsistent state at 
> return, so the caller will have to make it consistent by doing the claiming, 
> but that's arguably less confusing because we clearly communicate that this 
> is an optional step. So the function will do exactly what it says it'll do: 
> find gadgets and that's it.
> 
> Separately, we can probably consider not searching for fixable gadgets at all 
> when no warning gadgets are found. This could be a performance improvement, 
> but definitely a story for another time.
Thanks for the suggestion.  I moved the loop to the end of the call to 
`findGadgets` and landed it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155524/new/

https://reviews.llvm.org/D155524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
ziqingluo-90 marked an inline comment as done.
Closed by commit rGcfcf76c6ad72: [-Wunsafe-buffer-usage] Ignore the 
FixableGadgets that will not be fixed at an… (authored by ziqingluo-90).

Changed prior to commit:
  https://reviews.llvm.org/D155524?vs=542199=544151#toc

Repository:
  rG LLVM Github Monorepo

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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 d"
+  int * e;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span e"
+  int * f;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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
@@ -1090,16 +1090,6 @@
   }
 
   M.match(*D->getBody(), D->getASTContext());
-
-  // Gadgets "claim" variables they're responsible for. Once 

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

LGTM, thanks!!




Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1095-1102
   // 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.
   for (const auto  : CB.FixableGadgets) {
 for (const auto *DRE : G->getClaimedVarUseSites()) {
   CB.Tracker.claimUse(DRE);
 }

ziqingluo-90 wrote:
> NoQ wrote:
> > Let's also skip this part when there are no warning gadgets? Your initial 
> > patch was clearing the `FixableGadgets` list so it was naturally skipped, 
> > but now it's active again.
> Oh, I intentionally chose not to skip it:
>  - 1. To keep the `Tracker` consistent with `CB.FixableGadgets` in case in 
> the future we want to use these two objects even if there is no 
> `WarningGadget`; 
>  - 2. The `findGadgets` function can stay as straightforward as its name 
> suggests.  It doesn't have to know the specific optimization for empty 
> `WarningGadget`s.
> 
> But the two thoughts above probably aren't important enough.  I will change 
> it back to skipping the loop when there is no warnings.
> 
> 
Hmm, I agree that this turns into a confusing contract. Maybe move this loop 
out of the function, to the call site, so that it was more obvious that we skip 
it? This would leave the tracker in a mildly inconsistent state at return, so 
the caller will have to make it consistent by doing the claiming, but that's 
arguably less confusing because we clearly communicate that this is an optional 
step. So the function will do exactly what it says it'll do: find gadgets and 
that's it.

Separately, we can probably consider not searching for fixable gadgets at all 
when no warning gadgets are found. This could be a performance improvement, but 
definitely a story for another time.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155524/new/

https://reviews.llvm.org/D155524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 updated this revision to Diff 542199.
ziqingluo-90 added a comment.

Address comments


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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 d"
+  int * e;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span e"
+  int * f;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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,15 +1092,18 @@
 
   M.match(*D->getBody(), D->getASTContext());
 
-  // 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.
-  for (const auto  : CB.FixableGadgets) {
-for (const auto *DRE : G->getClaimedVarUseSites()) {
-  CB.Tracker.claimUse(DRE);
+  // If no `WarningGadget`s ever matched, there is no unsafe operations 

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1095-1102
   // 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.
   for (const auto  : CB.FixableGadgets) {
 for (const auto *DRE : G->getClaimedVarUseSites()) {
   CB.Tracker.claimUse(DRE);
 }

NoQ wrote:
> Let's also skip this part when there are no warning gadgets? Your initial 
> patch was clearing the `FixableGadgets` list so it was naturally skipped, but 
> now it's active again.
Oh, I intentionally chose not to skip it:
 - 1. To keep the `Tracker` consistent with `CB.FixableGadgets` in case in the 
future we want to use these two objects even if there is no `WarningGadget`; 
 - 2. The `findGadgets` function can stay as straightforward as its name 
suggests.  It doesn't have to know the specific optimization for empty 
`WarningGadget`s.

But the two thoughts above probably aren't important enough.  I will change it 
back to skipping the loop when there is no warnings.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155524/new/

https://reviews.llvm.org/D155524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1095-1102
   // 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.
   for (const auto  : CB.FixableGadgets) {
 for (const auto *DRE : G->getClaimedVarUseSites()) {
   CB.Tracker.claimUse(DRE);
 }

Let's also skip this part when there are no warning gadgets? Your initial patch 
was clearing the `FixableGadgets` list so it was naturally skipped, but now 
it's active again.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155524/new/

https://reviews.llvm.org/D155524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 updated this revision to Diff 541652.
ziqingluo-90 marked an inline comment as done.

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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 d"
+  int * e;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span e"
+  int * f;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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
@@ -2235,6 +2235,20 @@
 return;
   }
 
+  // If no `WarningGadget`s ever matched, there is no unsafe operations in the
+  // function under the analysis.  Thus, it early returns here as 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.
+  

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2249
   }
 
   UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets));

Maybe early-return here when there are no gadgets?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155524/new/

https://reviews.llvm.org/D155524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 marked an inline comment as done.
ziqingluo-90 added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2346
+  // 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();

t-rasmud wrote:
> Nit: Maybe also mention when a variable is neither warned nor is reachable? 
> Are there scenarios besides variables inside pragmas where this constraint is 
> satisfied? 
good question!  I add some explanation in the comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155524/new/

https://reviews.llvm.org/D155524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 updated this revision to Diff 541279.

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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 d"
+  int * e;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span e"
+  int * f;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 d"
+  int * e;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span e"
+  int * f;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Rashmi Mudduluru via Phabricator via cfe-commits]

t-rasmud accepted this revision.
t-rasmud added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2346
+  // 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();

Nit: Maybe also mention when a variable is neither warned nor is reachable? Are 
there scenarios besides variables inside pragmas where this constraint is 
satisfied? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155524/new/

https://reviews.llvm.org/D155524

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155524: [-Wunsafe-buffer-usage] Ignore the FixableGadgets that will not be fixed at an earlier stage

[Ziqing Luo via Phabricator via cfe-commits]

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 
 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 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 a"
+  int * b;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span b"
+  int * c;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span 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 d"
+  int * e;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span e"
+  int * f;
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:10}:"std::span f"
+