Skip to content

Commit b7eb40d

Browse files
Revert "[analyzer] Don't copy field-by-field conjured LazyCompoundVals (2/4) (llvm#115917)"
This reverts commit 4610e5c. Resolved conflicts in: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
1 parent 1775b58 commit b7eb40d

File tree

3 files changed

+7
-43
lines changed

3 files changed

+7
-43
lines changed

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,6 @@ class RegionStoreManager : public StoreManager {
714714
return getBinding(getRegionBindings(S), L, T);
715715
}
716716

717-
std::optional<SVal>
718-
getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const;
719-
720717
std::optional<SVal> getDefaultBinding(Store S, const MemRegion *R) override {
721718
RegionBindingsRef B = getRegionBindings(S);
722719
// Default bindings are always applied over a base region so look up the
@@ -2750,46 +2747,12 @@ RegionStoreManager::bindVector(LimitedRegionBindingsConstRef B,
27502747
return NewB;
27512748
}
27522749

2753-
std::optional<SVal>
2754-
RegionStoreManager::getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const {
2755-
const MemRegion *BaseR = LCV.getRegion();
2756-
2757-
// We only handle base regions.
2758-
if (BaseR != BaseR->getBaseRegion())
2759-
return std::nullopt;
2760-
2761-
const auto *Cluster = getRegionBindings(LCV.getStore()).lookup(BaseR);
2762-
if (!Cluster || !llvm::hasSingleElement(*Cluster))
2763-
return std::nullopt;
2764-
2765-
const auto [Key, Value] = *Cluster->begin();
2766-
return Key.isDirect() ? std::optional<SVal>{} : Value;
2767-
}
2768-
27692750
std::optional<LimitedRegionBindingsRef> RegionStoreManager::tryBindSmallStruct(
27702751
LimitedRegionBindingsConstRef B, const TypedValueRegion *R,
27712752
const RecordDecl *RD, nonloc::LazyCompoundVal LCV) {
27722753
if (B.hasExhaustedBindingLimit())
27732754
return B.withValuesEscaped(LCV);
27742755

2775-
// If we try to copy a Conjured value representing the value of the whole
2776-
// struct, don't try to element-wise copy each field.
2777-
// That would unnecessarily bind Derived symbols slicing off the subregion for
2778-
// the field from the whole Conjured symbol.
2779-
//
2780-
// struct Window { int width; int height; };
2781-
// Window getWindow(); <-- opaque fn.
2782-
// Window w = getWindow(); <-- conjures a new Window.
2783-
// Window w2 = w; <-- trivial copy "w", calling "tryBindSmallStruct"
2784-
//
2785-
// We should not end up with a new Store for "w2" like this:
2786-
// Direct [ 0..31]: Derived{Conj{}, w.width}
2787-
// Direct [32..63]: Derived{Conj{}, w.height}
2788-
// Instead, we should just bind that Conjured value instead.
2789-
if (std::optional<SVal> Val = getUniqueDefaultBinding(LCV)) {
2790-
return B.addBinding(BindingKey::Make(R, BindingKey::Default), Val.value());
2791-
}
2792-
27932756
FieldVector Fields;
27942757

27952758
if (const CXXRecordDecl *Class = dyn_cast<CXXRecordDecl>(RD))

clang/test/Analysis/ctor-trivial-copy.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,8 @@ void _02_structs_with_members() {
8181
clang_analyzer_dump_val(Aggr2); // expected-warning {{lazyCompoundVal}}
8282
clang_analyzer_dump_val(Aggr3); // expected-warning {{lazyCompoundVal}}
8383

84-
// We should have the same Conjured symbol for "Aggr", "Aggr2" and "Aggr3".
85-
// We used to have Derived symbols for the individual fields that were
86-
// copied as part of copying the whole struct.
84+
// We have fields in the struct we copy, thus we also have the entries for the copies
85+
// (and for all of their fields).
8786
clang_analyzer_printState();
8887
// CHECK: "store": { "pointer": "0x{{[0-9a-f]+}}", "items": [
8988
// CHECK-NEXT: { "cluster": "GlobalInternalSpaceRegion", "pointer": "0x{{[0-9a-f]+}}", "items": [
@@ -96,10 +95,12 @@ void _02_structs_with_members() {
9695
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "[[AGGR_CONJ:conj_\$[0-9]+{int, LC[0-9]+, S[0-9]+, #[0-9]+}]]" }
9796
// CHECK-NEXT: ]},
9897
// CHECK-NEXT: { "cluster": "Aggr2", "pointer": "0x{{[0-9a-f]+}}", "items": [
99-
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "[[AGGR_CONJ]]" }
98+
// CHECK-NEXT: { "kind": "Direct", "offset": 0, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.x}" },
99+
// CHECK-NEXT: { "kind": "Direct", "offset": 32, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.y}" }
100100
// CHECK-NEXT: ]},
101101
// CHECK-NEXT: { "cluster": "Aggr3", "pointer": "0x{{[0-9a-f]+}}", "items": [
102-
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "[[AGGR_CONJ]]" }
102+
// CHECK-NEXT: { "kind": "Direct", "offset": 0, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.x}" },
103+
// CHECK-NEXT: { "kind": "Direct", "offset": 32, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.y}" }
103104
// CHECK-NEXT: ]}
104105
// CHECK-NEXT: ]},
105106

clang/test/Analysis/store-dump-orders.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ void test_output(int n) {
4141
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$
4242
// CHECK-NEXT: ]},
4343
// CHECK-NEXT: { "cluster": "objfirst", "pointer": "0x{{[0-9a-f]+}}", "items": [
44-
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$
44+
// CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "lazyCompoundVal
4545
// CHECK-NEXT: { "kind": "Direct", "offset": 320, "value": "1 S32b" },
4646
// CHECK-NEXT: { "kind": "Direct", "offset": 352, "value": "2 S32b" },
4747
// CHECK-NEXT: { "kind": "Direct", "offset": 384, "value": "3 S32b" }

0 commit comments

Comments
 (0)