From 39018ad5c77c53156cb7cd672c05a19fb75a6af7 Mon Sep 17 00:00:00 2001 From: Yedidya Feldblum Date: Thu, 7 Dec 2017 12:38:59 -0800 Subject: [PATCH] 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 --- folly/ExceptionWrapper-inl.h | 9 +++++++++ folly/ExceptionWrapper.h | 19 ++++++++++++++----- folly/test/ExceptionWrapperTest.cpp | 7 ++++++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/folly/ExceptionWrapper-inl.h b/folly/ExceptionWrapper-inl.h index 6e4fe114..1d74c030 100644 --- a/folly/ExceptionWrapper-inl.h +++ b/folly/ExceptionWrapper-inl.h @@ -280,6 +280,15 @@ inline exception_wrapper exception_wrapper::SharedPtr::get_exception_ptr_( return that->sptr_.ptr_->get_exception_ptr_(); } +template +inline exception_wrapper::exception_wrapper( + ThrownTag, + in_place_type_t, + As&&... as) + : eptr_{std::make_exception_ptr(Ex(std::forward(as)...)), + reinterpret_cast(std::addressof(typeid(Ex))) + 1u}, + vptr_(&ExceptionPtr::ops_) {} + template inline exception_wrapper::exception_wrapper( OnHeapTag, diff --git a/folly/ExceptionWrapper.h b/folly/ExceptionWrapper.h index 36913d85..ff60aa34 100644 --- a/folly/ExceptionWrapper.h +++ b/folly/ExceptionWrapper.h @@ -235,16 +235,20 @@ class exception_wrapper final { Ex const& as() const noexcept; }; + struct ThrownTag {}; struct InSituTag {}; struct OnHeapTag {}; template using PlacementOf = _t())), - InSituTag, - OnHeapTag>>; + !IsStdException::value, + ThrownTag, + _t())), + InSituTag, + OnHeapTag>>>>; static std::exception const* as_exception_or_null_(std::exception const& ex); static std::exception const* as_exception_or_null_(AnyException); @@ -278,6 +282,7 @@ class exception_wrapper final { template struct InPlace { + static_assert(IsStdException::value, "only deriving std::exception"); static void copy_(exception_wrapper const* from, exception_wrapper* to); static void move_(exception_wrapper* from, exception_wrapper* to); static void delete_(exception_wrapper* that); @@ -306,6 +311,7 @@ class exception_wrapper final { }; template struct Impl final : public Base { + static_assert(IsStdException::value, "only deriving std::exception"); Ex ex_; Impl() = default; template @@ -334,6 +340,9 @@ class exception_wrapper final { }; VTable const* vptr_{&uninit_}; + template + exception_wrapper(ThrownTag, in_place_type_t, As&&... as); + template exception_wrapper(OnHeapTag, in_place_type_t, As&&... as); diff --git a/folly/test/ExceptionWrapperTest.cpp b/folly/test/ExceptionWrapperTest.cpp index 68dbc441..1615e6af 100644 --- a/folly/test/ExceptionWrapperTest.cpp +++ b/folly/test/ExceptionWrapperTest.cpp @@ -254,6 +254,7 @@ TEST(ExceptionWrapper, with_shared_ptr_test) { EXPECT_EQ(typeid(std::runtime_error), ew.type()); EXPECT_NE(nullptr, ew.get_exception()); EXPECT_NE(nullptr, ew.get_exception()); + EXPECT_STREQ("foo", ew.get_exception()->what()); EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_FALSE(ew.has_exception_ptr()); EXPECT_NE(nullptr, ew.to_exception_ptr()); @@ -286,6 +287,7 @@ TEST(ExceptionWrapper, with_exception_ptr_exn_test) { EXPECT_EQ(typeid(std::runtime_error), ew.type()); EXPECT_NE(nullptr, ew.get_exception()); EXPECT_NE(nullptr, ew.get_exception()); + EXPECT_STREQ("foo", ew.get_exception()->what()); EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_TRUE(ew.has_exception_ptr()); EXPECT_EQ(ep, ew.to_exception_ptr()); @@ -318,6 +320,7 @@ TEST(ExceptionWrapper, with_exception_ptr_any_test) { EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_NE(nullptr, ew.get_exception()); + EXPECT_EQ(12, *ew.get_exception()); EXPECT_TRUE(ew.has_exception_ptr()); EXPECT_EQ(ep, ew.to_exception_ptr()); EXPECT_TRUE(ew.has_exception_ptr()); @@ -348,7 +351,8 @@ TEST(ExceptionWrapper, with_non_std_exception_test) { EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_EQ(nullptr, ew.get_exception()); EXPECT_NE(nullptr, ew.get_exception()); - EXPECT_FALSE(ew.has_exception_ptr()); + EXPECT_EQ(42, *ew.get_exception()); + EXPECT_TRUE(ew.has_exception_ptr()); EXPECT_EQ("int", ew.class_name()); EXPECT_EQ("int", ew.what()); EXPECT_NE(nullptr, ew.to_exception_ptr()); @@ -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_NE(nullptr, ew.get_exception()); + EXPECT_EQ(12, *ew.get_exception()); EXPECT_EQ(ep, ew.to_exception_ptr()); EXPECT_EQ("", ew.class_name()); // because concrete type is // erased -- 2.34.1