majit: open the retrace path, and detect a stale LLBC before it ships wrong field offsets - #1062
Conversation
…tant arm
virtualstate.py:396-399 settles a LEVEL_CONSTANT incoming statically and
raises VirtualStatesCantMatch("different constants") before the runtime-box
test at :400-405. pyre had only the equal-constants arm, so two differing
constants fell through to the runtime test and emitted a GUARD_VALUE whenever
the concrete box happened to equal the target constant.
Add the `(Constant(_), Constant(_)) => Err(())` arm, and document that the
`info_type_matches` gate at :1470-1476 is what rejects a Virtual/VArray/
VStruct/VArrayStruct incoming against a NotVirtual expected (the port of
virtualstate.py:392-394 and :525-529), so the Constant arm is never entered
with a virtual incoming.
Two regression tests: differing constants with a runtime box equal to the
target constant, and a Constant target against a VArray incoming.
Assisted-by: Claude
…reachable The CloseLoop arm's `self.sym.as_ref()` match and the bridge-close `resolved_key.unwrap_or(bridge_key)` both carry a release fallback where upstream has none: `reached_loop_header` (pyjitpl.py:2974-2989) runs on a live MIFrame, and the cell token a close resolves is `jump_op.getdescr()` (unroll.py:196, :321-322), an object identity that cannot miss. Add a debug_assert at each site recording why the fallback is unreachable — `merge_point` returns early on `sym.is_none()` and `trace_fn` never receives `&mut self`; `current_trace_green_key()` always answers while `is_tracing()`. Release behaviour is unchanged. Assisted-by: Claude
…AG 54-56 Three sites on the merge-point path filter or fall back where upstream asserts or acts unconditionally. Each is now tallied where it FIRES: - 54 `has_merge_point_with_shape_assert` discards a same-green-key merge point whose `green_boxes` length differs from `live_args_len`; pyjitpl.py:3020 `assert len(original_boxes) == len(live_arg_boxes)` asserts instead. The `.any()` becomes an explicit reverse loop with the same short-circuit. - 55 `register_retrace_merge_point` returns without registering when a jump arg carries no intrinsic type; pyjitpl.py:3059-3060 appends unconditionally. The early return also suppresses the `keep_tracing_after_close` write, so the trace is abandoned without appearing in `loops_aborted`. - 56 `close_header_pc` falls back to `self.header_pc` when the walk recorded no close greens; pyjitpl.py:3021 `same_greenkey` always compares the actual closing boxes. The `map_or` becomes a `match`. Add `MC_DIAG_SLOTS` so `MC_DIAG` and `MC_DIAG_LABELS` cannot drift in length, and mirror the three labels in pyre-wasm-runner's hand-copied array. 55 and 56 are only reachable under a non-zero `retrace_limit` (default 0), so a default-parameter run reading 0 on them proves nothing. Assisted-by: Claude
…ntities compile.py:797-811 `ResumeGuardDescr.compile_and_attach` installs a finished retrace under ONE identity — `metainterp.resumekey_original_loop_token` — used for `new_loop.original_jitcell_token`, `send_bridge_to_backend` and `record_loop_or_bridge` alike. pyre binds the artifact to `source_jct` (`set_original_jitcell_token_number`, `record_loop_or_bridge`) but files `set_next_header_pc`, the `previous_tokens` read, `caller_recovery_layout`, `build_guard_metadata`'s key and the `compiled.traces` insert under the caller-supplied `green_key`. Those five name the right loop only while the two agree. They agree by construction: the bridge entry derives its key from this token via `descr_owning_jct(descr)?.green_key()` and seeds both `TraceCtx::green_key` and `BridgeTraceInfo::green_key` from it. Two debug_assert_eq! now state it, as `compile_bridge` already does for its own origin key. The retrace-budget charges (`record_target_token`, `set_retraced_count`) follow the loop being entered (unroll.py:213-215, :290-297) and are deliberately not covered. Assisted-by: Claude
…boehm fields gc.py:653-664 `get_ll_description(gcdescr)` selects `GcLLDescr_boehm` when `gcdescr is None`, so upstream has no "no collector" state — that configuration IS boehm, and gc.py:151-162 is its field block. Both backends already took the base-class answer for two of the four collector-sourced values (`can_use_nursery_malloc -> False` spelled `max_nursery_size: 0`, `write_barrier_descr = None`); the other two were hardcoded to their incminimark values. Retain the collector probe's `Option` to derive `is_boehm`, then: - `malloc_zero_filled` takes gc.py:153 `True` with no collector, matching what the raw malloc fallbacks (built on `libc::calloc`) actually do. Both settings are inert here (rewrite.py:499-500, :521-522). - `fielddescr_tid` takes gc.py:157 `None` with no collector, so gen_initialize_tid (rewrite.py:914-918) emits nothing — the raw malloc fallbacks stamp the header themselves. Assisted-by: Claude
The walker's interp-origin close falls back to `compile_trace(key, &live_args, None)` when `compile_trace_entry_data` answers `None`. That call cannot compile: `compile_trace`'s `None`-origin arm demands the entry-bridge payload (compile.py:1002-1021 `ResumeFromInterpDescr` needs an original green key to attach to) and returns `Cancelled` for every input — after deep-cloning the trace-so-far and lowering its snapshot pool. Answer `Cancelled` at the site so the give-up reads as one. Behaviour is unchanged. `compile_trace_entry_data` reads `trace_ctx().root_green_key()` and `trace_meta()`, both installed by every `JitDriver` trace start, so the arm is unreachable from a jd0 walk; only `MetaInterp::force_start_tracing` opens a tracer with no session envelope. Assisted-by: Claude
… last LABEL find_loop_label_index answered with the trace's trailing LABEL whenever the trace had one. x86/assembler.py:2461-2467 closing_jump instead tests `target_token in self.target_tokens_currently_compiling`: a JUMP naming a token this compilation does not define takes the :2467 `else` arm and jumps absolutely into another trace, even when this trace defines labels of its own. Follow the descr when the closing JUMP carries one; keep the last-LABEL answer when it does not. is_resumable_peeled and resumable_label_count delegate to the same function, so key_dispatch, num_labels and has_preamble follow. Guard the key-0 publication on the first LABEL sitting at op 0. Key 0 enters the function at its first op, so publishing a LABEL that has work before it would let a chaining bridge re-run that work. Assisted-by: Claude
… arity give-up in jump_to_preamble compile_retrace declined every trace whose closing JUMP did not name a label in that trace, which is every jump_to_preamble outcome and so every retrace on every backend. compile.py:341-394 compile_retrace declines only on InvalidLoop (:368-371), and both jump_to_preamble sites (unroll.py:156 and :171) return a full UnrollInfo. Delete the check; all three backends now decide local-vs-external on the token, as x86/assembler.py:2461-2467 does. Removing it exposed EXC_BAD_ACCESS on two of four repros on both native backends: a GuardClass read the class word of a register holding the unboxed loop counter. jump_to_preamble retargets the body JUMP at target_tokens[0] and keeps its args (unroll.py:238-242). Upstream's target_tokens[0] is the start label whose args are loop.inputargs, so compile.py:334's `assert jump.numargs() == label.numargs()` holds by construction. A pyre retrace emits no start label of its own, so target_tokens[0] is the original loop's start label -- the portal entry contract, one boxed Ref per frame local -- while the optimized loop-carried set is wider and mixed: measured 13 vs 8, 14 vs 9, 13 vs 8. Give up in jump_to_preamble when emit_start_label is clear and the two arities differ, returning InvalidLoop, which is unroll.py:242's own escape and lands on compile.py:368-371's cancel. A compile_loop unroll keeps its start label in the same artifact, so its preamble target is local and the mismatch cannot arise. Count the give-up in MC_DIAG slot 57 retrace_arity_giveup. The comment above preamble_target claimed force_box_for_end_of_preamble (unroll.py:126-127) leaves the body JUMP's args carrying the types the preamble Label declares. That is a per-arg property and says nothing about the arg list; restate it. Assisted-by: Claude
A loop-carried accumulator flips int -> float partway through, so the guard at that position fails repeatedly and the bridge grown there closes with a virtual state matching no existing target. The optimizer then asks for a retrace (compile.py:1085 retrace_needed). retrace_limit defaults to 0 (rpython/rlib/jit.py:595), so the fixture raises it to 5 through pypyjit.set_param, which is the only route that reaches the wasm guest. Coverage is retrace_needed -> cut_retrace_from -> the unroll pass -> jump_to_preamble -> the arity give-up; the retrace is not assembled. No other fixture in the corpus reaches compile_retrace at all. All three backends record the same row (loops_aborted=5, loops_compiled=1, bridges_compiled=1, guard_failures=1202). No max-pypy-ratio line. Assisted-by: Claude
…rces it models
preflight_llbc_or_fail tested only that build/llbc/{pyre-object,pyre-interpreter,
pyre-jit}.ullbc exist. The prepass reads those artefacts and never the Rust
sources (AGENTS.md:48), so a field inserted anywhere but last in a #[repr(C)]
struct shifts every following descr offset while the build stays green.
codegen_cache_key hashes every <crate>/src, so a source edit does invalidate the
codegen cache and re-run the prepass — over the same unchanged artefact.
Compare each artefact's stamp against the sources on the silent-success exit.
The oracle is the extractor's own: scripts/llbc_extract.py:815-817 prints the
same source_fingerprint() value that :474 writes into the stamp's `source=`
line, and :643-652 already skips a crate whose stamp still matches, so the
digest has one implementation and it is not in build.rs. `features=` and
`layout_targets=` are replayed from the stamp into CARGO_FEATURES and
LLBC_LAYOUT_TARGETS, because fingerprint_inputs (:255-360) walks the dependency
closure under the feature set and the cross-target layout set in force at
extraction time.
Warning by default; PYRE_LLBC_STRICT=1 promotes the same finding to
cargo::error, PYRE_LLBC_SKIP_FINGERPRINT_CHECK opts out. Every non-answer from
the oracle — spawn failure, non-zero exit, unparseable stdout, a 120s deadline —
is silence, so an unavailable python does not break the build. main() already
returns before preflight_llbc_or_fail when MAJIT_LLBC_EXTRACTION=1, so the check
cannot fire while the artefact it inspects is being produced.
All three stamps in this tree are stale: W_TypeObject gained five abi_* i64
fields in ff42f0e, positioned before weak_subclasses, terminator and
version_tag, after the installed artefacts were extracted.
Assisted-by: Claude
Both types were instantiable, subclassable, and named without their module. interp_scandir.py:468-487 gives W_DirEntry.typedef no __new__ and sets acceptable_as_base_class = False at :487; :172-180 does the same for W_ScandirIterator. typedef.py:55 `acceptable_as_base_class = '__new__' in rawdict` is the rule, and typedef.py:754 asserts it for PyFrame, whose port at typedef.rs:534-539 is the shape followed here. Name the typedefs posix.DirEntry (interp_scandir.py:469) and posix.ScandirIterator (:173), so __module__ reports posix and every type-name-bearing message is spelled the way the typedef spells it. Port descr_reduce_ex (interp_scandir.py:463-465). The flag-driven refusal in reduce_protocol_app.py:13-14 already fires, but spells the type with the bare __name__ where %T is error.py:592-593 `space.type(value).name`, the qualified name. Without it copy.copy(entry) returned a nameless entry whose repr then raised AttributeError. scandir_fn allocates through pyre_object::w_instance_new, which never enters type.__call__, so the producer is unaffected. name and path stay writable: scandir_fn populates them with setattr_str and discards the result, so a read-only getset would silently ship nameless entries — that needs a storage migration first. The new fixture is byte-identical to CPython 3.14 on nine probes across both backends. Assisted-by: Claude
…l flow upstream does not have Three copies of the comment on the dead `rd_loop_token` weakref said `compile.giveup()`'s `SwitchToBlackhole(ABORT_BRIDGE)` is "caught at pyjitpl.py:2906-2907, falling through to blackhole resume". The raise is at pyjitpl.py:2923 and sits above the `try:` at :2926, so the `except SwitchToBlackhole` at :2930-2931 does not catch it and the `finally:` at :2932-2935 does not run; it leaves handle_guard_failure with nothing traced. The cited lines were also stale: handle_guard_failure is at pyjitpl.py:2914 and the weakref read at :2921, not :2890/:2897. State the phase difference in compile_bridge as well. Upstream's check runs before create_history (pyjitpl.py:2925); this site is inside the compile, where upstream asserts instead (compile.py:800). Returning false becomes CompileOutcome::Cancelled and thence Declined, which keeps the trace alive — the shape compile.py:1085's `return None` has at a bridge close, since raise_if_successful does not raise on None. Comments only. Assisted-by: Claude
WalkthroughThe pull request updates JIT backend allocation behavior, trace and retrace validation, diagnostic counters, POSIX builtin parity, LLBC freshness checks, and a retrace benchmark. ChangesBackend runtime configuration
JIT trace control flow and diagnostics
POSIX builtin parity
LLBC artifact freshness
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0a50973a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .arg(crate_name) | ||
| .current_dir(repo_root) | ||
| .env("CARGO_FEATURES", features) | ||
| .env("LLBC_LAYOUT_TARGETS", layout_targets) |
There was a problem hiding this comment.
When a stamp was produced with more than one layout sidecar target, scripts/llbc_extract.py records it as space-separated (layout_targets={' '.join(...)}) but the CLI expects LLBC_LAYOUT_TARGETS to be comma-separated. Passing the raw stamp field here makes the fingerprint subprocess see a single invalid target like a b; it exits non-zero, llbc_source_fingerprint returns None, and the stale-LLBC warning/strict error is silently skipped for those cross-target artifacts, leaving frozen offsets undetected. Re-join the stamp targets with commas before setting the env.
AGENTS.md reference: AGENTS.md:L46-L50
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@majit/majit-backend-wasm/src/lib.rs`:
- Around line 2223-2228: Update the publishability handling around
first_label_at_entry so a rejected non-peeled retrace retracts the previously
published LABEL_TARGETS entry for the shared descriptor before continuing.
Ensure cleanup targets the stale non-peeled retrace entry without removing valid
targets from other loops, and add a regression test covering publication
followed by a failed same-target retrace and subsequent loop-closing JUMP
resolution.
In `@majit/majit-metainterp/src/optimizeopt/unroll.rs`:
- Around line 1844-1881: Update the body_jump_arity calculation near
body_terminal_op to fall back to the actual terminal Jump in body_ops when
body_terminal_op is None, using that Jump’s num_args() value. Keep the existing
body_terminal_op path unchanged so the mismatch check before the retrace
retargeting reflects the same Jump that lines 1904-1905 may retarget.
In `@pyre/extra_tests/parity_tests/dir_entry_uninstantiable_python314.py`:
- Around line 9-11: Extend the parity test for os.DirEntry/posix.DirEntry to
include pickle protocols 0 and 1, asserting TypeError with the same “cannot
pickle 'posix.DirEntry' object” behavior as other protocols. Update the generic
static-type reduction path used by copyreg._reduce_ex as needed so these
protocols produce the matching rejection instead of being excluded.
In `@pyre/pyre-jit-trace/build.rs`:
- Around line 289-306: Update llbc_fingerprint_output to retain access to the
child process until the timeout outcome is known; on recv_timeout expiration,
terminate the child and wait for it to be reaped before returning None. Preserve
the existing successful output parsing and non-success handling, ensuring every
timed-out fingerprint process is cleaned up.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 266ea48c-ab6e-449f-912b-e9955e96f935
📒 Files selected for processing (20)
majit/majit-backend-cranelift/src/compiler.rsmajit/majit-backend-dynasm/src/lib.rsmajit/majit-backend-dynasm/src/runner.rsmajit/majit-backend-wasm/src/codegen.rsmajit/majit-backend-wasm/src/lib.rsmajit/majit-metainterp/src/compile.rsmajit/majit-metainterp/src/jitdriver.rsmajit/majit-metainterp/src/lib.rsmajit/majit-metainterp/src/optimizeopt/unroll.rsmajit/majit-metainterp/src/optimizeopt/virtualstate.rsmajit/majit-metainterp/src/pyjitpl.rspyre/bench/synth/retrace_accumulator_type_flip.cranelift.jitstatspyre/bench/synth/retrace_accumulator_type_flip.dynasm.jitstatspyre/bench/synth/retrace_accumulator_type_flip.pypyre/bench/synth/retrace_accumulator_type_flip.wasm.jitstatspyre/extra_tests/parity_tests/dir_entry_uninstantiable_python314.pypyre/pyre-interpreter/src/module/posix/interp_posix.rspyre/pyre-jit-trace/build.rspyre/pyre-jit-trace/src/jitcode_dispatch/mod.rspyre/pyre-wasm-runner/src/main.rs
| let first_label_at_entry = ops | ||
| .iter() | ||
| .position(|op| op.opcode == majit_ir::OpCode::Label) | ||
| == Some(0); | ||
| let publishable = first_label_at_entry | ||
| && label_descrs.first().is_some_and(|&id| id != 0) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline majit/majit-backend-wasm/src/lib.rs --items all --type function \
| rg -n 'publish_label_target|resolve_cross_loop_jump_target|invalidate_loop' || true
fi
rg -n -C 10 \
'publish_label_target|resolve_cross_loop_jump_target|LABEL_TARGETS|unpublish|remove.*label.*target' \
majit --glob '*.rs'
rg -n -C 8 \
'label_descrs|set_label_block_id|invalidate_loop|jump_to_preamble|retrace|loop-closing' \
majit pyre --glob '*.{rs,py}'Repository: youknowone/pyre
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline around relevant symbols =="
ast-grep outline majit/majit-backend-wasm/src/lib.rs --items all --type function \
| rg -n 'publish_label_target|resolve_cross_loop_jump_target|invalidate_loop|compile_loop|compile_bridge|get_latest' || true
echo "== locate WASM WASM files =="
git ls-files majit/majit-backend-wasm/src | sed -n '1,120p'
echo "== relevant sections in WASM lib =="
sed -n '2160,2265p' majit/majit-backend-wasm/src/lib.rs
printf '\n--- jump resolver section ---\n'
rg -n -C 18 'pub fn resolve_cross_loop_jump_target|resolve_cross_loop_jump_target|LABEL_TARGETS|label.*target|publish_label_target' majit/majit-backend-wasm/src/lib.rs
echo "== backend trait declarations/usages =="
rg -n -C 8 'fn publish_label_target|publish_label_target|fn resolve_cross_loop_jump_target|resolve_cross_loop_jump_target|fn invalidate_loop|invalidate_loop' majit/majit-metainterp/src majit/majit-backend/src majit --glob '*.rs' | sed -n '1,240p'Repository: youknowone/pyre
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== failguard label target implementation =="
rg -n -C 12 'struct LabelTarget|fn publish_label_target|fn label_target|fn unpublish|fn d?e?retr?' majit/majit-backend-wasm/src/failguard.rs
echo "== label_target helper =="
rg -n -C 18 'pub fn label_target|fn label_target|LABEL_TARGETS|label_target' majit/majit-backend-wasm/src/lib.rs majit/majit-backend-wasm/src/failguard.rs
echo "== invalidate_loop implementation =="
rg -n -C 20 'fn invalidate_loop\(&self, token: &JitCellToken\)' majit/majit-metainterp/src majit/majit-backend/src majit/majit-backend-wasm/src --glob '*.rs'
echo "== remove_compiled_loop / target registry cleanup =="
rg -n -C 16 'remove_compiled_loop|compiled_meta|label.*target|LABEL_TARGETS|unpublish|drop.*label' majit/majit-metainterp/src/pyjitpl.rs majit/majit-metainterp/src/jitdriver.rs majit/majit-backend/src/lib.rs majit/majit-backend-wasm/src/failguard.rs --glob '*.rs'Repository: youknowone/pyre
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Compile WasmLoop layout =="
sed -n '2254,2285p' majit/majit-backend-wasm/src/lib.rs
echo "== WasmLoop drop =="
sed -n '2286,2316p' majit/majit-backend-wasm/src/lib.rs
echo "== CompiledWasmLoop Drop =="
sed -n '614,640p' majit/majit-backend-wasm/src/failguard.rs
echo "== stale-target reachable probe =="
python3 - <<'PY'
from pathlib import Path
import re
lib = Path("majit/majit-backend-wasm/src/lib.rs").read_text()
fail = Path("majit/majit-backend-wasm/src/failguard.rs").read_text()
# Extract the non-peeled publish predicate and any unpublish/remove_label_target calls.
predicate = re.search(
r'let first_label_at_entry = .*?\n\s*let publishable = .*?&& label_num_args\.first\(\) == Some\(&inputargs\.len\(\)\);',
lib,
re.S,
).group(0)
cleanup_names = re.findall(
r'\b(?:pub\s+)?(?:fn\s+)?([A-Za-z_][A-Za-z0-9_]*(?:<[^>]*>)?\b)\s*\([^)]*\)\s*\{',
fail + "\n" + lib,
)
cleanup = re.findall(
r'\b(?:pub\s+)?(?:fn\s+)?(unpublish[_a-z0-9]*|remove_label_target[s]?|retract_label_target[s]?|LABELS?[_a-z0-9]*\s*\[[^\]]*\]\s*=|LABEL_TARGETS.*remove|LABELS?[_a-z0-9]*.*remove|remove.*label.*target[s]?|retrac[ei]ng_siblings[_a-z0-9]*)\b',
fail + "\n" + lib,
re.I,
)
print("predicate:")
print(predicate.strip())
print()
print("direct label-target cleanup symbols:")
print("\n".join(sorted(set(c for c in cleanup if re.search(r'label|label.*target|target.*label', c, re.I)))))
print()
print("function names containing possible cleanup words:")
print("\n".join(sorted(set(c for c in cleanup_names if re.search(r'label|label.*target|target.*label|unpublish|retract|dropped|peer|retrace| sibling', c, re.I)))))
# Inspect non-peeled branch immediately for any explicit stale-target cleanup around publishable=false.
m = re.search(
r'} else \{(?P<else>.*?)let compiled = CompiledWasmLoop \{',
lib,
re.S,
).group("else")
print()
print("non-peeled branch first 24 lines:")
for i, line in enumerate(m.splitlines()[:24], 1):
print(f"{2189+i-1:5}: {line}")
PYRepository: youknowone/pyre
Length of output: 6052
Retract the stale label target for rejected non-peeled retraces.
LABEL_TARGETS has no unpublish/clean-up path for label targets, and CompiledWasmLoop::drop only removes targets whose func_handle matches the dropped loop. If a publishable non-peeled retrace is followed by the same loop target retrace that fails this branch, the previous entry remains visible to resolve_cross_loop_jump_target; a later loop-closing JUMP can chain to a trace that is not entry-compatible. Add a regression test and retract the previous target when publication is skipped for this shared descriptor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@majit/majit-backend-wasm/src/lib.rs` around lines 2223 - 2228, Update the
publishability handling around first_label_at_entry so a rejected non-peeled
retrace retracts the previously published LABEL_TARGETS entry for the shared
descriptor before continuing. Ensure cleanup targets the stale non-peeled
retrace entry without removing valid targets from other loops, and add a
regression test covering publication followed by a failed same-target retrace
and subsequent loop-closing JUMP resolution.
| let preamble_arity = exported_renamed_inputargs.len(); | ||
| let body_jump_arity = body_terminal_op.as_ref().map(|j| j.num_args()).unwrap_or(0); | ||
| if crate::majit_log_enabled() { | ||
| let body_jump_arity = body_terminal_op.as_ref().map(|j| j.num_args()).unwrap_or(0); | ||
| eprintln!( | ||
| "[jit] jump_to_preamble: body_jump_args={} preamble_arity={} start_label_args={:?}", | ||
| body_jump_arity, preamble_arity, exported_renamed_inputargs, | ||
| ); | ||
| } | ||
| // `compile.py:334 assert jump.numargs() == label.numargs()`. | ||
| // Upstream can assert because its `target_tokens[0]` is the start | ||
| // label whose args ARE `loop.inputargs` — the same loop-carried | ||
| // positions the body JUMP carries — so `unroll.py:238-242`'s | ||
| // arg-preserving retarget is sound by construction. | ||
| // | ||
| // A pyre RETRACE has no start label of its own (see | ||
| // `emit_start_label`), so `target_tokens[0]` is the ORIGINAL loop's | ||
| // start label: the portal's entry contract, one boxed Ref per frame | ||
| // local. The optimized loop-carried set is wider and mixed, so the | ||
| // retarget can land N values on an M-slot label and the target then | ||
| // reads a raw integer as a Ref — measured as EXC_BAD_ACCESS on the | ||
| // loop counter inside GuardClass. | ||
| // | ||
| // Give up, which is `unroll.py:242`'s own escape (it lets | ||
| // `send_extra_operation` raise `InvalidLoop`) and lands on | ||
| // `compile.py:368-371`'s cancel. Scoped to the retrace: a | ||
| // `compile_loop` unroll keeps its start label in the SAME artifact, | ||
| // so its preamble target is local and this mismatch cannot arise. | ||
| if !self.emit_start_label && body_jump_arity != preamble_arity { | ||
| crate::mc_diag_bump(57); | ||
| if crate::majit_log_enabled() { | ||
| eprintln!( | ||
| "[jit] jump_to_preamble giveup: body JUMP args {body_jump_arity} \ | ||
| != preamble LABEL args {preamble_arity}" | ||
| ); | ||
| } | ||
| return Err(crate::optimize::InvalidLoop( | ||
| "jump_to_preamble: body JUMP arity != preamble LABEL arity", | ||
| )); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Use the actual body Jump when checking arity.
Line 1845 reports zero when body_terminal_op is None. Lines 1904-1905 can still retarget a terminal Jump from body_ops.
A nonzero body Jump can bypass the mismatch check when preamble_arity is zero. A matching body Jump can also be rejected when preamble_arity is nonzero. Derive body_jump_arity from body_ops when body_terminal_op is absent.
Proposed fix
- let body_jump_arity = body_terminal_op.as_ref().map(|j| j.num_args()).unwrap_or(0);
+ let body_jump_arity = body_terminal_op
+ .as_ref()
+ .map(|jump| jump.num_args())
+ .or_else(|| {
+ body_ops
+ .iter()
+ .rfind(|op| op.opcode == OpCode::Jump)
+ .map(|jump| jump.num_args())
+ })
+ .unwrap_or(0);As per coding guidelines, “Port RPython/PyPy code with strict line-by-line structural parity.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let preamble_arity = exported_renamed_inputargs.len(); | |
| let body_jump_arity = body_terminal_op.as_ref().map(|j| j.num_args()).unwrap_or(0); | |
| if crate::majit_log_enabled() { | |
| let body_jump_arity = body_terminal_op.as_ref().map(|j| j.num_args()).unwrap_or(0); | |
| eprintln!( | |
| "[jit] jump_to_preamble: body_jump_args={} preamble_arity={} start_label_args={:?}", | |
| body_jump_arity, preamble_arity, exported_renamed_inputargs, | |
| ); | |
| } | |
| // `compile.py:334 assert jump.numargs() == label.numargs()`. | |
| // Upstream can assert because its `target_tokens[0]` is the start | |
| // label whose args ARE `loop.inputargs` — the same loop-carried | |
| // positions the body JUMP carries — so `unroll.py:238-242`'s | |
| // arg-preserving retarget is sound by construction. | |
| // | |
| // A pyre RETRACE has no start label of its own (see | |
| // `emit_start_label`), so `target_tokens[0]` is the ORIGINAL loop's | |
| // start label: the portal's entry contract, one boxed Ref per frame | |
| // local. The optimized loop-carried set is wider and mixed, so the | |
| // retarget can land N values on an M-slot label and the target then | |
| // reads a raw integer as a Ref — measured as EXC_BAD_ACCESS on the | |
| // loop counter inside GuardClass. | |
| // | |
| // Give up, which is `unroll.py:242`'s own escape (it lets | |
| // `send_extra_operation` raise `InvalidLoop`) and lands on | |
| // `compile.py:368-371`'s cancel. Scoped to the retrace: a | |
| // `compile_loop` unroll keeps its start label in the SAME artifact, | |
| // so its preamble target is local and this mismatch cannot arise. | |
| if !self.emit_start_label && body_jump_arity != preamble_arity { | |
| crate::mc_diag_bump(57); | |
| if crate::majit_log_enabled() { | |
| eprintln!( | |
| "[jit] jump_to_preamble giveup: body JUMP args {body_jump_arity} \ | |
| != preamble LABEL args {preamble_arity}" | |
| ); | |
| } | |
| return Err(crate::optimize::InvalidLoop( | |
| "jump_to_preamble: body JUMP arity != preamble LABEL arity", | |
| )); | |
| let preamble_arity = exported_renamed_inputargs.len(); | |
| let body_jump_arity = body_terminal_op | |
| .as_ref() | |
| .map(|jump| jump.num_args()) | |
| .or_else(|| { | |
| body_ops | |
| .iter() | |
| .rfind(|op| op.opcode == OpCode::Jump) | |
| .map(|jump| jump.num_args()) | |
| }) | |
| .unwrap_or(0); | |
| if crate::majit_log_enabled() { | |
| eprintln!( | |
| "[jit] jump_to_preamble: body_jump_args={} preamble_arity={} start_label_args={:?}", | |
| body_jump_arity, preamble_arity, exported_renamed_inputargs, | |
| ); | |
| } | |
| // `compile.py:334 assert jump.numargs() == label.numargs()`. | |
| // Upstream can assert because its `target_tokens[0]` is the start | |
| // label whose args ARE `loop.inputargs` — the same loop-carried | |
| // positions the body JUMP carries — so `unroll.py:238-242`'s | |
| // arg-preserving retarget is sound by construction. | |
| // | |
| // A pyre RETRACE has no start label of its own (see | |
| // `emit_start_label`), so `target_tokens[0]` is the ORIGINAL loop's | |
| // start label: the portal's entry contract, one boxed Ref per frame | |
| // local. The optimized loop-carried set is wider and mixed, so the | |
| // retarget can land N values on an M-slot label and the target then | |
| // reads a raw integer as a Ref — measured as EXC_BAD_ACCESS on the | |
| // loop counter inside GuardClass. | |
| // | |
| // Give up, which is `unroll.py:242`'s own escape (it lets | |
| // `send_extra_operation` raise `InvalidLoop`) and lands on | |
| // `compile.py:368-371`'s cancel. Scoped to the retrace: a | |
| // `compile_loop` unroll keeps its start label in the SAME artifact, | |
| // so its preamble target is local and this mismatch cannot arise. | |
| if !self.emit_start_label && body_jump_arity != preamble_arity { | |
| crate::mc_diag_bump(57); | |
| if crate::majit_log_enabled() { | |
| eprintln!( | |
| "[jit] jump_to_preamble giveup: body JUMP args {body_jump_arity} \ | |
| != preamble LABEL args {preamble_arity}" | |
| ); | |
| } | |
| return Err(crate::optimize::InvalidLoop( | |
| "jump_to_preamble: body JUMP arity != preamble LABEL arity", | |
| )); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@majit/majit-metainterp/src/optimizeopt/unroll.rs` around lines 1844 - 1881,
Update the body_jump_arity calculation near body_terminal_op to fall back to the
actual terminal Jump in body_ops when body_terminal_op is None, using that
Jump’s num_args() value. Keep the existing body_terminal_op path unchanged so
the mismatch check before the retrace retargeting reflects the same Jump that
lines 1904-1905 may retarget.
Source: Coding guidelines
| Pickle protocols 0 and 1 are deliberately NOT asserted: they never reach | ||
| `reduce_newobj`, they land in `copyreg._reduce_ex`, and that leg already | ||
| diverges for every static type. It is a separate, pre-existing defect. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a 'reduce_protocol_app.py' .
rg -n -C 8 '_reduce_ex|reduce_newobj|__reduce_ex__' pyre lib-python
rg -n -C 6 'pickle\.dumps\(entry|range\(2, pickle\.HIGHEST_PROTOCOL' \
pyre/extra_tests/parity_tests/dir_entry_uninstantiable_python314.pyRepository: youknowone/pyre
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file =="
cat -n pyre/extra_tests/parity_tests/dir_entry_uninstantiable_python314.py
echo "== reduce_protocol_app relevant lines =="
cat -n pyre/pyre-interpreter/src/reduce_protocol_app.py | sed -n '230,360p'
echo "== reduce_protocol.rs relevant lines =="
cat -n pyre/pyre-interpreter/src/reduce_protocol.rs | sed -n '308,380p'
cat -n pyre/pyre-interpreter/src/reduce_protocol.rs | sed -n '1,180p'
echo "== locate DirEntry / dir_entry tests in CPython lib-python =="
rg -n "DirEntry|dir_entry|copyreg|reduce" pyre/pyre-interpreter/extra_tests pyre/extra_tests lib-python -g '*python314*' -g '*.py' | head -200
echo "== object object reduce protocol relevant block =="
cat -n lib-python/3/copyreg.py | sed -n '55,145p'Repository: youknowone/pyre
Length of output: 34117
🌐 Web query:
CPython 3.14 copyreg._reduce_ex pickle protocol 0 1 object __reduce_ex__ TypeError
💡 Result:
In CPython, including version 3.14, the copyreg._reduce_ex function is a helper used by objects that do not explicitly define their own reduce or reduce_ex methods when using low-level pickle protocols (specifically protocols 0 and 1) [1][2][3]. A TypeError occurring within this context is typically triggered because the object's structure is incompatible with the legacy requirements of these protocols [4][3]. Common causes include: 1. Incompatibility of new: When using protocol 0 or 1, copyreg._reduce_ex attempts to reconstruct the object using a mechanism that relies on class.new [1][3]. If an object is implemented as a C-extension type (or has complex internal state that cannot be initialized by a standard object.new call), this reconstruction can fail with a TypeError, such as object.new(X) is not safe, use X.new [4]. 2. Improper use of slots: Classes that define slots without also providing a getstate method are prone to triggering a TypeError during pickling [1][3]. For protocols 0 and 1, copyreg._reduce_ex explicitly raises this error if it cannot access dict or the object state [1][3]. 3. Legacy Constraints: These protocols are limited and do not support many modern Python features [5][6]. Because copyreg._reduce_ex is designed for compatibility with older implementations, it often lacks the flexibility to handle complex, modern class structures, leading to serialization failures [1][4]. If you encounter this error, the most effective solution is to avoid protocols 0 and 1 entirely in favor of higher-performance, modern protocols (e.g., protocol 5, which is the default in Python 3.14) [5][6]. If you must support older protocols for legacy data, you may need to manually implement reduce_ex or reduce in your class to provide a clear, safe way for the pickler to reconstruct your object [5][7]. As a last resort, if you are attempting to unpickle data generated by an older version of your code, you may need to use a temporary migration script in an environment matching the original pickle time to unpickle the data and re-serialize it using a newer, more robust protocol [4].
Citations:
- 1: https://github.com/python/cpython/blob/main/Lib/copyreg.py
- 2: https://github.com/python/cpython/blob/master/Lib/copyreg.py
- 3: https://hg.python.org/cpython/file/f419b2e96a98/Lib/copy_reg.py
- 4: [3.13] copyreg._reconstructor failures for protocol 0 and 1 for
dateutil.tz.tzutcpython/cpython#135201 - 5: https://docs.python.org/3/library/pickle.html
- 6: https://docs.pythonlang.net/3/library/pickle.html
- 7: https://runebook.dev/en/docs/python/library/pickle/object.__reduce_ex__
🌐 Web query:
"cannot pickle 'posix.DirEntry' object" "copyreg._reduce_ex"
💡 Result:
The error "cannot pickle 'posix.DirEntry' object" occurs because os.DirEntry objects, which are returned by os.scandir, are designed to be short-lived, lightweight iterators that hold system-level file handles or cached state [1][2][3]. They are not intended to be persisted, serialized, or passed between processes via mechanisms like multiprocessing.Pool or pickle [1][2]. When you attempt to pickle such an object, the underlying copyreg module invokes _reduce_ex to determine how to serialize it [4][5]. Because os.DirEntry does not implement the necessary protocols to support pickling, this process fails, raising a TypeError [1][2][4]. To resolve this issue, you must avoid pickling the DirEntry object directly. Instead, extract the specific information you need from the object and serialize those primitive data types (e.g., strings, integers, or dicts) [1][2]. Recommended approach: 1. Extract the attributes you require (such as entry.name, entry.path, or the results of entry.is_dir and entry.stat) [1]. 2. Store this data in a dictionary or a custom class that is picklable [1][6]. 3. If you need to restore the "DirEntry-like" functionality later, you can create a simple wrapper or "PseudoDirEntry" class that accepts these stored values in its constructor to mimic the original object's interface [1][6]. Example of manual serialization: # Instead of: pickle.dumps(entry) # Use: data_to_pickle = { 'name': entry.name, 'path': entry.path, 'is_dir': entry.is_dir, 'stat': entry.stat } serialized_data = pickle.dumps(data_to_pickle) [1]
Citations:
- 1: https://stackoverflow.com/questions/39757325/how-to-serialize-a-scandir-direntry-in-python-for-sending-through-a-network-sock
- 2: https://stackoverflow.com/questions/57849854/os-scandir-and-multiprocessing-threadpool-works-but-multi-process-pool-doesn
- 3: https://peps.python.org/pep-0471/
- 4: https://github.com/python/cpython/blob/main/Lib/copyreg.py
- 5: https://github.com/python/cpython/blob/3.11/Lib/copyreg.py
- 6: https://www.exchangetuts.com/how-to-serialize-a-scandirdirentry-in-python-for-sending-through-a-network-socket-1768655702517261
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import copyreg, pickle, types, sys
class Foo(type):
pass
class DirEntry:
pass
DirEntry.__module__ = "posix"
DirEntry.__name__ = "DirEntry"
entry = DirEntry()
print("sys.version", sys.version.split()[0])
print("expected copyreg._reduce_ex for protocol 0:", repr(copyreg._reduce_ex(entry, 0)))
print("expected copyreg._reduce_ex for protocol 1:", repr(copyreg._reduce_ex(entry, 1)))
try:
copyreg._reduce_ex(entry, 2)
except Exception as exc:
print("copyreg._reduce_ex proto >= 2 exception type:", type(exc).__module__ + "." + type(exc).__name__)
print("copyreg._reduce_ex proto >= 2 message:", str(exc))
PY
python3 - <<'PY'
import pickle, sys
class Foo(type):
pass
class DirEntry:
pass
DirEntry.__module__ = "posix"
DirEntry.__name__ = "DirEntry"
entry = DirEntry()
for proto in range(5):
try:
dumps = pickle.dumps(entry, proto)
print("proto", proto, "dumps bytes", dumps, "len", len(dumps))
except Exception as exc:
print("proto", proto, "exception type", type(exc).__module__ + "." + type(exc).__name__)
print("proto", proto, "message", str(exc))
PYRepository: youknowone/pyre
Length of output: 1281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== _reduce_ex source =="
python3 - <<'PY'
import copyreg, inspect
print(inspect.getsource(copyreg._reduce_ex))
PY
echo "== objects involved in copyreg._reduce_ex lookup =="
python3 - <<'PY'
import copyreg
print("_new_type", copyreg._new_type)
print("type of builtin new:", callable(type(int.__new__)))
for name in ["object", "Foo", "DirEntry"]:
obj = eval(name)
print(f"{name}.__new__ type={type(getattr(obj, '__new__', None))!r}")
print(f"{name}.__new__.__self__={getattr(getattr(obj, '__new__', None), '__self__', None)!r}")
PY
echo "== inspect pyre reduce_protocol_app.py if available elsewhere =="
fd -a 'reduce_protocol_app.py' . -x sh -c 'echo "--- $1"; wc -l "$1"' sh {}
rg -n "def reduce_1|def reduce_2|HEAP|dir_entry|DirEntry|ScandirIterator|_reduce_ex" pyre pyre/pyre-interpreter pyre/extra_tests 2>/dev/null | head -240
echo "== inspect object reduction implementation in pyre =="
cat -n pyre/extra_tests/parity_tests/bytearray_python314.py | sed -n '33,39p' 2>/dev/null || true
cat -n pyre/extra_tests/snippets/pickle_instances.py | sed -n '1,25p' 2>/dev/null || trueRepository: youknowone/pyre
Length of output: 1749
🌐 Web query:
CPython Python 3.14 source Objects/typeobject.c Object_new_error new_is_unsafe
💡 Result:
In CPython 3.14, the source file Objects/typeobject.c manages type objects and their behavior [1][2]. While there is no single function named Object_new_error or new_is_unsafe, these terms relate to logic within tp_new_wrapper for generating error messages when attempting to instantiate types that cannot be created [3]. Specifically, in CPython 3.14, the function tp_new_wrapper includes logic to handle cases where a type's tp_new slot is NULL [3]. A recent change (gh-78465) updated this logic to improve error messages for instances where cls.new(cls,...) is called on a type that is not instantiable (such as certain builtin or extension types) [3]. When staticbase->tp_new is NULL, the interpreter now raises a TypeError with the message "cannot create '...' instances" instead of suggesting a potentially invalid operation [3]. Prior to this, error messages in this context would state that a method is "not safe," specifically using the format: "%s.new(%s) is not safe, use %s.new" [3]. This "not safe" phrasing remains in other parts of the tp_new_wrapper logic as a fallback for providing guidance when types are not properly compatible for instantiation [3]. These mechanisms are part of the broader system in typeobject.c that enforces type safety, manages inheritance, and ensures that the new protocol is followed correctly [2][4].
Citations:
- 1: https://github.com/python/cpython/blob/3.14/Objects/typeobject.c
- 2: https://gopy.tamnd.com/docs/annotations/objects/typeobject_detail
- 3: python/cpython@1b21868
- 4: https://gopy.tamnd.com/docs/annotations/objects/typeobject
🌐 Web query:
"cannot pickle 'posix.DirEntry' object" "cpython" "Python 3.14"
💡 Result:
The error "cannot pickle 'posix.DirEntry' object" occurs because os.DirEntry objects, which are returned by os.scandir(), are not designed to be serialized [1][2][3]. These objects are intended to be short-lived and hold internal, low-level data optimized for filesystem traversal; they do not support the pickling protocol [2][4]. This issue commonly arises when passing DirEntry objects to multiprocessing worker functions or attempting to store them in a serialized format [1][5]. To resolve this, you should extract the necessary information from the DirEntry object before passing it or pickling it [1][2][3]. Recommended approaches include: 1. Extract and pass only the required attributes: Pass a simple, picklable data type like the file's path (entry.path), name (entry.name), or a dictionary containing the specific file details you need [1][2]. 2. Use string or path representations: If you only need the file path, os.fspath(entry) or entry.path are efficient ways to obtain a string that can be easily pickled and sent to other processes or threads [4]. 3. Reconstruct the object if necessary: If you must maintain an object-like interface on the receiving end, create a custom "pseudo" class (e.g., a data class or simple class) that holds the data you serialized and mimics the original object's behavior [2][3]. Since os.DirEntry instances are tied to the specific system call and internal state of the scandir iterator, they cannot be reconstructed from scratch without the raw filesystem data they originally held [4]. Converting them to standard types (strings, integers, dictionaries) is the standard practice in Python 3.14 and all earlier versions [2][3].
Citations:
- 1: https://stackoverflow.com/questions/57849854/os-scandir-and-multiprocessing-threadpool-works-but-multi-process-pool-doesn
- 2: https://stackoverflow.com/questions/39757325/how-to-serialize-a-scandir-direntry-in-python-for-sending-through-a-network-sock
- 3: https://www.exchangetuts.com/how-to-serialize-a-scandirdirentry-in-python-for-sending-through-a-network-socket-1768655702517261
- 4: Make os.DirEntry exist python/cpython#71225
- 5: https://stackoverflow.com/questions/69676049/aosp-repo-sync-shutil-error-errno-13-permission-denied
🌐 Web query:
"gh-106529" "cannot pickle 'posix.DirEntry' object"
💡 Result:
The error "cannot pickle 'posix.DirEntry' object" occurs because os.DirEntry objects, which are returned by os.scandir, are designed to be low-level, short-lived, and platform-dependent objects [1][2][3]. They hold system-specific references (such as file descriptors or pointers from readdir) that cannot be serialized for inter-process communication or long-term storage [1][2]. Note that "gh-106529" as cited in your query refers to a CPython internal development issue related to the Tier 2 (uops) interpreter and the implementation of FOR_ITER instructions [4][5][6]; it is unrelated to the pickling of DirEntry objects. To resolve the pickling issue in your application, do not attempt to pickle the DirEntry object itself. Instead, extract and serialize the necessary data (such as the file name or path) as primitive types (strings, dictionaries, etc.) [1][7]. Recommended approach: 1. Extract attributes from the DirEntry object (e.g., entry.path, entry.name, or results from entry.is_dir and entry.stat) before serialization [1][7]. 2. Create a dictionary or a custom class that stores these values, and serialize that structure instead [1][3]. 3. If you need to "reconstruct" a similar object on the receiving end, create a lightweight helper class (a "PseudoDirEntry") that holds the serialized data and mimics the required methods [1][3].
Citations:
- 1: https://stackoverflow.com/questions/39757325/how-to-serialize-a-scandir-direntry-in-python-for-sending-through-a-network-sock
- 2: Make os.DirEntry exist python/cpython#71225
- 3: https://stackoverflow.com/questions/38307995/create-os-direntry
- 4: Branching design for Tier 2 (uops) interpreter python/cpython#106529
- 5: gh-106529: Make FOR_ITER a viable uop python/cpython#112134
- 6: gh-106529: Make FOR_ITER a viable uop python/cpython#112134
- 7: https://stackoverflow.com/questions/57849854/os-scandir-and-multiprocessing-threadpool-works-but-multi-process-pool-doesn
🌐 Web query:
"posix.DirEntry" "pickle" "cpython" "112134"
💡 Result:
In Python, os.DirEntry objects (which are implemented as posix.DirEntry on POSIX systems) cannot be pickled, and attempting to do so will raise a TypeError: can't pickle posix.DirEntry objects [1]. This is by design because os.DirEntry objects are intended to be short-lived, performance-optimized wrappers for low-level directory entry data provided by the operating system (e.g., from readdir or FindFirstFile) [2][3][4]. They encapsulate state that is tightly coupled to a specific system call and a specific point in time, and they are not intended for long-term storage or serialization [2][4]. If you need to pass file information across process boundaries or store it, you should extract the necessary attributes (such as path, name, or stat information) into serializable objects like dictionaries or custom data structures [1][2]. Regarding the reference 112134 mentioned in your query: there is no widely known CPython issue 112134 specifically documenting a request to make os.DirEntry picklable, as the inability to pickle them is considered a fundamental consequence of their design rather than a bug. Users encountering this error in environments like multiprocessing.Pool are advised to pass paths or other serializable metadata instead of the DirEntry object itself [1].
Citations:
- 1: https://stackoverflow.com/questions/57849854/os-scandir-and-multiprocessing-threadpool-works-but-multi-process-pool-doesn
- 2: https://stackoverflow.com/questions/39757325/how-to-serialize-a-scandir-direntry-in-python-for-sending-through-a-network-sock
- 3: Make os.DirEntry exist python/cpython#71225
- 4: https://hg.python.org/cpython/rev/b841972ed0bd
🌐 Web query:
site:github.com/python/cpython "posix.DirEntry" "pickle" "cpython" "gh-71225"
💡 Result:
The GitHub issue #71225 in the CPython repository, titled "Make os.DirEntry exist," refers to the task of exposing the os.DirEntry class, which was previously internal to the posix module [1]. It does not directly relate to pickling os.DirEntry objects [1]. os.DirEntry objects are deliberately designed not to be picklable [1]. They are intended to hold transient, low-level data retrieved from system calls like readdir() (on POSIX) or FindFirstFile() (on Windows) to provide performance optimizations during directory iteration [1]. Because they represent a specific point-in-time state of a file system entry and rely on this low-level context, they cannot be reconstructed independently [1]. If you need to work with directory entries in a way that requires serialization or persistence, you should use pathlib.Path objects or standard string/bytes path representations instead [1]. You can convert an os.DirEntry object to a string or bytes path using os.fspath() [1].
Citations:
Do not exclude pickle protocols 0 and 1 from parity coverage.
os.DirEntry / posix.DirEntry must refuse pickling on every CPython 3.14 protocol; the protocols 0/1 case is not a legitimate difference. Cover these protocols and assert the same TypeError, and fix the generic static-type reduction path if protocols 0/1 currently diverge from cannot pickle 'posix.DirEntry' object.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/extra_tests/parity_tests/dir_entry_uninstantiable_python314.py` around
lines 9 - 11, Extend the parity test for os.DirEntry/posix.DirEntry to include
pickle protocols 0 and 1, asserting TypeError with the same “cannot pickle
'posix.DirEntry' object” behavior as other protocols. Update the generic
static-type reduction path used by copyreg._reduce_ex as needed so these
protocols produce the matching rejection instead of being excluded.
| fn llbc_fingerprint_output(child: std::process::Child) -> Option<String> { | ||
| let (tx, rx) = std::sync::mpsc::channel(); | ||
| std::thread::spawn(move || { | ||
| let _ = tx.send(child.wait_with_output()); | ||
| }); | ||
| let output = rx | ||
| .recv_timeout(std::time::Duration::from_secs(120)) | ||
| .ok()? | ||
| .ok()?; | ||
| if !output.status.success() { | ||
| return None; | ||
| } | ||
| let value = String::from_utf8(output.stdout).ok()?.trim().to_string(); | ||
| // A bare lowercase sha256 and nothing else; anything else means the driver | ||
| // printed something this code does not model. | ||
| let is_hash = value.len() == 64 && value.bytes().all(|b| b.is_ascii_hexdigit()); | ||
| is_hash.then_some(value) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Terminate and reap the fingerprint child after a timeout.
At Line 291, the helper thread takes ownership of Child. At Line 295, a timeout drops the receiver but does not stop that child. wait_with_output() can then keep the extraction process alive indefinitely.
Keep the child handle available through the deadline. Kill and reap it before this function returns on timeout. The current loop can leave up to one process per LLBC_CRATES entry after Cargo exits.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/pyre-jit-trace/build.rs` around lines 289 - 306, Update
llbc_fingerprint_output to retain access to the child process until the timeout
outcome is known; on recv_timeout expiration, terminate the child and wait for
it to be reaped before returning None. Preserve the existing successful output
parsing and non-success handling, ensuring every timed-out fingerprint process
is cleaned up.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit a0a5097). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
Twelve commits on top of
6b843fa4b76, in three groups.1. The retrace path (#33)
closes_on_own_labeldeclined every retrace on every backend, because everyjump_to_preambleoutcome closes on a foreign token.compile.py:341-394 compile_retracedeclines only onInvalidLoop(:368-371), and bothjump_to_preamblesites (unroll.py:156,:171) return a fullUnrollInfo, so the check had no upstream counterpart.Removing it exposed
EXC_BAD_ACCESSon two of four repros on both native backends — aGuardClassreading the class word of a register holding the unboxed loop counter (ldr x16, [x8],x8 = 4405,INT_TYPEinx16/x17). Root cause:jump_to_preambleretargets the body JUMP attarget_tokens[0]and keeps its args (unroll.py:238-242). Upstream'starget_tokens[0]is the start label whose args areloop.inputargs, socompile.py:334'sassert jump.numargs() == label.numargs()holds by construction. A pyre retrace emits no start label of its own, sotarget_tokens[0]is the original loop's start label — the portal entry contract, one boxed Ref per frame local — while the optimized loop-carried set is wider and mixed: measured 13 vs 8, 14 vs 9, 13 vs 8.So the decline moves to
jump_to_preamble, where both correct numbers exist together, and raisesInvalidLoop— which isunroll.py:242's own escape, landing oncompile.py:368-371's cancel. Scoped to retraces: acompile_loopunroll keeps its start label in the same artifact. Counted asMC_DIAGslot 57retrace_arity_giveup.All three backends now decide local-vs-external on the token, which is
x86/assembler.py:2461-2467 closing_jump's own test. The wasm backend used a trace-global "does this trace have a LABEL" heuristic instead; a census over 362/362 existing synth fixtures reads 0 on a counter of exactly the affected traces, so the change is provably inert for the corpus.A new synth fixture reaches
retrace_needed→cut_retrace_from→ unroll →jump_to_preamble→ the arity give-up. It does not cover a compiled retrace, because no retrace in this tree satisfies the arity contract; before this, zero fixtures reachedcompile_retraceat all. All three backends record an identical row. The remaining work — unifying pyre's start-label contract with its loop-carried contract — is filed separately.2. A stale
build/llbcshipped wrong field offsets in silence (#44)preflight_llbc_or_failtested only that the artefacts exist. The prepass reads them and never the Rust sources (AGENTS.md:48), so a field inserted anywhere but last in a#[repr(C)]struct shifts every following descr offset while the build stays green.codegen_cache_keymakes it worse rather than better: it hashes every<crate>/src, so a source edit reliably invalidates the codegen cache and re-runs the prepass — over the same unchanged artefact.This tree is stale right now.
ff42f0e3317(#950) inserted fiveabi_*: i64fields intoW_TypeObjectbeforeweak_subclasses,terminatorandversion_tag; the installed artefacts predate it.version_tagis rescued at runtime byoffset_of!; the other two are not.The detector compares each artefact's stamp against the sources on the silent-success exit, using the extractor's own oracle —
scripts/llbc_extract.py:815-817prints the samesource_fingerprint()value that:474writes into the stamp, and:643-652already skips a crate whose stamp matches. One implementation of the digest, and it is not inbuild.rs. Warning by default;PYRE_LLBC_STRICT=1promotes it tocargo::error,PYRE_LLBC_SKIP_FINGERPRINT_CHECKopts out. Every non-answer from the oracle is silence, so an unavailablepython3cannot break the build.Re-extraction is deliberately not part of this PR.
3. Parity
posix.DirEntry/posix.ScandirIteratorwere instantiable, subclassable and named without their module.interp_scandir.py:468-487gives the typedef no__new__and setsacceptable_as_base_class = False;:172-180likewise for the iterator.descr_reduce_ex(:463-465) is ported too — without itcopy.copy(entry)returned a nameless entry whoserepr()then raisedAttributeError. New fixture is byte-identical to CPython 3.14 on nine probes across both backends.name/pathstay writable on purpose:scandir_fnpopulates them withsetattr_str, so a read-only getset would silently ship nameless entries.rd_loop_tokencomment claimedcompile.giveup()'sSwitchToBlackholeis "caught atpyjitpl.py:2906-2907and falls through to blackhole resume". The raise is at:2923, above thetry:at:2926, soexcept SwitchToBlackholeat:2930-2931does not catch it and thefinally:does not run. Comments only.reached_loop_headerfallbacks andattach_retrace_to_source_guard's two key identities are nowdebug_asserts;generate_guards'Constantarm rejects differing constants (virtualstate.py:392-394); the no-collectorGcRewriterImplgets its boehm fields (gc.py:151-162); the walker declines directly wherecompile_tracecould only cancel.Verification
pyre/check.pygreen on all three backends at each landing point — final rundynasm 380/380 · cranelift 380/380 · wasm 376/376,CHECKPY_RC=0, HEAD stamps equal at both ends, zero existing.jitstatsmodified (only the retrace fixture's three new ones added). Parity suite under CPython 3.14:all parity tests pass.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
posix.DirEntryandposix.ScandirIterator, including qualified type names and restrictions on instantiation, subclassing, copying, and pickling.Bug Fixes
Diagnostics