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

diff --git a/folly/ExceptionWrapper-inl.h b/folly/ExceptionWrapper-inl.h
index 6e4fe11482e282c936f1650a72d6691450133d43..1d74c030d9fbd5e06e98ee4d9b44e956d6731409 100644 (file)
--- 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 <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>
 inline exception_wrapper::exception_wrapper(
     OnHeapTag,

diff --git a/folly/ExceptionWrapper.h b/folly/ExceptionWrapper.h
index 36913d85db9509088ed10475a004d960e7b11eb9..ff60aa347117457aa9e123fc27850c30a398d1d7 100644 (file)
--- 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 <class T>
   using PlacementOf = _t<std::conditional<
-      sizeof(T) <= sizeof(Buffer::Storage) &&
-          alignof(T) <= alignof(Buffer::Storage) &&
-          noexcept(T(std::declval<T&&>())),
-      InSituTag,
-      OnHeapTag>>;
+      !IsStdException<T>::value,
+      ThrownTag,
+      _t<std::conditional<
+          sizeof(T) <= sizeof(Buffer::Storage) &&
+              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_(AnyException);
@@ -278,6 +282,7 @@ class exception_wrapper final {
 
   template <class Ex>
   struct InPlace {
+    static_assert(IsStdException<Ex>::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 <class Ex>
     struct Impl final : public Base {
+      static_assert(IsStdException<Ex>::value, "only deriving std::exception");
       Ex ex_;
       Impl() = default;
       template <typename... As>
@@ -334,6 +340,9 @@ class exception_wrapper final {
   };
   VTable const* vptr_{&uninit_};
 
+  template <class Ex, typename... As>
+  exception_wrapper(ThrownTag, in_place_type_t<Ex>, As&&... as);
+
   template <class Ex, typename... As>
   exception_wrapper(OnHeapTag, in_place_type_t<Ex>, As&&... as);
 

diff --git a/folly/test/ExceptionWrapperTest.cpp b/folly/test/ExceptionWrapperTest.cpp
index 68dbc441c7643dbb7d8a56a25e51ebf0480311ac..1615e6af1a482f2317081d7be53e320e54bf6b0d 100644 (file)
--- 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<std::exception>());
+  EXPECT_STREQ("foo", ew.get_exception<std::exception>()->what());
   EXPECT_EQ(nullptr, ew.get_exception<int>());
   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<std::exception>());
+  EXPECT_STREQ("foo", ew.get_exception<std::exception>()->what());
   EXPECT_EQ(nullptr, ew.get_exception<int>());
   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<std::exception>());
   EXPECT_NE(nullptr, ew.get_exception<int>());
+  EXPECT_EQ(12, *ew.get_exception<int>());
   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<std::exception>());
   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.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<std::exception>());
   EXPECT_NE(nullptr, ew.get_exception<int>());
+  EXPECT_EQ(12, *ew.get_exception<int>());
   EXPECT_EQ(ep, ew.to_exception_ptr());
   EXPECT_EQ("<unknown exception>", ew.class_name()); // because concrete type is
   // erased