[clang] 56cc697 - [clang][dataflow] Merge distinct pointer values in Environment::join

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-01-29T16:33:15Z
New Revision: 56cc697323445337134cc2bbe8ce8b1f60131574

URL: https://github.com/llvm/llvm-project/commit/56cc697323445337134cc2bbe8ce8b1f60131574
DIFF: https://github.com/llvm/llvm-project/commit/56cc697323445337134cc2bbe8ce8b1f60131574.diff

LOG: [clang][dataflow] Merge distinct pointer values in Environment::join

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

Reviewed-by: ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D118480

Added: 
    

Modified: 
    clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
    clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 938f7338b6403..8392f959d798e 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -89,8 +89,12 @@ LatticeJoinEffect Environment::join(const Environment &Other,
   if (ExprToLocSizeBefore != ExprToLoc.size())
     Effect = LatticeJoinEffect::Changed;
 
-  llvm::DenseMap<const StorageLocation *, Value *> MergedLocToVal;
-  for (auto &Entry : LocToVal) {
+  // Move `LocToVal` so that `Environment::Merger::merge` can safely assign
+  // values to storage locations while this code iterates over the current
+  // assignments.
+  llvm::DenseMap<const StorageLocation *, Value *> OldLocToVal =
+      std::move(LocToVal);
+  for (auto &Entry : OldLocToVal) {
     const StorageLocation *Loc = Entry.first;
     assert(Loc != nullptr);
 
@@ -103,19 +107,25 @@ LatticeJoinEffect Environment::join(const Environment &Other,
     assert(It->second != nullptr);
 
     if (It->second == Val) {
-      MergedLocToVal.insert({Loc, Val});
+      LocToVal.insert({Loc, Val});
       continue;
     }
 
+    if (auto *FirstVal = dyn_cast<PointerValue>(Val)) {
+      auto *SecondVal = cast<PointerValue>(It->second);
+      if (&FirstVal->getPointeeLoc() == &SecondVal->getPointeeLoc()) {
+        LocToVal.insert({Loc, FirstVal});
+        continue;
+      }
+    }
+
     // FIXME: Consider destroying `MergedValue` immediately if `Merger::merge`
     // returns false to avoid storing unneeded values in `DACtx`.
     if (Value *MergedVal = createValue(Loc->getType()))
       if (Merger.merge(Loc->getType(), *Val, *It->second, *MergedVal, *this))
-        MergedLocToVal.insert({Loc, MergedVal});
+        LocToVal.insert({Loc, MergedVal});
   }
-  const unsigned LocToValSizeBefore = LocToVal.size();
-  LocToVal = std::move(MergedLocToVal);
-  if (LocToValSizeBefore != LocToVal.size())
+  if (OldLocToVal.size() != LocToVal.size())
     Effect = LatticeJoinEffect::Changed;
 
   return Effect;

diff  --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index ee0bc3ed5e251..e9d5e129e32d6 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -494,4 +494,37 @@ TEST_F(WideningTest, JoinDistinctValuesWithSameProperties) {
       });
 }
 
+TEST_F(WideningTest, DistinctPointersToTheSameLocation) {
+  std::string Code = R"(
+    void target(int Foo, bool Cond) {
+      int *Bar = &Foo;
+      while (Cond) {
+        Bar = &Foo;
+      }
+      (void)0;
+      // [[p]]
+    }
+  )";
+  runDataflow(Code,
+              [](llvm::ArrayRef<
+                     std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                     Results,
+                 ASTContext &ASTCtx) {
+                ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+                const Environment &Env = Results[0].second.Env;
+
+                const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+                ASSERT_THAT(FooDecl, NotNull());
+
+                const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+                ASSERT_THAT(BarDecl, NotNull());
+
+                const auto *FooLoc = cast<ScalarStorageLocation>(
+                    Env.getStorageLocation(*FooDecl, SkipPast::None));
+                const auto *BarVal =
+                    cast<PointerValue>(Env.getValue(*BarDecl, SkipPast::None));
+                EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+              });
+}
+
 } // namespace