Author: Balazs Benics Date: 2024-11-14T16:28:37+01:00 New Revision: 4610e5c78647983f79d1bd5264afff254774e13e
URL: https://github.com/llvm/llvm-project/commit/4610e5c78647983f79d1bd5264afff254774e13e DIFF: https://github.com/llvm/llvm-project/commit/4610e5c78647983f79d1bd5264afff254774e13e.diff LOG: [analyzer] Don't copy field-by-field conjured LazyCompoundVals (2/4) (#115917) Split from #114835 Added: Modified: clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/test/Analysis/ctor-trivial-copy.cpp clang/test/Analysis/store-dump-orders.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index f46282a73fbe40..46e294a1741cfe 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<SVal> + getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const; + std::optional<SVal> 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<SVal> +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<SVal>{} : Value; +} + std::optional<RegionBindingsRef> 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<SVal> Val = getUniqueDefaultBinding(LCV)) { + return B.addBinding(BindingKey::Make(R, BindingKey::Default), Val.value()); + } + FieldVector Fields; if (const CXXRecordDecl *Class = dyn_cast<CXXRecordDecl>(RD)) diff --git a/clang/test/Analysis/ctor-trivial-copy.cpp b/clang/test/Analysis/ctor-trivial-copy.cpp index a1209c24136e20..ab623c1919e152 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 d99f581f00fe14..dbe93f1c5183a9 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" } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
