diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index f46282a73fbe4..46e294a1741cf 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -608,6 +608,9 @@ class RegionStoreManager : public StoreManager { return getBinding(getRegionBindings(S), L, T); } + std::optional + getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const; + std::optional getDefaultBinding(Store S, const MemRegion *R) override { RegionBindingsRef B = getRegionBindings(S); // Default bindings are always applied over a base region so look up the @@ -2605,9 +2608,43 @@ RegionBindingsRef RegionStoreManager::bindVector(RegionBindingsConstRef B, return NewB; } +std::optional +RegionStoreManager::getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const { + const MemRegion *BaseR = LCV.getRegion(); + + // We only handle base regions. + if (BaseR != BaseR->getBaseRegion()) + return std::nullopt; + + const auto *Cluster = getRegionBindings(LCV.getStore()).lookup(BaseR); + if (!Cluster || !llvm::hasSingleElement(*Cluster)) + return std::nullopt; + + const auto [Key, Value] = *Cluster->begin(); + return Key.isDirect() ? std::optional{} : Value; +} + std::optional RegionStoreManager::tryBindSmallStruct( RegionBindingsConstRef B, const TypedValueRegion *R, const RecordDecl *RD, nonloc::LazyCompoundVal LCV) { + // If we try to copy a Conjured value representing the value of the whole + // struct, don't try to element-wise copy each field. + // That would unnecessarily bind Derived symbols slicing off the subregion for + // the field from the whole Conjured symbol. + // + // struct Window { int width; int height; }; + // Window getWindow(); <-- opaque fn. + // Window w = getWindow(); <-- conjures a new Window. + // Window w2 = w; <-- trivial copy "w", calling "tryBindSmallStruct" + // + // We should not end up with a new Store for "w2" like this: + // Direct [ 0..31]: Derived{Conj{}, w.width} + // Direct [32..63]: Derived{Conj{}, w.height} + // Instead, we should just bind that Conjured value instead. + if (std::optional Val = getUniqueDefaultBinding(LCV)) { + return B.addBinding(BindingKey::Make(R, BindingKey::Default), Val.value()); + } + FieldVector Fields; if (const CXXRecordDecl *Class = dyn_cast(RD)) diff --git a/clang/test/Analysis/ctor-trivial-copy.cpp b/clang/test/Analysis/ctor-trivial-copy.cpp index a1209c24136e2..ab623c1919e15 100644 --- a/clang/test/Analysis/ctor-trivial-copy.cpp +++ b/clang/test/Analysis/ctor-trivial-copy.cpp @@ -83,8 +83,9 @@ void _02_structs_with_members() { clang_analyzer_dump_val(Aggr2); // expected-warning {{lazyCompoundVal}} clang_analyzer_dump_val(Aggr3); // expected-warning {{lazyCompoundVal}} - // We have fields in the struct we copy, thus we also have the entries for the copies - // (and for all of their fields). + // We should have the same Conjured symbol for "Aggr", "Aggr2" and "Aggr3". + // We used to have Derived symbols for the individual fields that were + // copied as part of copying the whole struct. clang_analyzer_printState(); // CHECK: "store": { "pointer": "0x{{[0-9a-f]+}}", "items": [ // CHECK-NEXT: { "cluster": "GlobalInternalSpaceRegion", "pointer": "0x{{[0-9a-f]+}}", "items": [ @@ -97,12 +98,10 @@ void _02_structs_with_members() { // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "[[AGGR_CONJ:conj_\$[0-9]+{int, LC[0-9]+, S[0-9]+, #[0-9]+}]]" } // CHECK-NEXT: ]}, // CHECK-NEXT: { "cluster": "Aggr2", "pointer": "0x{{[0-9a-f]+}}", "items": [ - // CHECK-NEXT: { "kind": "Direct", "offset": 0, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.x}" }, - // CHECK-NEXT: { "kind": "Direct", "offset": 32, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.y}" } + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "[[AGGR_CONJ]]" } // CHECK-NEXT: ]}, // CHECK-NEXT: { "cluster": "Aggr3", "pointer": "0x{{[0-9a-f]+}}", "items": [ - // CHECK-NEXT: { "kind": "Direct", "offset": 0, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.x}" }, - // CHECK-NEXT: { "kind": "Direct", "offset": 32, "value": "derived_${{[0-9]+}}{[[AGGR_CONJ]],Aggr.y}" } + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "[[AGGR_CONJ]]" } // CHECK-NEXT: ]} // CHECK-NEXT: ]}, diff --git a/clang/test/Analysis/store-dump-orders.cpp b/clang/test/Analysis/store-dump-orders.cpp index d99f581f00fe1..dbe93f1c5183a 100644 --- a/clang/test/Analysis/store-dump-orders.cpp +++ b/clang/test/Analysis/store-dump-orders.cpp @@ -41,7 +41,7 @@ void test_output(int n) { // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$ // CHECK-NEXT: ]}, // CHECK-NEXT: { "cluster": "objfirst", "pointer": "0x{{[0-9a-f]+}}", "items": [ - // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "lazyCompoundVal + // CHECK-NEXT: { "kind": "Default", "offset": 0, "value": "conj_$ // CHECK-NEXT: { "kind": "Direct", "offset": 320, "value": "1 S32b" }, // CHECK-NEXT: { "kind": "Direct", "offset": 352, "value": "2 S32b" }, // CHECK-NEXT: { "kind": "Direct", "offset": 384, "value": "3 S32b" }