From 48a54ead68d83a7a95f3e4889a0035ad780a56c2 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 9 Aug 2026 23:04:29 +0900 Subject: [PATCH] jit: keep the adopted terminal's result when a carrier-drain walk aborts `drive_bridge_carrier_walk`'s abort tail called `fbw_finish_payload_reset` after `try_adopt_blackhole`, which had just stored the adopted blackhole terminal's `DoneWithThisFrame*` value there. The reset is older than the adopt, which was inserted above it, so the drain cleared the frame's return value on every adopted abort. The drop is silent. The adopt also commits the walk-end state, so the caller finds no concrete to hand back (`fbw_finish_concrete_take`) and does not replay through the blackhole either; the frame's return value reaches Python as None. Clear the stash before the adopt instead. A stale sub-walk value is still discarded, and whatever the adopt installs now survives to the caller. The non-adopted leg keeps its post-discard reset, and a CRN adoption stores nothing, so neither leg can carry a stale value forward. `test.test_pickletools` `OptimizedPickleTests.test_ints` failed on this: `pickletools.optimize` returned None and `pickle.loads` raised `EOFError: Ran out of input`. The CPython suite gate goes from PASS 106/FAIL 2 to PASS 107/FAIL 1, the remaining failure being the pre-existing `test_re`. The parity test reproduces the defect in 0.2s. Assisted-by: Claude --- .../pickletools_optimize_bridge_abort.py | 36 +++++++++++++++++++ pyre/pyre-jit-trace/src/trace.rs | 16 ++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 pyre/extra_tests/parity_tests/pickletools_optimize_bridge_abort.py diff --git a/pyre/extra_tests/parity_tests/pickletools_optimize_bridge_abort.py b/pyre/extra_tests/parity_tests/pickletools_optimize_bridge_abort.py new file mode 100644 index 00000000000..a4308bcb7c3 --- /dev/null +++ b/pyre/extra_tests/parity_tests/pickletools_optimize_bridge_abort.py @@ -0,0 +1,36 @@ +"""A bridge walk that adopts a blackhole terminal must keep the frame's result. + +``pickletools.optimize`` ends in a single ``return out.getvalue()``. Under the +JIT, the guard-failure bridge walk over its opcode loop can stop inside the +reconstructed callee, adopt the blackhole chain it has already driven to a +return, and then abort the trace. The adopted terminal carries that frame's +``DoneWithThisFrame`` value, which is the function's return value; dropping it +leaves the caller with neither a result nor a replay, so ``optimize`` returns +``None`` and ``pickle.loads`` raises ``EOFError: Ran out of input``. + +Two rounds over the varying protocol/width matrix are required: the trace is +compiled during the first round and the guard fails in the second, and a fixed +input never reaches the failing shape. +""" + +import pickle +import pickletools +import sys + +for _round in range(2): + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + n = sys.maxsize + while n: + for expected in (-n, n): + data = pickletools.optimize(pickle.dumps(expected, proto)) + assert data is not None, ( + f"optimize() returned None (proto={proto} n={expected})" + ) + got = pickle.loads(data) + assert got == expected, ( + f"round-trip gave {got!r}, want {expected!r} " + f"(proto={proto} n={expected})" + ) + n = n >> 1 + +print("OK") diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index ee4f498780b..de944a205d6 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -2048,6 +2048,17 @@ fn drive_bridge_carrier_walk( // A declined adopt leaves everything to the rollback below, which is the // pre-existing behaviour. let live_root_addr = sym.live_vable_frame_addr(); + // Clear a stash the sub-walk left BEFORE adopting, not after. An adopted + // terminal stores the frame's `DoneWithThisFrame*` result here + // (`try_adopt_blackhole`), and that result IS this drain's answer: the + // guard's caller takes it as the bridge resolution + // (`fbw_finish_concrete_take` in `call_jit.rs`). Clearing after the adopt + // dropped it, and the drop is silent — the adopt also commits the walk-end + // state, so the caller neither finds a concrete nor replays through the + // blackhole, and the frame's return value reaches Python as `None`. + // Clearing first keeps the same protection against a stale stash while + // leaving whatever the adopt installs intact. + crate::jitcode_dispatch::fbw_finish_payload_reset(); let adopted = crate::jitcode_dispatch::fbw_executed_effect_count() != effects_at_entry && try_adopt_blackhole(ctx, cf_addr, live_root_addr, WalkEndCommitLeg::CarrierAbort); if crate::jitcode_dispatch::fbw_debug_abort_enabled() { @@ -2058,12 +2069,15 @@ fn drive_bridge_carrier_walk( } discard_bridge_carrier_walk(ctx, sym, entry_depth, pre_pos, &pre_virtualref_boxes); crate::jitcode_dispatch::bool_box_truth_reset(); - crate::jitcode_dispatch::fbw_finish_payload_reset(); if adopted { // The chain ran the callee forward from where the sub-walk stopped, so // the eager stores it journaled stand exactly once. crate::jitcode_dispatch::fbw_store_journal_commit(); } else { + // Nothing adopted the chain, so no result is owed to the caller and a + // stash `discard_bridge_carrier_walk` may have left must not leak into + // the next walk. + crate::jitcode_dispatch::fbw_finish_payload_reset(); // Non-commit epilogue: the sub-walk concrete-executed the reconstructed // callee, and the blackhole replays it from the guard, so restore the // pre-walk heap rather than dropping the journals (which would leave every