[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
Sat Sep 26 05:52:06 PDT 2026
https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/213319
>From babcc2b0c839cb6da8777c9049b8a49f99a9e205 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.
- Mirroring happens inside the pointer-flow search, so flows out of a mirrored
EPL are followed too.
- Type-constrained slots are never mirrored onto, so C3 still holds.
§4 of rdar://179151603
Assisted-By: claude
---
.../UnsafeBufferUsageAnalysis.cpp | 69 ++++-
.../UnsafeBufferReachableAnalysisTest.cpp | 240 ++++++++++++++++++
2 files changed, 295 insertions(+), 14 deletions(-)
diff --git a/clang/lib/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.cpp b/clang/lib/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.cpp
index eed4925a2b298..41c3dcaa4b736 100644
--- a/clang/lib/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.cpp
+++ b/clang/lib/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsageAnalysis.cpp
@@ -22,11 +22,14 @@
#include "clang/ScalableStaticAnalysis/Analyses/PointerFlow/PointerFlowAnalysis.h"
#include "clang/ScalableStaticAnalysis/Analyses/TypeConstrainedPointers/TypeConstrainedPointers.h"
#include "clang/ScalableStaticAnalysis/Analyses/UnsafeBufferUsage/UnsafeBufferUsage.h"
+#include "clang/ScalableStaticAnalysis/Analyses/VirtualMethodFamily/VirtualMethodFamily.h"
#include "clang/ScalableStaticAnalysis/Core/Model/EntityId.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/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/JSON.h"
@@ -142,11 +145,15 @@ JSONFormat::AnalysisResultRegistry::Add<UnsafeBufferReachableAnalysisResult>
/// the pointer flow graph (provided by `PointerFlowAnalysisResult`), it is
/// also unsafe.
/// 3. **C3 (Constrained):** Type-constrained entities are NOT unsafe.
+/// 4. **C4 (Family):** If a parameter or return slot of a virtual method is
+/// unsafe at some pointer level, so is every slot of its override family
+/// (provided by `VirtualMethodFamilyAnalysisResult`) at that level, because
+/// a virtual call can dispatch to any of the overrides.
class UnsafeBufferReachableAnalysis
- : public DerivedAnalysis<UnsafeBufferReachableAnalysisResult,
- PointerFlowAnalysisResult,
- TypeConstrainedPointersAnalysisResult,
- UnsafeBufferUsageAnalysisResult> {
+ : public DerivedAnalysis<
+ UnsafeBufferReachableAnalysisResult, PointerFlowAnalysisResult,
+ TypeConstrainedPointersAnalysisResult,
+ UnsafeBufferUsageAnalysisResult, VirtualMethodFamilyAnalysisResult> {
struct BoundsPropagationGraph {
EdgeSet PointerFlows;
@@ -163,10 +170,24 @@ class UnsafeBufferReachableAnalysis
std::map<EntityId, BoundsPropagationGraph> BPG;
+ /// Maps each virtual method slot to the ID of its override family.
+ const llvm::DenseMap<EntityId, EntityId> *FamilyOf = nullptr;
+
+ /// The slots of each override family, excluding type-constrained ones.
+ llvm::DenseMap<EntityId, llvm::SmallVector<EntityId, 2>> FamilyMembers;
+
// Use pointers for efficiency. EPLs are in tree-based containers that only
// grow. So pointers to them are stable.
using EPLPtr = const EntityPointerLevel *;
+ // Insert `EPL` into `Reachables`, and add it to `Worklist` if it is new:
+ void insertReachable(const EntityPointerLevel &EPL,
+ std::vector<EPLPtr> &WorkList) {
+ auto [It, Inserted] = getResult().Reachables.insert(EPL);
+ if (Inserted)
+ WorkList.push_back(&*It);
+ }
+
// Find all outgoing edges from `EPL` in the `Graph`, insert their
// destination nodes into `Reachables`, and add newly discovered nodes to
// `Worklist`:
@@ -175,16 +196,27 @@ class UnsafeBufferReachableAnalysis
for (auto &[Id, SubGraph] : BPG) {
auto R = SubGraph.getDestNodes(*EPL);
- for (const auto &Dst : R) {
- auto [It, Inserted] = getResult().Reachables.insert(Dst);
- if (Inserted)
- WorkList.push_back(&*It);
- }
+ for (const auto &Dst : R)
+ insertReachable(Dst, WorkList);
}
}
+ // Insert the slots of the override family of `EPL` at the pointer level of
+ // `EPL` into `Reachables`, and add newly discovered nodes to `Worklist`:
+ void updateReachablesWithFamily(EPLPtr EPL, std::vector<EPLPtr> &WorkList) {
+ auto FamilyIt = FamilyOf->find(EPL->getEntity());
+ if (FamilyIt == FamilyOf->end())
+ return;
+ auto MembersIt = FamilyMembers.find(FamilyIt->second);
+ if (MembersIt == FamilyMembers.end())
+ return;
+ for (EntityId Member : MembersIt->second)
+ insertReachable(buildEntityPointerLevel(Member, EPL->getPointerLevel()),
+ WorkList);
+ }
+
// Expand the initial set of C1 pointers in `getResult().Reachables` by
- // computing and appending all reachable pointers, satisfying both C1 and C2.
+ // computing and appending all reachable pointers, satisfying C1, C2 and C4.
void computeReachableUnsafePointers() {
auto &Reachables = getResult().Reachables;
// Simple DFS:
@@ -198,6 +230,7 @@ class UnsafeBufferReachableAnalysis
Worklist.pop_back();
updateReachablesWithOutgoings(Node, Worklist);
+ updateReachablesWithFamily(Node, Worklist);
}
}
@@ -205,7 +238,8 @@ class UnsafeBufferReachableAnalysis
llvm::Error
initialize(const PointerFlowAnalysisResult &PtrFlowGraph,
const TypeConstrainedPointersAnalysisResult &TypeConstraints,
- const UnsafeBufferUsageAnalysisResult &UnsafePtrs) override {
+ const UnsafeBufferUsageAnalysisResult &UnsafePtrs,
+ const VirtualMethodFamilyAnalysisResult &Families) override {
auto HasNoTypeConstraint =
[&TypeConstraints](const EntityPointerLevel &EPL) {
return !TypeConstraints.contains(EPL.getEntity());
@@ -237,13 +271,19 @@ class UnsafeBufferReachableAnalysis
getResult().Reachables.insert(FilteredRange.begin(), FilteredRange.end());
}
+
+ // Filter out type-constrained slots from the override families:
+ FamilyOf = &Families.RetAndParamData;
+ for (auto [Slot, FamilyId] : Families.RetAndParamData)
+ if (!TypeConstraints.contains(Slot))
+ FamilyMembers[FamilyId].push_back(Slot);
return llvm::Error::success();
}
llvm::Expected<bool> step() override {
// Compute the reachable EPLs from the C1 unsafe pointers over the
- // pointer-flow graph; both are already C3-filtered, so the result
- // satisfies C1, C2, and C3.
+ // pointer-flow graph and the override families; all three are already
+ // C3-filtered, so the result satisfies C1, C2, C3, and C4.
computeReachableUnsafePointers();
// This is not an iterative algorithm so stop iteration by retruning false:
return false;
@@ -252,7 +292,8 @@ class UnsafeBufferReachableAnalysis
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/unittests/ScalableStaticAnalysis/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp b/clang/unittests/ScalableStaticAnalysis/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
index d856bc14f436a..92cc76cadb875 100644
--- a/clang/unittests/ScalableStaticAnalysis/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
+++ b/clang/unittests/ScalableStaticAnalysis/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
@@ -16,6 +16,7 @@
#include "clang/ScalableStaticAnalysis/Analyses/TypeConstrainedPointers/TypeConstrainedPointers.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/ASTEntityMapping.h"
#include "clang/ScalableStaticAnalysis/Core/EntityLinker/EntityLinker.h"
#include "clang/ScalableStaticAnalysis/Core/EntityLinker/LUSummary.h"
@@ -43,12 +44,24 @@
#include <optional>
#include <set>
#include <string>
+#include <vector>
using namespace clang;
using namespace ssaf;
namespace {
+/// 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; ///< Entity the summary is stored under.
+ std::vector<char> Params; ///< VirtualMethodSummary::ParamEntities.
+ std::optional<char> Ret; ///< VirtualMethodSummary::ReturnEntity.
+ std::vector<char> Overrides; ///< VirtualMethodSummary::OverriddenMethods.
+};
+
class UnsafeBufferReachableAnalysisTest : public TestFixture {
protected:
using EPLEdge = std::pair<EntityPointerLevel, EntityPointerLevel>;
@@ -87,6 +100,22 @@ class UnsafeBufferReachableAnalysisTest : public TestFixture {
buildUnsafeBufferUsageEntitySummary(std::move(UnsafeBuffers)));
}
+ /// 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));
+ }
+
+ /// Insert a TypeConstrainedPointersEntitySummary for an entity.
+ void insertTypeConstrainedPointersSummary(LUSummary &LU, EntityId Id,
+ std::set<EntityId> Entities) {
+ auto Sum = std::make_unique<TypeConstrainedPointersEntitySummary>();
+ Sum->Entities = std::move(Entities);
+ getData(LU)[TypeConstrainedPointersEntitySummary::summaryName()][Id] =
+ std::move(Sum);
+ }
+
class LetterEntityBiMap {
std::map<char, EntityId> Forward;
std::map<EntityId, char> Reverse;
@@ -193,6 +222,95 @@ class UnsafeBufferReachableAnalysisTest : public TestFixture {
return Result;
}
+
+ /// Compute reachables for the virtual-method hierarchy described by
+ /// \p Methods, seeded with \p StarterLayout, over the pointer-flow edges in
+ /// \p EdgeLayout, and with the entities in \p Constrained being
+ /// type-constrained. Starters, edges and type constraints all belong to one
+ /// contributor, which no layout mentions.
+ std::set<Node> familyClosure(llvm::ArrayRef<MethodLayout> Methods,
+ llvm::ArrayRef<Node> StarterLayout,
+ llvm::ArrayRef<Edge> EdgeLayout,
+ llvm::ArrayRef<char> Constrained,
+ unsigned Line) {
+ constexpr char Contributor = '#';
+ auto LU = makeLUSummary();
+ auto Entities =
+ createEntities(*LU, entityDomainOf(Contributor, Methods, StarterLayout,
+ EdgeLayout, Constrained));
+ auto GetEPL = [&Entities](const Node &N) -> EntityPointerLevel {
+ return buildEntityPointerLevel(Entities[N.first], N.second);
+ };
+
+ auto GetIds = [&Entities](llvm::ArrayRef<char> Letters) {
+ std::vector<EntityId> Ids;
+ for (char L : Letters)
+ Ids.push_back(Entities[L]);
+ return Ids;
+ };
+ for (const MethodLayout &M : Methods) {
+ VirtualMethodSummary Sum;
+ Sum.ParamEntities = GetIds(M.Params);
+ if (M.Ret)
+ Sum.ReturnEntity = Entities[*M.Ret];
+ Sum.OverriddenMethods = GetIds(M.Overrides);
+ insertVirtualMethodSummary(*LU, Entities[M.Method], std::move(Sum));
+ }
+
+ std::vector<EPLEdge> Edges;
+ for (const auto &[F, T] : EdgeLayout)
+ Edges.push_back({GetEPL(F), GetEPL(T)});
+ std::vector<EntityPointerLevel> Starters;
+ for (const Node &N : StarterLayout)
+ Starters.push_back(GetEPL(N));
+ insertSummaries(*LU, Entities[Contributor], Edges, Starters);
+
+ std::vector<EntityId> ConstrainedIds = GetIds(Constrained);
+ insertTypeConstrainedPointersSummary(
+ *LU, Entities[Contributor],
+ {ConstrainedIds.begin(), ConstrainedIds.end()});
+
+ auto Reachables = computeReachables(std::move(LU), Line);
+ if (!Reachables)
+ return {};
+
+ std::set<Node> Result;
+ for (const EntityPointerLevel &EPL : *Reachables)
+ Result.insert({Entities[EPL.getEntity()], EPL.getPointerLevel()});
+ return Result;
+ }
+
+ std::set<Node> familyClosure(llvm::ArrayRef<MethodLayout> Methods,
+ llvm::ArrayRef<Node> StarterLayout,
+ unsigned Line) {
+ return familyClosure(Methods, StarterLayout, /*EdgeLayout=*/{},
+ /*Constrained=*/{}, Line);
+ }
+
+private:
+ /// Every letter the layouts mention, plus \p Contributor, deduplicated.
+ static std::vector<char> entityDomainOf(char Contributor,
+ llvm::ArrayRef<MethodLayout> Methods,
+ llvm::ArrayRef<Node> Starters,
+ llvm::ArrayRef<Edge> Edges,
+ llvm::ArrayRef<char> Constrained) {
+ std::set<char> Domain{Contributor};
+ 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 Node &N : Starters)
+ Domain.insert(N.first);
+ for (const auto &[From, To] : Edges) {
+ Domain.insert(From.first);
+ Domain.insert(To.first);
+ }
+ Domain.insert(Constrained.begin(), Constrained.end());
+ return {Domain.begin(), Domain.end()};
+ }
};
////////////////////////////////////////////////////////////////////////////////
@@ -772,4 +890,126 @@ TEST_F(UnsafeBufferReachableAnalysisSourceTest, 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) mirrors (p,1).
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureParamSlot) {
+ auto Reachables = familyClosure(
+ /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+ {'D', /*Params=*/{'q'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+ /* Starters */ {{'q', 1}}, __LINE__);
+
+ EXPECT_EQ(Reachables, (std::set<Node>{
+ {'q', 1},
+ {'p', 1}, // Up to the overridden method.
+ }));
+}
+
+// As above, but p and q are the methods' return slots rather than parameters.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureReturnSlot) {
+ auto Reachables = familyClosure(
+ /* Methods */ {{'B', /*Params=*/{}, /*Ret=*/{'p'}, /*Overrides=*/{}},
+ {'D', /*Params=*/{}, /*Ret=*/{'q'}, /*Overrides=*/{'B'}}},
+ /* Starters */ {{'q', 1}}, __LINE__);
+
+ EXPECT_EQ(Reachables, (std::set<Node>{
+ {'q', 1},
+ {'p', 1}, // Up to the overridden method.
+ }));
+}
+
+// No virtual methods at all, so no families. Closure adds nothing, so the
+// starters are all that is reachable.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureEmptyFamilyIsNoop) {
+ auto Reachables =
+ familyClosure(/* Methods */ {}, /* Starters */ {{'b', 1}}, __LINE__);
+
+ EXPECT_EQ(Reachables, (std::set<Node>{{'b', 1}}));
+}
+
+// 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) {
+ auto Reachables = familyClosure(
+ /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+ {'X', /*Params=*/{'x'}, /*Ret=*/{}, /*Overrides=*/{'B'}},
+ {'Y', /*Params=*/{'y'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+ /* Starters */ {{'x', 1}}, __LINE__);
+
+ EXPECT_EQ(Reachables, (std::set<Node>{
+ {'x', 1},
+ {'p', 1}, // Up to the base.
+ {'y', 1}, // Sideways to the sibling override.
+ }));
+}
+
+// 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) {
+ auto Reachables = familyClosure(
+ /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+ {'D', /*Params=*/{'q'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+ /* Starters */ {{'q', 3}}, __LINE__);
+
+ EXPECT_EQ(Reachables, (std::set<Node>{
+ {'q', 3},
+ {'p', 3}, // Level 3 only; neither p at 1 nor p at 2.
+ }));
+}
+
+// A single slot reachable at several levels propagates every one of those
+// levels onto its family members.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureMultipleLevelsSameSlot) {
+ auto Reachables = familyClosure(
+ /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+ {'D', /*Params=*/{'q'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+ /* Starters */ {{'q', 1}, {'q', 2}}, __LINE__);
+
+ EXPECT_EQ(Reachables, (std::set<Node>{
+ {'q', 1},
+ {'q', 2},
+ {'p', 1}, // Both levels, not just one of them.
+ {'p', 2},
+ }));
+}
+
+// (p,1) becomes reachable only via family closure, and (p,1) -> (z,1) is a
+// flow edge. Nothing is seeded at (p,1), so only the family closure can make
+// the pointer-flow search visit it.
+TEST_F(UnsafeBufferReachableAnalysisTest, FamilyClosureFeedsBackIntoDFS) {
+ auto Reachables = familyClosure(
+ /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+ {'D', /*Params=*/{'q'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+ /* Starters */ {{'q', 1}},
+ /* EdgeLayout */ {{{'p', 1}, {'z', 1}}},
+ /* Constrained */ {}, __LINE__);
+
+ EXPECT_EQ(Reachables, (std::set<Node>{
+ {'q', 1},
+ {'p', 1}, // Up to the base.
+ {'z', 1}, // Flow successor of the mirrored (p,1).
+ }));
+}
+
+// X and Y both override B, whose slot p is type-constrained. Seeding X reaches
+// the sibling Y through the family, but never the constrained p (C3).
+TEST_F(UnsafeBufferReachableAnalysisTest,
+ FamilyClosureSkipsTypeConstrainedMember) {
+ auto Reachables = familyClosure(
+ /* Methods */ {{'B', /*Params=*/{'p'}, /*Ret=*/{}, /*Overrides=*/{}},
+ {'X', /*Params=*/{'x'}, /*Ret=*/{}, /*Overrides=*/{'B'}},
+ {'Y', /*Params=*/{'y'}, /*Ret=*/{}, /*Overrides=*/{'B'}}},
+ /* Starters */ {{'x', 1}},
+ /* EdgeLayout */ {},
+ /* Constrained */ {'p'}, __LINE__);
+
+ EXPECT_EQ(Reachables, (std::set<Node>{
+ {'x', 1},
+ {'y', 1}, // Sideways, even though p is excluded.
+ }));
+}
+
} // namespace
More information about the llvm-branch-commits
mailing list