Author: Stanislav Gatev Date: 2022-03-22T08:35:34Z New Revision: 2ddd57ae1ec42c4aad8e70645cff82c877a94e3f
URL: https://github.com/llvm/llvm-project/commit/2ddd57ae1ec42c4aad8e70645cff82c877a94e3f DIFF: https://github.com/llvm/llvm-project/commit/2ddd57ae1ec42c4aad8e70645cff82c877a94e3f.diff LOG: [clang][dataflow] Model the behavior of optional and std swap Differential Revision: https://reviews.llvm.org/D122129 Reviewed-by: ymandel, xazax.hun Added: Modified: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 4d0b10435a094..bf325b04967c2 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -107,6 +107,12 @@ auto isOptionalNulloptAssignment() { hasArgument(1, hasNulloptType())); } +auto isStdSwapCall() { + return callExpr(callee(functionDecl(hasName("std::swap"))), + argumentCountIs(2), hasArgument(0, hasOptionalType()), + hasArgument(1, 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) { @@ -283,6 +289,50 @@ void transferNulloptAssignment(const CXXOperatorCallExpr *E, transferAssignment(E, State.Env.getBoolLiteralValue(false), State); } +void transferSwap(const StorageLocation &OptionalLoc1, + const StorageLocation &OptionalLoc2, + LatticeTransferState &State) { + auto *OptionalVal1 = State.Env.getValue(OptionalLoc1); + assert(OptionalVal1 != nullptr); + + auto *OptionalVal2 = State.Env.getValue(OptionalLoc2); + assert(OptionalVal2 != nullptr); + + State.Env.setValue(OptionalLoc1, *OptionalVal2); + State.Env.setValue(OptionalLoc2, *OptionalVal1); +} + +void transferSwapCall(const CXXMemberCallExpr *E, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(E->getNumArgs() == 1); + + auto *OptionalLoc1 = State.Env.getStorageLocation( + *E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer); + assert(OptionalLoc1 != nullptr); + + auto *OptionalLoc2 = + State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference); + assert(OptionalLoc2 != nullptr); + + transferSwap(*OptionalLoc1, *OptionalLoc2, State); +} + +void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(E->getNumArgs() == 2); + + auto *OptionalLoc1 = + State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference); + assert(OptionalLoc1 != nullptr); + + auto *OptionalLoc2 = + State.Env.getStorageLocation(*E->getArg(1), SkipPast::Reference); + assert(OptionalLoc2 != nullptr); + + transferSwap(*OptionalLoc1, *OptionalLoc2, State); +} + auto buildTransferMatchSwitch() { // FIXME: Evaluate the efficiency of matchers. If using matchers results in a // lot of duplicated work (e.g. string comparisons), consider providing APIs @@ -361,6 +411,13 @@ auto buildTransferMatchSwitch() { State.Env.getBoolLiteralValue(false)); }) + // optional::swap + .CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("swap"), + transferSwapCall) + + // std::swap + .CaseOf<CallExpr>(isStdSwapCall(), transferStdSwapCall) + .Build(); } diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index fbc17668447ff..76340aad5fd4d 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -507,6 +507,9 @@ namespace std { template <typename T> constexpr remove_reference_t<T>&& move(T&& x); +template <typename T> +void swap(T& a, T& b) noexcept; + } // namespace std #endif // UTILITY_H @@ -718,6 +721,8 @@ class optional : private __optional_storage_base<_Tp> { constexpr explicit operator bool() const noexcept; using __base::has_value; + + constexpr void swap(optional& __opt) noexcept; }; template <typename T> @@ -938,6 +943,8 @@ class optional { constexpr explicit operator bool() const noexcept; constexpr bool has_value() const noexcept; + + void swap(optional& rhs) noexcept; }; template <typename T> @@ -1129,6 +1136,8 @@ class Optional { constexpr explicit operator bool() const noexcept; constexpr bool has_value() const noexcept; + + void swap(Optional& other); }; template <typename T> @@ -1911,10 +1920,93 @@ TEST_P(UncheckedOptionalAccessTest, NulloptAssignment) { UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7"))); } +TEST_P(UncheckedOptionalAccessTest, OptionalSwap) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional<int> opt1 = $ns::nullopt; + $ns::$optional<int> opt2 = 3; + + opt1.swap(opt2); + + opt1.value(); + /*[[check-1]]*/ + + opt2.value(); + /*[[check-2]]*/ + } + )", + UnorderedElementsAre(Pair("check-1", "safe"), + Pair("check-2", "unsafe: input.cc:13:7"))); + + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional<int> opt1 = $ns::nullopt; + $ns::$optional<int> opt2 = 3; + + opt2.swap(opt1); + + opt1.value(); + /*[[check-3]]*/ + + opt2.value(); + /*[[check-4]]*/ + } + )", + UnorderedElementsAre(Pair("check-3", "safe"), + Pair("check-4", "unsafe: input.cc:13:7"))); +} + +TEST_P(UncheckedOptionalAccessTest, StdSwap) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional<int> opt1 = $ns::nullopt; + $ns::$optional<int> opt2 = 3; + + std::swap(opt1, opt2); + + opt1.value(); + /*[[check-1]]*/ + + opt2.value(); + /*[[check-2]]*/ + } + )", + UnorderedElementsAre(Pair("check-1", "safe"), + Pair("check-2", "unsafe: input.cc:13:7"))); + + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + void target() { + $ns::$optional<int> opt1 = $ns::nullopt; + $ns::$optional<int> opt2 = 3; + + std::swap(opt2, opt1); + + opt1.value(); + /*[[check-3]]*/ + + opt2.value(); + /*[[check-4]]*/ + } + )", + UnorderedElementsAre(Pair("check-3", "safe"), + Pair("check-4", "unsafe: input.cc:13:7"))); +} + // FIXME: Add support for: // - constructors (copy, move) // - assignment operators (default, copy, move) -// - swap // - invalidation (passing optional by non-const reference/pointer) // - `value_or(nullptr) != nullptr`, `value_or(0) != 0`, `value_or("").empty()` // - nested `optional` values _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
