address commentsusers/ziqingluo/PR-174874942-3

4 files changed, 51 insertions, 44 deletions

diff --git a/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowAnalysis.h b/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowAnalysis.h
index 0129c21501bc..e862df58a820 100644
--- a/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowAnalysis.h
+++ b/clang/include/clang/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowAnalysis.h
@@ -29,8 +29,8 @@ namespace clang::ssaf {
 
 constexpr llvm::StringLiteral PointerFlowAnalysisResultName =
     "PointerFlowAnalysisResult";
-constexpr llvm::StringLiteral PointerFlowReachableAnalysisResultName =
-    "PointerFlowReachableAnalysisResult";
+constexpr llvm::StringLiteral UnsafeBufferReachableAnalysisResultName =
+    "UnsafeBufferReachableAnalysisResult";
 
 struct PointerFlowAnalysisResult final : AnalysisResult {
   static AnalysisName analysisName() {
@@ -40,9 +40,9 @@ struct PointerFlowAnalysisResult final : AnalysisResult {
   std::map<EntityId, EdgeSet> Edges;
 };
 
-struct PointerFlowReachableAnalysisResult final : AnalysisResult {
+struct UnsafeBufferReachableAnalysisResult final : AnalysisResult {
   static AnalysisName analysisName() {
-    return AnalysisName(PointerFlowReachableAnalysisResultName.str());
+    return AnalysisName(UnsafeBufferReachableAnalysisResultName.str());
   }
 
   std::map<EntityId, EntityPointerLevelSet> Reachables;
diff --git a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowAnalysis.cpp b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowAnalysis.cpp
index ef5f1d2839c5..83652c2f1147 100644
--- a/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowAnalysis.cpp
+++ b/clang/lib/ScalableStaticAnalysisFramework/Analyses/PointerFlow/PointerFlowAnalysis.cpp
@@ -123,11 +123,11 @@ AnalysisRegistry::Add<PointerFlowAnalysis>
 //===----------------------------------------------------------------------===//
 
 json::Object serializePointerFlowReachableAnalysisResult(
-    const PointerFlowReachableAnalysisResult &R,
+    const UnsafeBufferReachableAnalysisResult &R,
     JSONFormat::EntityIdToJSONFn IdToJSON) {
   json::Object Result;
 
-  Result[PointerFlowReachableAnalysisResultName] =
+  Result[UnsafeBufferReachableAnalysisResultName] =
       entityPointerLevelMapToJSON(R.Reachables, IdToJSON);
   return Result;
 }
@@ -136,36 +136,36 @@ Expected<std::unique_ptr<AnalysisResult>>
 deserializePointerFlowReachableAnalysisResult(
     const json::Object &Obj, JSONFormat::EntityIdFromJSONFn IdFromJSON) {
   const json::Array *Content =
-      Obj.getArray(PointerFlowReachableAnalysisResultName);
+      Obj.getArray(UnsafeBufferReachableAnalysisResultName);
 
   if (!Content)
     return makeSawButExpectedError(
         Obj, "an object with a key %s",
-        PointerFlowReachableAnalysisResultName.data());
+        UnsafeBufferReachableAnalysisResultName.data());
 
   auto Reachables = entityPointerLevelMapFromJSON(*Content, IdFromJSON);
 
   if (!Reachables)
     return Reachables.takeError();
 
-  auto Ret = std::make_unique<PointerFlowReachableAnalysisResult>();
+  auto Ret = std::make_unique<UnsafeBufferReachableAnalysisResult>();
 
   Ret->Reachables = std::move(*Reachables);
   return Ret;
 }
 
-JSONFormat::AnalysisResultRegistry::Add<PointerFlowReachableAnalysisResult>
+JSONFormat::AnalysisResultRegistry::Add<UnsafeBufferReachableAnalysisResult>
     RegisterPointerFlowReachableResultForJSON(
         serializePointerFlowReachableAnalysisResult,
         deserializePointerFlowReachableAnalysisResult);
 
-// For any AnalysisResult that is a set of pointers, it can be the starting set
-// for reachable analysis.
-template <typename AnalysisResultInPointerSet>
-class PointerFlowReachableAnalysis
-    : public DerivedAnalysis<PointerFlowReachableAnalysisResult,
+/// 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`.
+class UnsafeBufferReachableAnalysis
+    : public DerivedAnalysis<UnsafeBufferReachableAnalysisResult,
                              PointerFlowAnalysisResult,
-                             AnalysisResultInPointerSet> {
+                             UnsafeBufferUsageAnalysisResult> {
   using GraphT = std::map<EntityId, EdgeSet>;
   const GraphT *Graph = nullptr;
 
@@ -196,10 +196,9 @@ class PointerFlowReachableAnalysis
   }
 
 public:
-  // FIXME: we need a unique analysis name for each instantiation
-
-  llvm::Error initialize(const PointerFlowAnalysisResult &Graph,
-                         const AnalysisResultInPointerSet &Starter) override {
+  llvm::Error
+  initialize(const PointerFlowAnalysisResult &Graph,
+             const UnsafeBufferUsageAnalysisResult &Starter) override {
     this->Graph = &Graph.Edges;
     assert(this->getResult().Reachables.empty());
     this->getResult().Reachables.insert(Starter.begin(), Starter.end());
@@ -225,8 +224,7 @@ public:
   }
 };
 
-AnalysisRegistry::Add<
-    PointerFlowReachableAnalysis<UnsafeBufferUsageAnalysisResult>>
+AnalysisRegistry::Add<UnsafeBufferReachableAnalysis>
     RegisterPointerFlowReachableAnalysis(
         "Reachable pointers from unsafe buffer usage in pointer flow graph");
 
diff --git a/clang/unittests/ScalableStaticAnalysisFramework/CMakeLists.txt b/clang/unittests/ScalableStaticAnalysisFramework/CMakeLists.txt
index 6d2a009da994..992a9eb0f285 100644
--- a/clang/unittests/ScalableStaticAnalysisFramework/CMakeLists.txt
+++ b/clang/unittests/ScalableStaticAnalysisFramework/CMakeLists.txt
@@ -26,7 +26,7 @@ add_distinct_clang_unittest(ClangScalableAnalysisTests
   TestFixture.cpp
   TUSummaryBuilderTest.cpp
   WholeProgramAnalysis/AnalysisDriverTest.cpp
-  WholeProgramAnalysis/PointerFlowReachableAnalysisTest.cpp
+  WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
 
   CLANG_LIBS
   clangAST
diff --git a/clang/unittests/ScalableStaticAnalysisFramework/WholeProgramAnalysis/PointerFlowReachableAnalysisTest.cpp b/clang/unittests/ScalableStaticAnalysisFramework/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
index 51b8c083d294..1e45e151dcea 100644
--- a/clang/unittests/ScalableStaticAnalysisFramework/WholeProgramAnalysis/PointerFlowReachableAnalysisTest.cpp
+++ b/clang/unittests/ScalableStaticAnalysisFramework/WholeProgramAnalysis/UnsafeBufferReachableAnalysisTest.cpp
@@ -1,4 +1,5 @@
-//===- PointerFlowReachableAnalysisTest.cpp -------------------------------===//
+//===- UnsafeBufferReachableAnalysisTest.cpp
+//-------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -74,7 +75,7 @@ static std::vector<PartitionT> enumeratePartition(unsigned N, unsigned NumGrps,
   return Partitions;
 }
 
-class PointerFlowReachableAnalysisTest : public TestFixture {
+class UnsafeBufferReachableAnalysisTest : public TestFixture {
 protected:
   using EPLEdge = std::pair<EntityPointerLevel, EntityPointerLevel>;
 
@@ -150,12 +151,12 @@ protected:
     AnalysisDriver Driver(std::move(LU));
     auto WPAOrErr =
         Driver.run<PointerFlowAnalysisResult, UnsafeBufferUsageAnalysisResult,
-                   PointerFlowReachableAnalysisResult>();
+                   UnsafeBufferReachableAnalysisResult>();
     if (!WPAOrErr) {
       ADD_FAILURE_AT(__FILE__, Line) << llvm::toString(WPAOrErr.takeError());
       return std::nullopt;
     }
-    auto ROrErr = WPAOrErr->get<PointerFlowReachableAnalysisResult>();
+    auto ROrErr = WPAOrErr->get<UnsafeBufferReachableAnalysisResult>();
     if (!ROrErr) {
       ADD_FAILURE_AT(__FILE__, Line) << llvm::toString(ROrErr.takeError());
       return std::nullopt;
@@ -232,11 +233,19 @@ protected:
   }
 };
 
-//- Test different graph topologic structures ----------------------//
+////////////////////////////////////////////////////////////////////////////////
+//  Tests below are written in a manner that focuses on pointer flow graph
+//  topology and the starter set, where numbers are used to represent distinct
+//  nodes (pointers).
+//  For example, `LinearChain` tests a graph forming a
+//  linear chain with 4 distinct nodes: 0 -> 1 -> 2 -> 3 with a starter set {0},
+//  where, for example, 0 -> 1 represents an edge where node 0 is the source and
+//  node 1 is the destination. Thus, {0, 1, 2, 3} is the expected reachable set.
+////////////////////////////////////////////////////////////////////////////////
 
 // Linear chain: 0 -> 1 -> 2 -> 3.
 // Start from {0} => {0, 1, 2, 3}
-TEST_F(PointerFlowReachableAnalysisTest, LinearChain) {
+TEST_F(UnsafeBufferReachableAnalysisTest, LinearChain) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {1, 2}, {2, 3}},
@@ -247,7 +256,7 @@ TEST_F(PointerFlowReachableAnalysisTest, LinearChain) {
 
 // Linear chain: 0 -> 1 -> 2 -> 3.
 // Start from mid-chain {2} => {2, 3}
-TEST_F(PointerFlowReachableAnalysisTest, LinearChainFromMiddle) {
+TEST_F(UnsafeBufferReachableAnalysisTest, LinearChainFromMiddle) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {1, 2}, {2, 3}},
@@ -260,7 +269,7 @@ TEST_F(PointerFlowReachableAnalysisTest, LinearChainFromMiddle) {
 
 // Diamond: 0 -> 1, 0 -> 2, 1 -> 3, 2 -> 3.
 // Start from {0} => {0, 1, 2, 3}
-TEST_F(PointerFlowReachableAnalysisTest, Diamond) {
+TEST_F(UnsafeBufferReachableAnalysisTest, Diamond) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {0, 2}, {1, 3}, {2, 3}},
@@ -271,7 +280,7 @@ TEST_F(PointerFlowReachableAnalysisTest, Diamond) {
 
 // Diamond: 0 -> 1, 0 -> 2, 1 -> 3, 2 -> 3.
 // Start from one branch {1} => {1, 3}
-TEST_F(PointerFlowReachableAnalysisTest, DiamondFromBranch) {
+TEST_F(UnsafeBufferReachableAnalysisTest, DiamondFromBranch) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {0, 2}, {1, 3}, {2, 3}},
@@ -284,7 +293,7 @@ TEST_F(PointerFlowReachableAnalysisTest, DiamondFromBranch) {
 
 // Disconnected subgraphs: 0 -> 1, 2 -> 3.
 // Start from {0} => {0, 1}
-TEST_F(PointerFlowReachableAnalysisTest, DisconnectedSubgraphs) {
+TEST_F(UnsafeBufferReachableAnalysisTest, DisconnectedSubgraphs) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {2, 3}},
@@ -297,7 +306,7 @@ TEST_F(PointerFlowReachableAnalysisTest, DisconnectedSubgraphs) {
 
 // Disconnected subgraphs: 0 -> 1, 2 -> 3.
 // Start from tail {1} => {1}
-TEST_F(PointerFlowReachableAnalysisTest, DisconnectedSubgraphsFromTail) {
+TEST_F(UnsafeBufferReachableAnalysisTest, DisconnectedSubgraphsFromTail) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {2, 3}},
@@ -309,7 +318,7 @@ TEST_F(PointerFlowReachableAnalysisTest, DisconnectedSubgraphsFromTail) {
 
 // Cycle: 0 -> 1 -> 2 -> 3 -> 0.
 // Start from {2} => {0, 1, 2, 3}
-TEST_F(PointerFlowReachableAnalysisTest, Cycle) {
+TEST_F(UnsafeBufferReachableAnalysisTest, Cycle) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {1, 2}, {2, 3}, {3, 0}},
@@ -323,7 +332,7 @@ TEST_F(PointerFlowReachableAnalysisTest, Cycle) {
 }
 
 // Empty graph: no edges, start from {0} => {0}
-TEST_F(PointerFlowReachableAnalysisTest, EmptyGraph) {
+TEST_F(UnsafeBufferReachableAnalysisTest, EmptyGraph) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {},
@@ -335,7 +344,7 @@ TEST_F(PointerFlowReachableAnalysisTest, EmptyGraph) {
 
 // Star: 0 -> 1, 0 -> 2, 0 -> 3.
 // Start from {0} => {0, 1, 2, 3}
-TEST_F(PointerFlowReachableAnalysisTest, StarFromHub) {
+TEST_F(UnsafeBufferReachableAnalysisTest, StarFromHub) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {0, 2}, {0, 3}},
@@ -346,7 +355,7 @@ TEST_F(PointerFlowReachableAnalysisTest, StarFromHub) {
 
 // Star: 0 -> 1, 0 -> 2, 0 -> 3.
 // Start from leaf {2} => {2}
-TEST_F(PointerFlowReachableAnalysisTest, StarFromLeaf) {
+TEST_F(UnsafeBufferReachableAnalysisTest, StarFromLeaf) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {0, 2}, {0, 3}},
@@ -358,7 +367,7 @@ TEST_F(PointerFlowReachableAnalysisTest, StarFromLeaf) {
 
 // Reverse star: 0 -> 3, 1 -> 3, 2 -> 3.
 // Start from {0} => {0, 3}
-TEST_F(PointerFlowReachableAnalysisTest, ReverseStarFromSource) {
+TEST_F(UnsafeBufferReachableAnalysisTest, ReverseStarFromSource) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 3}, {1, 3}, {2, 3}},
@@ -371,7 +380,7 @@ TEST_F(PointerFlowReachableAnalysisTest, ReverseStarFromSource) {
 
 // Reverse star: 0 -> 3, 1 -> 3, 2 -> 3.
 // Start from sink {3} => {3}
-TEST_F(PointerFlowReachableAnalysisTest, ReverseStarFromSink) {
+TEST_F(UnsafeBufferReachableAnalysisTest, ReverseStarFromSink) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 3}, {1, 3}, {2, 3}},
@@ -383,7 +392,7 @@ TEST_F(PointerFlowReachableAnalysisTest, ReverseStarFromSink) {
 
 // Self-loop: 0 -> 1, 1 -> 1, 1 -> 2, 2 -> 3.
 // Start from {0} => {0, 1, 2, 3}
-TEST_F(PointerFlowReachableAnalysisTest, SelfLoopFromRoot) {
+TEST_F(UnsafeBufferReachableAnalysisTest, SelfLoopFromRoot) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {1, 1}, {1, 2}, {2, 3}},
@@ -394,7 +403,7 @@ TEST_F(PointerFlowReachableAnalysisTest, SelfLoopFromRoot) {
 
 // Self-loop: 0 -> 1, 1 -> 1, 1 -> 2, 2 -> 3.
 // Start from {1} => {1, 2, 3}
-TEST_F(PointerFlowReachableAnalysisTest, SelfLoopFromLoopNode) {
+TEST_F(UnsafeBufferReachableAnalysisTest, SelfLoopFromLoopNode) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {1, 1}, {1, 2}, {2, 3}},
@@ -408,7 +417,7 @@ TEST_F(PointerFlowReachableAnalysisTest, SelfLoopFromLoopNode) {
 
 // Multiple starters: 0 -> 1, 2 -> 3 (disconnected).
 // Start from {0, 2} => {0, 1, 2, 3}
-TEST_F(PointerFlowReachableAnalysisTest, MultipleStartersBothChains) {
+TEST_F(UnsafeBufferReachableAnalysisTest, MultipleStartersBothChains) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {2, 3}},
@@ -419,7 +428,7 @@ TEST_F(PointerFlowReachableAnalysisTest, MultipleStartersBothChains) {
 
 // Multiple starters: 0 -> 1, 2 -> 3 (disconnected).
 // Start from leaves {1, 3} => {1, 3}
-TEST_F(PointerFlowReachableAnalysisTest, MultipleStartersLeaves) {
+TEST_F(UnsafeBufferReachableAnalysisTest, MultipleStartersLeaves) {
   auto Reachables = forEachPartition(
       /* NumEnt */ 4,
       /* EdgeLayout */ {{0, 1}, {2, 3}},