Skip to content

Commit ae7392b

Browse files
authored
Reapply "[analyzer][NFC] Make RegionStore dumps deterministic" (#115884)
This is reapplies #115615 without using tuples. The eager call of `getRegion()` and `getOffset()` could cause crashes when the Store had symbolic bindings. Here I'm fixing the crash by lazily calling those getters. Also, the tuple version poorly sorted the Clusters. The memory spaces should have come before the regular clusters. Now, that is also fixed here, demonstrated by the test.
1 parent 57c33ac commit ae7392b

File tree

2 files changed

+130
-13
lines changed

2 files changed

+130
-13
lines changed

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

Lines changed: 73 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,10 @@ class BindingKey {
6767
isa<ObjCIvarRegion, CXXDerivedObjectRegion>(r)) &&
6868
"Not a base");
6969
}
70-
public:
7170

71+
public:
7272
bool isDirect() const { return P.getInt() & Direct; }
73+
bool isDefault() const { return !isDirect(); }
7374
bool hasSymbolicOffset() const { return P.getInt() & Symbolic; }
7475

7576
const MemRegion *getRegion() const { return P.getPointer(); }
@@ -232,27 +233,86 @@ class RegionBindingsRef : public llvm::ImmutableMapRef<const MemRegion *,
232233

233234
void printJson(raw_ostream &Out, const char *NL = "\n",
234235
unsigned int Space = 0, bool IsDot = false) const {
235-
for (iterator I = begin(), E = end(); I != E; ++I) {
236-
// TODO: We might need a .printJson for I.getKey() as well.
236+
using namespace llvm;
237+
DenseMap<const MemRegion *, std::string> StringifyCache;
238+
auto ToString = [&StringifyCache](const MemRegion *R) {
239+
auto [Place, Inserted] = StringifyCache.try_emplace(R);
240+
if (!Inserted)
241+
return Place->second;
242+
std::string Res;
243+
raw_string_ostream OS(Res);
244+
OS << R;
245+
Place->second = Res;
246+
return Res;
247+
};
248+
249+
using Cluster =
250+
std::pair<const MemRegion *, ImmutableMap<BindingKey, SVal>>;
251+
using Binding = std::pair<BindingKey, SVal>;
252+
253+
const auto MemSpaceBeforeRegionName = [&ToString](const Cluster *L,
254+
const Cluster *R) {
255+
if (isa<MemSpaceRegion>(L->first) && !isa<MemSpaceRegion>(R->first))
256+
return true;
257+
if (!isa<MemSpaceRegion>(L->first) && isa<MemSpaceRegion>(R->first))
258+
return false;
259+
return ToString(L->first) < ToString(R->first);
260+
};
261+
262+
const auto SymbolicBeforeOffset = [&ToString](const BindingKey &L,
263+
const BindingKey &R) {
264+
if (L.hasSymbolicOffset() && !R.hasSymbolicOffset())
265+
return true;
266+
if (!L.hasSymbolicOffset() && R.hasSymbolicOffset())
267+
return false;
268+
if (L.hasSymbolicOffset() && R.hasSymbolicOffset())
269+
return ToString(L.getRegion()) < ToString(R.getRegion());
270+
return L.getOffset() < R.getOffset();
271+
};
272+
273+
const auto DefaultBindingBeforeDirectBindings =
274+
[&SymbolicBeforeOffset](const Binding *LPtr, const Binding *RPtr) {
275+
const BindingKey &L = LPtr->first;
276+
const BindingKey &R = RPtr->first;
277+
if (L.isDefault() && !R.isDefault())
278+
return true;
279+
if (!L.isDefault() && R.isDefault())
280+
return false;
281+
assert(L.isDefault() == R.isDefault());
282+
return SymbolicBeforeOffset(L, R);
283+
};
284+
285+
const auto AddrOf = [](const auto &Item) { return &Item; };
286+
287+
std::vector<const Cluster *> SortedClusters;
288+
SortedClusters.reserve(std::distance(begin(), end()));
289+
append_range(SortedClusters, map_range(*this, AddrOf));
290+
llvm::sort(SortedClusters, MemSpaceBeforeRegionName);
291+
292+
for (auto [Idx, C] : llvm::enumerate(SortedClusters)) {
293+
const auto &[BaseRegion, Bindings] = *C;
237294
Indent(Out, Space, IsDot)
238-
<< "{ \"cluster\": \"" << I.getKey() << "\", \"pointer\": \""
239-
<< (const void *)I.getKey() << "\", \"items\": [" << NL;
295+
<< "{ \"cluster\": \"" << BaseRegion << "\", \"pointer\": \""
296+
<< (const void *)BaseRegion << "\", \"items\": [" << NL;
297+
298+
std::vector<const Binding *> SortedBindings;
299+
SortedBindings.reserve(std::distance(Bindings.begin(), Bindings.end()));
300+
append_range(SortedBindings, map_range(Bindings, AddrOf));
301+
llvm::sort(SortedBindings, DefaultBindingBeforeDirectBindings);
240302

241303
++Space;
242-
const ClusterBindings &CB = I.getData();
243-
for (ClusterBindings::iterator CI = CB.begin(), CE = CB.end(); CI != CE;
244-
++CI) {
245-
Indent(Out, Space, IsDot) << "{ " << CI.getKey() << ", \"value\": ";
246-
CI.getData().printJson(Out, /*AddQuotes=*/true);
304+
for (auto [Idx, B] : llvm::enumerate(SortedBindings)) {
305+
const auto &[Key, Value] = *B;
306+
Indent(Out, Space, IsDot) << "{ " << Key << ", \"value\": ";
307+
Value.printJson(Out, /*AddQuotes=*/true);
247308
Out << " }";
248-
if (std::next(CI) != CE)
309+
if (Idx != SortedBindings.size() - 1)
249310
Out << ',';
250311
Out << NL;
251312
}
252-
253313
--Space;
254314
Indent(Out, Space, IsDot) << "]}";
255-
if (std::next(I) != E)
315+
if (Idx != SortedClusters.size() - 1)
256316
Out << ',';
257317
Out << NL;
258318
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s 2>&1 | FileCheck %s
2+
// expected-no-diagnostics
3+
4+
void clang_analyzer_printState();
5+
6+
struct Member {
7+
int large[10];
8+
};
9+
Member getMember();
10+
11+
struct Class {
12+
Member m;
13+
int first;
14+
int second;
15+
int third;
16+
};
17+
18+
19+
void test_output(int n) {
20+
Class objsecond;
21+
objsecond.m.large[n] = 20;
22+
23+
Class objfirst;
24+
25+
objfirst.m = getMember();
26+
objfirst.second = 2;
27+
objfirst.third = 3;
28+
objfirst.first = 1;
29+
30+
clang_analyzer_printState();
31+
// Default binding is before any direct bindings.
32+
// Direct bindings are increasing by offset.
33+
// Global memory space clusters come before any other clusters.
34+
// Otherwise, Clusters are in alphabetical order.
35+
36+
// CHECK: "store": { "pointer": "0x{{[0-9a-f]+}}", "items": [
37+
// CHECK-NEXT: { "cluster": "GlobalInternalSpaceRegion", "pointer": "0x{{[0-9a-f]+}}", "items": [
38+
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$
39+
// CHECK-NEXT: ]},
40+
// CHECK-NEXT: { "cluster": "GlobalSystemSpaceRegion", "pointer": "0x{{[0-9a-f]+}}", "items": [
41+
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$
42+
// CHECK-NEXT: ]},
43+
// CHECK-NEXT: { "cluster": "objfirst", "pointer": "0x{{[0-9a-f]+}}", "items": [
44+
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "lazyCompoundVal
45+
// CHECK-NEXT: { "kind": "Direct", "offset": 320, "value": "1 S32b" },
46+
// CHECK-NEXT: { "kind": "Direct", "offset": 352, "value": "2 S32b" },
47+
// CHECK-NEXT: { "kind": "Direct", "offset": 384, "value": "3 S32b" }
48+
// CHECK-NEXT: ]},
49+
// CHECK-NEXT: { "cluster": "objsecond", "pointer": "0x{{[0-9a-f]+}}", "items": [
50+
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "Unknown" },
51+
// CHECK-NEXT: { "kind": "Direct", "offset": null, "value": "20 S32b" }
52+
// CHECK-NEXT: ]}
53+
// CHECK-NEXT: ]},
54+
55+
(void)objfirst;
56+
(void)objsecond;
57+
}

0 commit comments

Comments
 (0)