Skip to content

Commit e44170e

Browse files
committed
Address Pablo's feedback
1 parent ac018d6 commit e44170e

File tree

10 files changed

+79
-29
lines changed

10 files changed

+79
-29
lines changed

Include/internal/pycore_ceval.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ typedef struct {
9494
void* (*init_state)(void);
9595
// Callback to register every trampoline being created
9696
void (*write_state)(void* state, const void *code_addr,
97-
unsigned int code_size, PyCodeObject* code);
97+
size_t code_size, PyCodeObject* code);
9898
// Callback to free the trampoline state
9999
int (*free_state)(void* state);
100100
} _PyPerf_Callbacks;
@@ -109,7 +109,7 @@ extern PyStatus _PyPerfTrampoline_AfterFork_Child(void);
109109
extern _PyPerf_Callbacks _Py_perfmap_callbacks;
110110
extern _PyPerf_Callbacks _Py_perfmap_jit_callbacks;
111111
extern void _PyPerfJit_WriteNamedCode(const void *code_addr,
112-
unsigned int code_size,
112+
size_t code_size,
113113
const char *entry,
114114
const char *filename);
115115
#endif

Include/internal/pycore_interp_structs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ struct code_arena_st;
6969
struct trampoline_api_st {
7070
void* (*init_state)(void);
7171
void (*write_state)(void* state, const void *code_addr,
72-
unsigned int code_size, PyCodeObject* code);
72+
size_t code_size, PyCodeObject* code);
7373
int (*free_state)(void* state);
7474
void *state;
7575
Py_ssize_t code_padding;

Include/internal/pycore_jit_unwind.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ size_t _PyJitUnwind_BuildEhFrame(uint8_t *buffer, size_t buffer_size,
1919
int absolute_addr);
2020

2121
void _PyJitUnwind_GdbRegisterCode(const void *code_addr,
22-
unsigned int code_size,
22+
size_t code_size,
2323
const char *entry,
2424
const char *filename);
2525

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,27 @@
11
# Sample script for use by test_gdb.test_jit
22

3+
import _testinternalcapi
34
import operator
4-
import sys
5+
6+
7+
WARMUP_ITERATIONS = _testinternalcapi.TIER2_THRESHOLD + 10
58

69

710
def jit_bt_hot(depth, warming_up_caller=False):
8-
if warming_up_caller:
9-
return
1011
if depth == 0:
11-
id(42)
12+
if not warming_up_caller:
13+
id(42)
1214
return
1315

14-
warming_up = True
15-
while warming_up:
16-
warming_up = sys._jit.is_enabled() & (not sys._jit.is_active())
17-
operator.call(jit_bt_hot, depth - 1, warming_up)
16+
for iteration in range(WARMUP_ITERATIONS):
17+
operator.call(
18+
jit_bt_hot,
19+
depth - 1,
20+
warming_up_caller or iteration + 1 != WARMUP_ITERATIONS,
21+
)
1822

1923

20-
jit_bt_hot(10)
24+
# Warm the shared shim once without hitting builtin_id so the real run uses
25+
# the steady-state shim path when GDB breaks inside id(42).
26+
jit_bt_hot(1, warming_up_caller=True)
27+
jit_bt_hot(1)

Lib/test/test_gdb/test_jit.py

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,30 @@
77

88

99
JIT_SAMPLE_SCRIPT = os.path.join(os.path.dirname(__file__), "gdb_jit_sample.py")
10+
# In batch GDB, break in builtin_id() while it is running under JIT,
11+
# then repeatedly "finish" until the selected frame is the executor.
12+
# That gives a deterministic backtrace starting with py::jit_executor:<jit>.
13+
#
14+
# builtin_id() sits only a few helper frames above the executor on this path.
15+
# This bound is just a generous upper limit so the test fails clearly if the
16+
# expected stack shape changes.
17+
MAX_FINISH_STEPS = 20
18+
# After landing on the executor frame, single-step a little further into the
19+
# blob so the backtrace is taken from executor code itself rather than the
20+
# immediate helper-return site.
21+
EXECUTOR_SINGLE_STEPS = 2
22+
23+
FINISH_TO_JIT_EXECUTOR = (
24+
"python exec(\"import gdb\\n"
25+
"target = 'py::jit_executor:<jit>'\\n"
26+
f"for _ in range({MAX_FINISH_STEPS}):\\n"
27+
" frame = gdb.selected_frame()\\n"
28+
" if frame is not None and frame.name() == target:\\n"
29+
" break\\n"
30+
" gdb.execute('finish')\\n"
31+
"else:\\n"
32+
" raise RuntimeError('did not reach %s' % target)\\n\")"
33+
)
1034

1135

1236
def setUpModule():
@@ -24,12 +48,30 @@ def test_bt_unwinds_through_jit_frames(self):
2448
cmds_after_breakpoint=["bt"],
2549
PYTHON_JIT="1",
2650
)
27-
self.assertIn("py::jit_executor:<jit>", gdb_output)
28-
self.assertIn("py::jit_shim:<jit>", gdb_output)
2951
self.assertRegex(
3052
gdb_output,
3153
re.compile(
32-
r"py::jit_executor:<jit>.*(_PyEval_EvalFrameDefault|_PyEval_Vector)",
54+
r"py::jit_executor:<jit>.*py::jit_shim:<jit>.*"
55+
r"(_PyEval_EvalFrameDefault|_PyEval_Vector)",
56+
re.DOTALL,
57+
),
58+
)
59+
60+
def test_bt_unwinds_from_inside_jit_executor(self):
61+
gdb_output = self.get_stack_trace(
62+
script=JIT_SAMPLE_SCRIPT,
63+
cmds_after_breakpoint=[
64+
FINISH_TO_JIT_EXECUTOR,
65+
*(["si"] * EXECUTOR_SINGLE_STEPS),
66+
"bt",
67+
],
68+
PYTHON_JIT="1",
69+
)
70+
self.assertRegex(
71+
gdb_output,
72+
re.compile(
73+
r"#0\s+py::jit_executor:<jit>.*#1\s+py::jit_shim:<jit>.*"
74+
r"(_PyEval_EvalFrameDefault|_PyEval_Vector)",
3375
re.DOTALL,
3476
),
3577
)

Modules/_testinternalcapi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1210,7 +1210,7 @@ write_perf_map_entry(PyObject *self, PyObject *args)
12101210
{
12111211
PyObject *code_addr_v;
12121212
const void *code_addr;
1213-
unsigned int code_size;
1213+
size_t code_size;
12141214
const char *entry_name;
12151215

12161216
if (!PyArg_ParseTuple(args, "OIs", &code_addr_v, &code_size, &entry_name))

Python/jit.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,11 @@ jit_record_code(const void *code_addr, size_t code_size,
7070
_PyPerfTrampoline_GetCallbacks(&callbacks);
7171
if (callbacks.write_state == _Py_perfmap_jit_callbacks.write_state) {
7272
_PyPerfJit_WriteNamedCode(
73-
code_addr, (unsigned int)code_size, entry, filename);
73+
code_addr, code_size, entry, filename);
7474
return;
7575
}
7676
_PyJitUnwind_GdbRegisterCode(
77-
code_addr, (unsigned int)code_size, entry, filename);
77+
code_addr, code_size, entry, filename);
7878
#else
7979
(void)code_addr;
8080
(void)code_size;

Python/jit_unwind.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -655,11 +655,12 @@ struct jit_descriptor {
655655
struct jit_code_entry *first_entry;
656656
};
657657

658-
static volatile struct jit_descriptor __jit_debug_descriptor = {
658+
Py_EXPORTED_SYMBOL volatile struct jit_descriptor __jit_debug_descriptor = {
659659
1, JIT_NOACTION, NULL, NULL
660660
};
661661

662-
static void __attribute__((noinline)) __jit_debug_register_code(void)
662+
Py_EXPORTED_SYMBOL void __attribute__((noinline))
663+
__jit_debug_register_code(void)
663664
{
664665
/* Keep this call visible to debuggers and not optimized away. */
665666
(void)__jit_debug_descriptor.action_flag;
@@ -684,7 +685,7 @@ gdb_jit_machine_id(void)
684685
static void
685686
gdb_jit_register_code(
686687
const void *code_addr,
687-
unsigned int code_size,
688+
size_t code_size,
688689
const char *symname,
689690
const uint8_t *eh_frame,
690691
size_t eh_frame_size
@@ -858,9 +859,9 @@ gdb_jit_register_code(
858859

859860
void
860861
_PyJitUnwind_GdbRegisterCode(const void *code_addr,
861-
unsigned int code_size,
862-
const char *entry,
863-
const char *filename)
862+
size_t code_size,
863+
const char *entry,
864+
const char *filename)
864865
{
865866
#if defined(__linux__) && defined(__ELF__)
866867
/* GDB expects a stable symbol name and absolute addresses in .eh_frame. */

Python/perf_jit_trampoline.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ static void* perf_map_jit_init(void) {
531531
static void perf_map_jit_write_entry_with_name(
532532
void *state,
533533
const void *code_addr,
534-
unsigned int code_size,
534+
size_t code_size,
535535
const char *entry,
536536
const char *filename
537537
)
@@ -724,7 +724,7 @@ static void perf_map_jit_write_entry_with_name(
724724
* and must not be changed without coordinating with core Python development.
725725
*/
726726
static void perf_map_jit_write_entry(void *state, const void *code_addr,
727-
unsigned int code_size, PyCodeObject *co)
727+
size_t code_size, PyCodeObject *co)
728728
{
729729
const char *entry = "";
730730
const char *filename = "";
@@ -741,7 +741,7 @@ static void perf_map_jit_write_entry(void *state, const void *code_addr,
741741
}
742742

743743
void
744-
_PyPerfJit_WriteNamedCode(const void *code_addr, unsigned int code_size,
744+
_PyPerfJit_WriteNamedCode(const void *code_addr, size_t code_size,
745745
const char *entry, const char *filename)
746746
{
747747
perf_map_jit_write_entry_with_name(

Python/perf_trampoline.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ perf_trampoline_code_watcher(PyCodeEvent event, PyCodeObject *co)
243243

244244
static void
245245
perf_map_write_entry(void *state, const void *code_addr,
246-
unsigned int code_size, PyCodeObject *co)
246+
size_t code_size, PyCodeObject *co)
247247
{
248248
const char *entry = "";
249249
if (co->co_qualname != NULL) {

0 commit comments

Comments
 (0)