[-Wunsafe-buffer-usage] Fix debug notes for unclaimed DREs (llvm#80787)

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.

(cherry picked from commit 2f49058)

Author: jkorous-apple

clang/lib/Analysis/UnsafeBufferUsage.cpp

@@ -2864,34 +2864,41 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
                                   it->first->getNameAsString() +
                                   "' : has a reference type"));
 #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()) {
+        it = FixablesForAllVars.byVar.erase(it);
+      } else if (Tracker.hasUnclaimedUses(it->first)) {
+        it = FixablesForAllVars.byVar.erase(it);
+      } else if (it->first->isInitCapture()) {
 #ifndef NDEBUG
       Handler.addDebugNoteForVar(it->first, it->first->getBeginLoc(),
                                  ("failed to produce fixit for '" +
                                   it->first->getNameAsString() +
                                   "' : init capture"));
 #endif
-      it = FixablesForAllVars.byVar.erase(it);
-    } else {
-      ++it;
+        it = FixablesForAllVars.byVar.erase(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 *>>;

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}}
+}