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
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
