bpo-29438: fixed use-after-free in key sharing dict by methane · Pull Request #17 · python/cpython
SonicField added a commit to SonicField/cpython that referenced this pull request
Push 47 (88/144 → 89/144 = 61.8%): bundled two-section commit per supervisor 04:48:57Z + theologian 04:48:55Z BUNDLE APPROVED. Section 1 closes a gatekeeper item python#17 violation from push 46; Section 2 ports emitResume to C. ================================================================ SECTION 1 — FOLLOW-UP FIX (item python#17): JIT_CHECK_C in emitTypeAnnotationGuards ================================================================ Push 46 (5f79084) ported emitTypeAnnotationGuards but the C body used a silent-skip workaround for the NULL-arg case: if (arg == NULL) continue; // BAD — silent skip, no diagnostic This violates the no-workarounds policy (feedback_no_workarounds.md): always root-cause, never bail-out/deopt. Supervisor 04:35:30Z + theologian 04:36:18Z + gatekeeper item python#17 (post-push-46 formalization at 04:37:14Z) mandate ALWAYS-ON form (NOT JIT_DCHECK_C, NOT silent-skip): JIT_CHECK_C(arg != NULL, "No register for argument %d", arg_idx); C++ baseline used JIT_CHECK with the same message format (builder.cpp:1751 pre-deletion). Phoenix C parity restored. Match-strings (gatekeeper item python#17 grep targets): - 'JIT_CHECK_C(arg != NULL' ← present - 'No register for argument' ← present Header dependency: added '#include "cinderx/Common/jit_log_c.h"' to builder_emit_c.c for the JIT_CHECK_C macro. No transitive include path existed. ================================================================ SECTION 2 — EMIT METHOD CONVERSION: emitResume → C (88/144 → 89/144) ================================================================ Bridge: hir_builder_emit_resume_c Purpose: RESUME opcode handler — inserts an eval-breaker check + periodic tasks block when oparg < 2. Oparg 2/3 are no-op resumes (coroutine sends/throws don't need periodic-task points). C++ source: builder.cpp:3175-3186 (replaced with 8-line delegating stub) PHASE 0 AUDIT: Type A (headers — all existing at HEAD): - PhxTranslationContext (builder_emit_c.c:22, local typedef) - HirFrameStateLayout (phx_frame_state.h) - Function/CFG/BasicBlock/FrameState (hir.h, used via void* cast in bridge) Type B (symbols — all existing at HEAD): - hir_cfg_alloc_block(void *func) — hir_c_api.h:746 ✓ EXISTS - hir_c_create_snapshot(void *frame_state) — copies FrameState (hir_c_api.cpp:926-934) ✓ EXISTS - phx_tc_emit(tc, instr) — local helper (builder_emit_c.c:39) ✓ EXISTS - HIRBuilder::insertRunPeriodicActivites — private member (builder.h:504-508). NEW friend bridge added: hir_builder_insert_run_periodic_activities_c mirrors hir_builder_get_block_at_off pattern (builder.h:18 fwd-decl + builder.h:105 friend ref). Phase 0.5 IFDEF AUDIT: - insertRunPeriodicActivites contains '#ifdef Py_GIL_DISABLED' block (line 5602-5604 emitAtQuiescentState). This stays in C++ helper — bridge inherits behavior automatically. NO ifdef in new C body. Phase 0 LESSON LEARNED (added during compile-check iteration): Initial attempt missed the file-scope FORWARD DECLARATION prerequisite for friend bridges. Audit must check BOTH: (a) friend-decl site (inside class) (b) file-scope forward-decl site (extern "C" block before class) Will codify in BRIDGE SPEC TEMPLATE meta-rules. PRIOR DECISIONS: - Bundle approved: supervisor 04:48:57Z + theologian 04:48:55Z (47.1 surgical 3-line fix; split would require unsafe stash-dance). - emitResume choice over emitKwNames: theologian's table LOW + LOW vs LOW + LOW-MED — pure velocity contribution per supervisor 04:14:48Z REBALANCE directive (no G-item closure value, that's fine). - Snapshot-on-succ-block trick (avoids temp PhxTranslationContext + frame copy/destroy): theologian 04:45:32Z verified equivalence using hir_c_create_snapshot copy semantics at hir_c_api.cpp:926-934 (snapshot captures frame value at call, subsequent mutations don't affect the snapshot — equivalent to C++ succ.frame semantics). INVARIANTS PRESERVED: 1. Early-return on oparg >= 2 (no Resume action — matches C++ guard). 2. Snapshot emitted into succ_block, NOT check_block (matches C++ succ.emitSnapshot semantic). Trick: capture original tc->block, switch tc->block = succ_block, emit snapshot, then restore via end-state assignment (final tc->block == succ_block). 3. insertRunPeriodicActivites runs full helper chain (eval-breaker check + body w/ RunPeriodicTasks + branches) — preserved via extern "C" delegation through hir_builder_insert_run_periodic_activities_c. 4. Py_GIL_DISABLED behavior preserved (ifdef stays in C++ helper). 5. Final tc->block == succ_block (caller continues on new block — matches C++ tc.block = succ.block). DESIGN EQUIVALENCE PROOF (snapshot-on-succ-block): C++: TranslationContext succ(cfg.AllocateBlock(), tc.frame); succ.emitSnapshot(); insertRunPeriodicActivites(cfg, tc.block, succ.block, tc.frame); tc.block = succ.block; C: void *check_block = tc->block; void *succ_block = hir_cfg_alloc_block(func); tc->block = succ_block; phx_tc_emit(tc, hir_c_create_snapshot(&tc->frame)); hir_builder_insert_run_periodic_activities_c( builder, func, check_block, succ_block, &tc->frame); EQUIVALENT because: (a) C++ succ.frame is a COPY of tc.frame at construction; emitSnapshot doesn't mutate frame; therefore succ.frame == tc.frame at point of insertRunPeriodicActivites call. (b) hir_c_create_snapshot makes a COPY of the FrameState (hir_c_api.cpp:926-934) — snapshot captures the value at the call, not a reference. Subsequent mutations to tc->frame do not affect the snapshot. (c) Snapshot lands in tc->block at time of phx_tc_emit, which we set to succ_block — same destination as C++ succ.emitSnapshot. (d) Final tc->block == succ_block (matches C++ tc.block = succ.block). Falsifier: any function with RESUME opcode (most functions, since RESUME is emitted at function entry by Python 3.12 compiler). JIT-compile, verify HIR shows 2-block sequence around RESUME(0|1): check_block ends with CondBranch on EvalBreaker, succ_block starts with Snapshot. If single-block or wrong successor, port has stripped invariants. ================================================================ DIFF ================================================================ Python/jit/hir/builder.cpp (-11 / +20): - emitResume body (10 lines) → 8-line delegating stub calling extern "C" hir_builder_emit_resume_c. - +13 lines: extern "C" hir_builder_insert_run_periodic_activities_c bridge wrapper (5-line static_cast chain calling private member). Python/jit/hir/builder.h (+5): - +3 lines: file-scope forward declaration of hir_builder_insert_run_periodic_activities_c inside existing extern "C" block (matches hir_builder_get_block_at_off pattern, line 18). - +2 lines: friend declaration inside class HIRBuilder (line 106-107). Python/jit/hir/builder_emit_c.c (+39 / -5): - +1 #include "cinderx/Common/jit_log_c.h" (JIT_CHECK_C macro for §1) - §1 fix: silent-skip replaced with JIT_CHECK_C(arg != NULL, ...) - +35 lines: hir_builder_emit_resume_c implementation + extern decls. Net diff stat: 3 files, +64/-12 (net +52 LOC). ================================================================ VERIFICATION (compile-clean pre-commit) ================================================================ cmake --build phoenix_jit: PASS, 0 errors (only 4 pre-existing warnings). Verified by testkeeper compile-check at 05:02:51Z (after one fix iteration for missing file-scope forward-decl). Build succeeded with friend declaration resolving correctly to global namespace function. Push 47 batch is 1 commit per BUNDLE APPROVED at 04:48:57Z. ABBA_pre baseline 1.07x captured at HEAD 5f79084 (testkeeper 04:59:13Z) for cap-20 ABBA_post comparison. Process discipline applied: - explicit `git add Python/jit/hir/builder.cpp Python/jit/hir/builder.h Python/jit/hir/builder_emit_c.c` (lesson from 6450421c93 over-broad commit; lesson from 04:27:32Z directive-compliance feedback) - `git diff --cached --stat` verification: only 3 files staged - `git status --short` confirmed file separation pre-staging: M Python/jit/hir/builder.cpp (mine, staged) M Python/jit/hir/builder.h (mine, staged) M Python/jit/hir/builder_emit_c.c (mine, staged) M docs/wiring_catches.md (testkeeper W13a, UNSTAGED) M scripts/gate_phoenix.sh (testkeeper W13a, UNSTAGED)