[clang] [analyzer] Don't copy field-by-field conjured LazyCompoundVals (2/4) (PR #115917)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 14 05:28:44 PST 2024


================
@@ -2609,9 +2611,42 @@ RegionBindingsRef RegionStoreManager::bindVector(RegionBindingsConstRef B,
   return NewB;
 }
 
+std::optional<SVal>
+RegionStoreManager::getUniqueDefaultBinding(Store S,
+                                            const MemRegion *BaseR) const {
+  assert(BaseR == BaseR->getBaseRegion() && "Expecting a base region");
+  const auto *Cluster = getRegionBindings(S).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 (LCV.getRegion()->getBaseRegion() == LCV.getRegion()) {
+    if (auto Val = getUniqueDefaultBinding(LCV.getStore(), LCV.getRegion())) {
----------------
steakhal wrote:

This was the motive, yes.

https://github.com/llvm/llvm-project/pull/115917


More information about the cfe-commits mailing list