Skip to content

Commit 0a9e622

Browse files
antonisclaude
andauthored
fix(android): Prevent New Arch build break linking libsentry-tm-perf-logger.so + CI ABI/link coverage (#6406)
* fix(android): Prevent New Arch build break linking libsentry-tm-perf-logger.so The perf-logger native library added in 8.17.0 (#6307) references facebook::react::TurboModulePerfLogger::enableLogging, which lives in React Native's `reactnative` prefab. Some New Architecture build setups (e.g. Expo builds on armeabi-v7a) do not satisfy that reference at link time, and because the NDK/AGP toolchain links with -Wl,-z,defs (--no-undefined) the unresolved symbol becomes a fatal "undefined symbol" error that breaks the whole Android build — even for hosts that never opt in to enableTurboModuleTracking, since the library is compiled whenever the New Architecture is enabled. Append -Wl,-z,undefs to the target's link options so undefined symbols are non-fatal, leaving them to resolve at load time against the loaded libreactnative.so. This is safe: the library is loaded lazily and only when tracking is opted in, and an unresolvable symbol surfaces as an UnsatisfiedLinkError from System.loadLibrary that the Java side already swallows (RNSentryTurboModulePerfTracker), degrading the experimental feature to a no-op rather than crashing. Verified with NDK 27 against the RN 0.86 prefab: - armeabi-v7a with --no-undefined and the prefab unresolved: FAILED before, links after the fix. - normal builds (prefab linked) keep libreactnative.so in DT_NEEDED and resolve the symbol at load time -- the flag is a no-op there. - x86 / arm64 still build and stay 16 KB aligned. Fixes #6398 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: Reference PR instead of issue in changelog entry Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(android): Add ABI coverage and link regression guard for perf-logger Two CI additions to catch the class of failures that shipped with the app-compiled libsentry-tm-perf-logger.so (#6394, #6398), both of which escaped because CI only built x86: - Build all four ABIs (armeabi-v7a, arm64-v8a, x86, x86_64) in the New Architecture sample Android build instead of x86 only, so per-ABI native breakage is exercised and the 16 KB alignment check runs across every ABI. - Add scripts/check-tm-perf-logger-link.sh, a lightweight guard that reproduces the #6398 link condition (armeabi-v7a, --no-undefined, reactnative prefab reference unresolved) using the real sources and the real link flags declared in CMakeLists.txt. It fails if -Wl,-z,undefs is ever removed. Wired into buildandtest.yml so it runs on every PR using the runner's preinstalled NDK. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(android): Build all ABIs in the Expo sample too #6398 (armeabi-v7a link failure) was reported on an Expo build, but the Expo sample only built x86 in CI. Build all four ABIs to keep coverage consistent with the bare React Native sample. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(android): Honor reactNativeArchitectures for the perf-logger native build Root-cause fix for #6398. The module hardcoded `abiFilters` to all four ABIs, ignoring the host app's `reactNativeArchitectures`. When the app builds a subset (e.g. `-PreactNativeArchitectures=arm64-v8a`, common in Expo/dev/CI builds), React Native only provides its `reactnative` prefab for that subset, so building the extra ABIs cannot resolve `TurboModulePerfLogger::enableLogging` at link time and breaks the build. Honor `reactNativeArchitectures` (falling back to all four when unset), matching react-native-screens, reanimated and the RN app template. Now the module builds only the ABIs the app requested. Verified locally against a subset build: with the old hardcoded filters, `:sentry_react-native:externalNativeBuildDebug -PreactNativeArchitectures=arm64-v8a` built arm64-v8a + armeabi-v7a + x86 + x86_64; with the fix it builds only arm64-v8a. The `-Wl,-z,undefs` link flag stays as defense-in-depth for any remaining setup where a built ABI's prefab reference is unresolved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d7d54c6 commit 0a9e622

7 files changed

Lines changed: 195 additions & 3 deletions

File tree

.github/workflows/buildandtest.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,28 @@ jobs:
3434
- name: Test
3535
run: yarn test
3636

37+
job_native_link_check:
38+
name: Android Native Link Check
39+
runs-on: ubuntu-latest
40+
needs: [diff_check]
41+
if: ${{ needs.diff_check.outputs.skip_ci != 'true' }}
42+
steps:
43+
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
44+
- run: corepack enable
45+
- uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
46+
with:
47+
package-manager-cache: false
48+
node-version: 18
49+
cache: 'yarn'
50+
cache-dependency-path: yarn.lock
51+
- name: Install Dependencies
52+
run: yarn install
53+
# Guards against regressing the #6398 fix: verifies libsentry-tm-perf-logger.so
54+
# still links for armeabi-v7a under --no-undefined when the reactnative prefab
55+
# reference is unresolved. Uses the runner's preinstalled NDK (ANDROID_NDK_LATEST_HOME).
56+
- name: Check libsentry-tm-perf-logger.so links (armeabi-v7a)
57+
run: ./scripts/check-tm-perf-logger-link.sh
58+
3759
job_lint:
3860
name: Lint
3961
runs-on: ubuntu-latest

.github/workflows/sample-application-expo.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,11 @@ jobs:
187187
run: |
188188
[[ "${{ matrix.build-type }}" == "production" ]] && CONFIG='Release' || CONFIG='Debug'
189189
echo "Building $CONFIG"
190-
./gradlew ":app:assemble$CONFIG" -PreactNativeArchitectures=x86
190+
# Build all ABIs (not just x86): the Sentry perf-logger native library is
191+
# compiled from source per ABI, and #6398 (armeabi-v7a link failure) was
192+
# reported on an Expo build. Keeps ABI coverage consistent with the bare
193+
# RN sample.
194+
./gradlew ":app:assemble$CONFIG" -PreactNativeArchitectures=armeabi-v7a,arm64-v8a,x86,x86_64
191195
192196
- name: Export Expo
193197
working-directory: samples/expo

.github/workflows/sample-application.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,12 @@ jobs:
185185
[[ "${{ matrix.build-type }}" == "production" ]] && export CONFIG='release' || export CONFIG='debug'
186186
187187
./scripts/set-dsn-aos.mjs
188-
./scripts/build-android.sh -PreactNativeArchitectures=x86
188+
# Build all ABIs (not just x86) so armeabi-v7a and the 64-bit ABIs are
189+
# exercised too. The Sentry perf-logger native library is compiled from
190+
# source per ABI, and ABI-specific breakage has shipped before (#6394
191+
# 16 KB alignment, #6398 armeabi-v7a link) precisely because CI only
192+
# built x86. The 16 KB alignment check below then runs across every ABI.
193+
./scripts/build-android.sh -PreactNativeArchitectures=armeabi-v7a,arm64-v8a,x86,x86_64
189194
190195
- name: Check 16 KB native library alignment
191196
# Only Sentry-owned libraries are checked: third-party/React Native

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
### Fixes
1212

13+
- Fix Android New Architecture build failing to link `libsentry-tm-perf-logger.so` with an undefined `TurboModulePerfLogger::enableLogging` symbol on some setups (e.g. armeabi-v7a) ([#6406](https://github.com/getsentry/sentry-react-native/pull/6406))
1314
- Fix Android New Architecture build failing at CMake configure on React Native 0.75 by gating the `libsentry-tm-perf-logger.so` native build to RN 0.76+ ([#6407](https://github.com/getsentry/sentry-react-native/pull/6407))
1415

1516
## 8.17.1

packages/core/android/build.gradle

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,18 @@ def isNewArchitectureEnabled() {
66
return project.hasProperty("newArchEnabled") && project.newArchEnabled == "true"
77
}
88

9+
// The ABIs the host app is building, from its `reactNativeArchitectures`
10+
// property (the same knob React Native, react-native-screens and reanimated
11+
// read). Falls back to all four when unset. We must honour this: when the app
12+
// builds a subset (e.g. `-PreactNativeArchitectures=arm64-v8a`), React Native
13+
// only provides its `reactnative` prefab for that subset, so building any other
14+
// ABI fails to resolve `TurboModulePerfLogger::enableLogging` at link time
15+
// (#6398). Hardcoding all four ABIs is what caused that failure.
16+
def reactNativeArchitectures() {
17+
def value = project.getProperties().get("reactNativeArchitectures")
18+
return value ? value.split(",") : ["armeabi-v7a", "x86", "x86_64", "arm64-v8a"]
19+
}
20+
921
// `libsentry-tm-perf-logger.so` links against React Native's `reactnative`
1022
// prefab target. The merged `libreactnative.so` (exposed as the
1123
// `ReactAndroid::reactnative` prefab) only ships from RN 0.76 onward; on RN
@@ -135,7 +147,7 @@ android {
135147
versionCode 1
136148
versionName "1.0"
137149
ndk {
138-
abiFilters "x86", "armeabi-v7a", "x86_64", "arm64-v8a"
150+
abiFilters(*reactNativeArchitectures())
139151
}
140152
compileOptions {
141153
sourceCompatibility JavaVersion.VERSION_1_8

packages/core/android/src/main/jni/CMakeLists.txt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,32 @@ target_link_options(
7878
"-Wl,-z,max-page-size=16384"
7979
)
8080

81+
# Do not fail the link when `facebook::react::TurboModulePerfLogger::enableLogging`
82+
# (and friends) cannot be resolved at link time. Those symbols live in React
83+
# Native's `reactnative` prefab, which we link above, and they are present in
84+
# every ABI's `libreactnative.so`. However, some New Architecture build setups
85+
# (e.g. Expo builds on certain ABIs such as armeabi-v7a) do not end up
86+
# satisfying the reference at link time, and the NDK/AGP toolchain links with
87+
# `-Wl,-z,defs` (a.k.a. `--no-undefined`), which turns that into a fatal
88+
# `undefined symbol` error and breaks the whole Android build — even for hosts
89+
# that never opt in to `enableTurboModuleTracking`, since the library is
90+
# compiled whenever the New Architecture is enabled.
91+
#
92+
# `-Wl,-z,undefs` (appended after the toolchain's flags, so it wins) makes
93+
# undefined symbols non-fatal, leaving them to be resolved at load time against
94+
# the already-loaded `libreactnative.so`. This is safe: the library is loaded
95+
# lazily and only when tracking is opted in (see RNSentryTurboModulePerfTracker),
96+
# and a genuinely unresolvable symbol surfaces as an `UnsatisfiedLinkError` from
97+
# `System.loadLibrary` that the Java side already swallows, degrading the
98+
# experimental feature to a no-op rather than crashing. When the prefab does
99+
# satisfy the reference at link time (the common case) this flag changes nothing.
100+
# https://github.com/getsentry/sentry-react-native/issues/6398
101+
target_link_options(
102+
sentry-tm-perf-logger
103+
PRIVATE
104+
"-Wl,-z,undefs"
105+
)
106+
81107
# Note: we deliberately do NOT pass `-Wl,--strip-all` (or similar) here.
82108
# Android Gradle Plugin's `StripDebugSymbolsTask` already strips the .so for
83109
# the packaged APK while preserving the unstripped artefact under
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
#!/bin/bash
2+
3+
# Regression guard for https://github.com/getsentry/sentry-react-native/issues/6398
4+
#
5+
# `libsentry-tm-perf-logger.so` is compiled from source in the consuming app and
6+
# references `facebook::react::TurboModulePerfLogger::enableLogging`, which lives
7+
# in React Native's `reactnative` prefab. On some New Architecture setups (e.g.
8+
# Expo builds on armeabi-v7a) that reference is not satisfied at link time, and
9+
# because the NDK/AGP toolchain links with `-Wl,-z,defs` (`--no-undefined`) the
10+
# unresolved symbol becomes a fatal error that breaks the whole Android build.
11+
# The fix (see src/main/jni/CMakeLists.txt) appends `-Wl,-z,undefs` so undefined
12+
# symbols are non-fatal and resolve at load time instead.
13+
#
14+
# This check reproduces that exact link condition for armeabi-v7a using the real
15+
# sources and the real link flags declared in CMakeLists.txt:
16+
# 1. Control — link WITHOUT the fix flags: must FAIL (proves the reproduction
17+
# is still valid; if it starts passing the check is stale).
18+
# 2. Fixed — link WITH the flags CMakeLists.txt actually declares: must PASS
19+
# (fails if `-Wl,-z,undefs` is ever removed from CMakeLists.txt).
20+
#
21+
# Requires an Android NDK (found via $ANDROID_NDK_* / $ANDROID_HOME/ndk) and a
22+
# React Native install (resolved from packages/core, or $REACT_NATIVE_DIR).
23+
24+
set -euo pipefail
25+
26+
repo_root="$(cd "$(dirname "$0")/.." && pwd)"
27+
cmakelists="$repo_root/packages/core/android/src/main/jni/CMakeLists.txt"
28+
cpp_dir="$repo_root/packages/core/cpp"
29+
jni_dir="$repo_root/packages/core/android/src/main/jni"
30+
31+
# --- Resolve the React Native source tree (for ReactCommon headers) ----------
32+
rn_dir="${REACT_NATIVE_DIR:-}"
33+
if [[ -z "$rn_dir" ]]; then
34+
rn_dir="$(cd "$repo_root/packages/core" && node -p "require('path').dirname(require.resolve('react-native/package.json'))" 2>/dev/null || true)"
35+
fi
36+
if [[ -z "$rn_dir" || ! -d "$rn_dir/ReactCommon" ]]; then
37+
echo "error: could not locate react-native (set \$REACT_NATIVE_DIR to its root)" >&2
38+
exit 2
39+
fi
40+
41+
# --- Locate the NDK clang++ --------------------------------------------------
42+
find_clang() {
43+
local host
44+
case "$(uname -s)" in
45+
Darwin) host="darwin-x86_64" ;;
46+
*) host="linux-x86_64" ;;
47+
esac
48+
local roots=()
49+
[[ -n "${ANDROID_NDK_HOME:-}" ]] && roots+=("$ANDROID_NDK_HOME")
50+
[[ -n "${ANDROID_NDK_LATEST_HOME:-}" ]] && roots+=("$ANDROID_NDK_LATEST_HOME")
51+
[[ -n "${ANDROID_NDK_ROOT:-}" ]] && roots+=("$ANDROID_NDK_ROOT")
52+
for base in "${ANDROID_HOME:-}" "${ANDROID_SDK_ROOT:-}" "$HOME/Library/Android/sdk" "$HOME/Android/Sdk"; do
53+
[[ -d "$base/ndk" ]] && while IFS= read -r d; do roots+=("$d"); done < <(find "$base/ndk" -maxdepth 1 -mindepth 1 -type d | sort -r)
54+
done
55+
for r in "${roots[@]}"; do
56+
local c="$r/toolchains/llvm/prebuilt/$host/bin/clang++"
57+
[[ -x "$c" ]] && { echo "$c"; return 0; }
58+
done
59+
return 1
60+
}
61+
clang="$(find_clang || true)"
62+
if [[ -z "$clang" ]]; then
63+
echo "error: could not find an Android NDK clang++ (set \$ANDROID_NDK_HOME)" >&2
64+
exit 2
65+
fi
66+
67+
# --- Extract the link flags the CMakeLists.txt actually declares --------------
68+
# Everything the target passes via target_link_options(... "-Wl,..."), verbatim.
69+
fix_flags=()
70+
while IFS= read -r f; do fix_flags+=("$f"); done < <(grep -oE '"-Wl,[^"]+"' "$cmakelists" | tr -d '"')
71+
if [[ ${#fix_flags[@]} -eq 0 ]]; then
72+
echo "error: no -Wl link flags found in $cmakelists (has the target changed?)" >&2
73+
exit 2
74+
fi
75+
76+
workdir="$(mktemp -d)"
77+
trap 'rm -rf "$workdir"' EXIT
78+
79+
includes=(
80+
-I "$cpp_dir"
81+
-I "$rn_dir/ReactCommon/react/nativemodule/core"
82+
-I "$rn_dir/ReactCommon/reactperflogger"
83+
-I "$rn_dir/ReactCommon"
84+
-I "$rn_dir/ReactCommon/callinvoker"
85+
)
86+
sources=("$cpp_dir/SentryTurboModulePerfLogger.cpp" "$jni_dir/OnLoad.cpp")
87+
88+
# Reproduce the failing condition: armeabi-v7a, -shared, `--no-undefined`, and
89+
# the `reactnative` prefab NOT linked (so `enableLogging` is unresolved).
90+
link() {
91+
"$clang" --target=armv7-none-linux-androideabi21 -std=c++20 -fPIC -shared \
92+
-DRCT_NEW_ARCH_ENABLED=1 "${includes[@]}" "${sources[@]}" \
93+
-Wl,--no-undefined "$@" -o "$workdir/out.so" 2>"$workdir/err.txt"
94+
}
95+
96+
echo "NDK clang: $clang"
97+
echo "React Native: $rn_dir"
98+
echo "CMakeLists link flags: ${fix_flags[*]}"
99+
echo
100+
101+
# --- 1) Control: without the fix flags, the link MUST fail -------------------
102+
rm -f "$workdir/out.so"
103+
if link; then
104+
echo "✖ CONTROL UNEXPECTEDLY LINKED — the reproduction is stale." >&2
105+
echo " '$0' no longer exercises the #6398 condition (did RN inline the symbol," >&2
106+
echo " or did the toolchain stop passing --no-undefined?). Update this check." >&2
107+
exit 1
108+
fi
109+
echo "✔ control link fails as expected (undefined symbol without the fix)"
110+
111+
# --- 2) Fixed: with the flags CMakeLists declares, the link MUST succeed ------
112+
rm -f "$workdir/out.so"
113+
if ! link "${fix_flags[@]}"; then
114+
echo "✖ FIXED LINK FAILED — CMakeLists.txt no longer prevents the #6398 build break." >&2
115+
echo " Ensure the sentry-tm-perf-logger target keeps '-Wl,-z,undefs'." >&2
116+
echo " Linker output:" >&2
117+
sed 's/^/ /' "$workdir/err.txt" >&2
118+
exit 1
119+
fi
120+
echo "✔ fixed link succeeds with CMakeLists flags (${fix_flags[*]})"
121+
echo
122+
echo "✔ #6398 link regression guard passed"

0 commit comments

Comments
 (0)