From: Stanislav Gatev <sgatev@google.com>
Date: Wed, 18 May 2022 21:57:40 +0000 (+0000)
Subject: [clang][dataflow] Add support for correlated branches to optional model
X-Git-Tag: upstream/15.0.7~4678
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8fcdd625856b2e7df2fdb3a4c57efedb35e4d7c1;p=platform%2Fupstream%2Fllvm.git

[clang][dataflow] Add support for correlated branches to optional model

Add support for correlated branches to the std::optional dataflow model.

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

Reviewed-by: ymandel, xazax.hun
---

diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
index 8cc3fa7..2102ed5 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -59,6 +59,14 @@ public:
   void transfer(const Stmt *Stmt, SourceLocationsLattice &State,
                 Environment &Env);
 
+  bool compareEquivalent(QualType Type, const Value &Val1,
+                         const Environment &Env1, const Value &Val2,
+                         const Environment &Env2) override;
+
+  bool merge(QualType Type, const Value &Val1, const Environment &Env1,
+             const Value &Val2, const Environment &Env2, Value &MergedVal,
+             Environment &MergedEnv) override;
+
 private:
   MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch;
 };
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 23a9d964..5029741 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -169,11 +169,17 @@ auto isCallReturningOptional() {
       optionalOrAliasType(), referenceType(pointee(optionalOrAliasType()))))));
 }
 
+/// Sets `HasValueVal` as the symbolic value that represents the "has_value"
+/// property of the optional value `OptionalVal`.
+void setHasValue(Value &OptionalVal, BoolValue &HasValueVal) {
+  OptionalVal.setProperty("has_value", HasValueVal);
+}
+
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
 /// symbolic value of its "has_value" property.
 StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
   auto OptionalVal = std::make_unique<StructValue>();
-  OptionalVal->setProperty("has_value", HasValueVal);
+  setHasValue(*OptionalVal, HasValueVal);
   return Env.takeOwnership(std::move(OptionalVal));
 }
 
@@ -274,11 +280,28 @@ void initializeOptionalReference(const Expr *OptionalExpr,
   if (auto *OptionalVal =
           State.Env.getValue(*OptionalExpr, SkipPast::Reference)) {
     if (OptionalVal->getProperty("has_value") == nullptr) {
-      OptionalVal->setProperty("has_value", State.Env.makeAtomicBoolValue());
+      setHasValue(*OptionalVal, State.Env.makeAtomicBoolValue());
     }
   }
 }
 
+/// Returns true if and only if `OptionalVal` is initialized and known to be
+/// empty in `Env.
+bool isEmptyOptional(const Value &OptionalVal, const Environment &Env) {
+  auto *HasValueVal =
+      cast_or_null<BoolValue>(OptionalVal.getProperty("has_value"));
+  return HasValueVal != nullptr &&
+         Env.flowConditionImplies(Env.makeNot(*HasValueVal));
+}
+
+/// Returns true if and only if `OptionalVal` is initialized and known to be
+/// non-empty in `Env.
+bool isNonEmptyOptional(const Value &OptionalVal, const Environment &Env) {
+  auto *HasValueVal =
+      cast_or_null<BoolValue>(OptionalVal.getProperty("has_value"));
+  return HasValueVal != nullptr && Env.flowConditionImplies(*HasValueVal);
+}
+
 void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
                         LatticeTransferState &State) {
   if (auto *OptionalVal =
@@ -651,5 +674,31 @@ void UncheckedOptionalAccessModel::transfer(const Stmt *S,
   TransferMatchSwitch(*S, getASTContext(), State);
 }
 
+bool UncheckedOptionalAccessModel::compareEquivalent(QualType Type,
+                                                     const Value &Val1,
+                                                     const Environment &Env1,
+                                                     const Value &Val2,
+                                                     const Environment &Env2) {
+  return isNonEmptyOptional(Val1, Env1) == isNonEmptyOptional(Val2, Env2);
+}
+
+bool UncheckedOptionalAccessModel::merge(QualType Type, const Value &Val1,
+                                         const Environment &Env1,
+                                         const Value &Val2,
+                                         const Environment &Env2,
+                                         Value &MergedVal,
+                                         Environment &MergedEnv) {
+  if (!IsOptionalType(Type))
+    return true;
+
+  auto &HasValueVal = MergedEnv.makeAtomicBoolValue();
+  if (isNonEmptyOptional(Val1, Env1) && isNonEmptyOptional(Val2, Env2))
+    MergedEnv.addToFlowCondition(HasValueVal);
+  else if (isEmptyOptional(Val1, Env1) && isEmptyOptional(Val2, Env2))
+    MergedEnv.addToFlowCondition(MergedEnv.makeNot(HasValueVal));
+  setHasValue(MergedVal, HasValueVal);
+  return true;
+}
+
 } // namespace dataflow
 } // namespace clang
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 3c37bec..912754e 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1861,7 +1861,26 @@ TEST_P(UncheckedOptionalAccessTest, Emplace) {
   )",
                          UnorderedElementsAre(Pair("check", "safe")));
 
-  // FIXME: Add tests that call `emplace` in conditional branches.
+  // FIXME: Add tests that call `emplace` in conditional branches:
+  //  ExpectLatticeChecksFor(
+  //      R"(
+  //    #include "unchecked_optional_access_test.h"
+  //
+  //    void target($ns::$optional<int> opt, bool b) {
+  //      if (b) {
+  //        opt.emplace(0);
+  //      }
+  //      if (b) {
+  //        opt.value();
+  //        /*[[check-1]]*/
+  //      } else {
+  //        opt.value();
+  //        /*[[check-2]]*/
+  //      }
+  //    }
+  //  )",
+  //      UnorderedElementsAre(Pair("check-1", "safe"),
+  //                           Pair("check-2", "unsafe: input.cc:12:9")));
 }
 
 TEST_P(UncheckedOptionalAccessTest, Reset) {
@@ -1892,7 +1911,27 @@ TEST_P(UncheckedOptionalAccessTest, Reset) {
   )",
       UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9")));
 
-  // FIXME: Add tests that call `reset` in conditional branches.
+  // FIXME: Add tests that call `reset` in conditional branches:
+  //  ExpectLatticeChecksFor(
+  //      R"(
+  //    #include "unchecked_optional_access_test.h"
+  //
+  //    void target(bool b) {
+  //      $ns::$optional<int> opt = $ns::make_optional(0);
+  //      if (b) {
+  //        opt.reset();
+  //      }
+  //      if (b) {
+  //        opt.value();
+  //        /*[[check-1]]*/
+  //      } else {
+  //        opt.value();
+  //        /*[[check-2]]*/
+  //      }
+  //    }
+  //  )",
+  //      UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:10:9"),
+  //                           Pair("check-2", "safe")));
 }
 
 TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
@@ -2347,6 +2386,284 @@ TEST_P(UncheckedOptionalAccessTest, AssignThroughLvalueReferencePtr) {
       UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) {
+  ExpectLatticeChecksFor(R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b, $ns::$optional<int> opt) {
+      if (b || opt.has_value()) {
+        if (!b) {
+          opt.value();
+          /*[[check-1]]*/
+        }
+      }
+    }
+  )code",
+                         UnorderedElementsAre(Pair("check-1", "safe")));
+
+  ExpectLatticeChecksFor(R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b, $ns::$optional<int> opt) {
+      if (b && !opt.has_value()) return;
+      if (b) {
+        opt.value();
+        /*[[check-2]]*/
+      }
+    }
+  )code",
+                         UnorderedElementsAre(Pair("check-2", "safe")));
+
+  ExpectLatticeChecksFor(
+      R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b, $ns::$optional<int> opt) {
+      if (opt.has_value()) b = true;
+      if (b) {
+        opt.value();
+        /*[[check-3]]*/
+      }
+    }
+  )code",
+      UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+
+  ExpectLatticeChecksFor(R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b, $ns::$optional<int> opt) {
+      if (b) return;
+      if (opt.has_value()) b = true;
+      if (b) {
+        opt.value();
+        /*[[check-4]]*/
+      }
+    }
+  )code",
+                         UnorderedElementsAre(Pair("check-4", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b, $ns::$optional<int> opt) {
+      if (opt.has_value() == b) {
+        if (b) {
+          opt.value();
+          /*[[check-5]]*/
+        }
+      }
+    }
+  )",
+                         UnorderedElementsAre(Pair("check-5", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b, $ns::$optional<int> opt) {
+      if (opt.has_value() != b) {
+        if (!b) {
+          opt.value();
+          /*[[check-6]]*/
+        }
+      }
+    }
+  )",
+                         UnorderedElementsAre(Pair("check-6", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b) {
+      $ns::$optional<int> opt1 = $ns::nullopt;
+      $ns::$optional<int> opt2;
+      if (b) {
+        opt2 = $ns::nullopt;
+      } else {
+        opt2 = $ns::nullopt;
+      }
+      if (opt2.has_value()) {
+        opt1.value();
+        /*[[check]]*/
+      }
+    }
+  )",
+                         UnorderedElementsAre(Pair("check", "safe")));
+
+  // FIXME: Add support for operator==.
+  // ExpectLatticeChecksFor(R"(
+  //   #include "unchecked_optional_access_test.h"
+  //
+  //   void target($ns::$optional<int> opt1, $ns::$optional<int> opt2) {
+  //     if (opt1 == opt2) {
+  //       if (opt1.has_value()) {
+  //         opt2.value();
+  //         /*[[check-7]]*/
+  //       }
+  //     }
+  //   }
+  // )",
+  //                     UnorderedElementsAre(Pair("check-7", "safe")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
+  ExpectLatticeChecksFor(
+      R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b) {
+      $ns::$optional<int> opt;
+      if (b) {
+        opt = Make<$ns::$optional<int>>();
+      } else {
+        opt = Make<$ns::$optional<int>>();
+      }
+      if (opt.has_value()) {
+        opt.value();
+        /*[[check-1]]*/
+      } else {
+        opt.value();
+        /*[[check-2]]*/
+      }
+    }
+  )code",
+      UnorderedElementsAre(Pair("check-1", "safe"),
+                           Pair("check-2", "unsafe: input.cc:15:9")));
+
+  ExpectLatticeChecksFor(R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b) {
+      $ns::$optional<int> opt;
+      if (b) {
+        opt = Make<$ns::$optional<int>>();
+        if (!opt.has_value()) return;
+      } else {
+        opt = Make<$ns::$optional<int>>();
+        if (!opt.has_value()) return;
+      }
+      opt.value();
+      /*[[check-3]]*/
+    }
+  )code",
+                         UnorderedElementsAre(Pair("check-3", "safe")));
+
+  ExpectLatticeChecksFor(
+      R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b) {
+      $ns::$optional<int> opt;
+      if (b) {
+        opt = Make<$ns::$optional<int>>();
+        if (!opt.has_value()) return;
+      } else {
+        opt = Make<$ns::$optional<int>>();
+      }
+      opt.value();
+      /*[[check-4]]*/
+    }
+  )code",
+      UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:12:7")));
+
+  ExpectLatticeChecksFor(
+      R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b) {
+      $ns::$optional<int> opt;
+      if (b) {
+        opt = 1;
+      } else {
+        opt = 2;
+      }
+      opt.value();
+      /*[[check-5]]*/
+    }
+  )code",
+      UnorderedElementsAre(Pair("check-5", "safe")));
+
+  ExpectLatticeChecksFor(
+      R"code(
+    #include "unchecked_optional_access_test.h"
+
+    void target(bool b) {
+      $ns::$optional<int> opt;
+      if (b) {
+        opt = 1;
+      } else {
+        opt = Make<$ns::$optional<int>>();
+      }
+      opt.value();
+      /*[[check-6]]*/
+    }
+  )code",
+      UnorderedElementsAre(Pair("check-6", "unsafe: input.cc:11:7")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, ReassignValueInLoop) {
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt = 3;
+      while (Make<bool>()) {
+        opt.value();
+        /*[[check-1]]*/
+      }
+    }
+  )",
+                         UnorderedElementsAre(Pair("check-1", "safe")));
+
+  ExpectLatticeChecksFor(R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt = 3;
+      while (Make<bool>()) {
+        opt.value();
+        /*[[check-2]]*/
+
+        opt = Make<$ns::$optional<int>>();
+        if (!opt.has_value()) return;
+      }
+    }
+  )",
+                         UnorderedElementsAre(Pair("check-2", "safe")));
+
+  ExpectLatticeChecksFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt = 3;
+      while (Make<bool>()) {
+        opt.value();
+        /*[[check-3]]*/
+
+        opt = Make<$ns::$optional<int>>();
+      }
+    }
+  )",
+      UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+
+  ExpectLatticeChecksFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+
+    void target() {
+      $ns::$optional<int> opt = 3;
+      while (Make<bool>()) {
+        opt.value();
+        /*[[check-4]]*/
+
+        opt = Make<$ns::$optional<int>>();
+        if (!opt.has_value()) continue;
+      }
+    }
+  )",
+      UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:7:9")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)