Fix crash in exception_wrapper::get_exception<>
authorYedidya Feldblum <yfeldblum@fb.com>
Thu, 7 Dec 2017 20:38:59 +0000 (12:38 -0800)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Thu, 7 Dec 2017 20:51:13 +0000 (12:51 -0800)
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
folly/ExceptionWrapper.h
folly/test/ExceptionWrapperTest.cpp

index 6e4fe11..1d74c03 100644 (file)
@@ -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,
index 36913d8..ff60aa3 100644 (file)
@@ -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);
 
index 68dbc44..1615e6a 100644 (file)
@@ -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