[clang] [clang][dataflow] Fix assignment of unknown values. (PR #178943)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-analysis

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