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 @@ -11,6 +11,8 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" #include +#include +#include namespace clang { namespace dataflow { @@ -41,6 +43,21 @@ callee(cxxMethodDecl(ofClass(optionalClass())))); } +static auto isMakeOptionalCall() { + return callExpr( + callee(functionDecl(hasAnyName( + "std::make_optional", "base::make_optional", "absl::make_optional"))), + hasOptionalType()); +} + +/// 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); + 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. static BoolValue *getHasValue(Value *Val) { @@ -75,6 +92,13 @@ State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc()); } +void transferMakeOptionalCall(const CallExpr *E, LatticeTransferState &State) { + auto &Loc = State.Env.createStorageLocation(*E); + State.Env.setStorageLocation(*E, Loc); + State.Env.setValue( + Loc, createOptionalValue(State.Env, State.Env.getBoolLiteralValue(true))); +} + static void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr, LatticeTransferState &State) { if (auto *OptionalVal = cast_or_null( @@ -89,6 +113,26 @@ } } +void transferEmplaceCall(const CXXMemberCallExpr *E, + LatticeTransferState &State) { + if (auto *OptionalLoc = State.Env.getStorageLocation( + *E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer)) { + State.Env.setValue( + *OptionalLoc, + createOptionalValue(State.Env, State.Env.getBoolLiteralValue(true))); + } +} + +void transferResetCall(const CXXMemberCallExpr *E, + LatticeTransferState &State) { + if (auto *OptionalLoc = State.Env.getStorageLocation( + *E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer)) { + State.Env.setValue( + *OptionalLoc, + createOptionalValue(State.Env, State.Env.getBoolLiteralValue(false))); + } +} + static auto buildTransferMatchSwitch() { return MatchSwitchBuilder() // Attach a symbolic "has_value" state to optional values that we see for @@ -96,6 +140,9 @@ .CaseOf(expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()), initializeOptionalReference) + // make_optional + .CaseOf(isMakeOptionalCall(), transferMakeOptionalCall) + // optional::value .CaseOf( isOptionalMemberCallWithName("value"), @@ -115,6 +162,16 @@ .CaseOf(isOptionalMemberCallWithName("has_value"), transferOptionalHasValueCall) + // optional::operator bool + .CaseOf(isOptionalMemberCallWithName("operator bool"), + transferOptionalHasValueCall) + + // optional::emplace + .CaseOf(isOptionalMemberCallWithName("emplace"), transferEmplaceCall) + + // optional::reset + .CaseOf(isOptionalMemberCallWithName("reset"), transferResetCall) + .Build(); } 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 @@ -29,9 +29,23 @@ using ::testing::Pair; using ::testing::UnorderedElementsAre; +// FIXME: Move header definitions in separate file(s). static constexpr char StdTypeTraitsHeader[] = R"( +#ifndef TYPE_TRAITS_H +#define TYPE_TRAITS_H + namespace std { +typedef decltype(sizeof(char)) size_t; + +template +struct integral_constant { + static constexpr T value = V; +}; + +using true_type = integral_constant; +using false_type = integral_constant; + template< class T > struct remove_reference {typedef T type;}; template< class T > struct remove_reference {typedef T type;}; template< class T > struct remove_reference {typedef T type;}; @@ -39,21 +53,135 @@ template using remove_reference_t = typename remove_reference::type; +template +struct remove_extent { + typedef T type; +}; + +template +struct remove_extent { + typedef T type; +}; + +template +struct remove_extent { + typedef T type; +}; + +template +struct is_array : false_type {}; + +template +struct is_array : true_type {}; + +template +struct is_array : true_type {}; + +template +struct is_function : false_type {}; + +template +struct is_function : true_type {}; + +namespace detail { + +template +struct type_identity { + using type = T; +}; // or use type_identity (since C++20) + +template +auto try_add_pointer(int) -> type_identity::type*>; +template +auto try_add_pointer(...) -> type_identity; + +} // namespace detail + +template +struct add_pointer : decltype(detail::try_add_pointer(0)) {}; + +template +struct conditional { + typedef T type; +}; + +template +struct conditional { + typedef F type; +}; + +template +struct remove_cv { + typedef T type; +}; +template +struct remove_cv { + typedef T type; +}; +template +struct remove_cv { + typedef T type; +}; +template +struct remove_cv { + typedef T type; +}; + +template +struct decay { + private: + typedef typename remove_reference::type U; + + public: + typedef typename conditional< + is_array::value, typename remove_extent::type*, + typename conditional::value, typename add_pointer::type, + typename remove_cv::type>::type>::type type; +}; + } // namespace std + +#endif // TYPE_TRAITS_H )"; static constexpr char StdUtilityHeader[] = R"( +#ifndef UTILITY_H +#define UTILITY_H + #include "std_type_traits.h" namespace std { template -constexpr std::remove_reference_t&& move(T&& x); +constexpr remove_reference_t&& move(T&& x); + +} // namespace std + +#endif // UTILITY_H +)"; + +static constexpr char StdInitializerListHeader[] = R"( +#ifndef INITIALIZER_LIST_H +#define INITIALIZER_LIST_H + +namespace std { + +template +class initializer_list { + public: + initializer_list() noexcept; +}; } // namespace std + +#endif // INITIALIZER_LIST_H )"; static constexpr char StdOptionalHeader[] = R"( +#include "std_initializer_list.h" +#include "std_type_traits.h" +#include "std_utility.h" + namespace std { template @@ -74,13 +202,41 @@ const T&& value() const&&; T&& value() &&; + template + constexpr T value_or(U&& v) const&; + template + T value_or(U&& v) &&; + + template + T& emplace(Args&&... args); + + template + T& emplace(std::initializer_list ilist, Args&&... args); + + void reset() noexcept; + + constexpr explicit operator bool() const noexcept; constexpr bool has_value() const noexcept; }; +template +constexpr optional::type> make_optional(T&& v); + +template +constexpr optional make_optional(Args&&... args); + +template +constexpr optional make_optional(std::initializer_list il, + Args&&... args); + } // namespace std )"; static constexpr char AbslOptionalHeader[] = R"( +#include "std_initializer_list.h" +#include "std_type_traits.h" +#include "std_utility.h" + namespace absl { template @@ -101,13 +257,41 @@ const T&& value() const&&; T&& value() &&; + template + constexpr T value_or(U&& v) const&; + template + T value_or(U&& v) &&; + + template + T& emplace(Args&&... args); + + template + T& emplace(std::initializer_list ilist, Args&&... args); + + void reset() noexcept; + + constexpr explicit operator bool() const noexcept; constexpr bool has_value() const noexcept; }; +template +constexpr optional::type> make_optional(T&& v); + +template +constexpr optional make_optional(Args&&... args); + +template +constexpr optional make_optional(std::initializer_list il, + Args&&... args); + } // namespace absl )"; static constexpr char BaseOptionalHeader[] = R"( +#include "std_initializer_list.h" +#include "std_type_traits.h" +#include "std_utility.h" + namespace base { template @@ -128,9 +312,33 @@ const T&& value() const&&; T&& value() &&; + template + constexpr T value_or(U&& v) const&; + template + T value_or(U&& v) &&; + + template + T& emplace(Args&&... args); + + template + T& emplace(std::initializer_list ilist, Args&&... args); + + void reset() noexcept; + + constexpr explicit operator bool() const noexcept; constexpr bool has_value() const noexcept; }; +template +constexpr Optional::type> make_optional(T&& v); + +template +constexpr Optional make_optional(Args&&... args); + +template +constexpr Optional make_optional(std::initializer_list il, + Args&&... args); + } // namespace base )"; @@ -177,6 +385,7 @@ ReplaceAllOccurrences(SourceCode, "$optional", GetParam().TypeName); std::vector> Headers; + Headers.emplace_back("std_initializer_list.h", StdInitializerListHeader); Headers.emplace_back("std_type_traits.h", StdTypeTraitsHeader); Headers.emplace_back("std_utility.h", StdUtilityHeader); Headers.emplace_back("std_optional.h", StdOptionalHeader); @@ -185,8 +394,12 @@ Headers.emplace_back("unchecked_optional_access_test.h", R"( #include "absl_optional.h" #include "base_optional.h" + #include "std_initializer_list.h" #include "std_optional.h" #include "std_utility.h" + + template + T Make(); )"); const tooling::FileContentMappings FileContents(Headers.begin(), Headers.end()); @@ -315,7 +528,7 @@ UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7"))); } -TEST_P(UncheckedOptionalAccessTest, UnwrapWithCheck) { +TEST_P(UncheckedOptionalAccessTest, HasValueCheck) { ExpectLatticeChecksFor(R"( #include "unchecked_optional_access_test.h" @@ -329,13 +542,179 @@ UnorderedElementsAre(Pair("check", "safe"))); } +TEST_P(UncheckedOptionalAccessTest, OperatorBoolCheck) { + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional opt) { + if (opt) { + opt.value(); + /*[[check]]*/ + } + } + )", + UnorderedElementsAre(Pair("check", "safe"))); +} + +TEST_P(UncheckedOptionalAccessTest, UnwrapFunctionCallResultNoCheck) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + Make<$ns::$optional>().value(); + (void)0; + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7"))); + + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional opt) { + std::move(opt).value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7"))); +} + +TEST_P(UncheckedOptionalAccessTest, DefaultConstructor) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional opt; + opt.value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7"))); +} + +TEST_P(UncheckedOptionalAccessTest, MakeOptional) { + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional opt = $ns::make_optional(0); + opt.value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "safe"))); + + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + struct Foo { + Foo(int, int); + }; + + void target() { + $ns::$optional opt = $ns::make_optional(21, 22); + opt.value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "safe"))); + + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + struct Foo { + constexpr Foo(std::initializer_list); + }; + + void target() { + char a = 'a'; + $ns::$optional opt = $ns::make_optional({a}); + opt.value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "safe"))); +} + +TEST_P(UncheckedOptionalAccessTest, ValueOr) { + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional opt; + opt.value_or(0); + (void)0; + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "safe"))); +} + +TEST_P(UncheckedOptionalAccessTest, Emplace) { + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional opt; + opt.emplace(0); + opt.value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "safe"))); + + ExpectLatticeChecksFor(R"( + #include "unchecked_optional_access_test.h" + + void target($ns::$optional *opt) { + opt->emplace(0); + opt->value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "safe"))); + + // FIXME: Add tests that call `emplace` in conditional branches. +} + +TEST_P(UncheckedOptionalAccessTest, Reset) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional opt = $ns::make_optional(0); + opt.reset(); + opt.value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:7"))); + + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional *opt; + *opt = $ns::make_optional(0); + opt->reset(); + opt->value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7"))); + + // FIXME: Add tests that call `reset` in conditional branches. +} + // FIXME: Add support for: -// - constructors (default, copy, move, non-standard) +// - constructors (copy, move, non-standard) // - assignment operators (default, copy, move, non-standard) -// - operator bool -// - emplace -// - reset -// - value_or // - swap -// - make_optional // - invalidation (passing optional by non-const reference/pointer) +// - `value_or(nullptr) != nullptr`, `value_or(0) != 0`, `value_or("").empty()` +// - nested `optional` values