Skip to content

Commit ceac02f

Browse files
committed
pythongh-149202: Fix frame pointer unwinding on s390x and ARM
-fno-omit-frame-pointer is not enough to make every target walkable by the simple manual frame pointer unwinder. 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. s390x stores the return address in the ABI register save area as saved GPR14, so read it from the named s390x frame-layout offset. 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 saved-fp and saved-lr slots.
1 parent 3efd2f4 commit ceac02f

7 files changed

Lines changed: 252 additions & 24 deletions

File tree

Doc/howto/perf_profiling.rst

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,9 @@ How to obtain the best results
219219

220220
For best results, keep frame pointers enabled. On supported GCC-compatible
221221
toolchains, CPython builds itself with ``-fno-omit-frame-pointer`` and, when
222-
available, ``-mno-omit-leaf-frame-pointer`` by default. These flags allow
222+
available, ``-mno-omit-leaf-frame-pointer`` by default. On 32-bit ARM,
223+
CPython also adds ``-marm`` when supported. On s390 platforms, CPython also
224+
adds ``-mbackchain`` when supported. These flags allow
223225
profilers to unwind using only the frame pointer and not on DWARF debug
224226
information. This is because as the code that is interposed to allow ``perf``
225227
support is dynamically generated it doesn't have any DWARF debugging information

Doc/using/configure.rst

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -784,9 +784,11 @@ also be used to improve performance.
784784

785785
Disable frame pointers, which are enabled by default (see :pep:`831`).
786786

787-
By default, the build appends ``-fno-omit-frame-pointer`` (and
788-
``-mno-omit-leaf-frame-pointer`` when the compiler supports it) to
789-
``BASECFLAGS`` so profilers, debuggers, and system tracing tools
787+
By default, the build appends ``-fno-omit-frame-pointer``,
788+
``-mno-omit-leaf-frame-pointer`` when the compiler supports it,
789+
``-marm`` on 32-bit ARM when supported, and ``-mbackchain`` on s390
790+
platforms when supported, to ``BASECFLAGS`` so
791+
profilers, debuggers, and system tracing tools
790792
(``perf``, ``eBPF``, ``dtrace``, ``gdb``) can walk the C call stack
791793
without DWARF metadata. The flags propagate to third-party C
792794
extensions through :mod:`sysconfig`. On compilers that do not

Doc/whatsnew/3.15.rst

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2305,8 +2305,9 @@ Build changes
23052305
(:pep:`831`). Pass :option:`--without-frame-pointers` to opt out.
23062306
Authors of C extensions and native libraries built with custom build
23072307
systems should add ``-fno-omit-frame-pointer`` and
2308-
``-mno-omit-leaf-frame-pointer`` to their own ``CFLAGS`` to keep the
2309-
unwind chain intact.
2308+
``-mno-omit-leaf-frame-pointer`` to their own ``CFLAGS``,
2309+
``-marm`` on 32-bit ARM, and ``-mbackchain`` on s390 platforms,
2310+
to keep the unwind chain intact.
23102311
(Contributed by Pablo Galindo Salgado and Savannah Ostrowski in :gh:`149201`.)
23112312

23122313
.. _whatsnew315-windows-tail-calling-interpreter:
Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
Enable frame pointers by default for GCC-compatible CPython builds, including
2-
``-mno-omit-leaf-frame-pointer`` when the compiler supports it, so profilers
3-
and debuggers can unwind native interpreter frames more reliably. Users can pass
2+
``-mno-omit-leaf-frame-pointer``, ``-marm`` on 32-bit ARM, and ``-mbackchain``
3+
on s390 platforms when the compiler supports them, so profilers and debuggers
4+
can unwind native interpreter frames more reliably. Users can pass
45
``--without-frame-pointers`` to opt out.

Modules/_testinternalcapi.c

Lines changed: 128 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,24 @@
5959

6060
static const uintptr_t min_frame_pointer_addr = 0x1000;
6161

62+
#ifdef __s390x__
63+
// s390x stores saved GPR14 112 bytes into the ABI register save area.
64+
# define S390X_FRAME_SAVED_GPR14_OFFSET 112
65+
#endif
66+
67+
#if defined(__arm__) && !defined(__thumb__) && !defined(__clang__)
68+
// GCC ARM mode keeps the caller's fp below fp and the saved LR at fp[0].
69+
# define FRAME_POINTER_NEXT_OFFSET (-1)
70+
# define FRAME_POINTER_RETURN_OFFSET 0
71+
#elif defined(__s390x__)
72+
# define FRAME_POINTER_NEXT_OFFSET 0
73+
# define FRAME_POINTER_RETURN_OFFSET \
74+
(S390X_FRAME_SAVED_GPR14_OFFSET / (Py_ssize_t)sizeof(uintptr_t))
75+
#else
76+
# define FRAME_POINTER_NEXT_OFFSET 0
77+
# define FRAME_POINTER_RETURN_OFFSET 1
78+
#endif
79+
6280

6381
static PyObject *
6482
_get_current_module(void)
@@ -325,16 +343,108 @@ get_jit_backend(PyObject *self, PyObject *Py_UNUSED(args))
325343
#endif
326344
}
327345

346+
static int
347+
stack_address_is_valid(uintptr_t addr, uintptr_t stack_min, uintptr_t stack_max)
348+
{
349+
if (addr < min_frame_pointer_addr) {
350+
return 0;
351+
}
352+
if (stack_min != 0 && (addr < stack_min || addr >= stack_max)) {
353+
return 0;
354+
}
355+
return 1;
356+
}
357+
358+
static int
359+
frame_pointer_slot_is_valid(uintptr_t *frame_pointer, Py_ssize_t offset,
360+
uintptr_t stack_min, uintptr_t stack_max)
361+
{
362+
uintptr_t fp_addr = (uintptr_t)frame_pointer;
363+
uintptr_t slot_addr;
364+
uintptr_t delta = (uintptr_t)Py_ABS(offset) * sizeof(uintptr_t);
365+
if (offset < 0) {
366+
if (fp_addr < delta) {
367+
return 0;
368+
}
369+
slot_addr = fp_addr - delta;
370+
}
371+
else {
372+
if (fp_addr > UINTPTR_MAX - delta) {
373+
return 0;
374+
}
375+
slot_addr = fp_addr + delta;
376+
}
377+
if (!stack_address_is_valid(slot_addr, stack_min, stack_max)) {
378+
return 0;
379+
}
380+
if (stack_max != 0) {
381+
if (slot_addr > UINTPTR_MAX - sizeof(uintptr_t)) {
382+
return 0;
383+
}
384+
if (slot_addr + sizeof(uintptr_t) > stack_max) {
385+
return 0;
386+
}
387+
}
388+
return 1;
389+
}
390+
391+
static int
392+
next_frame_pointer_is_valid(uintptr_t *frame_pointer, uintptr_t *next_fp,
393+
uintptr_t stack_min, uintptr_t stack_max)
394+
{
395+
uintptr_t fp_addr = (uintptr_t)frame_pointer;
396+
uintptr_t next_addr = (uintptr_t)next_fp;
397+
if (!stack_address_is_valid(next_addr, stack_min, stack_max)) {
398+
return 0;
399+
}
400+
if ((next_addr % sizeof(uintptr_t)) != 0) {
401+
return 0;
402+
}
403+
#if _Py_STACK_GROWS_DOWN
404+
return next_addr > fp_addr;
405+
#else
406+
return next_addr < fp_addr;
407+
#endif
408+
}
409+
410+
static uintptr_t *
411+
next_frame_pointer(uintptr_t *frame_pointer)
412+
{
413+
return (uintptr_t *)frame_pointer[FRAME_POINTER_NEXT_OFFSET];
414+
}
415+
416+
static uintptr_t
417+
frame_return_address(uintptr_t *frame_pointer)
418+
{
419+
#ifdef __s390x__
420+
Py_BUILD_ASSERT(S390X_FRAME_SAVED_GPR14_OFFSET % sizeof(uintptr_t) == 0);
421+
#endif
422+
return frame_pointer[FRAME_POINTER_RETURN_OFFSET];
423+
}
424+
328425
static PyObject *
329426
manual_unwind_from_fp(uintptr_t *frame_pointer)
330427
{
331428
Py_ssize_t max_depth = 200;
332-
int stack_grows_down = _Py_STACK_GROWS_DOWN;
429+
uintptr_t stack_min = 0;
430+
uintptr_t stack_max = 0;
333431

334432
if (frame_pointer == NULL) {
335433
return PyList_New(0);
336434
}
337435

436+
PyThreadState *tstate = _PyThreadState_GET();
437+
if (tstate != NULL) {
438+
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
439+
#if _Py_STACK_GROWS_DOWN
440+
stack_min = tstate_impl->c_stack_hard_limit;
441+
stack_max = tstate_impl->c_stack_top;
442+
#else
443+
stack_min = tstate_impl->c_stack_top;
444+
stack_max = tstate_impl->c_stack_hard_limit;
445+
#endif
446+
}
447+
338448
PyObject *result = PyList_New(0);
339449
if (result == NULL) {
340450
return NULL;
@@ -348,7 +458,21 @@ manual_unwind_from_fp(uintptr_t *frame_pointer)
348458
if ((fp_addr % sizeof(uintptr_t)) != 0) {
349459
break;
350460
}
351-
uintptr_t return_addr = frame_pointer[1];
461+
if (!stack_address_is_valid(fp_addr, stack_min, stack_max)) {
462+
break;
463+
}
464+
if (!frame_pointer_slot_is_valid(frame_pointer,
465+
FRAME_POINTER_NEXT_OFFSET,
466+
stack_min, stack_max)) {
467+
break;
468+
}
469+
if (!frame_pointer_slot_is_valid(frame_pointer,
470+
FRAME_POINTER_RETURN_OFFSET,
471+
stack_min, stack_max)) {
472+
break;
473+
}
474+
uintptr_t *next_fp = next_frame_pointer(frame_pointer);
475+
uintptr_t return_addr = frame_return_address(frame_pointer);
352476

353477
PyObject *addr_obj = PyLong_FromUnsignedLongLong(return_addr);
354478
if (addr_obj == NULL) {
@@ -362,22 +486,10 @@ manual_unwind_from_fp(uintptr_t *frame_pointer)
362486
}
363487
Py_DECREF(addr_obj);
364488

365-
uintptr_t *next_fp = (uintptr_t *)frame_pointer[0];
366-
// Stop if the frame pointer is extremely low.
367-
if ((uintptr_t)next_fp < min_frame_pointer_addr) {
489+
if (!next_frame_pointer_is_valid(frame_pointer, next_fp,
490+
stack_min, stack_max)) {
368491
break;
369492
}
370-
uintptr_t next_addr = (uintptr_t)next_fp;
371-
if (stack_grows_down) {
372-
if (next_addr <= fp_addr) {
373-
break;
374-
}
375-
}
376-
else {
377-
if (next_addr >= fp_addr) {
378-
break;
379-
}
380-
}
381493
frame_pointer = next_fp;
382494
}
383495

configure

Lines changed: 100 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

configure.ac

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2548,6 +2548,16 @@ AS_VAR_IF([ac_cv_gcc_compat], [yes], [
25482548
AX_CHECK_COMPILE_FLAG([-mno-omit-leaf-frame-pointer], [
25492549
frame_pointer_cflags="$frame_pointer_cflags -mno-omit-leaf-frame-pointer"
25502550
], [], [-Werror])
2551+
AS_CASE([$host_cpu], [arm|armv*], [
2552+
AX_CHECK_COMPILE_FLAG([-marm], [
2553+
frame_pointer_cflags="$frame_pointer_cflags -marm"
2554+
], [], [-Werror])
2555+
])
2556+
AS_CASE([$host_cpu], [s390*], [
2557+
AX_CHECK_COMPILE_FLAG([-mbackchain], [
2558+
frame_pointer_cflags="$frame_pointer_cflags -mbackchain"
2559+
], [], [-Werror])
2560+
])
25512561
], [], [-Werror])
25522562
if test -n "$frame_pointer_cflags" && test "x$with_frame_pointers" != xno; then
25532563
BASECFLAGS="$frame_pointer_cflags $BASECFLAGS"

0 commit comments

Comments
 (0)