This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG56cc69732344: [clang][dataflow] Merge distinct pointer values in Environment::join (authored by sgatev).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118480/new/ https://reviews.llvm.org/D118480 Files: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp =================================================================== --- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -494,4 +494,37 @@ }); } +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 Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -89,8 +89,12 @@ 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 @@ 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;
Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp =================================================================== --- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -494,4 +494,37 @@ }); } +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 Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -89,8 +89,12 @@ 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 @@ 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;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits