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.

Sign up for GitHub

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

Jump to bottom

exception handling doesn't work with static executables #593

Closed
DanAlbert opened this issue Dec 8, 2017 · 17 comments
Closed

exception handling doesn't work with static executables #593

DanAlbert opened this issue Dec 8, 2017 · 17 comments
Milestone
r17

Comments

@DanAlbert
Copy link
Member

Currently undiagnosed. Since it's a static executable, we don't get a stack trace in logcat.
@DanAlbert DanAlbert added this to the r18 milestone Dec 8, 2017
@DanAlbert DanAlbert self-assigned this Dec 8, 2017
@leebaok
Copy link

leebaok commented Jan 24, 2018

So exceptions not work with libc++ static ?
But for gnustl ? can exceptions work ?
@DanAlbert

@jmgao
Copy link
Contributor

jmgao commented Jan 24, 2018

re: no stack traces in logcat, try gdbclient?

@DanAlbert
Copy link
Member Author

https://android-review.googlesource.com/c/platform/bionic/+/600854

@DanAlbert DanAlbert modified the milestones: r18, r17 Jan 25, 2018
@DanAlbert
Copy link
Member Author

The patch above fixes arm32. The other architectures (arm64 at least, I haven't gotten to testing the Intel arches yet, but the fix is likely the same) are broken because Clang doesn't automatically pass --eh-frame-hdr to the linker for static executables: https://android.googlesource.com/toolchain/clang/+/0659c9e0352a35af1ea86cf11b19618543702974/lib/Driver/ToolChains/Gnu.cpp#368

This actually isn't unique to libc++. Exceptions don't work for any STL with static executables because of this.

I'm trying to dig into why Clang doesn't do that and get it fixed, but that will require a clang update. Until then, you can use exceptions in static executables by adding -Wl,--eh-frame-hdr to your ldflags.

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue Jan 25, 2018
@DanAlbert
Fix __gnu_Unwind_Find_exidx for static executables.
aeb5bc6 
__exidx_start and __exidx_end are not actual values to be read, just
symbols placed before and after .ARM.exidx.

The default linker script for ARM static executables includes these
lines:

    __exidx_start = .;
   .ARM.exidx   : { *(.ARM.exidx*) }
    __exidx_end = .;

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474c/BABEHEDA.html

Test: used __gnu_Unwind_Find_exidx in a static executable and showed
      non-bogus values
Test: verified that this fixes #593 in the NDK
Bug: android/ndk#593
Change-Id: Ib35a4b6c82920666ae8a45aa0a2d43e458699b51
@eddielong
Copy link

I'm encountering this problem too (using zxing which heavily uses exceptions for not detecting barcodes, yuck), looking at that link above, do you know when or in what NDK the fix will be released?

@DanAlbert
Copy link
Member Author

The milestone is accurate. This is fixed in r17, but not necessarily in beta 1.

@DanAlbert
Copy link
Member Author

The arm32 fix is now in r17 (will be in beta 1 since we're respinning that for a gradle workaround). The --eh-frame-hdr issue has a patch up for clang (https://reviews.llvm.org/D43203), but that won't land in the NDK until at least r18. We can still do a bit more work for r17 to add -Wl,--eh-frame-hdr to the default ldflags for CMake and ndk-build, but that won't be in beta 1.

@DanAlbert
Copy link
Member Author

https://android-review.googlesource.com/c/platform/ndk/+/618577 hoists this into the build systems until we get a clang with the fix.

@DanAlbert DanAlbert changed the title libc++ exception handling doesn't work with static executables exception handling doesn't work with static executables Feb 14, 2018
@DanAlbert
Copy link
Member Author

r17 beta 1 is being respun again, so this will be in after all. I'll leave the bug open to track the clang fix, but I'm moving this off the r17 milestone since we've done as much as we can for this release.

@DanAlbert DanAlbert modified the milestones: r17, r18 Feb 15, 2018
@DanAlbert DanAlbert assigned stephenhines and unassigned DanAlbert Feb 21, 2018
@DanAlbert
Copy link
Member Author

Change is submitted upstream. Over to @stephenhines to get the change into the next clang update.

@DanAlbert
Copy link
Member Author

DanAlbert commented Mar 6, 2018 •
edited

Fixed in r17 after all since we needed one last clang respin.

@DanAlbert DanAlbert modified the milestones: r18, r17 Mar 6, 2018
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
@DanAlbert
Work around Clang not passing --eh-frame-hdr.
a5b6299 
Clang doesn't pass this automatically for static executables for some
reason, and libgcc can't unwind without a .eh_frame_hdr section.

Also move the test to libc++ since gnustl is going away soon.

Test: ./run_tests.py --filter static-executable-exceptions
Bug: http://b/24468267
Bug: http://b/30101473
Bug: http://b/72512648
Bug: android/ndk#593
Change-Id: I153dfee4d22082560833517f1ee575742da21e21
(cherry picked from commit 2f7aa277079f13e431f355c33ccf1ab601d3e947)
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
@DanAlbert
Test fixes for modern static libraries.
4523043 
Unfortunately static executables with libc++ will require API > 26 to
avoid libandroid_support. Fortunately this doesn't actually cause any
issues except in the case of having a static executable in the same
project as other non-static executable modules.

Test: ./run_tests.py
Bug: http://b/24507500
Bug: android/ndk#593
Change-Id: I925dac426a41326c76bddec691ba8ce227ba5370
(cherry picked from commit d0dd54a20855dcb9cfdcda2c129edf1e4d2f42bb)
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
@DanAlbert
Hoist --eh-frame-hdr into the build systems.
3d72ae6 
Better than making all the users do it themselves. We can revert this
once we get a clang with https://reviews.llvm.org/D43203.

Test: ./checkbuild.py && ./run_tests.py
Bug: android/ndk#593
Bug: http://b/72512648
Change-Id: I53791d53cfab8eefe004f8e278527c80f1fa1240
(cherry picked from commit c8558428d993000a68fe0e72815b2bb85f228438)
miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
@DanAlbert
Update Clang to build 4630689.
824d75a 
Test: ./checkbuild.py && ./run_tests.py
Bug: android/ndk#593
Bug: http://b/72512648
Change-Id: I01df21b47da547efc6c8b029fc389423e9a63c23
(cherry picked from commit fa3562bbdad17357fb7fb01cb77ac546abfa99a2)
miodragdinic pushed a commit to MIPS/prebuilts-ndk that referenced this issue Apr 17, 2018
@DanAlbert
Update NDK platform prebuilts to build 4567395.
17b9430 
Test: ndk/checkbuild.py && ndk/run_tests.py
Bug: android/ndk#593
Change-Id: Ifee3ce4bdf810e68f0012a978b520e4222607939
(cherry picked from commit eaf16d99504a191679b5ca97466ee7aeaecd9089)
@trapacska trapacska mentioned this issue Jun 13, 2018
Merged
@FrankHB
Copy link

FrankHB commented Sep 7, 2019 •
edited

@DanAlbert This is not fully fixed, at least for old platforms (tested with NDK r20, target platform v21), because the definition of __gnu_Unwind_Find_exidx actually used by libc++ is still broken. It seems that the broken definition comes from the dynamically linked system libc .

Since statically linked libc is problematic, I have to backport this and pass -Wl,--wrap=__gnu_Unwind_Find_exidx to the linker command with explicitly specified crtbegin_so/crtend_so stuff to allow the object file containing the wrapper after -lc++, and it worksbut this still terminates at __cxxabiv1::failed_throw for an exception that should have been caught.

I've tested it in a real machine running Android 9.0 and it fails at __cxxabiv1::failed_throw with a different exception failed to catch.

_Unwind_RaiseException returns _URC_FAILURE in __cxa_throw. Not sure why.

Is this related to https://bugs.llvm.org/show_bug.cgi?id=21053?

@DanAlbert
Copy link
Member Author

It seems that the broken definition comes from the dynamically linked system libc .

Then it sounds like you aren't using a static executable. Could you verify that?

@FrankHB
Copy link

FrankHB commented Sep 9, 2019

@DanAlbert My fault. I was testing the standalone toolchain with different project configurations and occasionally got confused by makefiles having different linker flags (-shared/-static/-static -static-libstdc++/nothing).

The previously mentioned problem still exists in the configuration with a single .so linked with -static-libstdc++ loaded by NativeActivity. This should be indeed another topic, but let me summarize a little here. AFAIK the failure is due to a call to an unexpected personality routine in the unwinding phase 1, which should not happen after following the document. The failure only occur within some specific position of my code. (Not sure if this and this are related.) Since the original case is not easy to reduce, I'm still investigating the exact condition to reproduce the problem. I'd open an issue then. Sorry for the misreport.

@DanAlbert
Copy link
Member Author

Yeah, go ahead and open a new bug if you find something. It's almost always a broken dependency in this situation.

@FrankHB
Copy link

FrankHB commented Sep 11, 2019

I find it a code generation issue, reported upstream.

@DanAlbert DanAlbert mentioned this issue Sep 11, 2019
Open
@DanAlbert
Copy link
Member Author

Thanks! I filed #1079 to track that on our side.

sepehrst pushed a commit to spsforks/android-bionic-libc that referenced this issue Apr 22, 2024
@DanAlbert
Fix __gnu_Unwind_Find_exidx for static executables.
7fac86d 
__exidx_start and __exidx_end are not actual values to be read, just
symbols placed before and after .ARM.exidx.

The default linker script for ARM static executables includes these
lines:

    __exidx_start = .;
   .ARM.exidx   : { *(.ARM.exidx*) }
    __exidx_end = .;

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474c/BABEHEDA.html

Test: used __gnu_Unwind_Find_exidx in a static executable and showed
      non-bogus values
Test: verified that this fixes #593 in the NDK
Bug: android/ndk#593
Change-Id: Ib35a4b6c82920666ae8a45aa0a2d43e458699b51
sepehrst pushed a commit to spsforks/android-bionic-libc that referenced this issue Apr 22, 2024
@DanAlbert
Fix __gnu_Unwind_Find_exidx for static executables.
0414f5b 
__exidx_start and __exidx_end are not actual values to be read, just
symbols placed before and after .ARM.exidx.

The default linker script for ARM static executables includes these
lines:

    __exidx_start = .;
   .ARM.exidx   : { *(.ARM.exidx*) }
    __exidx_end = .;

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474c/BABEHEDA.html

Test: used __gnu_Unwind_Find_exidx in a static executable and showed
      non-bogus values
Bug: android/ndk#593
Change-Id: Ib35a4b6c82920666ae8a45aa0a2d43e458699b51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
r17
Development

No branches or pull requests
6 participants
@DanAlbert @jmgao @FrankHB @leebaok @eddielong @stephenhines