From a67c8767056c94acf8288e15d570c40cb3d6c6f0 Mon Sep 17 00:00:00 2001 From: Kudo Chien Date: Tue, 7 Feb 2023 09:33:44 -0800 Subject: [PATCH] update jsc-android to ndk r23 based (#36062) Summary: the current jsc-android is still built based on ndk r21, and react-native is now built based on ndk r23. the unwinder between r21 and r23 is incompatible (libgcc vs libunwind). if there's exceptions throwing from jsc, other react native libraries cannot catch these exceptions and cause runtime crash. this pr updates jsc-android to 235231.0.0 which is the same webkitgtk version as 235230.2.1 but only built by ndk r23. the jsc-android pr is from https://github.com/react-native-community/jsc-android-buildscripts/pull/179. note that the jsc is based on ndk r23c and react-native is based on ndk r23b. the reason is that i cannot get jsc building successfully on r23b. hopefully r23b and r23c are abi safe. there is another crash from libjscexecutor when testing the new jsc-android. to fix the issue, i have to explicitly link libunwind.a from libjscexecutor.so. supposedly ndk r23 should help to link libunwind under the hood, i still not figure out why it doesn't. but after linking libunwind.a, i can get new jsc-android work successfully. ``` E/art ( 2669): dlopen("/data/app/com.test-1/lib/x86_64/libjscexecutor.so", RTLD_LAZY) failed: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.test-1/lib/x86_64/libjscexecutor.so"... W/System.err( 2669): java.lang.UnsatisfiedLinkError: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.test-1/lib/x86_64/libjscexecutor.so"... W/System.err( 2669): at java.lang.Runtime.load(Runtime.java:331) W/System.err( 2669): at java.lang.System.load(System.java:982) W/System.err( 2669): at com.facebook.soloader.SoLoader$1.load(SoLoader.java:558) W/System.err( 2669): at com.facebook.soloader.DirectorySoSource.loadLibraryFrom(DirectorySoSource.java:110) W/System.err( 2669): at com.facebook.soloader.DirectorySoSource.loadLibrary(DirectorySoSource.java:63) W/System.err( 2669): at com.facebook.soloader.ApplicationSoSource.loadLibrary(ApplicationSoSource.java:91) W/System.err( 2669): at com.facebook.soloader.SoLoader.doLoadLibraryBySoName(SoLoader.java:1067) W/System.err( 2669): at com.facebook.soloader.SoLoader.loadLibraryBySoNameImpl(SoLoader.java:943) W/System.err( 2669): at com.facebook.soloader.SoLoader.loadLibraryBySoName(SoLoader.java:855) W/System.err( 2669): at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:802) W/System.err( 2669): at com.facebook.soloader.SoLoader.loadLibrary(SoLoader.java:772) W/System.err( 2669): at com.facebook.react.jscexecutor.JSCExecutor.loadLibrary(JSCExecutor.java:24) W/System.err( 2669): at com.facebook.react.jscexecutor.JSCExecutor.(JSCExecutor.java:20) W/System.err( 2669): at com.facebook.react.ReactInstanceManagerBuilder.getDefaultJSExecutorFactory(ReactInstanceManagerBuilder.java:363) W/System.err( 2669): at com.facebook.react.ReactInstanceManagerBuilder.build(ReactInstanceManagerBuilder.java:316) W/System.err( 2669): at com.facebook.react.ReactNativeHost.createReactInstanceManager(ReactNativeHost.java:94) W/System.err( 2669): at com.facebook.react.ReactNativeHost.getReactInstanceManager(ReactNativeHost.java:41) W/System.err( 2669): at com.test.MainApplication.onCreate(MainApplication.java:60) W/System.err( 2669): at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1011) W/System.err( 2669): at androidx.test.runner.MonitoringInstrumentation.callApplicationOnCreate(MonitoringInstrumentation.java:483) W/System.err( 2669): at android.app.ActivityThread.handleBindApplication(ActivityThread.java:4518) W/System.err( 2669): at android.app.ActivityThread.access$1500(ActivityThread.java:144) W/System.err( 2669): at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1339) W/System.err( 2669): at android.os.Handler.dispatchMessage(Handler.java:102) W/System.err( 2669): at android.os.Looper.loop(Looper.java:135) W/System.err( 2669): at android.app.ActivityThread.main(ActivityThread.java:5221) W/System.err( 2669): at java.lang.reflect.Method.invoke(Native Method) W/System.err( 2669): at java.lang.reflect.Method.invoke(Method.java:372) W/System.err( 2669): at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:899) W/System.err( 2669): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:694) ``` fixes https://github.com/facebook/react-native/issues/36052 ## Changelog [ANDROID][FIXED] - Fixed jscexecutor crash on Android which is caused from NDK incompatibility Pull Request resolved: https://github.com/facebook/react-native/pull/36062 Test Plan: tested on [jsc-android instrumented test](https://github.com/react-native-community/jsc-android-buildscripts/tree/2.26.1/test) (based on react-native 0.71.2) Reviewed By: cipolleschi Differential Revision: D43040295 Pulled By: cortinico fbshipit-source-id: e0e5b8fb7faa8ee5654d4cde5f274bef4b517376 --- .../src/main/jni/react/jscexecutor/CMakeLists.txt | 13 +++++++++++++ package.json | 2 +- yarn.lock | 8 ++++---- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/ReactAndroid/src/main/jni/react/jscexecutor/CMakeLists.txt b/ReactAndroid/src/main/jni/react/jscexecutor/CMakeLists.txt index 464aa19ccef868..67858803b3cff7 100644 --- a/ReactAndroid/src/main/jni/react/jscexecutor/CMakeLists.txt +++ b/ReactAndroid/src/main/jni/react/jscexecutor/CMakeLists.txt @@ -13,9 +13,22 @@ add_library(jscexecutor SHARED ${jscexecutor_SRC}) target_include_directories(jscexecutor PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) +# Patch from Expo: https://github.com/expo/react-native/blob/02714ab44d1e206fa80e81aef618e61017cccdc1/ReactAndroid/src/main/java/com/facebook/react/jscexecutor/CMakeLists.txt#L16-L25 +# Explicitly link libgcc.a to prevent undefined `_Unwind_Resume` symbol and crash from throwing c++ exceptions even someone tries to catch the exceptions. +# according to https://android.googlesource.com/platform/ndk/+/master/docs/BuildSystemMaintainers.md#unwinding, +# we should put the unwinder between static libs and shared libs. +# +# TODO(ncor): we don't need this patch anymore after upgrading to ndk r23 +if(ANDROID_NDK_REVISION VERSION_LESS "23.0.0") + set(LIB_UNWIND gcc) +else() + set(LIB_UNWIND unwind) +endif() + target_link_libraries(jscexecutor jsireact jscruntime + ${LIB_UNWIND} fb fbjni folly_runtime diff --git a/package.json b/package.json index 6119bd30689bfd..e17aa48a77c965 100644 --- a/package.json +++ b/package.json @@ -126,7 +126,7 @@ "event-target-shim": "^5.0.1", "invariant": "^2.2.4", "jest-environment-node": "^29.2.1", - "jsc-android": "^250230.2.1", + "jsc-android": "^250231.0.0", "memoize-one": "^5.0.0", "metro-react-native-babel-transformer": "0.73.5", "metro-runtime": "0.73.5", diff --git a/yarn.lock b/yarn.lock index 640749212e5e5c..742db74a6ba207 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5732,10 +5732,10 @@ jsbn@~0.1.0: resolved "https://registry.yarnpkg.com/jsbn/-/jsbn-0.1.1.tgz#a5e654c2e5a2deb5f201d96cefbca80c0ef2f513" integrity sha1-peZUwuWi3rXyAdls77yoDA7y9RM= -jsc-android@^250230.2.1: - version "250230.2.1" - resolved "https://registry.yarnpkg.com/jsc-android/-/jsc-android-250230.2.1.tgz#3790313a970586a03ab0ad47defbc84df54f1b83" - integrity sha512-KmxeBlRjwoqCnBBKGsihFtvsBHyUFlBxJPK4FzeYcIuBfdjv6jFys44JITAgSTbQD+vIdwMEfyZklsuQX0yI1Q== +jsc-android@^250231.0.0: + version "250231.0.0" + resolved "https://registry.yarnpkg.com/jsc-android/-/jsc-android-250231.0.0.tgz#91720f8df382a108872fa4b3f558f33ba5e95262" + integrity sha512-rS46PvsjYmdmuz1OAWXY/1kCYG7pnf1TBqeTiOJr1iDz7s5DLxxC9n/ZMknLDxzYzNVfI7R95MH10emSSG1Wuw== jscodeshift@^0.14.0: version "0.14.0"