[PATCH] D145993: [-Wunsafe-buffer-usage] Reducing non-determinism in diagnostics output stream

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: jkorous, NoQ, malavikasamak, t-rasmud, xazax.hun, ymandel, gribozavr, aaron.ballman.
Herald added subscribers: mgrang, rnkovacs.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `-Wunsafe-buffer-usage` analysis outputs diagnostics in the order of pointer values to associated `VarDecl`s.  This creates non-determinism in the order of diagnostics in output since it cannot be guaranteed in pointer values.  However, our fix-it tests were written under the assumption that diagnostics are output in source location order.      This results in non-deterministic failures in our tests when diagnostics are not output in source location order, such as 
buildbot fail <https://lab.llvm.org/buildbot/#/builders/60/builds/10631> (We disabled the test for windows but didn't understand the failure that time).

The reason that a test fail, albeit possible, is much less likely than a test pass is probably because pointer values to AST nodes nearly respect to the order of node creation/parse.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145993

Files:
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp


Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
@@ -1,4 +1,3 @@
-// REQUIRES: !system-windows
 // RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 typedef int * Int_ptr_t;
 typedef int Int_t;
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -661,7 +661,11 @@
 }
 
 struct WarningGadgetSets {
-  std::map<const VarDecl *, std::set<std::unique_ptr<WarningGadget>>> byVar;
+  std::map<const VarDecl *, std::set<std::unique_ptr<WarningGadget>>,
+           // To keep keys sorted by their locations in the map so that the
+           // order is deterministic:
+           clang::internal::CompareNode<VarDecl>>
+      byVar;
   // These Gadgets are not related to pointer variables (e. g. temporaries).
   llvm::SmallVector<std::unique_ptr<WarningGadget>, 16> noVar;
 };
Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
===================================================================
--- clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -61,6 +61,14 @@
 // conflict if they have overlapping source ranges.
 bool anyConflict(const llvm::SmallVectorImpl<FixItHint> &FixIts,
                  const SourceManager &SM);
+
+// Compares AST nodes by source locations.
+template <typename NodeTy> struct CompareNode {
+  bool operator()(const NodeTy *N1, const NodeTy *N2) const {
+    return N1->getBeginLoc().getRawEncoding() <
+           N2->getBeginLoc().getRawEncoding();
+  }
+};
 } // namespace internal
 } // end namespace clang
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145993.504860.patch
Type: text/x-patch
Size: 1984 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230313/894466ec/attachment-0001.bin>