Commit 39018ad5 authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook Github Bot

Fix crash in exception_wrapper::get_exception<>

Summary:
[Folly] Fix crash in `exception_wrapper::get_exception<>`.

When the contained object is unthrown and does not inherit `std::exception`, `get_exception` templated over a type that does not inherit `std::exception` must throw and catch internally and may then return a pointer to the internally thrown object, which has since been deallocated.

Attempting to dereference that pointer is undefined behavior is correctly caught by ASAN as heap-use-after-free.

Fix it by storing objects not inheriting `std::exception` using only the `std::exception_ptr` representation.

As a downside, we no longer get the small-object optimization or the non-throwing optimization for objects which do not inherit `std::exception`. But this is not likely to be terribly concerning in practice.

Reviewed By: ericniebler

Differential Revision: D6504911

fbshipit-source-id: 0065de911733b5cab87be55e7e4e47f0a9c09140
parent a335700f
...@@ -280,6 +280,15 @@ inline exception_wrapper exception_wrapper::SharedPtr::get_exception_ptr_( ...@@ -280,6 +280,15 @@ inline exception_wrapper exception_wrapper::SharedPtr::get_exception_ptr_(
return that->sptr_.ptr_->get_exception_ptr_(); return that->sptr_.ptr_->get_exception_ptr_();
} }
template <class Ex, typename... As>
inline exception_wrapper::exception_wrapper(
ThrownTag,
in_place_type_t<Ex>,
As&&... as)
: eptr_{std::make_exception_ptr(Ex(std::forward<As>(as)...)),
reinterpret_cast<std::uintptr_t>(std::addressof(typeid(Ex))) + 1u},
vptr_(&ExceptionPtr::ops_) {}
template <class Ex, typename... As> template <class Ex, typename... As>
inline exception_wrapper::exception_wrapper( inline exception_wrapper::exception_wrapper(
OnHeapTag, OnHeapTag,
......
...@@ -235,16 +235,20 @@ class exception_wrapper final { ...@@ -235,16 +235,20 @@ class exception_wrapper final {
Ex const& as() const noexcept; Ex const& as() const noexcept;
}; };
struct ThrownTag {};
struct InSituTag {}; struct InSituTag {};
struct OnHeapTag {}; struct OnHeapTag {};
template <class T> template <class T>
using PlacementOf = _t<std::conditional< using PlacementOf = _t<std::conditional<
sizeof(T) <= sizeof(Buffer::Storage) && !IsStdException<T>::value,
alignof(T) <= alignof(Buffer::Storage) && ThrownTag,
noexcept(T(std::declval<T&&>())), _t<std::conditional<
InSituTag, sizeof(T) <= sizeof(Buffer::Storage) &&
OnHeapTag>>; alignof(T) <= alignof(Buffer::Storage) &&
noexcept(T(std::declval<T&&>())),
InSituTag,
OnHeapTag>>>>;
static std::exception const* as_exception_or_null_(std::exception const& ex); static std::exception const* as_exception_or_null_(std::exception const& ex);
static std::exception const* as_exception_or_null_(AnyException); static std::exception const* as_exception_or_null_(AnyException);
...@@ -278,6 +282,7 @@ class exception_wrapper final { ...@@ -278,6 +282,7 @@ class exception_wrapper final {
template <class Ex> template <class Ex>
struct InPlace { struct InPlace {
static_assert(IsStdException<Ex>::value, "only deriving std::exception");
static void copy_(exception_wrapper const* from, exception_wrapper* to); static void copy_(exception_wrapper const* from, exception_wrapper* to);
static void move_(exception_wrapper* from, exception_wrapper* to); static void move_(exception_wrapper* from, exception_wrapper* to);
static void delete_(exception_wrapper* that); static void delete_(exception_wrapper* that);
...@@ -306,6 +311,7 @@ class exception_wrapper final { ...@@ -306,6 +311,7 @@ class exception_wrapper final {
}; };
template <class Ex> template <class Ex>
struct Impl final : public Base { struct Impl final : public Base {
static_assert(IsStdException<Ex>::value, "only deriving std::exception");
Ex ex_; Ex ex_;
Impl() = default; Impl() = default;
template <typename... As> template <typename... As>
...@@ -334,6 +340,9 @@ class exception_wrapper final { ...@@ -334,6 +340,9 @@ class exception_wrapper final {
}; };
VTable const* vptr_{&uninit_}; VTable const* vptr_{&uninit_};
template <class Ex, typename... As>
exception_wrapper(ThrownTag, in_place_type_t<Ex>, As&&... as);
template <class Ex, typename... As> template <class Ex, typename... As>
exception_wrapper(OnHeapTag, in_place_type_t<Ex>, As&&... as); exception_wrapper(OnHeapTag, in_place_type_t<Ex>, As&&... as);
......
...@@ -254,6 +254,7 @@ TEST(ExceptionWrapper, with_shared_ptr_test) { ...@@ -254,6 +254,7 @@ TEST(ExceptionWrapper, with_shared_ptr_test) {
EXPECT_EQ(typeid(std::runtime_error), ew.type()); EXPECT_EQ(typeid(std::runtime_error), ew.type());
EXPECT_NE(nullptr, ew.get_exception()); EXPECT_NE(nullptr, ew.get_exception());
EXPECT_NE(nullptr, ew.get_exception<std::exception>()); EXPECT_NE(nullptr, ew.get_exception<std::exception>());
EXPECT_STREQ("foo", ew.get_exception<std::exception>()->what());
EXPECT_EQ(nullptr, ew.get_exception<int>()); EXPECT_EQ(nullptr, ew.get_exception<int>());
EXPECT_FALSE(ew.has_exception_ptr()); EXPECT_FALSE(ew.has_exception_ptr());
EXPECT_NE(nullptr, ew.to_exception_ptr()); EXPECT_NE(nullptr, ew.to_exception_ptr());
...@@ -286,6 +287,7 @@ TEST(ExceptionWrapper, with_exception_ptr_exn_test) { ...@@ -286,6 +287,7 @@ TEST(ExceptionWrapper, with_exception_ptr_exn_test) {
EXPECT_EQ(typeid(std::runtime_error), ew.type()); EXPECT_EQ(typeid(std::runtime_error), ew.type());
EXPECT_NE(nullptr, ew.get_exception()); EXPECT_NE(nullptr, ew.get_exception());
EXPECT_NE(nullptr, ew.get_exception<std::exception>()); EXPECT_NE(nullptr, ew.get_exception<std::exception>());
EXPECT_STREQ("foo", ew.get_exception<std::exception>()->what());
EXPECT_EQ(nullptr, ew.get_exception<int>()); EXPECT_EQ(nullptr, ew.get_exception<int>());
EXPECT_TRUE(ew.has_exception_ptr()); EXPECT_TRUE(ew.has_exception_ptr());
EXPECT_EQ(ep, ew.to_exception_ptr()); EXPECT_EQ(ep, ew.to_exception_ptr());
...@@ -318,6 +320,7 @@ TEST(ExceptionWrapper, with_exception_ptr_any_test) { ...@@ -318,6 +320,7 @@ TEST(ExceptionWrapper, with_exception_ptr_any_test) {
EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_EQ(nullptr, ew.get_exception());
EXPECT_EQ(nullptr, ew.get_exception<std::exception>()); EXPECT_EQ(nullptr, ew.get_exception<std::exception>());
EXPECT_NE(nullptr, ew.get_exception<int>()); EXPECT_NE(nullptr, ew.get_exception<int>());
EXPECT_EQ(12, *ew.get_exception<int>());
EXPECT_TRUE(ew.has_exception_ptr()); EXPECT_TRUE(ew.has_exception_ptr());
EXPECT_EQ(ep, ew.to_exception_ptr()); EXPECT_EQ(ep, ew.to_exception_ptr());
EXPECT_TRUE(ew.has_exception_ptr()); EXPECT_TRUE(ew.has_exception_ptr());
...@@ -348,7 +351,8 @@ TEST(ExceptionWrapper, with_non_std_exception_test) { ...@@ -348,7 +351,8 @@ TEST(ExceptionWrapper, with_non_std_exception_test) {
EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_EQ(nullptr, ew.get_exception());
EXPECT_EQ(nullptr, ew.get_exception<std::exception>()); EXPECT_EQ(nullptr, ew.get_exception<std::exception>());
EXPECT_NE(nullptr, ew.get_exception<int>()); EXPECT_NE(nullptr, ew.get_exception<int>());
EXPECT_FALSE(ew.has_exception_ptr()); EXPECT_EQ(42, *ew.get_exception<int>());
EXPECT_TRUE(ew.has_exception_ptr());
EXPECT_EQ("int", ew.class_name()); EXPECT_EQ("int", ew.class_name());
EXPECT_EQ("int", ew.what()); EXPECT_EQ("int", ew.what());
EXPECT_NE(nullptr, ew.to_exception_ptr()); EXPECT_NE(nullptr, ew.to_exception_ptr());
...@@ -381,6 +385,7 @@ TEST(ExceptionWrapper, with_exception_ptr_any_nil_test) { ...@@ -381,6 +385,7 @@ TEST(ExceptionWrapper, with_exception_ptr_any_nil_test) {
EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_EQ(nullptr, ew.get_exception());
EXPECT_EQ(nullptr, ew.get_exception<std::exception>()); EXPECT_EQ(nullptr, ew.get_exception<std::exception>());
EXPECT_NE(nullptr, ew.get_exception<int>()); EXPECT_NE(nullptr, ew.get_exception<int>());
EXPECT_EQ(12, *ew.get_exception<int>());
EXPECT_EQ(ep, ew.to_exception_ptr()); EXPECT_EQ(ep, ew.to_exception_ptr());
EXPECT_EQ("<unknown exception>", ew.class_name()); // because concrete type is EXPECT_EQ("<unknown exception>", ew.class_name()); // because concrete type is
// erased // erased
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment