ymandel created this revision. ymandel added reviewers: xazax.hun, sgatev, li.zhe.hua. Herald added subscribers: tschuett, steakhal, rnkovacs. Herald added a project: All. ymandel requested review of this revision. Herald added a project: clang.
Ignore `MemberLocToStruct` in environment comparison. As an ancillary data structure, including it is redundant. We also can generate environments which differ in their `MemberLocToStruct` but are otherwise equivalent. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D126314 Files: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp =================================================================== --- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -3120,4 +3120,59 @@ }); } +TEST_F(TransferTest, LoopWithStructReferenceAssignmentConverges) { + std::string Code = R"( + struct Lookup { + int x; + }; + + void target(Lookup val, bool b) { + const Lookup* l = nullptr; + while (b) { + l = &val; + /*[[p-inner]]*/ + } + (void)0; + /*[[p-outer]]*/ + } + )"; + // The key property that we are verifying is implicit in `runDataflow` -- + // namely, that the analysis succeeds, rather than hitting the maximum number + // of iterations. + runDataflow( + Code, [](llvm::ArrayRef< + std::pair<std::string, DataflowAnalysisState<NoopLattice>>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, + ElementsAre(Pair("p-outer", _), Pair("p-inner", _))); + const Environment &OuterEnv = Results[0].second.Env; + const Environment &InnerEnv = Results[1].second.Env; + + const ValueDecl *ValDecl = findValueDecl(ASTCtx, "val"); + ASSERT_THAT(ValDecl, NotNull()); + + const ValueDecl *LDecl = findValueDecl(ASTCtx, "l"); + ASSERT_THAT(LDecl, NotNull()); + + // Inner. + auto *LVal = dyn_cast<IndirectionValue>( + InnerEnv.getValue(*LDecl, SkipPast::None)); + ASSERT_THAT(LVal, NotNull()); + + EXPECT_EQ(&LVal->getPointeeLoc(), + InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); + + // Outer. + LVal = dyn_cast<IndirectionValue>( + OuterEnv.getValue(*LDecl, SkipPast::None)); + ASSERT_THAT(LVal, NotNull()); + + // The loop body may not have been executed, so we should not conclude + // that `l` points to `val`. + EXPECT_NE(&LVal->getPointeeLoc(), + OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); +}); +} + } // namespace Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -239,9 +239,6 @@ if (ExprToLoc != Other.ExprToLoc) return false; - if (MemberLocToStruct != Other.MemberLocToStruct) - return false; - // Compare the contents for the intersection of their domains. for (auto &Entry : LocToVal) { const StorageLocation *Loc = Entry.first;
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp =================================================================== --- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -3120,4 +3120,59 @@ }); } +TEST_F(TransferTest, LoopWithStructReferenceAssignmentConverges) { + std::string Code = R"( + struct Lookup { + int x; + }; + + void target(Lookup val, bool b) { + const Lookup* l = nullptr; + while (b) { + l = &val; + /*[[p-inner]]*/ + } + (void)0; + /*[[p-outer]]*/ + } + )"; + // The key property that we are verifying is implicit in `runDataflow` -- + // namely, that the analysis succeeds, rather than hitting the maximum number + // of iterations. + runDataflow( + Code, [](llvm::ArrayRef< + std::pair<std::string, DataflowAnalysisState<NoopLattice>>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, + ElementsAre(Pair("p-outer", _), Pair("p-inner", _))); + const Environment &OuterEnv = Results[0].second.Env; + const Environment &InnerEnv = Results[1].second.Env; + + const ValueDecl *ValDecl = findValueDecl(ASTCtx, "val"); + ASSERT_THAT(ValDecl, NotNull()); + + const ValueDecl *LDecl = findValueDecl(ASTCtx, "l"); + ASSERT_THAT(LDecl, NotNull()); + + // Inner. + auto *LVal = dyn_cast<IndirectionValue>( + InnerEnv.getValue(*LDecl, SkipPast::None)); + ASSERT_THAT(LVal, NotNull()); + + EXPECT_EQ(&LVal->getPointeeLoc(), + InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); + + // Outer. + LVal = dyn_cast<IndirectionValue>( + OuterEnv.getValue(*LDecl, SkipPast::None)); + ASSERT_THAT(LVal, NotNull()); + + // The loop body may not have been executed, so we should not conclude + // that `l` points to `val`. + EXPECT_NE(&LVal->getPointeeLoc(), + OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference)); +}); +} + } // namespace Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp =================================================================== --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -239,9 +239,6 @@ if (ExprToLoc != Other.ExprToLoc) return false; - if (MemberLocToStruct != Other.MemberLocToStruct) - return false; - // Compare the contents for the intersection of their domains. for (auto &Entry : LocToVal) { const StorageLocation *Loc = Entry.first;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits