GH-126910: avoid reading the FP for getting the SP

[diegorusso]

This is needed for #146071

• Issue: Supporting stack unwinding in the JIT compiler #126910

[Fidget-Spinner]

To anyone reading this:

The TLDR it seems is that __builtin_frame_address forces the frame pointer to be materialized per-function as it requires reading the FP! This is obviously bad, as that means we get the FP prologue on every function that has this, which corrupts the real FP in the JIT code (the one from the shim).

Diego and I spent 1 hour pairing to find out this problem only by analyzing the generated asm. I am going to write a lightweight assembly verifier to make sure we don't regress on this and guarantee that don't corrupt the FP on the JIT anymore. This also ensures unwinders using FP will always properly work with the JIT.

[brandtbucher]

@Fidget-Spinner, would marking the function that calls the intrinsic as “noinline” work? IIUC the issue is that we’re inlining the intrinsic into JIT code.

[Fidget-Spinner]

@Fidget-Spinner, would marking the function that calls the intrinsic as “noinline” work? IIUC the issue is that we’re inlining the intrinsic into JIT code.

I'm inclined to not do that, as the JIT stencils will need to point to another extern call which will slow down this supposedly cheap check.

[Fidget-Spinner]

@brandtbucher it seems you're right and we need to PyAPI_DATA it as Brandt suggested. Some of the OS requires more precise addressees it seems :(.

@diegorusso This is the patch I have

diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h
index 189a8dde9f0..a66543cf1eb 100644
--- a/Include/internal/pycore_pystate.h
+++ b/Include/internal/pycore_pystate.h
@@ -312,18 +312,7 @@ static uintptr_t return_pointer_as_int(char* p) {
 }
 #endif
 
-static inline uintptr_t
-_Py_get_machine_stack_pointer(void) {
-#if _Py__has_builtin(__builtin_frame_address) || defined(__GNUC__)
-    return (uintptr_t)__builtin_frame_address(0);
-#elif defined(_MSC_VER)
-    return (uintptr_t)_AddressOfReturnAddress();
-#else
-    char here;
-    /* Avoid compiler warning about returning stack address */
-    return return_pointer_as_int(&here);
-#endif
-}
+PyAPI_DATA(uintptr_t) _Py_get_machine_stack_pointer(void);
 
 static inline intptr_t
 _Py_RecursionLimit_GetMargin(PyThreadState *tstate)
diff --git a/Python/pystate.c b/Python/pystate.c
index 143175da0f4..f974c82c391 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -3286,3 +3286,16 @@ _Py_GetMainConfig(void)
     }
     return _PyInterpreterState_GetConfig(interp);
 }
+
+uintptr_t
+_Py_get_machine_stack_pointer(void) {
+#if _Py__has_builtin(__builtin_frame_address) || defined(__GNUC__)
+    return (uintptr_t)__builtin_frame_address(0);
+#elif defined(_MSC_VER)
+    return (uintptr_t)_AddressOfReturnAddress();
+#else
+    char here;
+    /* Avoid compiler warning about returning stack address */
+    return return_pointer_as_int(&here);
+#endif
+}

[Fidget-Spinner]

Should be fine now.

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 CentOS9 NoGIL 3.x (tier-1) has failed when building commit b60b926.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1609/builds/5245) and take a look at the build logs.
4. Check if the failure is related to this commit (b60b926) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1609/builds/5245

Failed tests:

• test_perf_profiler

Failed subtests:

• test_python_calls_appear_in_the_stack_if_perf_activated - test.test_perf_profiler.TestPerfProfiler.test_python_calls_appear_in_the_stack_if_perf_activated

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/Lib/test/test_perf_profiler.py", line 451, in test_python_calls_appear_in_the_stack_if_perf_activated
    self.assertIn(f"py::foo:{script}", stdout)
    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'py::foo:/tmp/test_python_cirfvaet/tmplryg8m41/perftest.py' not found in 'python  801457 3112297.807703:          1 cycles:Pu: \n\t    7f8c92f53b60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.807724:          1 cycles:Pu: \n\t    7f8c92f53b60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.807741:          3 cycles:Pu: \n\t    7f8c92f53b60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.807758:          9 cycles:Pu: \n\t    7f8c92f53b60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.807774:         25 cycles:Pu: \n\t    7f8c92f53b60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.807792:         69 cycles:Pu: \n\tffffffffba400b80 [unknown] ([unknown])\n\t    7f8c92f53b60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.807814:        185 cycles:Pu: \n\t    7f8c92f53b63 _start+0x3 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.808264:        560 cycles:Pu: \n\t          421026 .plt+0x6 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          74698b _Py_PreInitializeFromPyArgv+0xfe (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          785a35 pymain_init+0x5f (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          787775 pymain_main+0x14 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          7877fb Py_BytesMain+0x2b (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          42294f main+0x9 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t    7f8c92c2a610 __libc_start_call_main+0x80 (/usr/lib64/libc.so.6)\n\npython  801457 3112297.808281:        528 cycles:Pu: \n\t    7f8c92f46e1f _dl_fixup+0x7f (/usr/lib64/ld-linux-x86-64.so.2)\n\t    7f8c92f6eac8 [unknown] ([unknown])\n\npython  801457 3112297.808297:       1423 cycles:Pu: \n\t    7f8c92f3f6e7 check_match+0x7 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.808313:       3949 cycles:Pu: \n\t    7f8c92f3fb30 do_lookup_x+0x2a0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.808331:      11026 cycles:Pu: \n\tffffffffba400b80 [unknown] ([unknown])\n\t    7f8c92f3fb30 do_lookup_x+0x2a0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  801457 3112297.808352:      29787 cycles:Pu: \n\t          576e3c mi_segment_reclaim_or_alloc+0x0 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          577445 _mi_segment_page_alloc+0xcd (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5775a7 mi_page_fresh_alloc+0x6f (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          577c66 mi_page_fresh+0x36 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          577fcd mi_page_queue_find_free_ex+0x171 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t       
/build/python)\n\t          74698b _Py_PreInitializeFromPyArgv+0xfe (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          785a35 pymain_init+0x5f (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          787775 pymain_main+0x14 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          7877fb Py_BytesMain+0x2b (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          42294f main+0x9 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t    7f8c92c2a610 __libc_start_call_main+0x80 (/usr/lib64/libc.so.6)\n\npython  801457 3112297.808392:      69321 cycles:Pu: \n\t    7f8c92f46e1f _dl_fixup+0x7f (/usr/lib64/ld-linux-x86-64.so.2)\n\t    7f8c92f6eac8 [unknown] ([unknown])\n\npython  801457 3112297.809530:     101595 cycles:Pu: \n\t          567a6b _mi_segment_page_of+0xdc (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          56f087 mi_usable_size+0x62 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          579e91 mi_heap_malloc_small_zero+0xd5 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          57a762 _PyMem_MiMalloc+0x38 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          56839d _PyMem_DebugRawAlloc+0xba (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5683de _PyMem_DebugRawMalloc+0x14 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5683f7 _PyMem_DebugMalloc+0x17 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          57f02c PyMem_Malloc+0x19 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b6877 mro_implementation_unlocked+0x48b (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b6ab0 mro_invoke+0xd8 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b6c4d mro_internal+0x6e (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b785e type_ready_mro+0xa8 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b7cbd type_ready+0xc6 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b8257 init_static_type+0x1cf (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b8365 _PyStaticType_InitBuiltin+0x2b (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          55c96e _PyTypes_InitTypes+0x90 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          739582 pycore_init_types+0x1a (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          73b241 pycore_interp_init+0xfb (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          73b440 pyinit_config+0x92 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          746d01 pyinit_core+0x111 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          746dea Py_InitializeFromConfig+0x96 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          785b15 pymain_init+0x13f (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          787775 pymain_main+0x14 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          7877fb Py_BytesMain+0x2b (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          42294f main+0x9 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t    7f8c92c2a610 __libc_start_call_main+0x80 (/usr/lib64/libc.so.6)\n\npython  801457 3112297.809588:      91687 cycles:Pu: \n\t          5a4205 add_operators+0xce (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5a4837 type_ready_fill_dict+0x11 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b7ce0 type_ready+0xe9 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5b


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/Lib/test/test_perf_profiler.py", line 451, in test_python_calls_appear_in_the_stack_if_perf_activated
    self.assertIn(f"py::foo:{script}", stdout)
    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'py::foo:/tmp/test_python_2b0nd734/tmp_011efys/perftest.py' not found in 'python  762391 3112004.511961:          1 cycles:Pu: \n\t    7f4ad055cb60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.511984:          1 cycles:Pu: \n\t    7f4ad055cb60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512001:          3 cycles:Pu: \n\t    7f4ad055cb60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512018:          9 cycles:Pu: \n\t    7f4ad055cb60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512034:         25 cycles:Pu: \n\t    7f4ad055cb60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512052:         69 cycles:Pu: \n\tffffffffba400b80 [unknown] ([unknown])\n\t    7f4ad055cb60 _start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512074:        185 cycles:Pu: \n\t    7f4ad055cb63 _start+0x3 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512113:        623 cycles:Pu: \n\t    7f4ad055d880 _dl_start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512130:       1050 cycles:Pu: \n\tffffffffba400b80 [unknown] ([unknown])\n\t    7f4ad055d880 _dl_start+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512152:       2838 cycles:Pu: \n\t    7f4ad055dc50 _dl_start+0x3d0 (/usr/lib64/ld-linux-x86-64.so.2)\n\t    7f4ad055cb68 _dl_start_user+0x0 (/usr/lib64/ld-linux-x86-64.so.2)\n\npython  762391 3112004.512171:       6514 cycl
buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          57a9c2 _PyMem_MiRawMalloc+0x2f (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          56839d _PyMem_DebugRawAlloc+0xba (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5683de _PyMem_DebugRawMalloc+0x14 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          57e690 PyMem_RawMalloc+0x19 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          7055f1 _Py_hashtable_set+0x28 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5e4b66 intern_static+0x9c (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          60b666 _PyUnicode_InternStatic+0x21 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          60b779 _PyUnicode_InitStaticStrings+0xd4 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          622b1a init_global_interned_strings+0x60 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          622d19 _PyUnicode_InitGlobalObjects+0x68 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          73950b pycore_init_global_objects+0x1a (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          73b17c pycore_interp_init+0x36 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          73b440 pyinit_config+0x92 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          746d01 pyinit_core+0x111 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          746dea Py_InitializeFromConfig+0x96 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          785b15 pymain_init+0x13f (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          787775 pymain_main+0x14 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          7877fb Py_BytesMain+0x2b (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          42294f main+0x9 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t    7f4ad022a610 __libc_start_call_main+0x80 (/usr/lib64/libc.so.6)\n\npython  762391 3112004.513429:     107877 cycles:Pu: \n\tffffffffba400b80 [unknown] ([unknown])\n\t          5d4aef unicode_hash+0xa9 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5d4b4c hashtable_unicode_hash+0x9 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          705285 _Py_hashtable_get_entry_generic+0x18 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          705703 _Py_hashtable_get+0x7 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          5e4b4b intern_static+0x81 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          60b666 _PyUnicode_InternStatic+0x21 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          60ce96 _PyUnicode_InitStaticStrings+0x17f1 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          622b1a init_global_interned_strings+0x60 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          622d19 _PyUnicode_InitGlobalObjects+0x68 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          73950b pycore_init_global_objects+0x1a (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          73b17c pycore_interp_init+0x36 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          73b440 pyinit_config+0x92 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          746d01 pyinit_core+0x111 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          746dea Py_InitializeFromConfig+0x96 (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/build/python)\n\t          785b15 pymain_init+0x13f (/home/buildbot/buildarea/3.x.itamaro-centos-aws.nogil/