_Unwind_ForcedUnwind is not mandated by the EHABI but for compatibilty
reasons adding so the interface to higher layers would be the same.
Dropping EHABI specific _Unwind_Stop_Fn definition since it is not defined by EHABI.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Add a simple test and fixes.
This version passes on arm32 - armv7l-unknown-linux-gnueabihf -
https://bugs.llvm.org/show_bug.cgi?id=47142 this effects this. the missing unwind info make forceunwind fragile.
Hi Daniel,
We tested this patch in Chrome OS and it resolved our build issues :) . We do have issues on ARM32 with some crashes but we believe that is a separate problem. And we'll continue to look into those.
But I feel it should be ok for you to submit this or request a review from more qualified folks.
@manojgupta Thanks for the feedback, let me know if you have updates.
libunwind/src/Unwind-EHABI.cpp | ||
---|---|---|
722 | Because there was no forceunwind and only in that case the reserved1 is written. | |
libunwind/test/forceunwind.pass.cpp | ||
41 | typo. | |
libunwind/test/libunwind/test/config.py | ||
46 โ | (On Diff #298857) | done already in upstream. |
I think ARMEH _Unwind_ForcedUnwind can probably just reuse the generic implementation.
@danielkiss Hi, I've added libc++abi forced unwinding tests in D95200. Does this implementation make the tests pass?
(Why does ARMEH uses a typedef so that struct _Unwind_Exception* does not compile? )
@MaskRay Thanks for the test case.
The personality handling is a bit different. Might not be much nicer with that many ifdefs.
It passes now.
(Why does ARMEH uses a typedef so that struct _Unwind_Exception* does not compile? )
I think the original typedef struct _Unwind_Control_Block comes from the spec:
https://github.com/ARM-software/abi-aa/blob/f52e1ad3f81254497a83578dc102f6aac89e52d0/ehabi32/ehabi32.rst#82language-independent-unwinding-types-and-functions
_Unwind_Control_Block could be just _Unwind_Exception.
IMHO there is no nice way to achieve the struct _Unwind_Exception* compile.
libunwind/include/unwind.h | ||
---|---|---|
58โ59 | this is not nice, but right know I don't see any other way around. please let me know if you have any better idea. |
libunwind/include/unwind.h | ||
---|---|---|
58โ59 | What's the issue that this solves? | |
libunwind/src/Unwind-EHABI.cpp | ||
727 | splitting hairs, but should long here be intptr_t? | |
libunwind/src/UnwindLevel1-gcc-ext.c | ||
33 | might be more clear to combine with if/else chain below and not rely on the PRIVATE_1 => private_1 => private_[0] expansion. | |
libunwind/test/forceunwind.pass.cpp | ||
41 | IIRC this needs special handling on baremetal, since there is no dladdr there. |
Rebased, retested on ToT.
Now we are confirmed the crash is a separate, unrelated problem.
ARM EHABI is very different from the Itanium implementation. Someone should move the implementation to a separate file.
Sorry, do you mean the content of the libunwind/include/unwind.h ?
libunwind/include/unwind.h | ||
---|---|---|
58โ59 | struct _Unwind_Exception* is used mostly but the definition of _Unwind_Control_Block comes from the Arm EHABI. with the typedef struct _Unwind_Exception* won't work. | |
libunwind/src/Unwind-EHABI.cpp | ||
727 | right, will fix. | |
libunwind/src/UnwindLevel1-gcc-ext.c | ||
33 | agree, I'll update it. | |
libunwind/test/forceunwind.pass.cpp | ||
41 | for now I add a filter for the test it should run only on linux as other tests here. |
libunwind/test/forceunwind.pass.cpp | ||
---|---|---|
41 | sounds good |
I mean all the #if defined(_LIBCXXABI_ARM_EHABI) branches make the file difficult to read. reduce sharing and move the implementation to a separate file.
Many sanitizer files are organized this way.
I understand Itanium _Unwind_ForcedUnwind. The ARM EH impl has a similar structure. LG
libunwind/include/unwind.h | ||
---|---|---|
70 | delete excess space | |
libunwind/src/Unwind-EHABI.cpp | ||
700 | This can trigger clang -Wunused-but-set-variable (from 2021-04 onwards) Don't define functionName in the #ifdef NDEBUG branch | |
713 | C++ doesn't need the elaborated type specifier struct | |
1096 | Delete the comment | |
libunwind/test/forceunwind.pass.cpp | ||
27 | Not needed |
We have been trying to debug 1 issue on ARM32 Chrome OS devices related to pthread_cancel.
After some debugging, I found that glibc uses char[8] for the exception class [1] but libunwind is using uin64_t.
This causes crashes because of ABI mismatch when pthread_cancel calls Unwind routines.
e.g. look at the following code in libunwind which calls unwind_stop from glibc:
(*stop)(1, action, exception_object->exception_class, exception_object, (_Unwind_Context *)(cursor), stop_parameter);
This mismatches the glibc's expected prototype for for unwind_stop [2] in argument 3 (exception class should be char * vs uint64_t).
Here is some data I got from a sample program.
(gdb) bt #0 unwind_stop (version=1, actions=10, exc_class=0x0, exc_obj=0x0, context=0xf74ff628, stop_parameter=0xf74fe648) at unwind.c:44 #1 0xf7f77bdc in unwind_phase2_forced (uc=0xf74fe7d0, cursor=0xf74fe648, exception_object=0xf74ff628, stop=0xf7fab3f1 <unwind_stop>, stop_parameter=0xf74fed78) at /build/elm/tmp/portage/sys-libs/llvm-libunwind-13.0_pre428724-r2/work/llvm-libunwind-13.0_pre428724/libunwind/src/Unwind-EHABI.cpp:716 #2 0xf7f77df2 in _Unwind_ForcedUnwind (exception_object=0xf74ff628, stop=0xf7fab3f1 <unwind_stop>, stop_parameter=0xf74fed78) at /build/elm/tmp/portage/sys-libs/llvm-libunwind-13.0_pre428724-r2/work/llvm-libunwind-13.0_pre428724/libunwind/src/Unwind-EHABI.cpp:1100 #3 0xf7fab4d4 in __GI___pthread_unwind (buf=<optimized out>) at unwind.c:121 #4 0xf7fa22b0 in __do_cancel () at ./pthreadP.h:313
unwind_stop parameters show context=0xf74ff628, it should be the next value shown for stop_parameter instead i.e. 0xf74fe648.
(gdb) ptype exc_class type = char *
this shows char * was expected in glibc instead of uint64_t.
[1] https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/arm/unwind.h;h=ffdea8047d5c6202bdc775548c27b640c36430ae;hb=HEAD#l194
[2] https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/unwind.c;h=8f157e49f4a088ac64722e85ff24514fff7f3c71;hb=HEAD#l40
this is not nice, but right know I don't see any other way around. please let me know if you have any better idea.