diff --git a/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h b/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h --- a/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h +++ b/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h @@ -70,28 +70,24 @@ /// \endcode template class MatchSwitchBuilder { public: - // An action is triggered by the match of a pattern against the input - // statement. For generality, actions take both the matched statement and the - // set of bindings produced by the match. - using Action = std::function; - - MatchSwitchBuilder &&CaseOf(ast_matchers::internal::Matcher M, - Action A) && { - Matchers.push_back(std::move(M)); - Actions.push_back(std::move(A)); - return std::move(*this); - } - - // Convenience function for the common case, where bound nodes are not - // needed. `Node` should be a subclass of `Stmt`. + /// Registers an action that will be triggered by the match of a pattern + /// against the input statement. + /// + /// Requirements: + /// + /// `Node` should be a subclass of `Stmt`. template - MatchSwitchBuilder &&CaseOf(ast_matchers::internal::Matcher M, - void (*Action)(const Node *, State &)) && { + MatchSwitchBuilder && + CaseOf(ast_matchers::internal::Matcher M, + std::function + A) && { Matchers.push_back(std::move(M)); - Actions.push_back([Action](const Stmt *Stmt, - const ast_matchers::MatchFinder::MatchResult &, - State &S) { Action(cast(Stmt), S); }); + Actions.push_back( + [A = std::move(A)](const Stmt *Stmt, + const ast_matchers::MatchFinder::MatchResult &R, + State &S) { A(cast(Stmt), R, S); }); return std::move(*this); } @@ -146,7 +142,9 @@ } std::vector Matchers; - std::vector Actions; + std::vector> + Actions; }; } // namespace dataflow } // namespace clang 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 @@ -22,7 +22,7 @@ using LatticeTransferState = TransferState; -static auto optionalClass() { +auto optionalClass() { return classTemplateSpecializationDecl( anyOf(hasName("std::optional"), hasName("std::__optional_storage_base"), hasName("__optional_destruct_base"), hasName("absl::optional"), @@ -30,26 +30,54 @@ hasTemplateArgument(0, refersToType(type().bind("T")))); } -static auto hasOptionalType() { return hasType(optionalClass()); } +auto hasOptionalType() { return hasType(optionalClass()); } -static auto isOptionalMemberCallWithName(llvm::StringRef MemberName) { +auto isOptionalMemberCallWithName(llvm::StringRef MemberName) { return cxxMemberCallExpr( on(expr(unless(cxxThisExpr()))), callee(cxxMethodDecl(hasName(MemberName), ofClass(optionalClass())))); } -static auto isOptionalOperatorCallWithName(llvm::StringRef OperatorName) { +auto isOptionalOperatorCallWithName(llvm::StringRef OperatorName) { return cxxOperatorCallExpr(hasOverloadedOperatorName(OperatorName), callee(cxxMethodDecl(ofClass(optionalClass())))); } -static auto isMakeOptionalCall() { +auto isMakeOptionalCall() { return callExpr( callee(functionDecl(hasAnyName( "std::make_optional", "base::make_optional", "absl::make_optional"))), hasOptionalType()); } +auto hasNulloptType() { + return hasType(namedDecl( + hasAnyName("std::nullopt_t", "absl::nullopt_t", "base::nullopt_t"))); +} + +auto inPlaceClass() { + return recordDecl( + hasAnyName("std::in_place_t", "absl::in_place_t", "base::in_place_t")); +} + +auto isOptionalNulloptConstructor() { + return cxxConstructExpr(hasOptionalType(), argumentCountIs(1), + hasArgument(0, hasNulloptType())); +} + +auto isOptionalInPlaceConstructor() { + return cxxConstructExpr(hasOptionalType(), + hasArgument(0, hasType(inPlaceClass()))); +} + +auto isOptionalValueOrConversionConstructor() { + return cxxConstructExpr( + hasOptionalType(), + unless(hasDeclaration( + cxxConstructorDecl(anyOf(isCopyConstructor(), isMoveConstructor())))), + argumentCountIs(1), hasArgument(0, unless(hasNulloptType()))); +} + /// 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) { @@ -60,15 +88,48 @@ /// 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) { +BoolValue *getHasValue(Value *Val) { if (auto *OptionalVal = cast_or_null(Val)) { return cast(OptionalVal->getProperty("has_value")); } return nullptr; } -static void initializeOptionalReference(const Expr *OptionalExpr, - LatticeTransferState &State) { +/// If `Type` is a reference type, returns the type of its pointee. Otherwise, +/// returns `Type` itself. +QualType stripReference(QualType Type) { + return Type->isReferenceType() ? Type->getPointeeType() : Type; +} + +/// Returns true if and only if `Type` is an optional type. +bool IsOptionalType(QualType Type) { + if (!Type->isRecordType()) + return false; + // FIXME: Optimize this by avoiding the `getQualifiedNameAsString` call. + auto TypeName = Type->getAsCXXRecordDecl()->getQualifiedNameAsString(); + return TypeName == "std::optional" || TypeName == "absl::optional" || + TypeName == "base::Optional"; +} + +/// Returns the number of optional wrappers in `Type`. +/// +/// For example, if `Type` is `optional>`, the result of this +/// function will be 2. +int countOptionalWrappers(const ASTContext &ASTCtx, QualType Type) { + if (!IsOptionalType(Type)) + return 0; + return 1 + countOptionalWrappers( + ASTCtx, + cast(Type->getAsRecordDecl()) + ->getTemplateArgs() + .get(0) + .getAsType() + .getDesugaredType(ASTCtx)); +} + +void initializeOptionalReference(const Expr *OptionalExpr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { if (auto *OptionalVal = cast_or_null( State.Env.getValue(*OptionalExpr, SkipPast::Reference))) { if (OptionalVal->getProperty("has_value") == nullptr) { @@ -77,8 +138,8 @@ } } -static void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr, - LatticeTransferState &State) { +void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr, + LatticeTransferState &State) { if (auto *OptionalVal = cast_or_null( State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer))) { auto *HasValueVal = getHasValue(OptionalVal); @@ -92,15 +153,18 @@ State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc()); } -void transferMakeOptionalCall(const CallExpr *E, LatticeTransferState &State) { +void transferMakeOptionalCall(const CallExpr *E, + const MatchFinder::MatchResult &, + 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) { +void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { if (auto *OptionalVal = cast_or_null( State.Env.getValue(*CallExpr->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer))) { @@ -113,64 +177,110 @@ } } -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 assignOptionalValue(const Expr &E, LatticeTransferState &State, + BoolValue &HasValueVal) { + if (auto *OptionalLoc = + State.Env.getStorageLocation(E, SkipPast::ReferenceThenPointer)) { + State.Env.setValue(*OptionalLoc, + createOptionalValue(State.Env, HasValueVal)); } } -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))); - } +void transferValueOrConversionConstructor( + const CXXConstructExpr *E, const MatchFinder::MatchResult &MatchRes, + LatticeTransferState &State) { + assert(E->getConstructor()->getTemplateSpecializationArgs()->size() > 0); + assert(E->getNumArgs() > 0); + + const int TemplateParamOptionalWrappersCount = countOptionalWrappers( + *MatchRes.Context, stripReference(E->getConstructor() + ->getTemplateSpecializationArgs() + ->get(0) + .getAsType())); + const int ArgTypeOptionalWrappersCount = countOptionalWrappers( + *MatchRes.Context, stripReference(E->getArg(0)->getType())); + auto *HasValueVal = + (TemplateParamOptionalWrappersCount == ArgTypeOptionalWrappersCount) + // This is a constructor call for optional with argument of type U + // such that T is constructible from U. + ? &State.Env.getBoolLiteralValue(true) + // This is a constructor call for optional with argument of type + // optional such that T is constructible from U. + : getHasValue(State.Env.getValue(*E->getArg(0), SkipPast::Reference)); + if (HasValueVal == nullptr) + HasValueVal = &State.Env.makeAtomicBoolValue(); + + assignOptionalValue(*E, State, *HasValueVal); } -static auto buildTransferMatchSwitch() { +auto buildTransferMatchSwitch() { return MatchSwitchBuilder() // Attach a symbolic "has_value" state to optional values that we see for // the first time. - .CaseOf(expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()), - initializeOptionalReference) + .CaseOf(expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()), + initializeOptionalReference) // make_optional - .CaseOf(isMakeOptionalCall(), transferMakeOptionalCall) + .CaseOf(isMakeOptionalCall(), transferMakeOptionalCall) + + // constructors: + .CaseOf( + isOptionalInPlaceConstructor(), + [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assignOptionalValue(*E, State, State.Env.getBoolLiteralValue(true)); + }) + .CaseOf( + isOptionalNulloptConstructor(), + [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assignOptionalValue(*E, State, + State.Env.getBoolLiteralValue(false)); + }) + .CaseOf(isOptionalValueOrConversionConstructor(), + transferValueOrConversionConstructor) // optional::value - .CaseOf( + .CaseOf( isOptionalMemberCallWithName("value"), - +[](const CXXMemberCallExpr *E, LatticeTransferState &State) { + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, + LatticeTransferState &State) { transferUnwrapCall(E, E->getImplicitObjectArgument(), State); }) // optional::operator*, optional::operator-> - .CaseOf( - expr(anyOf(isOptionalOperatorCallWithName("*"), - isOptionalOperatorCallWithName("->"))), - +[](const CallExpr *E, LatticeTransferState &State) { - transferUnwrapCall(E, E->getArg(0), State); - }) + .CaseOf(expr(anyOf(isOptionalOperatorCallWithName("*"), + isOptionalOperatorCallWithName("->"))), + [](const CallExpr *E, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + transferUnwrapCall(E, E->getArg(0), State); + }) // optional::has_value - .CaseOf(isOptionalMemberCallWithName("has_value"), - transferOptionalHasValueCall) + .CaseOf(isOptionalMemberCallWithName("has_value"), + transferOptionalHasValueCall) // optional::operator bool - .CaseOf(isOptionalMemberCallWithName("operator bool"), - transferOptionalHasValueCall) + .CaseOf(isOptionalMemberCallWithName("operator bool"), + transferOptionalHasValueCall) // optional::emplace - .CaseOf(isOptionalMemberCallWithName("emplace"), transferEmplaceCall) + .CaseOf( + isOptionalMemberCallWithName("emplace"), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assignOptionalValue(*E->getImplicitObjectArgument(), State, + State.Env.getBoolLiteralValue(true)); + }) // optional::reset - .CaseOf(isOptionalMemberCallWithName("reset"), transferResetCall) + .CaseOf( + isOptionalMemberCallWithName("reset"), + [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assignOptionalValue(*E->getImplicitObjectArgument(), State, + State.Env.getBoolLiteralValue(false)); + }) .Build(); } diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -179,6 +179,11 @@ Env.setValue(ExprLoc, *SubExprVal); break; } + case CK_UncheckedDerivedToBase: + case CK_ConstructorConversion: + case CK_UserDefinedConversion: + // FIXME: Add tests that excercise CK_UncheckedDerivedToBase, + // CK_ConstructorConversion, and CK_UserDefinedConversion. case CK_NoOp: { // FIXME: Consider making `Environment::getStorageLocation` skip noop // expressions (this and other similar expressions in the file) instead of @@ -191,8 +196,6 @@ break; } default: - // FIXME: Add support for CK_UserDefinedConversion, - // CK_ConstructorConversion, CK_UncheckedDerivedToBase. break; } } diff --git a/clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp b/clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp @@ -88,6 +88,7 @@ } void TransferSetTrue(const DeclRefExpr *, + const ast_matchers::MatchFinder::MatchResult &, TransferState &State) { State.Lattice = BooleanLattice(true); } @@ -107,9 +108,10 @@ using namespace ast_matchers; TransferSwitch = MatchSwitchBuilder>() - .CaseOf(declRefExpr(to(varDecl(hasName("X")))), TransferSetTrue) - .CaseOf(callExpr(callee(functionDecl(hasName("Foo")))), - TransferSetFalse) + .CaseOf(declRefExpr(to(varDecl(hasName("X")))), + TransferSetTrue) + .CaseOf(callExpr(callee(functionDecl(hasName("Foo")))), + TransferSetFalse) .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 @@ -31,8 +31,8 @@ // FIXME: Move header definitions in separate file(s). static constexpr char StdTypeTraitsHeader[] = R"( -#ifndef TYPE_TRAITS_H -#define TYPE_TRAITS_H +#ifndef STD_TYPE_TRAITS_H +#define STD_TYPE_TRAITS_H namespace std { @@ -127,6 +127,9 @@ typedef T type; }; +template +using remove_cv_t = typename remove_cv::type; + template struct decay { private: @@ -139,9 +142,196 @@ typename remove_cv::type>::type>::type type; }; +template +struct enable_if {}; + +template +struct enable_if { + typedef T type; +}; + +template +using enable_if_t = typename enable_if::type; + +template +struct is_same : false_type {}; + +template +struct is_same : true_type {}; + +template +struct is_void : is_same::type> {}; + +namespace detail { + +template +auto try_add_rvalue_reference(int) -> type_identity; +template +auto try_add_rvalue_reference(...) -> type_identity; + +} // namespace detail + +template +struct add_rvalue_reference : decltype(detail::try_add_rvalue_reference(0)) { +}; + +template +typename add_rvalue_reference::type declval() noexcept; + +namespace detail { + +template +auto test_returnable(int) + -> decltype(void(static_cast(nullptr)), true_type{}); +template +auto test_returnable(...) -> false_type; + +template +auto test_implicitly_convertible(int) + -> decltype(void(declval()(declval())), true_type{}); +template +auto test_implicitly_convertible(...) -> false_type; + +} // namespace detail + +template +struct is_convertible + : integral_constant(0))::value && + decltype(detail::test_implicitly_convertible( + 0))::value) || + (is_void::value && is_void::value)> {}; + +template +inline constexpr bool is_convertible_v = is_convertible::value; + +template +using void_t = void; + +template +struct is_constructible_ : false_type {}; + +template +struct is_constructible_()...))>, T, Args...> + : true_type {}; + +template +using is_constructible = is_constructible_, T, Args...>; + +template +inline constexpr bool is_constructible_v = is_constructible::value; + +template +struct __uncvref { + typedef typename remove_cv::type>::type type; +}; + +template +using __uncvref_t = typename __uncvref<_Tp>::type; + +template +using _BoolConstant = integral_constant; + +template +using _IsSame = _BoolConstant<__is_same(_Tp, _Up)>; + +template +using _IsNotSame = _BoolConstant; + +template +struct _MetaBase; +template <> +struct _MetaBase { + template + using _SelectImpl = _Tp; + template