https://github.com/jkorous-apple updated 
https://github.com/llvm/llvm-project/pull/80787

>From 1abbea30614c299219391770431b8226b8f7a88a Mon Sep 17 00:00:00 2001
From: Jan Korous <jkor...@apple.com>
Date: Mon, 5 Feb 2024 18:21:50 -0800
Subject: [PATCH] [-Wunsafe-buffer-usage] Fix debug notes for unclaimed DREs

Debug notes for unclaimed DeclRefExpr should report any DRE of an unsafe 
variable that is not covered by a Fixable (i. e. fixit for the particular AST 
pattern isn't implemented for whatever reason).
Currently not all unclaimed DeclRefExpr-s are reported which is a bug. The 
debug notes report only those DREs where the referred VarDecl has at least one 
other DeclRefExpr which is claimed (covered by a fixit).
If there is an unsafe VarDecl that has exactly one DRE and the DRE isn't 
claimed then the debug note about missing fixit won't be emitted.
That is because the debug note is emitted from within a loop over set of 
successfully matched FixableGadgets which by-definition is missing those DRE 
that are not matched at all.

The new code simply iterates over all unsafe VarDecls and all of their 
unclaimed DREs.
---
 clang/lib/Analysis/UnsafeBufferUsage.cpp      | 37 +++++++++++--------
 .../warn-unsafe-buffer-usage-debug.cpp        |  7 ++++
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 823cd2a7b9969..dd32c6c2c6e01 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -2870,19 +2870,6 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
 #endif
         it = FixablesForAllVars.byVar.erase(it);
       } else if (Tracker.hasUnclaimedUses(it->first)) {
-#ifndef NDEBUG
-        auto AllUnclaimed = Tracker.getUnclaimedUses(it->first);
-        for (auto UnclaimedDRE : AllUnclaimed) {
-        std::string UnclaimedUseTrace =
-            getDREAncestorString(UnclaimedDRE, D->getASTContext());
-
-        Handler.addDebugNoteForVar(
-            it->first, UnclaimedDRE->getBeginLoc(),
-            ("failed to produce fixit for '" + it->first->getNameAsString() +
-             "' : has an unclaimed use\nThe unclaimed DRE trace: " +
-             UnclaimedUseTrace));
-        }
-#endif
         it = FixablesForAllVars.byVar.erase(it);
       } else if (it->first->isInitCapture()) {
 #ifndef NDEBUG
@@ -2892,11 +2879,31 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
                                     "' : init capture"));
 #endif
         it = FixablesForAllVars.byVar.erase(it);
-      }else {
-      ++it;
+      } else {
+        ++it;
     }
   }
 
+#ifndef NDEBUG
+  for (const auto &it : UnsafeOps.byVar) {
+    const VarDecl *const UnsafeVD = it.first;
+    auto UnclaimedDREs = Tracker.getUnclaimedUses(UnsafeVD);
+    if (UnclaimedDREs.empty())
+      continue;
+    const auto UnfixedVDName = UnsafeVD->getNameAsString();
+    for (const clang::DeclRefExpr *UnclaimedDRE : UnclaimedDREs) {
+      std::string UnclaimedUseTrace =
+          getDREAncestorString(UnclaimedDRE, D->getASTContext());
+
+      Handler.addDebugNoteForVar(
+          UnsafeVD, UnclaimedDRE->getBeginLoc(),
+          ("failed to produce fixit for '" + UnfixedVDName +
+           "' : has an unclaimed use\nThe unclaimed DRE trace: " +
+           UnclaimedUseTrace));
+    }
+  }
+#endif
+
   // Fixpoint iteration for pointer assignments
   using DepMapTy = DenseMap<const VarDecl *, llvm::SetVector<const VarDecl *>>;
   DepMapTy DependenciesMap{};
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp 
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
index e08f70d97e391..5fff0854d4546 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-debug.cpp
@@ -98,3 +98,10 @@ void test_struct_claim_use() {
   x[6] = 8;  // expected-warning{{unsafe buffer access}}
   x++;  // expected-warning{{unsafe pointer arithmetic}}
 }
+
+void use(int*);
+void array2d(int idx) {
+  int buffer[10][5]; // expected-warning{{'buffer' is an unsafe buffer that 
does not perform bounds checks}}
+  use(buffer[idx]);  // expected-note{{used in buffer access here}} \
+  // debug-note{{safe buffers debug: failed to produce fixit for 'buffer' : 
has an unclaimed use}}
+}

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

Reply via email to