[cpullvm] Enable libunwind tests#479
Conversation
* llvm-libunwind-picolibc.cfg.in: wrap `-lc -lm -lclang_rt.builtins` in --start-group/--end-group. Some libunwind tests introduce back-references between libc and compiler-rt builtins that a single-pass link cannot resolve on bare-metal. * xfails.py: xfail signal_unwind.pass.cpp and unwind_leaffunction.pass.cpp on aarch64a_tlsie and the seven riscv64 variants. picolibc's semihost kill() is a stub that calls _exit(128+sig) instead of delivering the signal to the registered handler, so signal-driven unwind tests cannot run on these targets. * patches/picolibc/0004: align .except_unordered to 4 bytes instead of @DEFAULT_ALIGNMENT@ (8). When the prior section ends 4-byte-aligned, ALIGN(8) bumps __exidx_start by +4 but ELD emits .ARM.exidx content at the section base, so __exidx_start lands half an entry into the table and libunwind's binary search over .ARM.exidx misses. IHI0038 §6 defines each entry as two 4-byte words, so 4-byte alignment is sufficient. Signed-off-by: Shreeyash Pandey <shrpand@qti.qualcomm.com>
|
This PR is incumbent of these issues being fixed and brought in their respective projects: |
There was a problem hiding this comment.
Can you move this patch file to start 0006-? We already have patches 1-5 now. Hopefully it still applies cleanly.
| @@ -32,8 +32,10 @@ config.substitutions.append(('%{link_flags}', | |||
| ' -nostdlib++ -L %{lib}' | |||
| ' -lc++ -lc++abi -lunwind' | |||
| ' -nostdlib -L %{libc-lib}' | |||
| ' -Wl,--start-group' | |||
| ' -lc -lm -lclang_rt.builtins' | |||
There was a problem hiding this comment.
-lm does not need to be in the linker group, it should be able to come before -Wl,--start-group, and this is what we should probably do because this is what the driver will do.
| (half an entry) into the table; libunwind decodes every entry off-by-4, | ||
| the binary search in getInfoFromEHABISection() misses, and unwinding | ||
| stops at frame 1. Breaks libunwind_01, libunwind_02, and | ||
| backtrace/forced-unwind on all ARM EHABI variants. |
There was a problem hiding this comment.
Is this a permanent picolibc linker script fix or temporary while waiting some ELD change or do you plan to upstream this change to picolibc project?
There was a problem hiding this comment.
This is supposed to be a permanent change. Now that I think of it, I guess its better to have it upstreamed. I'll create a PR in picolibc for this.
There was a problem hiding this comment.
Did you check with any of the eld folks on this? Or check if lld/bfd do the same thing?
I'm not familiar with the Arm exidx stuff, but based on the description in the .patch file, this doesn't really sound like a picolibc problem to me.
but ELD emits the ARM_EXIDX content at the section base
What is eld emitting (what does ARM_EXIDX content mean?)? AFAIK .except_unordered isn't a special section eld should do anything special for.
If you're saying the input sections that should be grabbed by *(.ARM.exidx*) are being placed weirdly and __exidx_start then points partially into it, that seems like a bug in eld to me?
There was a problem hiding this comment.
Consider the executable for libunwind01.pass.cpp:
$ llvm-readelf -SW t.tmp.exe
There are 17 section headers, starting at offset 0x8020:
Section Headers:
[Nr] Name Type Address Off Size ES Flg Lk Inf Al
[ 0] NULL 00000000 000000 000000 00 0 0 0
[ 1] .init PROGBITS 00000000 000100 000040 00 A 0 0 128
[ 2] .text PROGBITS 00000040 000140 003548 00 AX 0 0 16
[ 3] .rodata PROGBITS 00003588 003688 000700 00 A 0 0 8
[ 4] .except_ordered PROGBITS 00003c88 003d88 000024 00 A 0 0 4
[ 5] .except_unordered ARM_EXIDX 00003cac 003dac 0003ac 00 AL 2 0 4
[ 6] .data PROGBITS 20000000 004158 000018 00 WA 0 0 4
...
Key to Flags:
W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
L (link order), O (extra OS processing required), G (group), T (TLS),
C (compressed), x (unknown), o (OS specific), E (exclude),
R (retain), y (purecode), p (processor specific)
$ llvm-nm t.tmp.exe | rg "__exidx_"
00004058 R __exidx_end
00003cb0 R __exidx_start
Here, so far as I understand, __exidx_start should match .except_unordereds (which contains the EHABI exception index table) start address.
But there's a 4 byte gap. __exidx_start is 4 bytes ahead of .except_unordered.
How I saw this:
The linker script contains this:
.except_unordered : {
. = ALIGN(@DEFAULT_ALIGNMENT@);
PROVIDE(@PREFIX@__exidx_start = .);
*(.ARM.exidx*)
PROVIDE(@PREFIX@__exidx_end = .);
At the start, . (current pointer) is 0x3c88 (which is the end of previous section). ALIGN(8) makes it 0x3cb0 and attaches that value
to __exidx_start. But the contents of .ARM.exidx (.except_unordered) are being placed at the old value (0x3c88). If the section placement
by the linker is wrong, this should be universal and other sections should also be effected. Therefore, it is probably the alignment that
the user is passing in the linker script that's going wrong. Changing the alignment fixed this as 0x3c88 aligned to 4 bytes is 0x3c88 (unchanged).
But now that I think about it, your suggestion that this is an eld bug sounds plausible too. When the current pointer moves from 0x3c88 to 0x3cb0,
the linker should acknowledge this and place the contents at the new address.
There was a problem hiding this comment.
I linked the same test with lld.
[ 5] .except_ordered PROGBITS 00003c90 013c90 000024 00 A 0 0 4
[ 6] .except_unordered ARM_EXIDX 00003cb4 013cb4 0001a4 00 AL 2 0 4
[ 7] .data PROGBITS 20000000 020000 000018 00 WA 0 0 4
00003cb8 R __exidx_start
__exidx_start and the section address dont match but the binary is padded with 00s between 0x3cb4 and 0x3cb8. These tests pass with lld.
Jonathon Penix (@jonathonpenix) I guess you're right and this is an eld bug.
There was a problem hiding this comment.
Hi Shreeyash Pandey (@bojle), Hi Jonathon Penix (@jonathonpenix)
Shreeyash Pandey (@bojle) Thanks for the analysis.
I am adding some comments, but from what I have read so far, the address/value of ARM_EXIDX/__exidx_start does not seem to be an eld bug.
When the prior section ends 4-byte (not
8-byte) aligned, ALIGN(8) bumps the symbol +4, but ELD emits the
ARM_EXIDX content at the section base. So __exidx_start lands 4 bytes
(half an entry) into the table
Can you please tell how did you verify that eld emits ARM_EXIDX contents at the section base?
Here, so far as I understand, __exidx_start should match .except_unordereds (which contains the EHABI exception index table) start address.
But there's a 4 byte gap. __exidx_start is 4 bytes ahead of .except_unordered.
__exidx_start should not match .except_unordered. It should match the starting address of the first section that matched *(.ARM.exidx). Please note that the .except_unordered is an output section and the linker script is incrementing the location counter inside it and then storing its value in __exidx_start. Hence, the output section address and the value of __exidx_start need not be the same.
As per the readelf and nm output, the values looks correct. .except_unordered is 0x3cac, after aligning to 0x8, it becomes 0x3cb0, and that is the value of __exidx_start.
[ 4] .except_ordered PROGBITS 00003c88 003d88 000024 00 A 0 0 4
[ 5] .except_unordered ARM_EXIDX 00003cac 003dac 0003ac 00 AL 2 0 4
[ 6] .data PROGBITS 20000000 004158 000018 00 WA 0 0 4
...
...
00004058 R __exidx_end
00003cb0 R __exidx_start
Can you please check the map-file to verify how eld is filling the output section .except_unordered? It should also show if eld is indeed placing *(.ARM.exidx) input sections at the output section base.
There was a problem hiding this comment.
Here's the relevant snippet from the map file:
.except_unordered 0x3cac 0x3ac # Offset: 0x3dac, LMA: 0x3cac, Alignment: 0x4, Flags: SHF_ALLOC|SHF_LINK_ORDER, Type: SHT_ARM_EXIDX, Segments : [ text ], Memory : [fl
ash, flash]
.(0x3cb0) = ALIGN(.(0x3cac), 0x8); # . = ALIGN(., 0x8); /local/mnt/workspace/shrp
and/cpullvm-toolchain/qualcomm-software/build/multilib-builds/runtimes/armv7m_soft_nofp_nopic/build/tmp_install/lib/picolibcpp.ld
PROVIDE(__exidx_start(0x3cb0) = .(0x3cb0)); # PROVIDE(__exidx_start = .); /local/mnt/workspace/shrpand/cpullvm-toolchain/qualcomm-software/build/multilib-builds/runtimes/armv7m_soft_nofp_nopic/build/tmp_install/lib/picolibcpp.ld
*(.ARM.exidx*) #Rule 47, /local/mnt/workspace/shrpand/cpullvm-toolchain/qualcomm-software/build/multilib-builds/runtimes/armv7m_soft_nofp_nopic/build/tmp_install/lib/picolibcpp.ld [110:0]
PROVIDE(__exidx_end(0x4058) = .(0x4058)); # PROVIDE(__exidx_end = .); /local/mnt/workspace/shrpand/cpullvm-toolchain/qualcomm-software/build/multilib-builds/runtimes/armv7m_soft_nofp_nopic/build/tmp_install/lib/picolibcpp.ld
*(.except_unordered) #Rule 48, Internal-LinkerScript (Implicit rule inserted by Linker) [0:0]
.ARM.exidx.text.unlikely._Unwind_Resume 0x3cac 0x8 /local/mnt/workspace/shrpand/cpullvm-toolchain/qualcomm-software/build/multilib-builds/cxxlibs/armv7m_soft_nofp_nopic/build/libunwind/test-suite-install/lib/libunwind.a(Unwind-EHABI.cpp.obj) #SHT_ARM_EXIDX,SHF_ALLOC|SHF_LINK_ORDER,4
You can see that .except_unordered starts at 0x3cac but __exidx_start is 0x3cb0 which 4 bytes in to the section. I agree that these numbers shouldn't necessarily be the same, I've checked with lld too. However, starting from address 0x3cac (.execpt_unordered base), eld emits real data (94c3ff7f a837b280, verified with readelf -x) whereas lld pads the first 4 bytes.
There was a problem hiding this comment.
"The addresses must be same for __exidx_start and .except_unordered" was my initial assumption on the basis of which I went with fixing the alignment in the linker script. This discussion makes me suspect it could be an eld bug.
There was a problem hiding this comment.
Thanks Shreeyash Pandey (@bojle) for sharing the map-file snippet. Yes, it seems to be an eld bug. .ARM.exidx.text.unlikely._Unwind_Resume section is incorrectly starting from 0x3cac and it seems to be incorrectly matching *(.except_unordered) instead of *(.ARM.exidx*).
There was a problem hiding this comment.
qualcomm/eld#1194 patch fixes this issue as well.
llvm-libunwind-picolibc.cfg.in: wrap
-lc -lm -lclang_rt.builtinsin --start-group/--end-group. Some libunwind tests introduce back-references between libc and compiler-rt builtins that a single-pass link cannot resolve on bare-metal.xfails.py: xfail signal_unwind.pass.cpp and unwind_leaffunction.pass.cpp on aarch64a_tlsie and the seven riscv64 variants. picolibc's semihost kill() is a stub that calls _exit(128+sig) instead of delivering the signal to the registered handler, so signal-driven unwind tests cannot run on these targets.
patches/picolibc/0004: align .except_unordered to 4 bytes instead of @DEFAULT_ALIGNMENT@ (8). When the prior section ends 4-byte-aligned, ALIGN(8) bumps __exidx_start by +4 but ELD emits .ARM.exidx content at the section base, so __exidx_start lands half an entry into the table and libunwind's binary search over .ARM.exidx misses. IHI0038 §6 defines each entry as two 4-byte words, so 4-byte alignment is sufficient.