← 返回首页
bpo-29438: fixed use-after-free in key sharing dict by methane · Pull Request #17 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

bpo-29438: fixed use-after-free in key sharing dict#17

Merged
methane merged 1 commit into
python:masterfrom
methane:bpo29438/fix-dict-useafterfree
Feb 12, 2017
Merged

bpo-29438: fixed use-after-free in key sharing dict#17
methane merged 1 commit into
python:masterfrom
methane:bpo29438/fix-dict-useafterfree

Conversation

methane commented Feb 11, 2017

Copy link
Copy Markdown
Member

No description provided.

methane commented Feb 11, 2017

Copy link
Copy Markdown
Member Author

Patch for Python 3.5 is slightly differ from this PR.
But I'm waiting 3.5 branch has .travis.yml

methane force-pushed the bpo29438/fix-dict-useafterfree branch from c38d9fe to 2dd62f6 Compare February 11, 2017 03:22
methane added the type-bug An unexpected behavior, bug, or error label Feb 11, 2017
Comment thread Objects/dictobject.c Outdated

zhangyangyu Feb 11, 2017
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Just as I said this is somewhat misleading and it's better to make it clear.

methane force-pushed the bpo29438/fix-dict-useafterfree branch from 2dd62f6 to bf8bd7e Compare February 11, 2017 05:48

codecov Bot commented Feb 11, 2017

Copy link
Copy Markdown

Codecov Report

Merging #17 into master will increase coverage by <.01%.

@@ Coverage Diff @@ ## master #17 +/- ## ========================================== + Coverage 82.37% 82.37% +<.01% ========================================== Files 1427 1427 Lines 350948 350948 ========================================== + Hits 289088 289095 +7 + Misses 61860 61853 -7

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7ffb99...bf8bd7e. Read the comment docs.

methane merged commit 2294f3a into python:master Feb 12, 2017
methane deleted the bpo29438/fix-dict-useafterfree branch February 12, 2017 04:51
paulmon added a commit to paulmon/cpython that referenced this pull request Jan 10, 2019
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
17: warn for ssl r=ltratt a=nanjekyejoannah Warn for `ssl` features. Co-authored-by: Joannah Nanjekye <jnanjekye@python.org>
pablogsal pushed a commit to DinoV/cpython that referenced this pull request Oct 25, 2025
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 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Footer

© 2026 GitHub, Inc.