llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Artem Dergachev (haoNoQ) <details> <summary>Changes</summary> Just because the right-hand side of the assignment doesn't have a known value, doesn't mean the left-hand side gets to keep its old value. --- Full diff: https://github.com/llvm/llvm-project/pull/178943.diff 2 Files Affected: - (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+1-1) - (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+76) ``````````diff diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 9d1b00293f394..51cc1f9bc26ab 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -170,7 +170,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { auto *RHSVal = Env.getValue(*RHS); if (RHSVal == nullptr) - break; + RHSVal = Env.createValue(LHS->getType()); // Assign a value to the storage location of the left-hand side. Env.setValue(*LHSLoc, *RHSVal); diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index a6308d115aa70..5ff0983543369 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -935,6 +935,82 @@ TEST(TransferTest, BinaryOperatorAssignIntegerLiteral) { }); } +TEST(TransferTest, BinaryOperatorAssignUnknown) { + std::string Code = R"( + int unknown(); + + void target() { + int Foo = unknown(); + int FooAtA = Foo; + + Foo = unknown(); + int FooAtB = Foo; + + Foo += unknown(); + int FooAtC = Foo; + + // [[p]] + + if (FooAtA != FooAtB) { + (void)0; + // [[q]] + } + + if (FooAtB != FooAtC) { + (void)0; + // [[r]] + } + } + )"; + using ast_matchers::binaryOperator; + using ast_matchers::match; + using ast_matchers::selectFirst; + runDataflow( + Code, + [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p", "q", "r")); + + // Check that the second unknown value is different. + const Environment &EnvP = getEnvironmentAtAnnotation(Results, "p"); + + const ValueDecl *FooAtADecl = findValueDecl(ASTCtx, "FooAtA"); + ASSERT_THAT(FooAtADecl, NotNull()); + const Value *FooAtAVal = EnvP.getValue(*FooAtADecl); + ASSERT_TRUE(isa_and_nonnull<IntegerValue>(FooAtAVal)); + + const ValueDecl *FooAtBDecl = findValueDecl(ASTCtx, "FooAtB"); + ASSERT_THAT(FooAtBDecl, NotNull()); + const Value *FooAtBVal = EnvP.getValue(*FooAtBDecl); + ASSERT_TRUE(isa_and_nonnull<IntegerValue>(FooAtBVal)); + + const ValueDecl *FooAtCDecl = findValueDecl(ASTCtx, "FooAtC"); + ASSERT_THAT(FooAtCDecl, NotNull()); + const Value *FooAtCVal = EnvP.getValue(*FooAtCDecl); + ASSERT_TRUE(isa_and_nonnull<IntegerValue>(FooAtCVal)); + + EXPECT_NE(FooAtAVal, FooAtBVal); + // FIXME: Should be NE too. + EXPECT_EQ(FooAtBVal, FooAtCVal); + + // Check that the storage location is correctly propagated. + auto MatchResult = match(binaryOperator().bind("bo"), ASTCtx); + const auto *BO = selectFirst<BinaryOperator>("bo", MatchResult); + EXPECT_NE(BO, nullptr); + const StorageLocation *BOLoc = EnvP.getStorageLocation(*BO); + EXPECT_NE(BOLoc, nullptr); + + // Check that the branches are reachable + // with a non-false flow condition. + const Environment &EnvQ = getEnvironmentAtAnnotation(Results, "q"); + const Environment &EnvR = getEnvironmentAtAnnotation(Results, "r"); + + EXPECT_FALSE(EnvQ.proves(EnvQ.arena().makeLiteral(false))); + // FIXME: Should be FALSE too. + EXPECT_TRUE(EnvR.proves(EnvR.arena().makeLiteral(false))); + }); +} + TEST(TransferTest, VarDeclInitAssign) { std::string Code = R"( void target() { `````````` </details> https://github.com/llvm/llvm-project/pull/178943 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
