[llvm-branch-commits] [clang] [SSAF] Close unsafe-buffer reachability over override families (PR #213319)

Balázs Benics via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Fri Aug 7 04:59:09 PDT 2026


https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/213319

>From e732ce41a9f04f49ed9e8f8fd0bba13f13582a3f Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Fri, 31 Jul 2026 16:20:01 +0100
Subject: [PATCH] [SSAF] Close unsafe-buffer reachability over override
 families
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

An unsafe pointer reaching one override's parameter is equally unsafe in every
sibling and base override of that method, because the call site picks the
target dynamically. Without closing over the families, reachability depended
on which override the extractor happened to see the flow through, so a fix
suggested for the base could be contradicted by a derived override.

Mirroring is level-preserving: families relate slot entities, so a reachable
EPL propagates only to the same pointer level on its family members.

The closure runs after the pointer-flow DFS has converged and does not feed
its own output back in, so a flow edge out of a newly discovered EPL is still
missed. FamilyClosureDoesNotRerunDFS pins that gap.

§4 of rdar://179151603
---
 .../UnsafeBufferUsageAnalysis.cpp             |  98 +++++-
 .../external-inline-function-in-multi-tu.test |   2 +-
 .../PointerFlow/lref-to-rref-cast.test        |   2 +-
 .../PointerFlow/multi-decl-contributor.cpp    |   2 +-
 .../multi-dim-pointer-flow-constraint.test    |   2 +-
 .../UnsafeBufferReachableAnalysisTest.cpp     | 327 +++++++++++++++++-
 6 files changed, 416 insertions(+), 17 deletions(-)

diff --git a/clang/lib/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.cpp b/clang/lib/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.cpp
index e404eb294ee4b..5781c892e5aea 100644
--- a/clang/lib/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.cpp
+++ b/clang/lib/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.cpp
@@ -17,12 +17,18 @@
 #include "clang/ScalableStaticAnalysis/Analyses/EntityPointerLevel/EntityPointerLevelFormat.h"
 #include "clang/ScalableStaticAnalysis/Analyses/PointerFlow/PointerFlowAnalysis.h"
 #include "clang/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h"
+#include "clang/ScalableStaticAnalysis/Analyses/VirtualMethodFamily/VirtualMethodFamily.h"
 #include "clang/ScalableStaticAnalysis/Core/Serialization/JSONFormat.h"
 #include "clang/ScalableStaticAnalysis/Core/WholeProgramAnalysis/AnalysisRegistry.h"
 #include "clang/ScalableStaticAnalysis/Core/WholeProgramAnalysis/SummaryAnalysis.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/JSON.h"
 #include <memory>
+#include <utility>
 
 using namespace clang::ssaf;
 using namespace llvm;
@@ -127,10 +133,14 @@ JSONFormat::AnalysisResultRegistry::Add<UnsafeBufferReachableAnalysisResult>
 /// Computes all the reachable "nodes" (pointers) in a pointer flow graph from a
 /// provided starter node set.  Specifically, the starter set is the unsafe
 /// pointers found by `UnsafeBufferUsageAnalysis`.
+///
+/// After the forward DFS converges, the analysis runs a family-closure pass
+/// using the `VirtualMethodFamilyAnalysisResult` to propagate reachability
+/// across virtual-method override equivalence classes.
 class UnsafeBufferReachableAnalysis
-    : public DerivedAnalysis<UnsafeBufferReachableAnalysisResult,
-                             PointerFlowAnalysisResult,
-                             UnsafeBufferUsageAnalysisResult> {
+    : public DerivedAnalysis<
+          UnsafeBufferReachableAnalysisResult, PointerFlowAnalysisResult,
+          UnsafeBufferUsageAnalysisResult, VirtualMethodFamilyAnalysisResult> {
 
   /// BoundsPropagationGraph adds bounds propagation semantics to the
   /// pointer-flow graph, which represents the set of static pointer assignment
@@ -186,6 +196,7 @@ class UnsafeBufferReachableAnalysis
   };
 
   std::map<EntityId, BoundsPropagationGraph> BPG;
+  const VirtualMethodFamilyAnalysisResult *Family = nullptr;
 
   // Use pointers for efficiency. EPLs are in tree-based containers that only
   // grow. So pointers to them are stable.
@@ -207,12 +218,74 @@ class UnsafeBufferReachableAnalysis
     }
   }
 
+  // Run the family-closure pass: propagate reachability across virtual-method
+  // override equivalence classes recorded in `Family`.
+  //
+  // `Family` records equivalence at *slot-entity* granularity. So if slot
+  // entity `e` is equivalent to slot entity `e'`, then EPL(e, L) is equivalent
+  // to EPL(e', L) at *every* level L. This pass is therefore level-preserving:
+  // it reconstructs the per-level relationship by keeping the level L of the
+  // reachable EPL it already holds.
+  //
+  // A single pass suffices: mirroring EPL(e, L) onto every member of e's
+  // family already covers every (family, level) pair the input contains, and
+  // the EPLs it inserts map back to the same family at the same level.
+  //
+  // FIXME: This pass runs after the pointer-flow DFS has converged and
+  // `step()` returns false, so the EPLs discovered here are never fed back
+  // through the pointer-flow graph. If closure discovers EPL(e', L) and an
+  // edge EPL(e', L) -> EPL(x, L) exists, EPL(x, L) is missed. Fixing this
+  // requires iterating the DFS and this pass to a fixpoint; see
+  // UnsafeBufferReachableAnalysisTest.FamilyClosureDoesNotRerunDFS.
+  void runFamilyClosurePass() {
+    auto &Reachables = getResult().Reachables;
+    const auto &RetAndParamData = Family->RetAndParamData;
+
+    if (RetAndParamData.empty())
+      return;
+
+    struct Member {
+      EntityId RetOrParamId;
+      EntityId OwnerMethodId;
+    };
+    llvm::DenseMap<EntityId, llvm::SmallVector<Member, 2>> MembersOfFamily;
+    for (const auto &[ParamId, Data] : RetAndParamData) {
+      MembersOfFamily[Data.FamilyId].push_back({ParamId, Data.OwnerMethodId});
+    }
+
+    // Collect the (family, level) pairs to mirror. This has to be a snapshot
+    // because the loop below inserts into `Reachables`.
+    DenseSet<std::pair<EntityId, unsigned>> FamilyLevels;
+    for (const EntityPointerLevelSet &EPLs : make_second_range(Reachables)) {
+      for (const EntityPointerLevel &E : EPLs) {
+        if (auto It = RetAndParamData.find(E.getEntity());
+            It != RetAndParamData.end()) {
+          FamilyLevels.insert({It->second.FamilyId, E.getPointerLevel()});
+        }
+      }
+    }
+
+    // A reachable EPL(e, L) makes EPL(e', L) reachable for
+    // every entity e' in family(e).
+    for (auto [FamilyId, Level] : FamilyLevels) {
+      auto It = MembersOfFamily.find(FamilyId);
+      if (It == MembersOfFamily.end())
+        continue;
+      for (const Member &M : It->second) {
+        Reachables[M.OwnerMethodId].insert(
+            buildEntityPointerLevel(M.RetOrParamId, Level));
+      }
+    }
+  }
+
 public:
   llvm::Error
   initialize(const PointerFlowAnalysisResult &PtrFlowGraph,
-             const UnsafeBufferUsageAnalysisResult &Starter) override {
+             const UnsafeBufferUsageAnalysisResult &Starter,
+             const VirtualMethodFamilyAnalysisResult &Family) override {
     for (auto &[Id, SubGraph] : PtrFlowGraph.Edges)
       BPG.try_emplace(Id, BoundsPropagationGraph(SubGraph));
+    this->Family = &Family;
     assert(getResult().Reachables.empty());
     getResult().Reachables.insert(Starter.begin(), Starter.end());
     return llvm::Error::success();
@@ -233,14 +306,27 @@ class UnsafeBufferReachableAnalysis
 
       updateReachablesWithOutgoings(Node, Worklist);
     }
-    // This is not an iterative algorithm so stop iteration by retruning false:
+
+    // After the forward DFS converges, run a family-closure pass that
+    // propagates reachability across virtual-method override equivalence
+    // classes.
+    runFamilyClosurePass();
+
+    // The DFS is not an iterative algorithm, so stop iterating by returning
+    // false.
+    // FIXME: `runFamilyClosurePass()` above may have made new EPLs reachable,
+    // and those are never pushed back through the pointer-flow graph. Reaching
+    // a true fixpoint requires alternating the DFS and the closure pass until
+    // neither makes progress; see
+    // UnsafeBufferReachableAnalysisTest.FamilyClosureDoesNotRerunDFS.
     return false;
   }
 };
 
 AnalysisRegistry::Add<UnsafeBufferReachableAnalysis>
     RegisterUnsafeBufferReachableAnalysis(
-        "Reachable pointers from unsafe buffer usage in pointer flow graph");
+        "Reachable pointers from unsafe buffer usage in pointer flow graph, "
+        "family-closed across virtual method overrides");
 
 } // namespace
 
diff --git a/clang/test/Analysis/Scalable/PointerFlow/external-inline-function-in-multi-tu.test b/clang/test/Analysis/Scalable/PointerFlow/external-inline-function-in-multi-tu.test
index b0ffc1b3cf947..33a7921ccbfc6 100644
--- a/clang/test/Analysis/Scalable/PointerFlow/external-inline-function-in-multi-tu.test
+++ b/clang/test/Analysis/Scalable/PointerFlow/external-inline-function-in-multi-tu.test
@@ -4,7 +4,7 @@
 // during bounds propagation.
 
 
-// DEFINE: %{extract} = %clang_cc1 -fsyntax-only -I %t --ssaf-extract-summaries=PointerFlow,UnsafeBufferUsage
+// DEFINE: %{extract} = %clang_cc1 -fsyntax-only -I %t --ssaf-extract-summaries=PointerFlow,UnsafeBufferUsage,VirtualMethod
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t
diff --git a/clang/test/Analysis/Scalable/PointerFlow/lref-to-rref-cast.test b/clang/test/Analysis/Scalable/PointerFlow/lref-to-rref-cast.test
index ca5df041240aa..1a7b168168cab 100644
--- a/clang/test/Analysis/Scalable/PointerFlow/lref-to-rref-cast.test
+++ b/clang/test/Analysis/Scalable/PointerFlow/lref-to-rref-cast.test
@@ -6,7 +6,7 @@
 
 // Extract per-TU PointerFlow + UnsafeBufferUsage summaries.
 // RUN: %clang_cc1 -fsyntax-only %t/tu.cpp \
-// RUN:   --ssaf-extract-summaries=PointerFlow,UnsafeBufferUsage \
+// RUN:   --ssaf-extract-summaries=PointerFlow,UnsafeBufferUsage,VirtualMethod \
 // RUN:   --ssaf-tu-summary-file=%t/tu.summary.json \
 // RUN:   --ssaf-compilation-unit-id="tu-1"
 
diff --git a/clang/test/Analysis/Scalable/PointerFlow/multi-decl-contributor.cpp b/clang/test/Analysis/Scalable/PointerFlow/multi-decl-contributor.cpp
index 717a2875636b2..3acc5f604b5f7 100644
--- a/clang/test/Analysis/Scalable/PointerFlow/multi-decl-contributor.cpp
+++ b/clang/test/Analysis/Scalable/PointerFlow/multi-decl-contributor.cpp
@@ -2,7 +2,7 @@
 
 
 // RUN: %clang_cc1 -fsyntax-only %s \
-// RUN:   --ssaf-extract-summaries=PointerFlow,UnsafeBufferUsage \
+// RUN:   --ssaf-extract-summaries=PointerFlow,UnsafeBufferUsage,VirtualMethod \
 // RUN:   --ssaf-tu-summary-file=%t/tu.summary.json \
 // RUN:   --ssaf-compilation-unit-id="tu-1"
 
diff --git a/clang/test/Analysis/Scalable/PointerFlow/multi-dim-pointer-flow-constraint.test b/clang/test/Analysis/Scalable/PointerFlow/multi-dim-pointer-flow-constraint.test
index 511b375c05f6c..be74f68d09ddf 100644
--- a/clang/test/Analysis/Scalable/PointerFlow/multi-dim-pointer-flow-constraint.test
+++ b/clang/test/Analysis/Scalable/PointerFlow/multi-dim-pointer-flow-constraint.test
@@ -6,7 +6,7 @@
 
 
 // RUN: %clang_cc1 -fsyntax-only %t/src.cpp \
-// RUN:   --ssaf-extract-summaries=PointerFlow,UnsafeBufferUsage \
+// RUN:   --ssaf-extract-summaries=PointerFlow,UnsafeBufferUsage,VirtualMethod \
 // RUN:   --ssaf-compilation-unit-id="tu-1" \
 // RUN:   --ssaf-tu-summary-file=%t/src.summary.json
 
diff --git a/clang/unittests/ScalableStaticAnalysis/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp b/clang/unittests/ScalableStaticAnalysis/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
index cf5420b35e7dc..e773398d300b7 100644
--- a/clang/unittests/ScalableStaticAnalysis/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
+++ b/clang/unittests/ScalableStaticAnalysis/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
@@ -12,6 +12,7 @@
 #include "clang/ScalableStaticAnalysis/Analyses/PointerFlow/PointerFlowAnalysis.h"
 #include "clang/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h"
 #include "clang/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.h"
+#include "clang/ScalableStaticAnalysis/Analyses/VirtualMethodFamily/VirtualMethodFamily.h"
 #include "clang/ScalableStaticAnalysis/Core/EntityLinker/LUSummary.h"
 #include "clang/ScalableStaticAnalysis/Core/Model/BuildNamespace.h"
 #include "clang/ScalableStaticAnalysis/Core/Model/EntityId.h"
@@ -24,6 +25,10 @@
 #include <map>
 #include <memory>
 #include <optional>
+#include <ostream>
+#include <set>
+#include <utility>
+#include <vector>
 
 using namespace clang;
 using namespace ssaf;
@@ -36,6 +41,76 @@ extern UnsafeBufferUsageEntitySummary
 
 namespace {
 
+/// An entity at a pointer level, with the entity spelled as the letter naming
+/// it in a test's layout.
+using Node = std::pair<char, unsigned>;
+using Edge = std::pair<Node, Node>;
+
+/// One VirtualMethodSummary, field for field, with entities spelled as letters.
+/// By convention tests use uppercase letters for method entities and lowercase
+/// ones for the slots they own, but the two are not distinguished: every letter
+/// is just an entity.
+struct MethodLayout {
+  char Method;                 ///< Scope the summary is stored under.
+  std::vector<char> Params;    ///< VirtualMethodSummary::ParamEntities.
+  std::optional<char> Ret;     ///< VirtualMethodSummary::ReturnEntity.
+  std::vector<char> Overrides; ///< VirtualMethodSummary::OverriddenMethods.
+};
+
+/// An edge, and the scope (method entity) whose pointer-flow graph owns it.
+using ScopedEdge = std::pair<char, Edge>;
+
+/// Nodes grouped by the scope (method entity) that owns them: the starter
+/// layout of a family-closure test, the entries its closure pass is expected to
+/// add, and the `Reachables` map it produces are all spelled this way.
+///
+/// Wrapped in a class rather than used as a plain map so that it can be added
+/// to with `operator+` and so that a failing comparison prints
+/// "{B: p at 1, p at 2; D: q at 1}" instead of gtest's raw char dump.
+class ScopedNodes {
+  std::map<char, std::set<Node>> Scopes;
+
+public:
+  using value_type = std::map<char, std::set<Node>>::value_type;
+
+  ScopedNodes() = default;
+  ScopedNodes(std::initializer_list<value_type> Init) : Scopes(Init) {}
+
+  void insert(char Scope, Node N) { Scopes[Scope].insert(N); }
+
+  const std::map<char, std::set<Node>> &scopes() const { return Scopes; }
+
+  bool operator==(const ScopedNodes &Other) const {
+    return Scopes == Other.Scopes;
+  }
+  bool operator!=(const ScopedNodes &Other) const { return !(*this == Other); }
+};
+
+/// The union of \p L and \p R. Lets a test state its expectation as "the
+/// starters, plus what closure adds".
+ScopedNodes operator+(const ScopedNodes &L, const ScopedNodes &R) {
+  ScopedNodes Result = L;
+  for (const auto &[Scope, Nodes] : R.scopes())
+    for (const Node &N : Nodes)
+      Result.insert(Scope, N);
+  return Result;
+}
+
+void PrintTo(const ScopedNodes &SN, std::ostream *OS) {
+  *OS << "{";
+  const char *ScopeSep = "";
+  for (const auto &[Scope, Nodes] : SN.scopes()) {
+    *OS << ScopeSep << Scope << ": ";
+    ScopeSep = "; ";
+    const char *NodeSep = "";
+    for (const auto &[Slot, Level] : Nodes) {
+      *OS << NodeSep << Slot << "@" << Level;
+      NodeSep = ", ";
+    }
+  }
+  *OS << "}";
+}
+
 class UnsafeBufferReachableAnalysisTest : public TestFixture {
 protected:
   using EPLEdge = std::pair<EntityPointerLevel, EntityPointerLevel>;
@@ -59,6 +134,25 @@ class UnsafeBufferReachableAnalysisTest : public TestFixture {
     return Id;
   }
 
+  /// Insert a VirtualMethodSummary keyed by the method's own EntityId.
+  void insertVirtualMethodSummary(LUSummary &LU, EntityId Id,
+                                  VirtualMethodSummary Sum) {
+    getData(LU)[VirtualMethodSummary::summaryName()][Id] =
+        std::make_unique<VirtualMethodSummary>(std::move(Sum));
+  }
+
+  /// Build a VirtualMethodSummary from its parameter entities, optional
+  /// return-slot entity, and direct override edges.
+  VirtualMethodSummary makeMethodSummary(std::vector<EntityId> ParamEntities,
+                                         std::optional<EntityId> RetEntity,
+                                         std::vector<EntityId> Overridden) {
+    VirtualMethodSummary S;
+    S.ParamEntities = std::move(ParamEntities);
+    S.ReturnEntity = RetEntity;
+    S.OverriddenMethods = std::move(Overridden);
+    return S;
+  }
+
   /// Insert a PointerFlowEntitySummary for an entity.
   void insertPointerFlowSummary(LUSummary &LU, EntityId Id, EdgeSet Edges) {
     getData(LU)[PointerFlowEntitySummary::summaryName()][Id] =
@@ -115,9 +209,9 @@ class UnsafeBufferReachableAnalysisTest : public TestFixture {
     insertUnsafeBufferUsageSummary(LU, Id, std::move(Starters));
   }
 
-  /// Run the driver and return the flattened reachable EPL set.
-  std::optional<EntityPointerLevelSet>
-  computeReachables(std::unique_ptr<LUSummary> LU, unsigned Line) {
+  /// Run the driver and return the full per-scope `Reachables` map.
+  std::optional<std::map<EntityId, EntityPointerLevelSet>>
+  computeReachablesByScope(std::unique_ptr<LUSummary> LU, unsigned Line) {
     AnalysisDriver Driver(std::move(LU));
     auto WPAOrErr =
         Driver.run<PointerFlowAnalysisResult, UnsafeBufferUsageAnalysisResult,
@@ -131,15 +225,22 @@ class UnsafeBufferReachableAnalysisTest : public TestFixture {
       ADD_FAILURE_AT(__FILE__, Line) << llvm::toString(ROrErr.takeError());
       return std::nullopt;
     }
+    return ROrErr->Reachables;
+  }
+
+  /// Run the driver and return the reachable EPLs of every scope, flattened
+  /// into a single set.
+  std::optional<EntityPointerLevelSet>
+  computeReachables(std::unique_ptr<LUSummary> LU, unsigned Line) {
+    auto ByScope = computeReachablesByScope(std::move(LU), Line);
+    if (!ByScope)
+      return std::nullopt;
     EntityPointerLevelSet Result;
-    for (const auto &[Id, EPLs] : ROrErr->Reachables)
+    for (const EntityPointerLevelSet &EPLs : llvm::make_second_range(*ByScope))
       Result.insert(EPLs.begin(), EPLs.end());
     return Result;
   }
 
-  using Node = std::pair<char, unsigned>;
-  using Edge = std::pair<Node, Node>;
-
   // FIXME: When we use more advanced search algorithms, it may involve
   // a divide-and-conquer approach on sub-graphs organized by contributors.
   // In that case, we may want to enumerate all possible partitions of
@@ -183,6 +284,101 @@ class UnsafeBufferReachableAnalysisTest : public TestFixture {
 
     return Result;
   }
+
+  /// Compute reachables per scope for the virtual-method hierarchy described by
+  /// \p Methods, seeded with \p StarterLayout and, optionally, the pointer-flow
+  /// edges in \p EdgeLayout. Both starters and edges name the scope that owns
+  /// them, because family closure is about which scope an EPL ends up under.
+  ///
+  /// The entity domain is the set of letters the layouts mention. Only scopes
+  /// named by a starter or an edge get PointerFlow/UnsafeBufferUsage summaries:
+  /// UnsafeBufferUsageAnalysis records even an empty summary, which would then
+  /// show up as an empty entry in `Reachables`.
+  ScopedNodes familyClosure(llvm::ArrayRef<MethodLayout> Methods,
+                            const ScopedNodes &StarterLayout,
+                            llvm::ArrayRef<ScopedEdge> EdgeLayout,
+                            unsigned Line) {
+    auto LU = makeLUSummary();
+    auto Entities =
+        createEntities(*LU, entityDomainOf(Methods, StarterLayout, EdgeLayout));
+    auto GetEPL = [&Entities](const Node &N) -> EntityPointerLevel {
+      return buildEntityPointerLevel(Entities[N.first], N.second);
+    };
+
+    for (const MethodLayout &M : Methods) {
+      std::vector<EntityId> Params;
+      for (char P : M.Params)
+        Params.push_back(Entities[P]);
+      std::vector<EntityId> Overrides;
+      for (char O : M.Overrides)
+        Overrides.push_back(Entities[O]);
+      std::optional<EntityId> Ret;
+      if (M.Ret)
+        Ret = Entities[*M.Ret];
+      insertVirtualMethodSummary(
+          *LU, Entities[M.Method],
+          makeMethodSummary(std::move(Params), Ret, std::move(Overrides)));
+    }
+
+    std::map<char, std::vector<EPLEdge>> EdgesOfScope;
+    std::map<char, std::vector<EntityPointerLevel>> StartersOfScope;
+    for (const auto &[Scope, E] : EdgeLayout)
+      EdgesOfScope[Scope].push_back({GetEPL(E.first), GetEPL(E.second)});
+    for (const auto &[Scope, Nodes] : StarterLayout.scopes())
+      for (const Node &N : Nodes)
+        StartersOfScope[Scope].push_back(GetEPL(N));
+
+    std::set<char> Scopes;
+    for (char Scope : llvm::make_first_range(EdgesOfScope))
+      Scopes.insert(Scope);
+    for (char Scope : llvm::make_first_range(StartersOfScope))
+      Scopes.insert(Scope);
+    for (char Scope : Scopes)
+      insertSummaries(*LU, Entities[Scope], EdgesOfScope[Scope],
+                      StartersOfScope[Scope]);
+
+    auto Reachables = computeReachablesByScope(std::move(LU), Line);
+    if (!Reachables)
+      return {};
+
+    ScopedNodes Result;
+    for (const auto &[Scope, EPLs] : *Reachables)
+      for (const EntityPointerLevel &EPL : EPLs)
+        Result.insert(Entities[Scope],
+                      {Entities[EPL.getEntity()], EPL.getPointerLevel()});
+    return Result;
+  }
+
+  ScopedNodes familyClosure(llvm::ArrayRef<MethodLayout> Methods,
+                            const ScopedNodes &StarterLayout, unsigned Line) {
+    return familyClosure(Methods, StarterLayout, /*EdgeLayout=*/{}, Line);
+  }
+
+private:
+  /// Every letter the layouts mention, deduplicated.
+  static std::vector<char> entityDomainOf(llvm::ArrayRef<MethodLayout> Methods,
+                                          const ScopedNodes &Starters,
+                                          llvm::ArrayRef<ScopedEdge> Edges) {
+    std::set<char> Domain;
+    for (const MethodLayout &M : Methods) {
+      Domain.insert(M.Method);
+      Domain.insert(M.Params.begin(), M.Params.end());
+      Domain.insert(M.Overrides.begin(), M.Overrides.end());
+      if (M.Ret)
+        Domain.insert(*M.Ret);
+    }
+    for (const auto &[Scope, Nodes] : Starters.scopes()) {
+      Domain.insert(Scope);
+      for (const Node &N : Nodes)
+        Domain.insert(N.first);
+    }
+    for (const auto &[Scope, E] : Edges) {
+      Domain.insert(Scope);
+      Domain.insert(E.first.first);
+      Domain.insert(E.second.first);
+    }
+    return {Domain.begin(), Domain.end()};
+  }
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -579,4 +775,121 @@ TEST_F(UnsafeBufferReachableAnalysisTest, MultipleKeysSameEntity) {
   EXPECT_EQ(Reachables, (std::set<Node>{{'a', 3}, {'b', 3}, {'c', 2}}));
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// Family-closure tests
+////////////////////////////////////////////////////////////////////////////////
+
+// Method B owns param slot p; D overrides B and owns param slot q.
+// Seeding (q,1) in D's scope mirrors (p,1) into B's scope.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureParamSlot) {
+  const ScopedNodes Starters{{'D', {{'q', 1}}}};
+  auto Reachables = familyClosure(
+      /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+                     {'D', /*Params=*/{'q'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+      Starters, __LINE__);
+
+  const ScopedNodes AddedByClosure{
+      {'B', {{'p', 1}}}, // Up to the overridden method.
+  };
+  EXPECT_EQ(Reachables, Starters + AddedByClosure);
+}
+
+// As above, but p and q are the methods' return slots rather than parameters.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureReturnSlot) {
+  const ScopedNodes Starters{{'D', {{'q', 1}}}};
+  auto Reachables = familyClosure(
+      /* Methods */ {{'B', /*Params=*/{}, /*Ret=*/{'p'}, /*Overrides=*/{}},
+                     {'D', /*Params=*/{}, /*Ret=*/{'q'}, /*Overrides=*/{'B'}}},
+      Starters, __LINE__);
+
+  const ScopedNodes AddedByClosure{
+      {'B', {{'p', 1}}}, // Up to the overridden method.
+  };
+  EXPECT_EQ(Reachables, Starters + AddedByClosure);
+}
+
+// No virtual methods at all, so no families: f is an ordinary function and b a
+// buffer it owns. Closure adds nothing, so the starters are all that is
+// reachable.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureEmptyFamilyIsNoop) {
+  const ScopedNodes Starters{{'f', {{'b', 1}}}};
+  auto Reachables = familyClosure(/* Methods */ {}, Starters, __LINE__);
+
+  EXPECT_EQ(Reachables, Starters + /*AddedByClosure=*/ScopedNodes{});
+}
+
+// X and Y both override B, so all three slots share one family.
+// Seeding X propagates up to B *and* sideways to the sibling override Y.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureThreeMemberFamily) {
+  const ScopedNodes Starters{{'X', {{'x', 1}}}};
+  auto Reachables = familyClosure(
+      /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+                     {'X', /*Params=*/{'x'}, /*Ret=*/{}, /*Overrides=*/{'B'}},
+                     {'Y', /*Params=*/{'y'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+      Starters, __LINE__);
+
+  const ScopedNodes AddedByClosure{
+      {'B', {{'p', 1}}}, // Up to the base.
+      {'Y', {{'y', 1}}}, // Sideways to the sibling override.
+  };
+  EXPECT_EQ(Reachables, Starters + AddedByClosure);
+}
+
+// Family closure is level-preserving: an EPL reachable at level 3 propagates to
+// the family member at level 3 only, not to the levels below it.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosurePreservesPointerLevel) {
+  const ScopedNodes Starters{{'D', {{'q', 3}}}};
+  auto Reachables = familyClosure(
+      /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+                     {'D', /*Params=*/{'q'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+      Starters, __LINE__);
+
+  const ScopedNodes AddedByClosure{
+      {'B', {{'p', 3}}}, // Level 3 only; neither p at 1 nor p at 2.
+  };
+  EXPECT_EQ(Reachables, Starters + AddedByClosure);
+}
+
+// A single slot reachable at several levels propagates every one of those
+// levels onto its family members.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureMultipleLevelsSameSlot) {
+  const ScopedNodes Starters{{'D', {{'q', 1}, {'q', 2}}}};
+  auto Reachables = familyClosure(
+      /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+                     {'D', /*Params=*/{'q'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+      Starters, __LINE__);
+
+  const ScopedNodes AddedByClosure{
+      {'B', {{'p', 1}, {'p', 2}}}, // Both levels, not just one of them.
+  };
+  EXPECT_EQ(Reachables, Starters + AddedByClosure);
+}
+
+// Pins the known limitation documented by the FIXME on `runFamilyClosurePass`:
+// the closure pass runs *after* the pointer-flow DFS has converged, and
+// `step()` returns false unconditionally, so EPLs discovered by the closure are
+// never fed back through the pointer-flow graph.
+//
+// Here (p,1) becomes reachable only via family closure, and B owns the flow
+// edge (p,1) -> (z,1). Nothing is seeded in B's scope, so the DFS never visits
+// (p,1) on its own. A true fixpoint over DFS + closure would also reach (z,1).
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureDoesNotRerunDFS) {
+  const ScopedNodes Starters{{'D', {{'q', 1}}}};
+  auto Reachables = familyClosure(
+      /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+                     {'D', /*Params=*/{'q'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+      Starters,
+      /* EdgeLayout */ {{'B', {{'p', 1}, {'z', 1}}}}, __LINE__);
+
+  // FIXME: z at 1 belongs in B's set below -- it is a pointer-flow successor of
+  // (p,1), which closure just discovered. It is missed because the pass does
+  // not re-run the DFS over its own output.
+  const ScopedNodes AddedByClosure{
+      {'B', {{'p', 1}}}, // Up to the base, but no z at 1.
+  };
+  EXPECT_EQ(Reachables, Starters + AddedByClosure)
+      << "If z at 1 shows up in B's scope, the DFS/closure fixpoint gap was "
+         "fixed; add it to AddedByClosure above";
+}
+
 } // namespace



More information about the llvm-branch-commits mailing list