Uh oh!
There was an error while loading. Please reload this page.
[Native] Restore indexed GC temporary peers - #12509
Merged
jonathanpeppers merged 3 commits intoAug 25, 2026
Merged
Conversation
Restore the allocation-backed temporary peer map removed while unifying GC bridge processing. This keeps the unified IGCUserPeer path while avoiding std::unordered_map in NativeAOT. Fixes the drop-libc++ regression introduced by #12133. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores indexed temporary-peer storage in the unified GC bridge processing code by replacing the previous std::unordered_map<size_t, jobject> temporary-peer tracking with a fixed-size calloc-allocated jobject array and SCC Count field index-encoding, ensuring temporary peers and marker restoration are handled via RAII before weak-reference processing.
Changes:
- Introduces
TemporaryPeerMapto allocate exact-size temporary peer storage and encode indexes into SCCCountusing a marker bit. - Refactors bridge preparation to scope temporary-peer lifetime to cross-reference processing and automatically clean up local refs + SCC markers.
- Removes the
std::unordered_mapdependency fromBridgeProcessingto avoid pulling in unwanted C++ standard library roots.
Show a summary per file
| File | Description |
|---|---|
| src/native/clr/include/host/bridge-processing.hh | Adds TemporaryPeerMap API and updates bridge helper method signatures to use it instead of std::unordered_map. |
| src/native/clr/host/bridge-processing.cc | Implements TemporaryPeerMap with calloc storage + SCC count marker encoding, and refactors bridge preparation to use RAII cleanup. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
commented
Aug 25, 2026
MemberAuthor
/review |
Contributor
✅ Android PR Reviewer completed successfully!
|
Contributor
There was a problem hiding this comment.
✅ LGTM
No blocking issues found. Findings: 0 errors, 0 warnings, 1 suggestion.
The scoped allocation preserves the prior bridge behavior while removing the std::unordered_map dependency, and the RAII cleanup restores SCC counts before weak-reference processing. All 44 reported checks are green. The inline suggestion requests focused regression coverage for the temporary marker invariant.
Generated by Android PR Reviewer for #12509 · gpt56 · 102 AIC · ⌖ 9.06 AIC · ⊞ 25.7K
Comment /review to run again
| // Count is unsigned, so encode the temporary peer index as ~index. This stores the same bit | ||
| // pattern as -(index + 1), giving us a sign bit marker while preserving index 0. | ||
| // The destructor resets every marker before returning cross_refs to the runtime. | ||
| static constexpr size_t temporary_peer_index_sign_bit = ~(~size_t { 0 } >> 1); |
Contributor
There was a problem hiding this comment.
🤖 💡 Testing — This introduces a non-trivial invariant that temporarily overwrites every empty SCC count and relies on scoped cleanup to restore it, but the PR only validates native builds and symbol removal. Please consider focused coverage for index 0, multiple empty SCCs, preservation of non-empty counts, and restoration after the map leaves scope so a future refactor cannot silently corrupt the runtime-owned argument block.
Rule: Native runtime behavior requires focused regression coverage
jonathanpeppers
approved these changes
Aug 25, 2026
Uh oh!
There was an error while loading. Please reload this page.
jonathanpeppers
deleted the
dev/simonrozsival/fix-drop-libcxx-regression
branch
August 25, 2026 17:07
simonrozsival added a commit
that referenced
this pull request
Sep 2, 2026
Closes#12139Closes#12146 Removes `libc++_static.a`, `libc++abi.a` and `libunwind.a` from the NativeAOT link. This is the payoff for the local-string removal work in the rest of this stack: with `strings.hh`, `dynamic_local_string` and `static_local_string` gone, nothing in `libnaot-android` uses libc++ any more. For 32-bit ARM, ILC output still references the EHABI personality routines `__aeabi_unwind_cpp_pr0` and `__aeabi_unwind_cpp_pr1`. The NativeAOT runtime already contains their implementations in `libRuntime.WorkstationGC.a`, but dotnet/runtime localizes those symbols. `LinkNativeAotSharedLibrary` now makes an intermediate copy of that archive and promotes `pr0`, `pr1` and `pr2` to weak globals with `llvm-objcopy`. This adds no shim implementation and avoids linking or shipping a second copy of libunwind. ### Who was actually using the C++ runtime? Linking with libc++ removed leaves exactly six undefined symbols. Only one of them came from our code: | Symbol | Referenced from | | --- | --- | | `operator new[](size_t)` | `android-system-shared.cc` — **ours** | | `operator delete(void*)` | `gcenv.ee.cpp`, `UnixNativeCodeManager.cpp` | | `operator delete[](void*)` | `gcenv.ee.cpp`, `interoplibinterface_java.cpp` | | `operator new(size_t, nothrow_t const&)` | `gcenv.ee.cpp`, `TypeManager.cpp` | | `operator new[](size_t, nothrow_t const&)` | `gcenv.ee.cpp`, `RhConfig.cpp` | | `std::nothrow` | `gcenv.ee.cpp`, `UnixNativeCodeManager.cpp` | The five runtime-owned ones belong to the NativeAOT runtime from dotnet/runtime, and the ILC SDK already ships definitions for all of them in `libstdc++compat.a`. Our targets were unconditionally *removing* that archive with the comment *"This library conflicts with static libc++"* — which is only true while libc++ is linked. With libc++ gone there is no conflict, so we simply stop removing it. The one symbol that was ours came from a dead code path in `monodroid__system_property_get`, which is now removed earlier in the stack by #12517 (the PR that makes that path unreachable in the first place). The result is **no shim implementation** in this repo and a link with zero undefined symbols. ### 32-bit ARM EHABI `android-arm` uses ARM EHABI unwind tables. The generated NativeAOT object references `__aeabi_unwind_cpp_pr0` and `__aeabi_unwind_cpp_pr1`; removing the NDK `libunwind.a` initially exposed these as XA3007 linker failures on both macOS and Windows CI. `libRuntime.WorkstationGC.a` already carries the same `pr0`, `pr1` and `pr2` implementations in `Runtime.PrivateLibunwind.o`, together with the private unwinder they call. They are local symbols, so the application object cannot resolve them directly. Before linking an `armeabi-v7a` application, the build task creates `libRuntime.WorkstationGC.arm-ehabi.a` in the intermediate directory and promotes only those three symbols to weak globals. Weak binding preserves compatibility with applications that provide their own strong EHABI personalities, while the app export script keeps them local to the final DSO. No extra unwind code is linked: this reuses the object that the NativeAOT runtime already extracts. ### Size impact Default MAUI app (`dotnet new maui`), `net11.0-android`, `android-arm64`, Release, `PublishAot=true`. Both sides built clean from the same tree. | | before | after | delta | | --- | ---: | ---: | ---: | | `.so` | 25,229,320 | 25,029,728 | **−199,592** (−0.79%) | | `.so` deflated in APK | 9,829,143 | 9,761,291 | **−67,852** (−0.69%) | | APK | 14,903,624 | 14,833,992 | **−69,632** (−0.47%) | ### Commits 1. **Stop linking libc++ and libunwind** — the targets change. 2. **Update stale libc++ linker comments.** 3. **Stop shipping libc++ archives in the runtime packs** — see below. 4. **Reuse private ARM unwind personalities** — promote the runtime's existing EHABI symbols instead of restoring `libunwind.a`. ### Notes * This PR depends on #12509: before it, the GC bridge used a `std::unordered_map`, which drags in `__next_prime` and `__libcpp_verbose_abort` from libc++ internals. The rest of this stack is already rebased on top of it, so nothing further is needed. * This only affects **NativeAOT**. The Mono and CoreCLR runtimes still link libc++, and `NativeRuntimeComponents.cs` (the unified-runtime archive list) is deliberately untouched. * ARM64 and x64 do not need the NDK unwind archive. On 32-bit ARM, the application resolves EHABI personalities from the NativeAOT runtime's existing private unwinder after symbol promotion. ### Testing Built, installed and launched a default MAUI app on an API 36 arm64 emulator. Cold start with no crashes. `llvm-nm --undefined-only` on the resulting `libnaot-android.release-static-release.a` reports no `operator new`/`operator delete`, `__cxa_*`, `_Unwind_*` or `__libcpp_*` references. The only remaining `std::` symbols are `string_view` appearing in mangled names, which is header-only and carries no runtime dependency. The 32-bit ARM path is covered locally in both linker configurations: * `BuildNativeAot_AndroidArm_WithoutNdk` — workload linker. * `BuildNativeAot_AndroidArm_WithNdkLinker` — NDK linker. Both tests build successfully and assert that the linker response uses `libRuntime.WorkstationGC.arm-ehabi.a` and does not contain `libunwind.a`. --- ## Also: stop shipping the archives in the runtime packs Previously a separate PR stacked directly on this one; folded in here because "stop linking it" and "stop shipping it" are the same change to the reader, and reviewing them apart means reading the same targets twice. The NativeAOT runtime packs still shipped `libc++_static.a`, `libc++abi.a` and `libunwind.a` even though, after the change above, nothing links them any more. ### Why this needs a new item kind `_AndroidNdkRedistributable` (in `build-tools/scripts/Ndk.targets`) tagged NDK files with just two kinds: * `System` — `libc.so`, `libdl.so`, `liblog.so`, `libm.so`, `libz.so` — shipped to every runtime. * `Toolchain` — `crtbegin_so.o`, `crtend_so.o`, `libc++_static.a`, `libc++abi.a`, `libclang_rt.builtins-*.a`, `libunwind.a` — shipped to CoreCLR and NativeAOT, since both do native linking. NativeAOT still needs `crtbegin_so.o`, `crtend_so.o` and `libclang_rt.builtins-*.a`, so the `Toolchain` group cannot just be dropped for NativeAOT. This adds a third kind, `CplusPlus`, for the three C++ archives, and ships it only for CoreCLR. Both packaging sites are updated: * `src/native/native.targets` — the local `bin/<Config>/lib/packs` layout. * `build-tools/create-packs/Microsoft.Android.Runtime.proj` — the shipped NuGet packs. ### Size Per ABI, removed from the NativeAOT runtime pack: | Archive | Size | | --- | ---: | | `libc++_static.a` | 15,182,348 | | `libc++abi.a` | 3,125,348 | | `libunwind.a` | 91,152 | | **Total** | **18,398,848** | Across `android-arm`, `android-arm64` and `android-x64` that is roughly **55 MB** of pack content. This does not change application size — that is the linker change above — but it shrinks what users restore. ### Testing Deleted each pack directory and regenerated it via `_CopyToPackDirs`, rather than checking a pack that could still contain stale files. NativeAOT (`android-arm64`) — the three archives are gone, and everything NativeAOT links is still present: ``` crtbegin_so.o crtend_so.o libc.so libclang_rt.builtins-aarch64-android.a libdl.so liblog.so libm.so libz.so libnaot-android.debug-static-debug.a libnaot-android.debug.so libnaot-android.release-static-release.a libnaot-android.release.so libxa-java-interop-release.a ``` CoreCLR (`android-arm64`) — all three are still shipped: ``` crtbegin_so.o crtend_so.o libarchive-dso-stub.so libc.so libc++_static.a libc++abi.a libclang_rt.builtins-aarch64-android.a libdl.so liblog.so libm.so libunwind.a libz.so ``` Mono is unaffected — it only ever received the `System` kind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restore the allocation-backed
TemporaryPeerMapremoved while unifying GC bridge processing in #12133.This preserves the unified concrete
BridgeProcessingimplementation andmono.android.IGCUserPeerpath from #12133 while removing the accidentally reintroducedstd::unordered_map<size_t, jobject>dependency from the NativeAOT host.Part of #12139. This ports the temporary-peer storage previously merged in #12145 into the unified bridge rather than restoring the old bridge inheritance or legacy JavaInterop1 hooks.
Changes
std::unordered_mapwith one exact-sizecallocallocation;Countfields during the scoped cross-reference pass;IGCUserPeermethod lookup and dispatch unchanged.Validation
dotnet build src/native/native-nativeaot.csproj -c Release --no-restore --nologo -p:BuildProjectReferences=falsedotnet build src/native/native-clr.csproj -c Release --no-restore --nologo -p:BuildProjectReferences=falseunordered_map,hash_table, or__next_primesymbols.