[clang] [analyzer][NFC] Make RegionStore dumps deterministic (PR #115615)
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 12 05:50:04 PST 2024
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/115615
>From 26f0cfabe3328c8eb8a861dd5d1d541921499f0c Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Sat, 9 Nov 2024 15:55:08 +0100
Subject: [PATCH 1/3] [analyzer][NFC] Make RegionStore dumps deterministic
Dump the memory space clusters before the other clusters, in
alphabetical order. Then default bindings over direct bindings, and if
any has symbolic offset, then those should come before the ones with
concrete offsets.
In theory, we should either have a symbolic offset OR concrete offsets,
but never both at the same time.
---
clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 86 ++++++++++++++++---
1 file changed, 73 insertions(+), 13 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 674099dd7e1f0f..6bad9a93a30169 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -67,9 +67,10 @@ class BindingKey {
isa<ObjCIvarRegion, CXXDerivedObjectRegion>(r)) &&
"Not a base");
}
-public:
+public:
bool isDirect() const { return P.getInt() & Direct; }
+ bool isDefault() const { return !isDirect(); }
bool hasSymbolicOffset() const { return P.getInt() & Symbolic; }
const MemRegion *getRegion() const { return P.getPointer(); }
@@ -232,27 +233,86 @@ class RegionBindingsRef : public llvm::ImmutableMapRef<const MemRegion *,
void printJson(raw_ostream &Out, const char *NL = "\n",
unsigned int Space = 0, bool IsDot = false) const {
- for (iterator I = begin(), E = end(); I != E; ++I) {
- // TODO: We might need a .printJson for I.getKey() as well.
+ using namespace llvm;
+ DenseMap<const MemRegion *, std::string> StringifyCache;
+ auto ToString = [&StringifyCache](const MemRegion *R) {
+ auto [Place, Inserted] = StringifyCache.try_emplace(R);
+ if (!Inserted)
+ return Place->second;
+ std::string Res;
+ raw_string_ostream OS(Res);
+ OS << R;
+ Place->second = Res;
+ return Res;
+ };
+
+ using Cluster =
+ std::pair<const MemRegion *, ImmutableMap<BindingKey, SVal>>;
+ using Binding = std::pair<BindingKey, SVal>;
+
+ const auto MemSpaceBeforeRegionName = [&ToString](const Cluster *L,
+ const Cluster *R) {
+ if (isa<MemSpaceRegion>(L->first) && !isa<MemSpaceRegion>(R->first))
+ return true;
+ if (!isa<MemSpaceRegion>(L->first) && isa<MemSpaceRegion>(R->first))
+ return false;
+ return ToString(L->first) < ToString(R->first);
+ };
+
+ const auto SymbolicBeforeOffset = [&ToString](const BindingKey &L,
+ const BindingKey &R) {
+ if (L.hasSymbolicOffset() && !R.hasSymbolicOffset())
+ return true;
+ if (!L.hasSymbolicOffset() && R.hasSymbolicOffset())
+ return false;
+ if (L.hasSymbolicOffset() && R.hasSymbolicOffset())
+ return ToString(L.getRegion()) < ToString(R.getRegion());
+ return L.getOffset() < R.getOffset();
+ };
+
+ const auto DefaultBindingBeforeDirectBindings =
+ [&SymbolicBeforeOffset](const Binding *LPtr, const Binding *RPtr) {
+ const BindingKey &L = LPtr->first;
+ const BindingKey &R = RPtr->first;
+ if (L.isDefault() && !R.isDefault())
+ return true;
+ if (!L.isDefault() && R.isDefault())
+ return false;
+ assert(L.isDefault() == R.isDefault());
+ return SymbolicBeforeOffset(L, R);
+ };
+
+ const auto AddrOf = [](const auto &Item) { return &Item; };
+
+ std::vector<const Cluster *> SortedClusters;
+ SortedClusters.reserve(std::distance(begin(), end()));
+ append_range(SortedClusters, map_range(*this, AddrOf));
+ llvm::sort(SortedClusters, MemSpaceBeforeRegionName);
+
+ for (auto [Idx, C] : llvm::enumerate(SortedClusters)) {
+ const auto &[BaseRegion, Bindings] = *C;
Indent(Out, Space, IsDot)
- << "{ \"cluster\": \"" << I.getKey() << "\", \"pointer\": \""
- << (const void *)I.getKey() << "\", \"items\": [" << NL;
+ << "{ \"cluster\": \"" << BaseRegion << "\", \"pointer\": \""
+ << (const void *)BaseRegion << "\", \"items\": [" << NL;
+
+ std::vector<const Binding *> SortedBindings;
+ SortedBindings.reserve(std::distance(Bindings.begin(), Bindings.end()));
+ append_range(SortedBindings, map_range(Bindings, AddrOf));
+ llvm::sort(SortedBindings, DefaultBindingBeforeDirectBindings);
++Space;
- const ClusterBindings &CB = I.getData();
- for (ClusterBindings::iterator CI = CB.begin(), CE = CB.end(); CI != CE;
- ++CI) {
- Indent(Out, Space, IsDot) << "{ " << CI.getKey() << ", \"value\": ";
- CI.getData().printJson(Out, /*AddQuotes=*/true);
+ for (auto [Idx, B] : llvm::enumerate(SortedBindings)) {
+ const auto &[Key, Value] = *B;
+ Indent(Out, Space, IsDot) << "{ " << Key << ", \"value\": ";
+ Value.printJson(Out, /*AddQuotes=*/true);
Out << " }";
- if (std::next(CI) != CE)
+ if (Idx != SortedBindings.size() - 1)
Out << ',';
Out << NL;
}
-
--Space;
Indent(Out, Space, IsDot) << "]}";
- if (std::next(I) != E)
+ if (Idx != SortedClusters.size() - 1)
Out << ',';
Out << NL;
}
>From 48d25454ba3a5a1aebcfa2edd386028fe747c026 Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Mon, 11 Nov 2024 18:04:37 +0100
Subject: [PATCH 2/3] NFC Use std::tuple as suggested
---
clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 39 +++++++------------
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 6bad9a93a30169..112a56764f98cd 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -250,36 +250,25 @@ class RegionBindingsRef : public llvm::ImmutableMapRef<const MemRegion *,
std::pair<const MemRegion *, ImmutableMap<BindingKey, SVal>>;
using Binding = std::pair<BindingKey, SVal>;
- const auto MemSpaceBeforeRegionName = [&ToString](const Cluster *L,
- const Cluster *R) {
- if (isa<MemSpaceRegion>(L->first) && !isa<MemSpaceRegion>(R->first))
- return true;
- if (!isa<MemSpaceRegion>(L->first) && isa<MemSpaceRegion>(R->first))
- return false;
- return ToString(L->first) < ToString(R->first);
+ const auto ClusterSortKey = [&ToString](const Cluster *C) {
+ const MemRegion *Key = C->first;
+ return std::tuple{isa<MemSpaceRegion>(Key), ToString(Key)};
};
- const auto SymbolicBeforeOffset = [&ToString](const BindingKey &L,
- const BindingKey &R) {
- if (L.hasSymbolicOffset() && !R.hasSymbolicOffset())
- return true;
- if (!L.hasSymbolicOffset() && R.hasSymbolicOffset())
- return false;
- if (L.hasSymbolicOffset() && R.hasSymbolicOffset())
- return ToString(L.getRegion()) < ToString(R.getRegion());
- return L.getOffset() < R.getOffset();
+ const auto MemSpaceBeforeRegionName = [&ClusterSortKey](const Cluster *L,
+ const Cluster *R) {
+ return ClusterSortKey(L) < ClusterSortKey(R);
+ };
+
+ const auto BindingSortKey = [&ToString](const Binding *BPtr) {
+ const BindingKey &Key = BPtr->first;
+ return std::tuple{Key.isDirect(), !Key.hasSymbolicOffset(),
+ ToString(Key.getRegion()), Key.getOffset()};
};
const auto DefaultBindingBeforeDirectBindings =
- [&SymbolicBeforeOffset](const Binding *LPtr, const Binding *RPtr) {
- const BindingKey &L = LPtr->first;
- const BindingKey &R = RPtr->first;
- if (L.isDefault() && !R.isDefault())
- return true;
- if (!L.isDefault() && R.isDefault())
- return false;
- assert(L.isDefault() == R.isDefault());
- return SymbolicBeforeOffset(L, R);
+ [&BindingSortKey](const Binding *LPtr, const Binding *RPtr) {
+ return BindingSortKey(LPtr) < BindingSortKey(RPtr);
};
const auto AddrOf = [](const auto &Item) { return &Item; };
>From 35eb4fb85a50bc19bc91feeeeb0908dc8d5e8b8e Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Tue, 12 Nov 2024 14:49:25 +0100
Subject: [PATCH 3/3] NFC Drop the unused isDefault()
---
clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 112a56764f98cd..085f0ef9a5fb96 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -70,7 +70,6 @@ class BindingKey {
public:
bool isDirect() const { return P.getInt() & Direct; }
- bool isDefault() const { return !isDirect(); }
bool hasSymbolicOffset() const { return P.getInt() & Symbolic; }
const MemRegion *getRegion() const { return P.getPointer(); }
More information about the cfe-commits
mailing list