From 81a25afb05099faa3cac42bec14a7cf7deebea06 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 2 Sep 2026 20:38:21 +0900 Subject: [PATCH] jit-trace, interp: name the unpaired-address disarm; extra_tests: the getsizeof ABI split, an exact module-name oracle, and a slice script that runs `disarm_unpaired_build_addrs` was called `reject_unpaired_build_addrs` in `pyre-interpreter/src/jit_fnaddr.rs` and `pyre-jit-trace/build/prepass.rs`, and both said it "refuses the load", which the function does not do: it writes zero over the address and leaves the body published. Its own doc justified zero as a value none of the pools' uses can mistake for a real target, a call through it faulting on the first instruction. Zero is instead what `jitcode.py JitCode.__init__ fnaddr=None` spells, and both consumers test for it before they branch: a call target goes through `is_callable_fnaddr`, so the blackhole declines it and hands the continuation back to the interpreter while the walker declines it as `ResidualDecline::Symbolic`, and a type operand is only ever compared, so a comparison matching no object fails the guard reading it. `stdlib_sys` charged a two-word `PyGC_Head` to every collector-tracked type. That header exists only in a build with a global interpreter lock; without one the collector keeps its bits in the object header, which is what `get_sizeof` implements and what `Py_GIL_DISABLED` names, so the expectation reads that config var and each interpreter asserts the layout it was built with. Behind that assertion sat a second one, which now fails here and says why: `sys__getframemodulename_impl` reads `PyFunction_GetModule(f->f_funcobj)`, so a `__module__` reassigned after definition is what CPython answers, while a frame here carries the globals it executes in and no link back to the function. `pypy/interpreter/pyframe.py class PyFrame` carries none either -- its `createframe(code, w_globals, outer_func)` takes the function to read its closure alone -- so closing this is a frame-model change rather than a missing port, and the assertion states CPython's answer rather than accepting both. The script is not gated, so the failure is recorded where it is read rather than turning a run red. `builtin_slice` called `test_all_slices`, which imported a `cpython_generated_slices` module that has never existed in this tree, so the script ended in ModuleNotFoundError on every runner. The function is removed rather than given the table it wanted: every question it asks is answered more widely by `test.test_slice`'s `test_indices`, which the CPython suite gate runs and records as PASS, and which walks `itertools.product` over twelve start/stop/step values against six lengths -- including `2**100` bounds and the zero length -- against a reference implementation. What remains is the slice construction, repr, comparison, subscript-protocol and `__index__` coverage that the suite does not carry, and the script now passes under cpython, dynasm and cranelift for 0.4s, so it carries `gate=1`. Assisted-by: Claude --- pyre/extra_tests/snippets/builtin_slice.py | 39 +------------------ pyre/extra_tests/snippets/stdlib_sys.py | 20 ++++++++-- pyre/pyre-interpreter/src/jit_fnaddr.rs | 5 ++- pyre/pyre-jit-trace/build/prepass.rs | 4 +- .../src/runtime_fnaddr_patch.rs | 19 ++++++--- 5 files changed, 35 insertions(+), 52 deletions(-) diff --git a/pyre/extra_tests/snippets/builtin_slice.py b/pyre/extra_tests/snippets/builtin_slice.py index 9a5c1bc78d8..16896be4f9a 100644 --- a/pyre/extra_tests/snippets/builtin_slice.py +++ b/pyre/extra_tests/snippets/builtin_slice.py @@ -1,3 +1,4 @@ +# pyre-check: gate=1 from testutils import assert_raises a = slice(10) @@ -165,41 +166,3 @@ def __index__(self): assert c[CustomIndex(1) : CustomIndex(3)] == [1, 2] assert d[CustomIndex(1) : CustomIndex(3)] == "23" - - -def test_all_slices(): - """ - test all possible slices except big number - """ - - mod = __import__("cpython_generated_slices") - - ll = mod.LL - start = mod.START - end = mod.END - step = mod.STEP - slices_res = mod.SLICES_RES - - count = 0 - failures = [] - for s in start: - for e in end: - for t in step: - lhs = ll[s:e:t] - try: - assert lhs == slices_res[count] - except AssertionError: - failures.append( - "start: {} ,stop: {}, step {}. Expected: {}, found: {}".format( - s, e, t, lhs, slices_res[count] - ) - ) - count += 1 - - if failures: - for f in failures: - print(f) - print(len(failures), "slices failed") - - -test_all_slices() diff --git a/pyre/extra_tests/snippets/stdlib_sys.py b/pyre/extra_tests/snippets/stdlib_sys.py index 041083589a4..ea0411c3aa7 100644 --- a/pyre/extra_tests/snippets/stdlib_sys.py +++ b/pyre/extra_tests/snippets/stdlib_sys.py @@ -184,11 +184,20 @@ def safe_path_flag(env, *opts): with assert_raises(TypeError): sys.getsizeof("x", 1, 2) -# CPython 3.14 adds a two-word PyGC_Head according to the type's GC flag, not -# the object's current tracked state, plus two managed-prefix words for heap -# instances with a managed dict or weakref slot. +# `sys.getsizeof` adds a pre-header the object's type asks for, read off the +# type's flags rather than off the object's current tracked state: two +# managed-prefix words for a heap instance with a managed dict or weakref slot, +# and a two-word PyGC_Head for a type the collector tracks. +# +# The PyGC_Head half exists only in a build that has a global interpreter lock. +# Without one the collector keeps its bits in the object header, so the header +# a tracked type already carries covers it and the pre-header charges nothing; +# `Py_GIL_DISABLED` is the config var that names which of the two layouts the +# running build was compiled with. +import sysconfig + pointer_size = (sys.maxsize.bit_length() + 7) // 8 -gc_head = 2 * pointer_size +gc_head = 0 if sysconfig.get_config_var("Py_GIL_DISABLED") else 2 * pointer_size managed_prefix = 2 * pointer_size @@ -261,4 +270,7 @@ def test_getframemodulename(): test_getframemodulename.__module__ = "awesome_module" +# `sys__getframemodulename_impl` reads the module off the frame's function +# object (`PyFunction_GetModule`), so a `__module__` reassigned after definition +# is what the call answers. assert test_getframemodulename() == "awesome_module" diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index a93e5999b5c..eabe49fc003 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -4598,8 +4598,9 @@ pub fn jit_static_pytype_addrs() -> Vec<(&'static str, i64)> { /// appears in the first and not the second. A name bound at build time and /// missing at run time keeps the build-process address baked in the constant /// pool, because `runtime_fnaddr_patch::patch_static_addr_constants` re-pairs -/// only names present in both; `reject_unpaired_build_addrs` refuses the load -/// once such an address actually reaches a constant pool. +/// only names present in both; `disarm_unpaired_build_addrs` writes zero over +/// such an address, which is the "no address" value every call site already +/// declines. pub fn pyre_class_pytype_addrs() -> Vec<(&'static str, i64)> { let mut rows = Vec::new(); pyre_object::lltype::for_each_class_descriptor(|d| { diff --git a/pyre/pyre-jit-trace/build/prepass.rs b/pyre/pyre-jit-trace/build/prepass.rs index 56a6c106e0a..4156f11fdb1 100644 --- a/pyre/pyre-jit-trace/build/prepass.rs +++ b/pyre/pyre-jit-trace/build/prepass.rs @@ -1117,8 +1117,8 @@ fn real_main() { // an unmatched name keeps this process's address. The `#[pyre_class]` // registry populates on every target for that reason; where the two pools // can still disagree is the module set, which this process cannot read for - // another target, and `reject_unpaired_build_addrs` refuses the load if - // one of those names reaches a constant pool. + // another target, and `disarm_unpaired_build_addrs` writes zero over an + // address of that kind so the call sites reading it decline. let registry_rows = pyre_interpreter::pyre_class_pytype_addrs(); let registry_keys: std::collections::HashSet<&str> = registry_rows.iter().map(|&(key, _)| key).collect(); diff --git a/pyre/pyre-jit-trace/src/runtime_fnaddr_patch.rs b/pyre/pyre-jit-trace/src/runtime_fnaddr_patch.rs index 1dbbf392aa7..37a3bf6d426 100644 --- a/pyre/pyre-jit-trace/src/runtime_fnaddr_patch.rs +++ b/pyre/pyre-jit-trace/src/runtime_fnaddr_patch.rs @@ -294,12 +294,19 @@ static UNPAIRED_BUILD_ADDRS: LazyLock> = LazyLock::new(|| { /// /// Such a value is a pointer into the build-script process, and every use the /// pools have for one is wrong with it: as a residual call target it enters -/// whatever this process holds at that address, as a `PyType` operand it is -/// dereferenced by a type test, and as a pointer-`eq` operand it answers "not -/// this type" for every object. Zero is the substitute because it is the one -/// value none of those can mistake for a real target — a call through it faults -/// on the first instruction, a dereference faults at the first field, and a -/// comparison never matches. +/// whatever this process holds at that address, and as a `PyType` operand it is +/// dereferenced by a type test or compared against every object's type. +/// +/// Zero is not a chosen poison, it is the spelling this already has for "no +/// address" — `jitcode.py JitCode.__init__ fnaddr=None` — and both consumers +/// test for it before they branch. A call target goes through +/// `is_callable_fnaddr`, so the blackhole's `residual_call_*` and +/// `inline_call_*` handlers decline it and hand the continuation back to the +/// interpreter, and the walker's residual path declines the same value as +/// `ResidualDecline::Symbolic`. A type operand is only ever compared, and a +/// comparison against zero matches no object, so the guard reading it fails +/// and the path leaves the JIT before anything can dereference it. That is +/// what makes zero an answer here rather than a trap laid for later. /// /// All three pools are cleared, `constants_r` included: that pool already /// carries words that are not gcrefs — patched host statics and pre-patch