Author: Sam Estep Date: 2022-06-10T14:52:05Z New Revision: cd0d52610d80116324f19983320ec6db2048450f
URL: https://github.com/llvm/llvm-project/commit/cd0d52610d80116324f19983320ec6db2048450f DIFF: https://github.com/llvm/llvm-project/commit/cd0d52610d80116324f19983320ec6db2048450f.diff LOG: [clang][dataflow] In `optional` model, match call return via hasType Currently the unchecked-optional-access model fails on this example: #include <memory> #include <optional> void foo() { std::unique_ptr<std::optional<float>> x; *x = std::nullopt; } You can verify the failure by saving the file as `foo.cpp` and running this command: clang-tidy -checks='-*,bugprone-unchecked-optional-access' foo.cpp -- -std=c++17 The failing `assert` is in the `transferAssignment` function of the `UncheckedOptionalAccessModel.cpp` file: assert(OptionalLoc != nullptr); The symptom can be treated by replacing that `assert` with an early `return`: if (OptionalLoc == nullptr) return; That would be better anyway since we cannot expect to always cover all possible LHS expressions, but it is out of scope for this patch and left as a followup. Note that the failure did not occur on this very similar example: #include <optional> template <typename T> struct smart_ptr { T& operator*() &; T* operator->(); }; void foo() { smart_ptr<std::optional<float>> x; *x = std::nullopt; } The difference is caused by the `isCallReturningOptional` matcher, which was previously checking the `functionDecl` of the `callee`. This patch changes it to instead use `hasType` directly on the call expression, fixing the failure for the `std::unique_ptr` example above. Reviewed By: sgatev Differential Revision: https://reviews.llvm.org/D127434 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 00d22fbe8bb80..993d56e44a87d 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -165,8 +165,8 @@ auto isValueOrNotEqX() { } auto isCallReturningOptional() { - return callExpr(callee(functionDecl(returns(anyOf( - optionalOrAliasType(), referenceType(pointee(optionalOrAliasType()))))))); + return callExpr(hasType(qualType(anyOf( + optionalOrAliasType(), referenceType(pointee(optionalOrAliasType())))))); } /// Creates a symbolic value for an `optional` value using `HasValueVal` as the diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 72efc724776b3..3c37bec82139c 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -179,6 +179,11 @@ struct is_void : is_same<void, typename remove_cv<T>::type> {}; namespace detail { +template <class T> +auto try_add_lvalue_reference(int) -> type_identity<T&>; +template <class T> +auto try_add_lvalue_reference(...) -> type_identity<T>; + template <class T> auto try_add_rvalue_reference(int) -> type_identity<T&&>; template <class T> @@ -186,6 +191,10 @@ auto try_add_rvalue_reference(...) -> type_identity<T>; } // namespace detail +template <class T> +struct add_lvalue_reference : decltype(detail::try_add_lvalue_reference<T>(0)) { +}; + template <class T> struct add_rvalue_reference : decltype(detail::try_add_rvalue_reference<T>(0)) { }; @@ -2318,6 +2327,26 @@ TEST_P(UncheckedOptionalAccessTest, OptionalValueInitialization) { UnorderedElementsAre(Pair("merge", "unsafe: input.cc:19:7"))); } +TEST_P(UncheckedOptionalAccessTest, AssignThroughLvalueReferencePtr) { + ExpectLatticeChecksFor( + R"( + #include "unchecked_optional_access_test.h" + + template <typename T> + struct smart_ptr { + typename std::add_lvalue_reference<T>::type operator*() &; + }; + + void target() { + smart_ptr<$ns::$optional<float>> x; + *x = $ns::nullopt; + (*x).value(); + /*[[check]]*/ + } + )", + UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7"))); +} + // FIXME: Add support for: // - constructors (copy, move) // - assignment operators (default, copy, move) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
