gh-149202: Fix frame pointer unwinding on s390x and ARM#149362
gh-149202: Fix frame pointer unwinding on s390x and ARM#149362pablogsal wants to merge 1 commit intopython:mainfrom
Conversation
pablogsal
requested review from
AA-Turner,
corona10,
emmatyping and
erlend-aasland
as code owners
|
!buildbot ARM |
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 9dcfaa3 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F149362%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
|
!buildbot S390x |
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 9dcfaa3 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F149362%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
Documentation build overview
14 files changed ·
|
|
!buildbot S390x |
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 9dcfaa3 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F149362%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
pablogsal
force-pushed
the
gh-149202-2
branch
3 times, most recently
from
87aa290 to
ef67c5d
Compare
-fno-omit-frame-pointer is not enough to make every target walkable by the simple manual frame pointer unwinder. The helper used by test_frame_pointer_unwind used to assume the frame pointer named a two-word record where fp[0] was the previous frame pointer and fp[1] was the return address. That is only the generic layout used by some targets. This patch keeps that default, but moves the slots behind named offsets so architecture-specific layouts can describe where the backchain and return address really live. On s390x, GCC and Clang do not emit a usable backchain unless -mbackchain is also enabled. Without it, the unwinder stops at the current C frame and the test reports no Python frames. Once backchains are present, the helper must also stop at the current thread's known C stack bounds; otherwise it can follow the final backchain far enough to dereference an invalid frame and segfault. For Linux s390x backchain frames, the documented z/Architecture stack-frame layout saves r14, the return-address register, at byte offset 112 from the frame pointer, so read the return address from that named slot instead of fp[1]. The 112-byte offset comes from Linux's s390 debugging documentation: its Stack Frame Layout table shows z/Architecture backchain frames with the backchain at offset 0 and saved r14 of the caller function at offset 112: https://www.kernel.org/doc/html/v5.3/s390/debugging390.html#stack-frame-layout This helper remains scoped to Linux s390x backchain frames. GNU SFrame's s390x notes state that the s390x ELF ABI does not generally mandate where RA and FP are saved, or whether they are saved at all: https://sourceware.org/binutils/docs/sframe-spec.html#s390x On 32-bit ARM, GCC defaults to Thumb mode on common armhf toolchains. The Thumb prologue keeps the saved frame pointer and link register at offsets that depend on the generated frame, which breaks the fp[0]/fp[1] walk used by the helper. Use -marm when it is supported for frame-pointer builds, and teach the helper the GCC ARM-mode slots where the previous frame pointer is at fp[-1] and the saved LR return address is at fp[0].
pablogsal
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @pablogsal for commit c68855d 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F149362%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
On s390x, it would be best to use only As background, the primary purpose of a "frame pointer" in compiler-generated code is to allow accessing local variables and spill slots in the current stack frame, if variable-sized stack allocations (alloca) is used. Normally, the stack frame is accessed via constant offsets to the stack pointer, but that doesn't work when alloca is in place, so a separate register (the frame pointer) is used instead. Now, on some architectures (like x86), that frame pointer refers to the top of the stack frame (close to the stack pointer at function entry), and therefore happens to be implicitly usable for stack backtracing. But on other platforms (like s390x) the frame pointer instead refers to the bottom of the stack frame (close to the stack pointer after the function prolog), so it is completely useless for stack backtracing. [ The main reason for this choice is that on s390x, some instructions only allow register+displacement addressing for positive values of the displacement, so a pointer to the bottom of the stack frame can be used more efficiently than a pointer to the top of the stack frame. ] So the only thing |
|
I don't have the context to review this properly. In case naive questions are helpful: Adding As PEP 831's impact analysis doesn't cover these platforms, would it be better to skip the test, and defer PEP 831 for Thumb & s390x? |
| ]) | ||
| AS_CASE([$host_cpu], [s390*], [ | ||
| AX_CHECK_COMPILE_FLAG([-mbackchain], [ | ||
| frame_pointer_cflags="$frame_pointer_cflags -mbackchain" |
There was a problem hiding this comment.
-fno-omit-frame-pointer should be avoided on s390 64-bit (s390x), as frame pointer based unwinding is not supported. When using the s390 back chain as alternative to frame pointers the use of -mbackchain should be sufficient. See my talk s390: Stack tracing using Frame Pointer, Back Chain, and SFrame for details.
Therefore the line above should be changed as follows:
frame_pointer_cflags="-mbackchain"
|
Thanks for the questions, they're not naive at all so happy to give context. To the second one first: I'd rather not skip the test and defer. If we skip the test, we don't make the underlying problem go away, we just stop noticing it: a 3.15 built with naive On 32-bit ARM, GCC can target two different instruction encodings: the classic fixed-width 32-bit "ARM" encoding ( The two modes generate materially different prologues. In ARM mode, GCC emits a fixed Forcing This is the only way to make the PEP compliant in these platforms. And if any user really doesn't want |
6 participants
This was particularly hard to get right :(
-fno-omit-frame-pointer is not enough to make every target walkable by the simple manual frame pointer unwinder.
The helper used by test_frame_pointer_unwind used to assume the frame pointer named a two-word record where fp[0] was the previous frame pointer and fp[1] was the return address. That is only the generic layout used by some targets. This patch keeps that default, but moves the slots behind named offsets so architecture-specific layouts can describe where the backchain and return address really live.
On s390x, GCC and Clang do not emit a usable backchain unless -mbackchain is also enabled. Without it, the unwinder stops at the current C frame and the test reports no Python frames. Once backchains are present, the helper must also stop at the current thread's known C stack bounds; otherwise it can follow the final backchain far enough to dereference an invalid frame and segfault. For Linux s390x backchain frames, the documented z/Architecture stack-frame layout saves r14, the return-address register, at byte offset 112 from the frame pointer, so read the return address from that named slot instead of fp[1].
On 32-bit ARM, GCC defaults to Thumb mode on common armhf toolchains. The Thumb prologue keeps the saved frame pointer and link register at offsets that depend on the generated frame, which breaks the fp[0]/fp[1] walk used by the helper. Use -marm when it is supported for frame-pointer builds, and teach the helper the GCC ARM-mode slots where the previous frame pointer is at fp[-1] and the saved LR return address is at fp[0].