Skip to content

Commit

Permalink
Fix crash in Lyra when throwing an exception without a destructor
Browse files Browse the repository at this point in the history
Summary:
With the `__cxa_throw` hook attached for gnustl, apps will crash if an object without a destructor is thrown:

```
try { throw 0; } catch(...) {}

// or

class SimpleException {};
try { throw SimpleException{}; } catch(...) {}
```

The issue is that the `__cxa_throw` abi has a destructor function pointer parameter, which may be null according to the documentation. In the gnustl region of the code, Lyra doesn't do a null check before invoking it at the following stack:

```
[???] 0X0 [unknown] + 0x0
+libfbjni.so  facebook::lyra::(anonymous namespace)::HijackedExceptionTypeInfo::destructor(void*) (./fbandroid/libraries/fbjni/cxx/lyra/cxa_throw.cpp:213)
libgnustl_shared.so 0X759F86716C [unknown] + 0x6116c
libgnustl_shared.so 0X759F8DD2E0 _Unwind_DeleteException + 0x18
```

I wrote a test which repros the issue, and without the fix it crashes  with a similar stack:

```
backtrace:
      #00 pc 00000000  <unknown>
      #01 pc 00022e01  /data/app/com.facebook.builds.fb4a-vhhkGO4NTAZmUTmfS5wpfw==/lib/arm/libfbjni.so (BuildId: 7a0f9db0801e4f451162c28a392cd35d4ba46b9a)
      facebook#2 pc 0004fb2d  /data/app/com.facebook.lyra.tests-MJNqCBmU7StfOjPoAyfEgg==/lib/arm/libgnustl_shared.so!libgnustl_shared.so (offset 0x4f000) (BuildId: 059ab3ea3d764339fe3ceea2904f54637c89594e)
      facebook#3 pc 0009b8e3  /data/app/com.facebook.lyra.tests-MJNqCBmU7StfOjPoAyfEgg==/lib/arm/libgnustl_shared.so!libgnustl_shared.so (offset 0x50000) (_Unwind_DeleteException+12) (BuildId: 059ab3ea3d764339fe3ceea2904f54637c89594e)
      facebook#4 pc 0000aeb9  /data/app/com.facebook.lyra.tests-MJNqCBmU7StfOjPoAyfEgg==/lib/arm/liblyra-tests.so (testThatCxaThrowHookThrowsAndSetsTraceTrivialException(facebook::jni::alias_ref<_jclass*>)+220) (BuildId: 31cc0552d685f1ce9e896d1628656cd97096678d)
```

To fix, add a null check before invoking `mutable_info->orig_dest_`, which originates from the destructor parameter.

Reviewed By: smeenai

Differential Revision: D20841003

fbshipit-source-id: 907a13ebf994c5bad511b13ab9684efcbc2ce474
  • Loading branch information
jwmcglynn authored and facebook-github-bot committed Apr 3, 2020
1 parent 632df0f commit dadc0d7
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion first-party/fbjni/cxx/lyra/cxa_throw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,9 @@ struct HijackedExceptionTypeInfo : public abi::__class_type_info {
auto exc_ptr = reinterpret_cast<std::exception_ptr*>(&obj);
auto info = reinterpret_cast<const::std::type_info*>(exc_ptr->__cxa_exception_type());
auto mutable_info = static_cast<HijackedExceptionTypeInfo*>(const_cast<std::type_info*>(info));
mutable_info->orig_dest_(obj);
if (mutable_info->orig_dest_) {
mutable_info->orig_dest_(obj);
}
delete mutable_info;
}

Expand Down

0 comments on commit dadc0d7

Please sign in to comment.