From f193a7e397b220a9d1ecc34183b6cffc279c9275 Mon Sep 17 00:00:00 2001 From: Eric Niebler Date: Thu, 7 Dec 2017 22:36:49 -0800 Subject: [PATCH] prevent folly::Function from being initialized with a function that returns a prvalue Summary: If the folly::Function returns by const& and the wrapped function returns a prvalue, it is guaranteed to return a dangling reference. Prevent it. Reviewed By: yfeldblum, aary Differential Revision: D6487447 fbshipit-source-id: 61700b4688e29409eefa27f546b31ecac258cfdd --- folly/Function.h | 56 +++++++++++++++++++++++-------------- folly/test/FunctionTest.cpp | 19 +++++++++++++ 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/folly/Function.h b/folly/Function.h index ec145688..a35564e6 100644 --- a/folly/Function.h +++ b/folly/Function.h @@ -284,6 +284,16 @@ inline bool uninitNoop(Op, Data*, Data*) { return false; } +template +using CallableResult = decltype(std::declval()(std::declval()...)); + +template < + typename From, + typename To, + typename = typename std::enable_if< + !std::is_reference::value || std::is_reference::value>::type> +using SafeResultOf = decltype(static_cast(std::declval())); + template struct FunctionTraits; @@ -295,9 +305,9 @@ struct FunctionTraits { using NonConstSignature = ReturnType(Args...); using OtherSignature = ConstSignature; - template ::type> - using ResultOf = decltype( - static_cast(std::declval()(std::declval()...))); + template + using ResultOf = + SafeResultOf>&, Args...>, ReturnType>; template static ReturnType callSmall(Data& p, Args&&... args) { @@ -340,9 +350,10 @@ struct FunctionTraits { using NonConstSignature = ReturnType(Args...); using OtherSignature = NonConstSignature; - template ::type> - using ResultOf = decltype(static_cast( - std::declval()(std::declval()...))); + template + using ResultOf = SafeResultOf< + CallableResult>&, Args...>, + ReturnType>; template static ReturnType callSmall(Data& p, Args&&... args) { @@ -386,9 +397,9 @@ struct FunctionTraits { using NonConstSignature = ReturnType(Args...) noexcept; using OtherSignature = ConstSignature; - template ::type> - using ResultOf = decltype( - static_cast(std::declval()(std::declval()...))); + template + using ResultOf = + SafeResultOf>&, Args...>, ReturnType>; template static ReturnType callSmall(Data& p, Args&&... args) noexcept { @@ -431,9 +442,10 @@ struct FunctionTraits { using NonConstSignature = ReturnType(Args...) noexcept; using OtherSignature = NonConstSignature; - template ::type> - using ResultOf = decltype(static_cast( - std::declval()(std::declval()...))); + template + using ResultOf = SafeResultOf< + CallableResult>&, Args...>, + ReturnType>; template static ReturnType callSmall(Data& p, Args&&... args) noexcept { @@ -607,14 +619,15 @@ class Function final : private detail::function::FunctionTraits { * signature matches (i.e. it returns an object convertible to `R` when called * with `Args...`). * - * \note `typename = ResultOf` prevents this overload from being - * selected by overload resolution when `fun` is not a compatible function. + * \note `typename Traits::template ResultOf` prevents this overload + * from being selected by overload resolution when `fun` is not a compatible + * function. * - * \note The noexcept requires some explanation. IsSmall is true when the + * \note The noexcept requires some explanation. `IsSmall` is true when the * decayed type fits within the internal buffer and is noexcept-movable. But * this ctor might copy, not move. What we need here, if this ctor does a * copy, is that this ctor be noexcept when the copy is noexcept. That is not - * checked in IsSmall, and shouldn't be, because once the Function is + * checked in `IsSmall`, and shouldn't be, because once the `Function` is * constructed, the contained object is never copied. This check is for this * ctor only, in the case that this ctor does a copy. */ @@ -624,7 +637,7 @@ class Function final : private detail::function::FunctionTraits { typename = typename Traits::template ResultOf> /* implicit */ Function(Fun fun) noexcept( IsSmall::value && noexcept(Fun(std::declval()))) - : Function(static_cast(fun), IsSmall{}) {} + : Function(std::move(fun), IsSmall{}) {} /** * For move-constructing from a `folly::Function`. @@ -700,8 +713,9 @@ class Function final : private detail::function::FunctionTraits { * Assigns a callable object to this `Function`. If the operation fails, * `*this` is left unmodified. * - * \note `typename = ResultOf` prevents this overload from being - * selected by overload resolution when `fun` is not a compatible function. + * \note `typename = decltype(Function(std::declval()))` prevents this + * overload from being selected by overload resolution when `fun` is not a + * compatible function. */ template ()))> Function& operator=(Fun fun) noexcept( @@ -711,10 +725,10 @@ class Function final : private detail::function::FunctionTraits { // Q: Why is is safe to destroy and reconstruct this object in place? // A: See the explanation in the move assignment operator. this->~Function(); - ::new (this) Function(static_cast(fun)); + ::new (this) Function(std::move(fun)); } else { // Construct a temporary and (nothrow) swap. - Function(static_cast(fun)).swap(*this); + Function(std::move(fun)).swap(*this); } return *this; } diff --git a/folly/test/FunctionTest.cpp b/folly/test/FunctionTest.cpp index 34217a2c..0d2dcd94 100644 --- a/folly/test/FunctionTest.cpp +++ b/folly/test/FunctionTest.cpp @@ -175,6 +175,25 @@ static_assert( Function>::value, ""); +static_assert( + !std::is_constructible, int (*)()>::value, + ""); + +static_assert( + !std::is_constructible, int (*)()>::value, + ""); + +#if FOLLY_HAVE_NOEXCEPT_FUNCTION_TYPE +static_assert( + !std::is_constructible, int (*)()>::value, + ""); + +static_assert( + !std::is_constructible, int (*)()>:: + value, + ""); +#endif + // TEST ===================================================================== // InvokeFunctor & InvokeReference -- 2.34.1