Guard unused llvm-libunwind symbols to avoid duplicates on Android#128667
Conversation
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
Pull request overview
This PR updates the NativeAOT build of the vendored llvm-libunwind to avoid duplicate symbol conflicts (notably on Android NDK r29) by compiling out libunwind’s public unw_* entrypoints and related exception-dispatch code that NativeAOT doesn’t use, and by routing NativeAOT’s unwind-section discovery through a runtime-owned helper instead of libunwind’s LocalAddressSpace::sThisAddressSpace singleton.
Changes:
- Add
_LIBUNWIND_NATIVEAOTand use it to guard outunw_*APIs (and theLocalAddressSpace::sThisAddressSpacesingleton) inlibunwind.cpp, plus dependent EHABI exception-dispatch exports inUnwind-EHABI.cpp. - Introduce
UnwindHelpers::FindUnwindSections()and switchUnixNativeCodeManagerto use it (avoiding the now-guardedsThisAddressSpace). - Record the patch application in
llvm-libunwind-version.txtand enable the new define in the NativeAOT runtime CMake configuration.
Show a summary per file
| File | Description |
|---|---|
| src/native/external/llvm-libunwind/src/Unwind-EHABI.cpp | Guards EHABI exception-dispatch exports when _LIBUNWIND_NATIVEAOT is set to avoid references to removed __unw_* APIs. |
| src/native/external/llvm-libunwind/src/libunwind.cpp | Guards the public unw_* API surface and the LocalAddressSpace::sThisAddressSpace singleton under _LIBUNWIND_NATIVEAOT. |
| src/native/external/llvm-libunwind/src/AddressSpace.hpp | Guards the LocalAddressSpace::sThisAddressSpace static member declaration under _LIBUNWIND_NATIVEAOT. |
| src/native/external/llvm-libunwind-version.txt | Notes the applied commit associated with this local patch. |
| src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.h | Declares the new UnwindHelpers::FindUnwindSections accessor. |
| src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp | Implements FindUnwindSections by delegating to the existing file-scope _addressSpace. |
| src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp | Replaces use of LocalAddressSpace::sThisAddressSpace.findUnwindSections with UnwindHelpers::FindUnwindSections. |
| src/coreclr/nativeaot/Runtime/CMakeLists.txt | Defines _LIBUNWIND_NATIVEAOT=1 for Unix NativeAOT builds to enable the guarding behavior. |
Copilot's findings
- Files reviewed: 8/8 changed files
- Comments generated: 0
|
If it's a negative |
Deleting the code creates more conflicts during updates. We won't be able to upstream the deletion either. I think ifdefing is better. |
NativeAOT uses llvm-libunwind's internal C++ classes directly (DwarfInstructions, CompactUnwinder, UnwindCursor, LocalAddressSpace) and does not call the public unw_* C API. These symbols conflict with platform libunwind on Android NDK r29. Add a _LIBUNWIND_NATIVEAOT define that guards out the public API symbols NativeAOT does not use: - libunwind.cpp: wrap the entire file (except debug logging) in #if !defined(_LIBUNWIND_NATIVEAOT) as a purely additive patch, making it resilient to upstream llvm-libunwind updates - Unwind-EHABI.cpp: the C++ exception dispatch functions are also guarded because they depend on the now-guarded unw_* functions and would cause undefined symbol errors on ARM32 Switch UnixNativeCodeManager to use a new UnwindHelpers::FindUnwindSections() accessor instead of the now-guarded sThisAddressSpace singleton. This delegates to the existing file-local LocalAddressSpace instance in UnwindHelpers.cpp. Co-authored-by: Jan Kotas <jkotas@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reference fde4a5b (Guard unused llvm-libunwind symbols). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@jkotas can you please help merge this without squashing? |
Split the single #if !defined(_LIBUNWIND_NATIVEAOT) guard into two blocks so that _Unwind_VRS_Get, _Unwind_VRS_Get_Internal, _Unwind_VRS_Set, _Unwind_VRS_Pop, and ValueAsBitPattern are compiled into the NativeAOT build. These helpers are called by _Unwind_VRS_Interpret which is the ARM32 EHABI managed-frame unwinding path in NativeAOT. PR #128667 accidentally included them inside the guard, causing the linker to resolve them against libgcc_s.so.1 instead of the in-tree implementations that understand the NativeAOT ArmUnwindCursor shim, resulting in SIGSEGV during ARM32 stack unwinding. The genuinely-unused C++ exception dispatch functions (__aeabi_unwind_cpp_pr*, unwind_phase1/2, _Unwind_RaiseException, _Unwind_ForcedUnwind, etc.) that depend on __unw_step and other libunwind.cpp public API remain guarded. Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
… _LIBUNWIND_NATIVEAOT guard PR #128667 accidentally excluded _Unwind_VRS_Get/Set/Pop (and their __unw_get/set_reg/__unw_save_vfp_as_X dependencies) from the NativeAOT build while leaving _Unwind_VRS_Interpret (their caller) in place. The linker resolved the exported VRS symbols from libgcc_s.so.1 which treats ArmUnwindCursor* as a real libgcc context → SIGSEGV on ARM32. Unwind-EHABI.cpp: Split single NATIVEAOT guard into two blocks so ValueAsBitPattern and _Unwind_VRS_Set/Get_Internal/Get/Pop sit between the two guards (always compiled). libunwind.cpp: Split single NATIVEAOT guard into smaller inner guards keeping __unw_get_reg, __unw_set_reg, __unw_get_fpreg, __unw_set_fpreg, __unw_save_vfp_as_X unconditional so they are compiled for NativeAOT. The singleton, __unw_init_local, __unw_step, __unw_resume, DWARF/FDE functions, and Apple dynamic sections remain behind the guard. Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
|
This broke uwinding on arm32. The failure mode is a rather mysterious sigsegv, but pulling down a binary from a failed test run and running Copilot is trying to do something at #128830. |
NativeAOT uses llvm-libunwind's internal C++ classes directly
(DwarfInstructions, CompactUnwinder, UnwindCursor, LocalAddressSpace)
and does not call the public unw_* C API. These symbols conflict with
platform libunwind on Android NDK r29.
Add a _LIBUNWIND_NATIVEAOT define that guards out the public API
symbols NativeAOT does not use:
the LocalAddressSpace::sThisAddressSpace singleton
guarded because they depend on the now-guarded unw_* functions
and would cause undefined symbol errors on ARM32
Switch UnixNativeCodeManager to use a new UnwindHelpers::FindUnwindSections()
accessor instead of the now-guarded sThisAddressSpace singleton. This
delegates to the existing file-local LocalAddressSpace instance in
UnwindHelpers.cpp
Fixes #121172