Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move fbjni tests into fbjni directory #2

Closed
wants to merge 2 commits into from
Closed

Move fbjni tests into fbjni directory #2

wants to merge 2 commits into from

Conversation

dreiss
Copy link
Contributor

@dreiss dreiss commented Aug 30, 2019

Summary: Want these to be included in Open Source fbjni.

Differential Revision: D17139151

fbshipit-source-id: 30e4bb76d748f135bfa3619f62983ee39bf9969c
Summary: Want these to be included in Open Source fbjni.

Differential Revision: D17139151

fbshipit-source-id: 4d28c59f27c8585d1aa13c7ebbfe8b141b945523
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 30, 2019
facebook-github-bot pushed a commit to facebook/hermes that referenced this pull request Sep 4, 2019
Summary:
Pull Request resolved: facebookincubator/fbjni#2

Want these to be included in Open Source fbjni.

Reviewed By: passy

Differential Revision: D17139151

fbshipit-source-id: 18d549fa4cbcc6860e9804abe2b8acdeb405d082
facebook-github-bot pushed a commit that referenced this pull request Sep 4, 2019
Summary:
Pull Request resolved: #2

Want these to be included in Open Source fbjni.

Reviewed By: passy

Differential Revision: D17139151

fbshipit-source-id: 18d549fa4cbcc6860e9804abe2b8acdeb405d082
facebook-github-bot pushed a commit that referenced this pull request Apr 3, 2020
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>
      #1 pc 00022e01  /data/app/com.facebook.builds.fb4a-vhhkGO4NTAZmUTmfS5wpfw==/lib/arm/libfbjni.so (BuildId: 7a0f9db0801e4f451162c28a392cd35d4ba46b9a)
      #2 pc 0004fb2d  /data/app/com.facebook.lyra.tests-MJNqCBmU7StfOjPoAyfEgg==/lib/arm/libgnustl_shared.so!libgnustl_shared.so (offset 0x4f000) (BuildId: 059ab3ea3d764339fe3ceea2904f54637c89594e)
      #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)
      #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
EduardoCostaom pushed a commit to EduardoCostaom/profilo that referenced this pull request Nov 2, 2022
Summary:
Pull Request resolved: facebookincubator/fbjni#2

Want these to be included in Open Source fbjni.

Reviewed By: passy

Differential Revision: D17139151

fbshipit-source-id: 18d549fa4cbcc6860e9804abe2b8acdeb405d082
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants