Uh oh!
There was an error while loading. Please reload this page.
bpo-29438: fixed use-after-free in key sharing dict - #17
Merged
Conversation
methane
commented
Feb 11, 2017
MemberAuthor
Patch for Python 3.5 is slightly differ from this PR. |
methaneforce-pushed
the
bpo29438/fix-dict-useafterfree
branch
from
February 11, 2017 03:22
c38d9fe to
2dd62f6CompareMember
There was a problem hiding this comment.
Just as I said this is somewhat misleading and it's better to make it clear.
methaneforce-pushed
the
bpo29438/fix-dict-useafterfree
branch
from
February 11, 2017 05:48
2dd62f6 to
bf8bd7eCompareCodecov Report@@ Coverage Diff @@## master #17 +/- ##
==========================================
+ Coverage 82.37% 82.37% +<.01%
==========================================
Files 1427 1427 Lines 350948 350948 ==========================================
+ Hits 289088 289095 +7 + Misses 61860 61853 -7Continue to review full report at Codecov.
|
zhangyangyu
approved these changes
Feb 11, 2017
methane referenced
this pull request
in methane/cpython
Feb 12, 2017
paulmon added a commit
to paulmon/cpython
that referenced
this pull request
Jan 10, 2019
Win arm32 master
jaraco pushed a commit
that referenced
this pull request
Dec 2, 2022
The `html_url` field takes us to the actual comment on GitHub. `url` field took us to an API JSON response.
nanjekyejoannah added a commit
to nanjekyejoannah/cpython
that referenced
this pull request
Dec 13, 2022
This was referenced Feb 11, 2025
pablogsal pushed a commit
to DinoV/cpython
that referenced
this pull request
Oct 25, 2025
Grab builtins dict from module
youknowone referenced
this pull request
in youknowone/cpython
Nov 25, 2025
Also make PyObject an UnsafeCell<ffi::_object> so it can be passed around by & reference
SonicField added a commit
to SonicField/cpython
that referenced
this pull request
Apr 19, 2026
Env initialization used hir_chase_assign (only follows Assign instrs) to compute model registers for support bit assignment. But the pass processing uses phx_rc_model_reg (follows ALL passthroughs except GuardIs). When a phi input goes through a non-Assign passthrough (CheckField, CheckVar, etc.), the two functions return different model registers. The env assigns a bit for the chase_assign model, but the pass looks up using the modelReg model — returning -1 (not found). This -1 was then cast to size_t (becoming SIZE_MAX) and passed to phx_bv_set_bit, causing out-of-bounds memory access → SIGSEGV. Fix: use phx_rc_model_reg in env initialization, matching the pass. Bug python#17: triggered by nbody's phi inputs through non-Assign passthrough instructions (list subscript operations producing CheckVar intermediaries).
SonicField added a commit
to SonicField/cpython
that referenced
this pull request
Apr 22, 2026
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)
SonicField added a commit
to SonicField/cpython
that referenced
this pull request
Apr 22, 2026
Push 48 Tier 5 emit-method conversion: port emitKwNames from C++ to C per supervisor 04:14:48Z REBALANCE directive (velocity focus, single emit-method push). No deletion-gate item closure (post-G2 velocity arc). ================================================================ BRIDGE SPEC TEMPLATE ================================================================ Bridge: hir_builder_emit_kw_names_c Purpose: KW_NAMES opcode handler — saves the keyword-names tuple from co_consts[oparg] into the HIRBuilder kwnames_ slot for the next CALL/CALL_KW to consume as part of the call's operands. C++ source: builder.cpp:3201-3220 (deleted in this commit) PHASE 0 AUDIT: Type A (headers — all existing at HEAD): - PyCodeObject (Python.h) — co_consts access - PyTupleObject — PyTuple_Size, PyTuple_GET_ITEM - PhxTranslationContext (builder_emit_c.c:22, local typedef) Type B (symbols — all existing at HEAD): - hir_func_alloc_register(func) ✓ EXISTS — equivalent to TempAllocator::AllocateNonStack since the latter just calls env_->AllocateRegister at builder.cpp:298. NO new bridge needed. - hir_c_create_load_const(reg, type) ✓ EXISTS (builder_emit_c.c:723) - hir_type_from_object(PyObject*) ✓ EXISTS — equivalent of Type::fromObject - phx_tc_emit(tc, instr) ✓ EXISTS (local helper, builder_emit_c.c:39) - JIT_CHECK_C ✓ EXISTS (header included since push 47) Type B (symbols — NEW BRIDGES, 2): - hir_builder_get_kwnames(builder) → void* (Register*) - hir_builder_set_kwnames(builder, void *reg) Reason for getter+setter pair (vs. single 'do everything' bridge): emitCall (next conversion target after kwnames cluster) reads kwnames_ and clears it post-consume (builder.cpp:3088-3093). Independent getter+setter enables that future conversion without re-architecting. Phase 0.5 IFDEF AUDIT: N/A (no #ifdef-guarded code in emitKwNames) Phase 0 LESSON FROM PUSH 47 APPLIED: Both new bridges have: (a) friend declarations in HIRBuilder class (builder.h:108-109) (b) file-scope forward declarations in extern "C" block (builder.h:28-29) Both sites verified BEFORE first compile-check this time. No friend-decl resolution error (vs. push 47 first attempt). PRIOR DECISIONS: - emitKwNames choice over CLUSTER (post-G2 velocity arc): supervisor 05:13:54Z + theologian 04:14:15Z pre-analysis (LOW + LOW-MED). No theologian CLUSTER analysis posted before push 48 announcement, so default per supervisor. - LOW-MED risk caveat is about kwnames touching CALL_KW argument-passing semantics. emitKwNames is the PRODUCER side (saves the names tuple); the CONSUMER (emitCall) is unchanged in this push. So push 48 risk surface is producer-only path, NOT call-graph integration. INVARIANTS PRESERVED: 1. Index bounds check: oparg < co_consts length, otherwise abort with diagnostic. Matches C++ JIT_CHECK at builder.cpp:3206-3210. Implemented as JIT_CHECK_C (ALWAYS-ON, item python#17 form). 2. Single-slot invariant: kwnames_ MUST be NULL when emitKwNames runs; otherwise prior KW_NAMES wasn't consumed by a CALL* opcode. Matches C++ JIT_CHECK at builder.cpp:3211-3215. Implemented as JIT_CHECK_C. 3. Non-stack allocation: kwnames_reg is allocated as a non-stack temp (hir_func_alloc_register, NOT phx_ptr_arr_push to stack). Matches C++ temps_.AllocateNonStack semantic. 4. LoadConst with object-type: tuple type captured via hir_type_from_object (Type::fromObject equivalent) — gives the consumer (emitCall) access to the full PyObject spec for resolveKwargs static-resolution path. 5. NO stack push: kwnames_ is NOT a stack value. The reg lives on the HIRBuilder for the next CALL to read. emitLoadConst on TC vs. the stack-pushing path (hir_builder_emit_load_const_c) is a critical difference — hir_c_create_load_const + phx_tc_emit alone (no phx_ptr_arr_push to stack). Falsifier: Python function with keyword-arg call (e.g. `f(x=1, y=2)`). JIT-compile, verify HIR shows: KW_NAMES instruction emits LoadConst<TTuple> to a non-stack reg, followed by CALL with that reg as last operand. If the kwnames reg appears on the stack OR if CALL receives wrong tuple, port has stripped invariants. ================================================================ DIFF ================================================================ Python/jit/hir/builder.cpp (-15 / +33): - emitKwNames body (~16 lines) → 8-line delegating stub calling extern "C" hir_builder_emit_kw_names_c. - +12 lines: extern "C" hir_builder_get_kwnames + hir_builder_set_kwnames bridge wrappers (3 lines each, static_cast + field access). Python/jit/hir/builder.h (+4): - +2 lines: file-scope forward decls of hir_builder_get_kwnames + hir_builder_set_kwnames inside existing extern "C" block (matches pattern from push 47). - +2 lines: friend decls inside class HIRBuilder. Python/jit/hir/builder_emit_c.c (+37): - +2 extern decls (kwnames getter/setter) - hir_builder_emit_kw_names_c implementation (~22 lines) - Comment block describing semantics + bridge rationale. Net diff stat: 3 files, +59/-15 (net +44 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:19:22Z. Friend declaration + file-scope forward decl both resolved correctly (Phase 0 audit refinement from push 47 applied successfully). Push 48 batch is 1 commit per supervisor 04:14:48Z REBALANCE directive (single emit-method push, velocity focus). 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)
This was referenced Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.