Visitar URL original
Add bytecode_offset=-1 self-enforcing invariant + regression test · SonicField/cpython@13f3720 · GitHub
Skip to content

Commit 13f3720

Browse files
committed
Add bytecode_offset=-1 self-enforcing invariant + regression test
Move bytecode_offset=-1 initialization into hir_c_alloc_instr so the invariant is structurally enforced — even if hir_c_init_instr is accidentally skipped, bytecode_offset will be -1 (not 0 from calloc). Add runtime regression test in hir_instr_c_verify.cpp that verifies the invariant for all three allocation paths (raw alloc, init_instr, init_deopt). Runs at startup via __attribute__((constructor)). Addresses gap flagged 5 times (Pythia python#14, python#40, python#59, librarian x2).
1 parent 3a1987e commit 13f3720

2 files changed

Lines changed: 42 additions & 0 deletions

File tree

‎Python/jit/hir/hir_instr_c.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,10 @@ static inline void *hir_c_alloc_instr(size_t struct_size, size_t num_operands) {
624624
void **node = (void **)ptr;
625625
node[0] = ptr; /* prev_ = self */
626626
node[1] = ptr; /* next_ = self */
627+
/* bytecode_offset defaults to -1 in C++ (BCOffset{-1}), but calloc
628+
* gives 0. Set here so the invariant is self-enforcing — even if
629+
* hir_c_init_instr is accidentally skipped. */
630+
((HirInstrLayout *)ptr)->bytecode_offset = -1;
627631
return ptr;
628632
}
629633

‎Python/jit/hir/hir_instr_c_verify.cpp‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,46 @@ static void verify_is_replayable_table() {
283283
assert(hir_instr_info_is_replayable(static_cast<int>(Opcode::kVectorCall)) == 0);
284284
}
285285

286+
/* Verify bytecode_offset=-1 invariant for C-allocated instructions.
287+
* calloc gives 0 for bytecode_offset; hir_c_alloc_instr and hir_c_init_instr
288+
* must both set it to -1 (matching C++ Instr::Instr default: BCOffset{-1}).
289+
* Regression test for the bug that caused wrong deopt behavior when
290+
* _cinderx module was loaded before compilation. */
291+
static void verify_bytecode_offset_invariant() {
292+
/* Test 1: hir_c_alloc_instr alone sets bytecode_offset=-1 */
293+
void *raw = hir_c_alloc_instr(sizeof(HirInstrLayout), 0);
294+
assert(raw != NULL);
295+
assert(hir_c_bytecode_offset(raw) == -1 &&
296+
"hir_c_alloc_instr must set bytecode_offset to -1");
297+
free((char *)raw - sizeof(size_t)); /* free from preamble start */
298+
299+
/* Test 2: hir_c_init_instr preserves bytecode_offset=-1 */
300+
void *instr = hir_c_alloc_instr(sizeof(HirInstrLayout), 1);
301+
assert(instr != NULL);
302+
hir_c_init_instr(instr, HIR_OP_Assign);
303+
assert(hir_c_bytecode_offset(instr) == -1 &&
304+
"hir_c_init_instr must set bytecode_offset to -1");
305+
free((char *)instr - 1 * sizeof(void *) - sizeof(size_t));
306+
307+
/* Test 3: hir_c_init_deopt preserves bytecode_offset=-1 */
308+
void *deopt = hir_c_alloc_instr(sizeof(HirDeoptLayout), 2);
309+
assert(deopt != NULL);
310+
hir_c_init_deopt(deopt, HIR_OP_BinaryOp);
311+
assert(hir_c_bytecode_offset(deopt) == -1 &&
312+
"hir_c_init_deopt must set bytecode_offset to -1");
313+
/* Clean up placement-new'd C++ containers before freeing */
314+
{
315+
HirDeoptLayout *d = (HirDeoptLayout *)deopt;
316+
using RegStateVec = std::vector<jit::hir::RegState>;
317+
reinterpret_cast<RegStateVec *>(&d->live_regs_storage)->~RegStateVec();
318+
reinterpret_cast<std::string *>(&d->descr_storage)->~basic_string();
319+
}
320+
free((char *)deopt - 2 * sizeof(void *) - sizeof(size_t));
321+
}
322+
286323
__attribute__((constructor))
287324
static void hir_instr_runtime_check() {
288325
verify_hir_instr_read_through_cast();
289326
verify_is_replayable_table();
327+
verify_bytecode_offset_invariant();
290328
}

0 commit comments

Comments
 (0)