diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h --- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h +++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h @@ -59,6 +59,10 @@ void transfer(const Stmt *Stmt, SourceLocationsLattice &State, Environment &Env); + bool merge(QualType Type, const Value &Val1, const Environment &Env1, + const Value &Val2, const Environment &Env2, Value &MergedVal, + Environment &MergedEnv) override; + private: MatchSwitch> TransferMatchSwitch; }; diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -158,18 +158,24 @@ ComparesToSame(integerLiteral(equals(0))))); } +/// Sets `HasValueVal` as the symbolic value that represents the "has_value" +/// property of the optional value `OptionalVal`. +void setHasValue(StructValue &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(); - OptionalVal->setProperty("has_value", HasValueVal); + setHasValue(*OptionalVal, HasValueVal); return Env.takeOwnership(std::move(OptionalVal)); } /// Returns the symbolic value that represents the "has_value" property of the /// optional value `Val`. Returns null if `Val` is null. -BoolValue *getHasValue(Value *Val) { - if (auto *OptionalVal = cast_or_null(Val)) { +BoolValue *getHasValue(const Value *Val) { + if (const auto *OptionalVal = cast_or_null(Val)) { return cast(OptionalVal->getProperty("has_value")); } return nullptr; @@ -213,7 +219,7 @@ if (auto *OptionalVal = cast_or_null( State.Env.getValue(*OptionalExpr, SkipPast::Reference))) { if (OptionalVal->getProperty("has_value") == nullptr) { - OptionalVal->setProperty("has_value", State.Env.makeAtomicBoolValue()); + setHasValue(*OptionalVal, State.Env.makeAtomicBoolValue()); } } } @@ -570,5 +576,28 @@ TransferMatchSwitch(*S, getASTContext(), State); } +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 *HasValueVal1 = getHasValue(&Val1); + assert(HasValueVal1 != nullptr); + + auto *HasValueVal2 = getHasValue(&Val2); + assert(HasValueVal2 != nullptr); + + setHasValue( + *cast(&MergedVal), + MergedEnv.makeOr( + MergedEnv.makeAnd(Env1.getFlowConditionToken(), *HasValueVal1), + MergedEnv.makeAnd(Env2.getFlowConditionToken(), *HasValueVal2))); + return true; +} + } // namespace dataflow } // namespace clang diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -1852,7 +1852,25 @@ )", UnorderedElementsAre(Pair("check", "safe"))); - // FIXME: Add tests that call `emplace` in conditional branches. + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional 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) { @@ -1883,7 +1901,26 @@ )", UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9"))); - // FIXME: Add tests that call `reset` in conditional branches. + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target(bool b) { + $ns::$optional 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) { @@ -2150,6 +2187,106 @@ UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe"))); } +TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) { + ExpectLatticeChecksFor(R"code( + #include "unchecked_optional_access_test.h" + + void target(bool b, $ns::$optional opt) { + if (b || opt.has_value()) { + if (!b) { + opt.value(); + /*[[check]]*/ + } + } + } + )code", + UnorderedElementsAre(Pair("check", "safe"))); + + ExpectLatticeChecksFor(R"code( + #include "unchecked_optional_access_test.h" + + void target(bool b, $ns::$optional opt) { + if (b && !opt.has_value()) return; + if (b) { + opt.value(); + /*[[check]]*/ + } + } + )code", + UnorderedElementsAre(Pair("check", "safe"))); + + ExpectLatticeChecksFor( + R"code( + #include "unchecked_optional_access_test.h" + + void target(bool b, $ns::$optional opt) { + if (opt.has_value()) b = true; + if (b) { + opt.value(); + /*[[check]]*/ + } + } + )code", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9"))); + + ExpectLatticeChecksFor(R"code( + #include "unchecked_optional_access_test.h" + + void target(bool b, $ns::$optional opt) { + if (b) return; + if (opt.has_value()) b = true; + if (b) { + opt.value(); + /*[[check]]*/ + } + } + )code", + UnorderedElementsAre(Pair("check", "safe"))); + + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + void target(bool b, $ns::$optional opt) { + if (opt.has_value() == b) { + if (b) { + opt.value(); + /*[[check]]*/ + } + } + } + )", + UnorderedElementsAre(Pair("check", "safe"))); + + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + void target(bool b, $ns::$optional opt) { + if (opt.has_value() != b) { + if (!b) { + opt.value(); + /*[[check]]*/ + } + } + } + )", + UnorderedElementsAre(Pair("check", "safe"))); + + // FIXME: Add support for operator==. + // ExpectLatticeChecksFor(R"( + // #include "unchecked_optional_access_test.h" + // + // void target($ns::$optional opt1, $ns::$optional opt2) { + // if (opt1 == opt2) { + // if (opt1.has_value()) { + // opt2.value(); + // /*[[check]]*/ + // } + // } + // } + // )", + // UnorderedElementsAre(Pair("check", "safe"))); +} + // FIXME: Add support for: // - constructors (copy, move) // - assignment operators (default, copy, move)