diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index eac2ebfca8b..866b0167ac0 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -10509,9 +10509,34 @@ impl CraneliftBackend { // earlier LABEL. Re-materialize it from the forwarded root // slot at this header so the fall-through transfer can pass // it onward without restoring a loop phi. + // + // That later LABEL is the only reader of the SSA variable. + // The loop body never `use_var`s a demoted ref — the reason + // `spill_ref_roots` and `reload_ref_roots` both skip one — + // a guard exit reaches the value through + // `demoted_failarg_slots`, and a JUMP filters demoted + // positions out of its args. With no later LABEL demoting + // the same raw the load has no use, and a load is never + // dead code to Cranelift: `MemFlags::trusted()` is not + // `readonly`, so the egraph keeps it, and it would sit in + // the header on every iteration of the loop. if let Some(positions) = demoted_ref_positions_by_label.get(&op_idx) { - let cur_jf = builder.ins().get_pinned_reg(ptr_type); + let mut cur_jf = None; for &(_, raw, ofs) in positions { + let passed_on = + demoted_ref_positions_by_label + .iter() + .any(|(&later_idx, later)| { + later_idx > op_idx + && later + .iter() + .any(|&(_, later_raw, _)| later_raw == raw) + }); + if !passed_on { + continue; + } + let cur_jf = *cur_jf + .get_or_insert_with(|| builder.ins().get_pinned_reg(ptr_type)); let value = builder.ins().load( cl_types::I64, MemFlagsData::trusted(), diff --git a/majit/majit-metainterp/src/blackhole.rs b/majit/majit-metainterp/src/blackhole.rs index 2282cf83372..33f7afffc40 100644 --- a/majit/majit-metainterp/src/blackhole.rs +++ b/majit/majit-metainterp/src/blackhole.rs @@ -2645,7 +2645,7 @@ pub fn run_forever( bh: BlackholeInterpreter, current_exc: i64, ) -> JitException { - run_forever_with_portal(builder, bh, current_exc, None, None, None) + run_forever_with_portal(builder, bh, current_exc, None, None, None, None) } /// blackhole.py:1752 _run_forever with optional portal runner callback. @@ -2668,6 +2668,7 @@ pub fn run_forever_with_portal( mut current_exc: i64, portal_runner: Option<&dyn Fn(&JitException) -> Result<(BhReturnType, i64), JitException>>, on_enter_level: Option<&dyn Fn(i64)>, + on_leave_level: Option<&dyn Fn(i64)>, mut terminal_out: Option<&mut Option>, ) -> JitException { loop { @@ -2717,6 +2718,18 @@ pub fn run_forever_with_portal( // blackhole.py:1759 let next = bh.nextblackholeinterp.take(); + // `pyopcode.py:239-241 RETURN_VALUE` (`frame_finished_execution = True`) + // and `pyopcode.py:184 handle_operation_error` (the same store on the + // no-handler propagation): the level reached here has returned to its + // caller by one of those two routes, so its frame's execution is over. + // Threaded from the interpreter side for the same reason as + // `on_enter_level` — the transition is a property of the embedder's + // frame object, which majit-metainterp cannot name. The bottommost + // level never arrives: it leaves through `handle_jitexception`'s + // propagating arm, which returns above. + if let Some(on_leave_level) = on_leave_level { + on_leave_level(bh.virtualizable_ptr); + } builder.release_interp(bh); // blackhole.py:1760 // RPython: blackholeinterp = blackholeinterp.nextblackholeinterp @@ -2747,6 +2760,17 @@ pub struct PyjitplBlackholeFrameConfig<'a> { /// the resumed frame chain. Threaded from the interpreter side because /// majit-metainterp cannot reference `ExecutionContext`. pub on_enter_level: Option<&'a dyn Fn(i64)>, + /// The `frame_finished_execution` store `pyopcode.py:239-241 RETURN_VALUE` + /// and `pyopcode.py:184 handle_operation_error` perform before leaving a + /// frame. Threaded from the interpreter side for the same reason as + /// [`Self::on_enter_level`]; called once per level that returns to its + /// caller, with that level's `virtualizable_ptr`. + /// + /// Set it only alongside [`Self::per_frame`], which is what makes that + /// pointer name the level's OWN frame. Without it every level shares the + /// portal's virtualizable, and a nested level would hand back the frame + /// ABOVE it — marking a caller that is still running as finished. + pub on_leave_level: Option<&'a dyn Fn(i64)>, } pub fn convert_and_run_from_pyjitpl( @@ -2761,6 +2785,7 @@ pub fn convert_and_run_from_pyjitpl( let mut next_bh: Option> = None; let roots_depth = majit_gc::shadow_stack::resume_ref_roots_depth(); let on_enter_level = config.as_ref().and_then(|config| config.on_enter_level); + let on_leave_level = config.as_ref().and_then(|config| config.on_leave_level); for (frame_index, frame) in framestack.frames.iter().enumerate() { let mut cur_bh = builder.acquire_interp(); @@ -2807,6 +2832,7 @@ pub fn convert_and_run_from_pyjitpl( current_exc, None, on_enter_level, + on_leave_level, terminal_out, ); majit_gc::shadow_stack::pop_resume_ref_roots_to(roots_depth); @@ -4222,6 +4248,7 @@ mod tests { Some(&portal_runner), None, None, + None, ); assert!( matches!( diff --git a/majit/majit-metainterp/src/jitdriver.rs b/majit/majit-metainterp/src/jitdriver.rs index 913297939e4..43a6b1293cc 100644 --- a/majit/majit-metainterp/src/jitdriver.rs +++ b/majit/majit-metainterp/src/jitdriver.rs @@ -310,6 +310,7 @@ pub fn drive_multi_frame_blackhole( raising_exception: bool, per_frame: Option<&[(i64, usize)]>, on_enter_level: Option<&dyn Fn(i64)>, + on_leave_level: Option<&dyn Fn(i64)>, ) -> MultiFrameBlackholeResult { let mut ref_locations = Vec::new(); let mut packed_ref_roots = Vec::new(); @@ -361,6 +362,7 @@ pub fn drive_multi_frame_blackhole( virtualizable_stack_base, per_frame, on_enter_level, + on_leave_level, }), Some(&mut terminal), ); @@ -2308,6 +2310,7 @@ impl JitDriver { raising_exception, None, None, + None, ); let MultiFrameBlackholeResult { outcome, terminal } = outcome; if crate::majit_log_enabled() { @@ -8053,6 +8056,7 @@ impl JitDriver { }), None, None, + None, ); // compile.py:716 assert 0, "unreachable" if crate::majit_log_enabled() { diff --git a/pyre/bench/synth/getattr_hook_binding.py b/pyre/bench/synth/getattr_hook_binding.py index 0ed24321dbd..89966d49b9c 100644 --- a/pyre/bench/synth/getattr_hook_binding.py +++ b/pyre/bench/synth/getattr_hook_binding.py @@ -1,8 +1,15 @@ -# pyre-check: max-pypy-ratio=90 +# pyre-check: max-pypy-ratio=25 # objspace.py:710 get_and_call_function: a __getattr__ (or __getattribute__) # defined as a classmethod or staticmethod must be bound through __get__ before # being called, exactly like any other special method, so it receives the # arguments the descriptor protocol gives it. +# +# Each of the three accesses below used to cost one opaque residual holding the +# whole `object_getattr_miss` walk plus a fresh frame for the hook. Inlining +# the hook against the version-tag and map pins that make the miss constant +# dropped the ratio from 48.6x/59.5x (dynasm/wasm) to 7.4x/10.4x/8.7x +# (dynasm/cranelift/wasm); the bound is twice the slowest of those (10.4x), +# rounded up to the next multiple of five. class ClassmethodGetattr: diff --git a/pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats b/pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats index a53022ed6c8..1432ba1cbbb 100644 --- a/pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats +++ b/pyre/bench/synth/inline_freevar_after_mayforce.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=1008 +guard_failures=1009 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/bench/synth/inlined_helper_mutation.py b/pyre/bench/synth/inlined_helper_mutation.py index 9228c6d48fa..7b6c54e1189 100644 --- a/pyre/bench/synth/inlined_helper_mutation.py +++ b/pyre/bench/synth/inlined_helper_mutation.py @@ -1,8 +1,15 @@ -# pyre-check: max-pypy-ratio=145 +# pyre-check: max-pypy-ratio=60 # The trip count now puts pypy above the startup-subtraction floor, so this -# ratio is a measurement rather than pyre divided by the floor constant. The -# ceiling is twice the slowest of the three backends observed unclamped -# (71.1x on wasm); the previous 45 was fitted against the clamp and fails. +# ratio is a measurement rather than pyre divided by the floor constant; a +# ceiling fitted against the clamp (the 45 this bench once carried) fails. +# The bound is twice the slowest of the three backends (27.7x), rounded up to +# the next multiple of ten. +# +# `push` binds `a.append` inside an inlined callee, and the folds that shape a +# bound-method load used to decline for the whole of such a sub-walk. They now +# decline only where a guard would collapse its resume to the caller's CALL, +# so the binding folds here: the ratio fell from 39.4x/70.2x/60.8x to +# 15.1x/27.7x/26.4x (dynasm/cranelift/wasm). # Inlined-callee shared-heap mutation parity, in both helper orderings. # # A tiny helper mutates a caller-owned list/instance inside a hot while-loop, diff --git a/pyre/bench/synth/property_accessor_invalidation.cranelift.jitstats b/pyre/bench/synth/property_accessor_invalidation.cranelift.jitstats new file mode 100644 index 00000000000..68b5ccdf447 --- /dev/null +++ b/pyre/bench/synth/property_accessor_invalidation.cranelift.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=6 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=6 +retraces_compiled=0 diff --git a/pyre/bench/synth/property_accessor_invalidation.dynasm.jitstats b/pyre/bench/synth/property_accessor_invalidation.dynasm.jitstats new file mode 100644 index 00000000000..68b5ccdf447 --- /dev/null +++ b/pyre/bench/synth/property_accessor_invalidation.dynasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=6 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=6 +retraces_compiled=0 diff --git a/pyre/bench/synth/property_accessor_invalidation.py b/pyre/bench/synth/property_accessor_invalidation.py new file mode 100644 index 00000000000..937dccbcde3 --- /dev/null +++ b/pyre/bench/synth/property_accessor_invalidation.py @@ -0,0 +1,93 @@ +# pyre-check: no-cpython +# `descriptor.py:175 W_Property._immutable_fields_ = ["w_fget?", "w_fset?", +# "w_fdel?"]`. The `?` is what lets a tracer bake the accessor and equally what +# registers the invalidation an assignment to the slot owes, so re-initialising +# an installed property revokes every loop that folded it. +# +# CPython is not an oracle for this: its `LOAD_ATTR_PROPERTY` specialization +# caches `fget` under the receiver type's version alone, and `property.__init__` +# on an installed descriptor bumps no type's version, so a specialized read +# keeps answering with the previous getter. Cold, CPython sees the new one — +# the divergence is the specialization's, and pyre follows pypy's `?` instead. +# +# Each rebind happens INSIDE its loop: a read after the loop is interpreted and +# would not consult what the trace baked. The accessor bodies are residual-free +# so the folds stand rather than aborting. +N = 400000 +SWITCH = N // 2 + + +def first_getter(self): + return 1 + + +def second_getter(self): + return 2 + + +def first_setter(self, value): + self.slot = 1 + + +def second_setter(self, value): + self.slot = 2 + + +class Getter: + x = property(first_getter) + + +class Setter: + slot = 0 + y = property(None, first_setter) + + +def rebind_getter(): + obj = Getter() + descr = Getter.__dict__['x'] + total = 0 + i = 0 + while i < N: + total += obj.x + if i == SWITCH: + descr.__init__(second_getter) + i += 1 + # SWITCH+1 reads of 1, then N-SWITCH-1 reads of 2. + print('getter', total) + + +def rebind_setter(): + obj = Setter() + descr = Setter.__dict__['y'] + total = 0 + i = 0 + while i < N: + obj.y = i + total += obj.slot + if i == SWITCH: + descr.__init__(None, second_setter) + i += 1 + print('setter', total) + + +def drop_getter(): + # The sharper case: the re-init leaves no getter at all, and `W_Property.get` + # (descriptor.py:224-225) raises rather than calling the old function. + obj = Getter() + descr = Getter.__dict__['x'] + raised = 0 + i = 0 + while i < N: + try: + obj.x + except AttributeError: + raised += 1 + if i == SWITCH: + descr.__init__(None) + i += 1 + print('dropped', raised) + + +rebind_getter() +rebind_setter() +drop_getter() diff --git a/pyre/bench/synth/property_accessor_invalidation.wasm.jitstats b/pyre/bench/synth/property_accessor_invalidation.wasm.jitstats new file mode 100644 index 00000000000..68b5ccdf447 --- /dev/null +++ b/pyre/bench/synth/property_accessor_invalidation.wasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=6 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=6 +retraces_compiled=0 diff --git a/pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats b/pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats index bf884686e62..9d94498bfa5 100644 --- a/pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats +++ b/pyre/bench/synth/range_ctor_in_loop.cranelift.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=0 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=0 +guard_failures=812 internal_compile_panics=0 loops_aborted=0 -loops_compiled=1 +loops_compiled=3 +retraces_compiled=0 diff --git a/pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats b/pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats index bf884686e62..9d94498bfa5 100644 --- a/pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats +++ b/pyre/bench/synth/range_ctor_in_loop.dynasm.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=0 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=0 +guard_failures=812 internal_compile_panics=0 loops_aborted=0 -loops_compiled=1 +loops_compiled=3 +retraces_compiled=0 diff --git a/pyre/bench/synth/range_ctor_in_loop.py b/pyre/bench/synth/range_ctor_in_loop.py index 00f253820a3..d580276e150 100644 --- a/pyre/bench/synth/range_ctor_in_loop.py +++ b/pyre/bench/synth/range_ctor_in_loop.py @@ -1,4 +1,4 @@ -# pyre-check: max-pypy-ratio=190 +# pyre-check: max-pypy-ratio=96 # Pins virtual range construction for one-, two-, and three-bound calls while # retaining correct residual behavior for exceptional, subclass, index, and # escaping-object shapes. @@ -9,11 +9,17 @@ # pypy spends 0.10s, clearing the floor even on the platform with the # coarsest timer. # -# The ceiling rose from 50 because that bound was fitted to the floored -# denominator, not because anything got slower: the honest ratio here is -# 83x as a median of interleaved pairwise runs. It is dominated by the four -# deliberately residual shapes below rather than by the virtualized loops -- -# each iteration also raises and catches a ValueError. +# `main` used to run interpreted end to end. The `try: range(0, 3, 0)` below +# puts an out-of-line handler after the trailing comprehension, and the loop +# region that gates the back edge grew across the gap between them and picked +# up that comprehension's call-bearing `FOR_ITER` -- an opcode this loop never +# reaches. With the region built from the exception table instead, the while +# loop and the three `for` loops compile, and this gate's own metric falls +# from 122x to 23.2x dynasm / 30.2x cranelift / 24.7x wasm. A separate +# min-of-five interleaved harness reads the same move as 158x to 35.2x / +# 38.3x, so the ceiling is set from the slower of the two readings: 2.5x of +# 38.3x, the slack every bench here carries against a runner 2.5x slower than +# an idle local box. N = 400000 diff --git a/pyre/bench/synth/range_ctor_in_loop.wasm.jitstats b/pyre/bench/synth/range_ctor_in_loop.wasm.jitstats index bf884686e62..9d94498bfa5 100644 --- a/pyre/bench/synth/range_ctor_in_loop.wasm.jitstats +++ b/pyre/bench/synth/range_ctor_in_loop.wasm.jitstats @@ -1,4 +1,4 @@ -bridges_compiled=0 +bridges_compiled=3 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 @@ -8,7 +8,8 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=0 +guard_failures=812 internal_compile_panics=0 loops_aborted=0 -loops_compiled=1 +loops_compiled=3 +retraces_compiled=0 diff --git a/pyre/extra_tests/parity_tests/abcmeta_type_call_inline.py b/pyre/extra_tests/parity_tests/abcmeta_type_call_inline.py new file mode 100644 index 00000000000..ab4a7490138 --- /dev/null +++ b/pyre/extra_tests/parity_tests/abcmeta_type_call_inline.py @@ -0,0 +1,70 @@ +# CPython-suite gap: `test_abc` never instantiates an ABCMeta-built class in a +# loop hot enough to compile, and no suite test installs `__call__` on a +# metaclass after such a loop has run. +# `typeobject.py:_type_call` runs unless the metatype supplies a `__call__` of +# its own. An `ABCMeta` subclass supplies `__instancecheck__`, +# `__subclasscheck__` and `register` and leaves `__call__` alone, so a class +# built with one instantiates through the same path a plain class does. The +# inline emit used to ask whether the metatype WAS `type`, which refused every +# such class -- and `Fraction`, `Decimal` and every `collections.abc` subclass +# with it. +# +# parity-tests reason: the admission is only sound while the answer holds, and +# what makes it hold is a pin on the METACLASS's version tag -- the class's own +# tag does not move when its metaclass gains an attribute. A `__call__` +# installed on the metaclass mid-loop must take over on the next iteration, so +# this belongs where a stale answer is visible as a wrong number rather than as +# a missed optimisation. +# +# Each rebind happens INSIDE its loop: a call after the loop is interpreted and +# would not consult what the trace baked. +import abc + +N = 40000 +SWITCH = N // 2 + + +class Meta(abc.ABCMeta): + pass + + +class Point(metaclass=Meta): + def __init__(self, x): + self.x = x + + +class Fixed: + x = 7 + + +def inlines(): + # The plain shape: default `__new__`, an `__init__` the walk can enter, and + # a metaclass that overrides neither. + total = 0 + i = 0 + while i < N: + total += Point(i).x + i += 1 + assert total == N * (N - 1) // 2, 'wrong sum: %r' % (total,) + + +def metaclass_gains_call(): + total = 0 + i = 0 + while i < N: + total += Point(i).x + if i == SWITCH: + Meta.__call__ = lambda cls, x: Fixed() + i += 1 + # `i == SWITCH` is assigned before the rebind, so iterations 0..SWITCH read + # their own index and the rest read `Fixed.x`. + expected = SWITCH * (SWITCH + 1) // 2 + (N - SWITCH - 1) * 7 + assert total == expected, 'baked a stale metaclass __call__: %r != %r' % ( + total, + expected, + ) + + +inlines() +metaclass_gains_call() +print('OK') diff --git a/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py b/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py new file mode 100644 index 00000000000..71511c29059 --- /dev/null +++ b/pyre/extra_tests/parity_tests/function_code_reassigned_midloop.py @@ -0,0 +1,144 @@ +# CPython-suite gap: `test_funcattrs` reassigns `__code__` and then calls the +# function once, never from inside a loop hot enough to have inlined the old +# body. +# parity-tests reason: this is a pyre JIT inline-lever regression. + +# `function.py:47 _immutable_fields_ = ['code?', 'w_func_globals?', +# 'closure?[*]', 'defs_w?[*]']`. The `?` is what lets the inline lever bake +# `code` and equally what registers the invalidation an assignment to the slot +# owes. The lever bakes it in the strongest form there is — `code` selects +# which callee body the trace walks into — so without the `?` a loop keeps +# running a body the function no longer has. +# +# The per-iteration `getfield_gc_r` + `guard_value` the lever emits elsewhere +# cannot stand in here: it reads the field off the pinned operand, and for a +# constant callable that operand is a baked `ConstPtr`. +# +# Each reassignment happens INSIDE its loop. A call after the loop is +# interpreted and would not consult what the trace baked. + +N = 40000 +SWITCH = N // 2 + + +def small(): + return 1 + + +def big(): + return 500 + + +def module_level_callee(): + # The constant-callable shape: `small` is resolved once and baked. + total = 0 + i = 0 + while i < N: + total += small() + if i == SWITCH: + small.__code__ = big.__code__ + i += 1 + expected = (SWITCH + 1) + (N - SWITCH - 1) * 500 + assert total == expected, 'baked a stale __code__: %r != %r' % (total, expected) + + +class Holder: + def m(self): + return 1 + + +def m_big(self): + return 500 + + +def method_callee(): + # The method shape: the receiver's type version pins the descriptor, which + # says nothing about the function's own `code` slot. + obj = Holder() + total = 0 + i = 0 + while i < N: + total += obj.m() + if i == SWITCH: + Holder.m.__code__ = m_big.__code__ + i += 1 + expected = (SWITCH + 1) + (N - SWITCH - 1) * 500 + assert total == expected, 'baked a stale method __code__: %r != %r' % (total, expected) + + +def getter_small(self): + return 1 + + +def getter_big(self): + return 500 + + +class WithProperty: + x = property(getter_small) + + +def property_accessor_callee(): + # The property fold resolves the accessor to a trace constant, so it lands + # on the same arm the module-level callee does. + obj = WithProperty() + total = 0 + i = 0 + while i < N: + total += obj.x + if i == SWITCH: + getter_small.__code__ = getter_big.__code__ + i += 1 + expected = (SWITCH + 1) + (N - SWITCH - 1) * 500 + assert total == expected, 'baked a stale accessor __code__: %r != %r' % (total, expected) + + +def hook_small(self, name): + return 1 + + +def hook_big(self, name): + return 500 + + +class WithHook: + pass + + +WithHook.__getattr__ = hook_small + + +def getattr_hook_callee(): + # The `__getattr__` fold resolves its callee the same way. + obj = WithHook() + total = 0 + i = 0 + while i < N: + total += obj.absent + if i == SWITCH: + hook_small.__code__ = hook_big.__code__ + i += 1 + expected = (SWITCH + 1) + (N - SWITCH - 1) * 500 + assert total == expected, 'baked a stale hook __code__: %r != %r' % (total, expected) + + +def fresh_callee_still_inlines(): + # A `MAKE_FUNCTION` in the loop body allocates a fresh callee every + # iteration, so the lever keeps re-proving `code` off the live function + # instead. Here only to catch that arm being given up along the way. + total = 0 + i = 0 + while i < N: + def helper(x): + return x + 1 + total += helper(i) + i += 1 + assert total == N * (N + 1) // 2, 'fresh-callee inline changed answer: %r' % (total,) + + +module_level_callee() +method_callee() +property_accessor_callee() +getattr_hook_callee() +fresh_callee_still_inlines() +print("OK") diff --git a/pyre/extra_tests/parity_tests/getattr_hook_descriptor_binding.py b/pyre/extra_tests/parity_tests/getattr_hook_descriptor_binding.py new file mode 100644 index 00000000000..fd68fc3c837 --- /dev/null +++ b/pyre/extra_tests/parity_tests/getattr_hook_descriptor_binding.py @@ -0,0 +1,88 @@ +# CPython-suite gap: no suite test rebinds a descriptor under a hot JIT loop +# that folded it, and none installs a classmethod subclass as `__getattr__`. +# parity-tests reason: these are pyre JIT descriptor-binding regressions. + +# The inline of a type's `__getattr__` hook resolves the descriptor spelling at +# record time — a plain function is called with the receiver, a `classmethod` or +# `staticmethod` is unwrapped to the callable inside it. Two things that +# resolution must not assume, both of which produced a stale answer from the +# compiled trace while the interpreter answered correctly: +# +# * `get_and_call_function` (`descroperation.py:169-187`) takes the descriptor +# shortcut only for the EXACT type and routes everything else through +# `space.get`. A `classmethod` subclass overriding `__get__` binds through +# that override, so unwrapping its `w_function` calls the wrong callable. +# +# * `function.py:673`/`:720` `_immutable_fields_ = ['w_function?']`. The `?` +# registers the invalidation an assignment owes, so re-initialising an +# installed wrapper has to be observed. It changes no type's version tag, +# which is the only pin the fold holds over the descriptor. +# +# The rebinds happen INSIDE each loop, because a read after the loop is +# interpreted and would not consult what the trace baked. Every hook body +# returns a constant so the fold stands rather than aborting on a residual. + +N = 12000 +SWITCH = N // 2 + + +class Subclassed(classmethod): + def __get__(self, obj, objtype=None): + return lambda name: 2 + + +def first(cls_or_name, name=None): + return 1 + + +def second(cls_or_name, name=None): + return 2 + + +def exact_type_is_required(): + class Owner: + __getattr__ = Subclassed(first) + + owner = Owner() + last = None + for _ in range(N): + last = owner.miss + return last + + +def rebind(wrapper): + class Owner: + __getattr__ = wrapper(first) + + owner = Owner() + seen = [] + for index in range(N): + value = owner.miss + if index == SWITCH: + Owner.__dict__['__getattr__'].__init__(second) + elif index in (SWITCH - 1, N - 1): + seen.append(value) + return seen + + +def rebind_plain(): + class Owner: + __getattr__ = first + + owner = Owner() + seen = [] + for index in range(N): + value = owner.miss + if index == SWITCH: + Owner.__getattr__ = second + elif index in (SWITCH - 1, N - 1): + seen.append(value) + return seen + + +assert exact_type_is_required() == 2, 'overridden __get__ was bypassed' +assert rebind(classmethod) == [1, 2], 'classmethod w_function stayed baked' +assert rebind(staticmethod) == [1, 2], 'staticmethod w_function stayed baked' +assert rebind_plain() == [1, 2], 'rebound plain hook stayed baked' + +print("OK") diff --git a/pyre/extra_tests/parity_tests/getattr_hook_devolved_dict.py b/pyre/extra_tests/parity_tests/getattr_hook_devolved_dict.py new file mode 100644 index 00000000000..a9763dfabff --- /dev/null +++ b/pyre/extra_tests/parity_tests/getattr_hook_devolved_dict.py @@ -0,0 +1,64 @@ +# CPython-suite gap: no suite test reads an attribute of a devolved instance of +# a class that also defines `__getattr__`, under a loop hot enough to specialize. +# parity-tests reason: this is a pyre JIT `__getattr__`-fold regression. + +# The fold that replaces `obj.name` with the type's `__getattr__` proves the +# name is absent from the instance by asking `find_map_attr(name, DICT)` and +# taking None for absence. mapdict.py:1534-1536 states that call "will always +# return None if attrkind==DICT" once the map is rooted at a +# `DevolvedDictTerminator`, so for a devolved instance the answer is the same +# whether or not the attribute is there. Upstream's own case of pinning a map +# to cache a negative instance lookup — `LOAD_METHOD_mapdict_fill_cache_method` +# — refuses the shape outright (mapdict.py:1569). +# +# The map guard cannot stand in: the devolved terminator is a per-class +# singleton, so the pinned map word is identical for every devolved instance of +# the class and unchanged by a later attribute assignment. + +N = 12000 + + +class Hooked: + def __getattr__(self, name): + return 'hook' + + +def non_string_key_devolves(): + obj = Hooked() + # A non-str `__dict__` key forces the object strategy at any attribute + # count, without waiting for the attribute-count limit. + obj.__dict__[1] = 'sentinel' + obj.__dict__['present'] = 'real' + seen = set() + for _ in range(N): + seen.add(obj.present) + assert seen == {'real'}, 'devolved instance answered from the hook: %r' % (seen,) + + +def assignment_after_devolving_is_seen(): + obj = Hooked() + obj.__dict__[1] = 'sentinel' + seen = [] + for i in range(N): + seen.append(obj.later) + if i == N // 2: + obj.later = 'assigned' + assert seen[0] == 'hook', 'absent attribute did not reach the hook: %r' % (seen[0],) + assert seen[-1] == 'assigned', ( + 'assignment on a devolved instance was not seen: %r' % (seen[-1],) + ) + + +def hook_still_answers_a_real_miss(): + obj = Hooked() + obj.__dict__[1] = 'sentinel' + seen = set() + for _ in range(N): + seen.add(obj.missing) + assert seen == {'hook'}, 'the decline swallowed the hook: %r' % (seen,) + + +non_string_key_devolves() +assignment_after_devolving_is_seen() +hook_still_answers_a_real_miss() +print("OK") diff --git a/pyre/extra_tests/parity_tests/getattr_hook_inline_deopt.py b/pyre/extra_tests/parity_tests/getattr_hook_inline_deopt.py new file mode 100644 index 00000000000..6d8bcc47100 --- /dev/null +++ b/pyre/extra_tests/parity_tests/getattr_hook_inline_deopt.py @@ -0,0 +1,165 @@ +# CPython-suite gap: the suite exercises __getattr__ semantics but never runs a +# hooked access hot enough to be compiled, so nothing covers the compiled form. +# parity-tests reason: this targets the pyre-specific guards a compiled +# __getattr__ hook rests on. + +"""A compiled `__getattr__` hook answers to the two pins that admitted it. + +`objspace.py:710 get_and_call_function` reaches the hook only after the +attribute resolves nowhere, so the compiled form pins the receiver's type +version tag (the type keeps lacking the name, and keeps this hook) and the +instance map (the receiver keeps lacking the name). Each loop below runs long +enough to be compiled and then invalidates exactly one of those pins mid-loop: +the values recorded before and after must differ at the iteration the pin was +broken, which is what proves the guard deopts rather than the compiled answer +being reused. + +The AttributeError case is here for the same reason: a hook that raises for an +unknown name is an ordinary outcome of an inlined body, not a shape the fold may +quietly turn into a returned value. +""" + +N = 40000 +SWAP = N // 2 + + +class Instance: + def __getattr__(self, name): + return "hook:" + name + + +class Hooked: + @classmethod + def __getattr__(cls, name): + return "cm:%s:%s" % (cls.__name__, name) + + +class Static: + @staticmethod + def __getattr__(name): + return "sm:" + name + + +class Raiser: + def __getattr__(self, name): + if name == "absent": + raise AttributeError("no " + name) + return "ok:" + name + + +class Installer: + def __getattr__(self, name): + # The hook itself gives the instance the attribute, so every later + # access must read the instance rather than hook again. + self.installed = "real" + return "hook:" + name + + +def instance_shadow(): + """A store during the loop puts the name on the instance.""" + obj = Instance() + seen = [] + i = 0 + while i < N: + seen.append(obj.later) + if i == SWAP: + obj.later = "instance" + i += 1 + assert seen[0] == "hook:later", seen[0] + assert seen[SWAP] == "hook:later", seen[SWAP] + assert seen[SWAP + 1] == "instance", seen[SWAP + 1] + assert seen[-1] == "instance", seen[-1] + + +def hook_replaced(): + """Reassigning `__getattr__` bumps the type's version tag.""" + + class Swapped(Hooked): + pass + + obj = Swapped() + seen = [] + i = 0 + while i < N: + seen.append(obj.zed) + if i == SWAP: + Swapped.__getattr__ = classmethod(lambda cls, name: "replaced") + i += 1 + assert seen[0] == "cm:Swapped:zed", seen[0] + assert seen[SWAP + 1] == "replaced", seen[SWAP + 1] + + +def name_shadowed_on_type(): + """A class attribute added during the loop wins over the hook.""" + + class Shadowed(Static): + pass + + obj = Shadowed() + seen = [] + i = 0 + while i < N: + seen.append(obj.zed) + if i == SWAP: + Shadowed.zed = "class" + i += 1 + assert seen[0] == "sm:zed", seen[0] + assert seen[SWAP + 1] == "class", seen[SWAP + 1] + + +def bound_argument_follows_the_receiver_type(): + """A classmethod hook binds the receiver's own class, not the base.""" + + class Sub(Hooked): + pass + + base = Hooked() + sub = Sub() + i = 0 + while i < N: + assert base.q == "cm:Hooked:q" + assert sub.q == "cm:Sub:q" + i += 1 + + +def hook_raises(): + """An AttributeError out of the hook reaches the caller every iteration.""" + obj = Raiser() + hits = 0 + misses = 0 + i = 0 + while i < N: + hits += len(obj.present) + try: + obj.absent + except AttributeError as exc: + assert str(exc) == "no absent", exc + misses += 1 + i += 1 + assert hits == N * len("ok:present"), hits + assert misses == N, misses + + +def hook_installs_the_attribute(): + obj = Installer() + seen = [] + i = 0 + while i < N: + seen.append(obj.installed) + i += 1 + assert seen[0] == "hook:installed", seen[0] + assert seen[1] == "real", seen[1] + assert seen[-1] == "real", seen[-1] + + +def main(): + instance_shadow() + hook_replaced() + name_shadowed_on_type() + bound_argument_follows_the_receiver_type() + hook_raises() + hook_installs_the_attribute() + print("OK") + + +main() diff --git a/pyre/extra_tests/parity_tests/load_attr_inline_abort_operand_stack.py b/pyre/extra_tests/parity_tests/load_attr_inline_abort_operand_stack.py new file mode 100644 index 00000000000..ddb7c03faac --- /dev/null +++ b/pyre/extra_tests/parity_tests/load_attr_inline_abort_operand_stack.py @@ -0,0 +1,59 @@ +# CPython-suite gap: no suite test resumes a JIT frame at a LOAD_ATTR whose +# inlined descriptor body aborted. +# parity-tests reason: this is a pyre trace-abort operand-stack regression. + +# The operand stack an aborted inline sub-walk hands back to the interpreter, +# for an inline entered from LOAD_ATTR rather than from CALL. +# +# A `__getattr__` hook and a `property` getter are both inlined in place of the +# attribute residual, so both enter the inline lever from LOAD_ATTR. When the +# sub-walk gives up, the caller's frame is flushed at that opcode and the +# interpreter re-executes it, which means the flush has to rebuild the operand +# stack the LOAD_ATTR pops. One of the sources it rebuilds from is the encoded +# residual's Ref operand list. For a CALL that list is exactly the stack image +# (`[callable, null_or_self, args...]`); for `load_attr_fn(obj, code, name_idx)` +# it is `[obj, code]`, whose `code` is a code object the Python stack never +# held. Publishing it resumed the LOAD_ATTR with the code object as receiver: +# `AttributeError: 'code' object has no attribute 'missing'` for an attribute +# the hook answers. +# +# Reaching the abort needs all three of: a FOR_ITER caller (the same body under +# `while` is admitted through an arm that does not abort), a body admitted as +# deferred-call safe, and a residual inside that body which does not inline — +# the string concatenations below. + +N = 3000 + + +class HookOwner: + def __getattr__(self, name): + return "hook:" + name + + +class PropertyOwner: + def __init__(self): + self._value = "v" + + @property + def value(self): + return "prop:" + self._value + + +def read_hook(owner): + last = None + for _ in range(N): + last = owner.missing + return last + + +def read_property(owner): + last = None + for _ in range(N): + last = owner.value + return last + + +assert read_hook(HookOwner()) == "hook:missing" +assert read_property(PropertyOwner()) == "prop:v" + +print("OK") diff --git a/pyre/extra_tests/parity_tests/property_subclass_descriptor_protocol.py b/pyre/extra_tests/parity_tests/property_subclass_descriptor_protocol.py new file mode 100644 index 00000000000..07393fffd95 --- /dev/null +++ b/pyre/extra_tests/parity_tests/property_subclass_descriptor_protocol.py @@ -0,0 +1,94 @@ +# CPython-suite gap: the suite's property tests subclass `property` to add +# methods, never to override `__get__` / `__set__` / `__delete__`. +# parity-tests reason: this is a pyre descriptor-dispatch divergence, shared by +# the interpreter and the JIT fold over it. + +# `get_and_call_function` (`descroperation.py:169-176`) takes a descriptor +# shortcut only for the EXACT type — "isinstance(typ, Function) would not be +# correct here" — and routes everything else through `space.get`, i.e. +# `type(w_descr).__get__` off the MRO. A `property` subclass keeps the base +# layout and retags only its class word, so a layout test admits it and calls +# the wrapped `fget` in place of the override. +# +# The hot loop is here because the JIT's property fold applies the same +# resolution: it must decline for a subclass rather than bake the wrapped +# accessor. + +N = 12000 + + +class Overriding(property): + def __get__(self, obj, objtype=None): + return 'override-get' + + def __set__(self, obj, value): + obj.recorded = 'override-set' + + def __delete__(self, obj): + obj.recorded = 'override-del' + + +def base_getter(self): + return 'wrapped-get' + + +def base_setter(self, value): + self.recorded = 'wrapped-set' + + +def base_deleter(self): + self.recorded = 'wrapped-del' + + +class WithOverride: + recorded = None + x = Overriding(base_getter, base_setter, base_deleter) + + +class Plain: + # A subclass that overrides nothing still reaches `property`'s own + # `__get__` through the MRO. + recorded = None + x = type('Inert', (property,), {})(base_getter, base_setter, base_deleter) + + +def overridden_accessors_run(): + obj = WithOverride() + seen = set() + for _ in range(N): + seen.add(obj.x) + assert seen == {'override-get'}, 'overridden __get__ was bypassed: %r' % (seen,) + + for _ in range(N): + obj.x = 1 + assert obj.recorded == 'override-set', ( + 'overridden __set__ was bypassed: %r' % (obj.recorded,) + ) + + del obj.x + assert obj.recorded == 'override-del', ( + 'overridden __delete__ was bypassed: %r' % (obj.recorded,) + ) + + +def inert_subclass_still_works(): + obj = Plain() + seen = set() + for _ in range(N): + seen.add(obj.x) + assert seen == {'wrapped-get'}, 'inert subclass lost its getter: %r' % (seen,) + + obj.x = 1 + assert obj.recorded == 'wrapped-set', ( + 'inert subclass lost its setter: %r' % (obj.recorded,) + ) + + del obj.x + assert obj.recorded == 'wrapped-del', ( + 'inert subclass lost its deleter: %r' % (obj.recorded,) + ) + + +overridden_accessors_run() +inert_subclass_still_works() +print("OK") diff --git a/pyre/gate-triage.md b/pyre/gate-triage.md index 33e23b5119b..44573661b8b 100644 --- a/pyre/gate-triage.md +++ b/pyre/gate-triage.md @@ -1014,7 +1014,7 @@ the folds it selects, not before them. `PYRE_FBW_SPEC_CENSUS` in §6c is its read-only half: the per-fold consulted/fired tallies. -### §6c — Default-OFF diagnostics, censuses and probes (66): keep, cost nothing +### §6c — Default-OFF diagnostics, censuses and probes (68): keep, cost nothing Each is inert unless set, so none is a removal target by this file's already-ON criterion. They are listed so they cannot be missed again. @@ -1027,7 +1027,8 @@ already-ON criterion. They are listed so they cannot be missed again. `PYRE_DYNASM_EXEC_DIAG`, `PYRE_FBW_CENSUS`, `PYRE_FBW_DEPTH_CENSUS`, `PYRE_FBW_INLINE_DIAG`, `PYRE_FBW_LOOPBODY_SCAN_FULL`, `PYRE_FBW_LOOPBODY_SCAN_LOOP_ONLY`, -`PYRE_FBW_MF_DIAG`, `PYRE_FBW_SPEC_CENSUS`, `PYRE_FBW_STRICT_DIAG`, +`PYRE_FBW_MF_DIAG`, `PYRE_FBW_REPLAY_DIRTY_BODY`, `PYRE_FBW_SPEC_CENSUS`, +`PYRE_FBW_STRICT_DIAG`, `PYRE_FIELD_IDENTITY_CENSUS`, `PYRE_FORITER_INFLIGHT_CENSUS`, `PYRE_FOR_ITER_GATE_DIAG`, `PYRE_GC_DIAG`, `PYRE_GC_FREELIST_DIAG`, `PYRE_JD1_DEBUG`, `PYRE_JD1_DUMP`, @@ -1053,6 +1054,12 @@ value knobs bound the capture window, sampling rate, and report size. This is a diagnostic tool rather than a temporary runtime experiment, so it retires only if the example itself is removed. +`PYRE_FBW_REPLAY_DIRTY_BODY` is a sub-knob of `PYRE_FBW_INLINE_DIAG` rather +than a gate of its own: `replay_safety_dump_body` returns unless both are set, +so setting it alone prints nothing. It lists each callee body as it is scanned, +which is what lets the `pc` on a following `[replay-dirty]` line be matched to +an op. It goes with the inline diagnostic it extends. + `PYRE_VSTACK_NO_EXACT` and `PYRE_VSTACK_KEEP_REORDER` are A/B switches over the walk-level operand-stack mirror, each restoring the behaviour its default replaced: resolving the mirror's Python-PC coordinate from the floor tier rather diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index 038ac5fbe48..66c31bdda70 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -10938,8 +10938,13 @@ pub(crate) unsafe fn get( } } - // property: PyPy W_Property.get → call fget(obj) - if is_property(descr) { + // property: PyPy W_Property.get → call fget(obj). Exact type only, for + // the reason `descroperation.py:169-176` gives its own shortcut: calling + // the accessor in place of `type(w_descr).__get__` is licensed only where + // the type cannot have overridden `__get__`. A subclass falls through to + // the general MRO lookup at the end of this function, which finds either + // its override or `property`'s own typedef entry. + if is_exact_property(descr) { // W_Property.get receives `space.w_None` for class access. Internally // that state is a null pointer so the actual None singleton can still // be a property-bearing instance. @@ -11053,7 +11058,8 @@ unsafe fn set( // raise AttributeError ("can't set attribute") rather than falling // through to the instance dict (`descrobject.c property_descr_set`, // mirrored at `pypy/module/__builtin__/descriptor.py W_Property.set`). - if is_property(descr) { + // Exact type only — see the `__get__` twin. + if is_exact_property(descr) { let fset = w_property_get_fset(descr); if fset.is_null() || is_none(fset) { return Err(property_no_accessor(descr, obj, "setter")?); @@ -11105,18 +11111,13 @@ unsafe fn set( return Ok(true); } - // General __set__: look up on descriptor's type MRO. GetSetProperty - // is no longer INSTANCE_TYPE-shaped (it carries `GETSET_DESCRIPTOR - // _TYPE` so its GetSetProperty payload is GC-traced), so resolve - // the type through `crate::typedef::r#type` rather than the - // `is_instance` branch. - let descr_type = if pyre_object::typedef::is_getset_property(descr) { - crate::typedef::r#type(descr).map_or(std::ptr::null_mut(), |p| p.as_ptr()) - } else if is_instance(descr) { - w_instance_get_type(descr) - } else { - std::ptr::null_mut() - }; + // General __set__: `space.lookup(w_descr, '__set__')` is an MRO lookup on + // whatever `type(w_descr)` is, so resolve the type the same way for every + // descriptor kind — the `__get__` twin at the end of `get` already does. + // The narrower `is_getset_property` / `is_instance` pair this replaces left + // a native-layout subclass instance (a `property` subclass, say) with a + // null type and so no MRO lookup at all. + let descr_type = crate::typedef::r#type(descr).map_or(std::ptr::null_mut(), |p| p.as_ptr()); if !descr_type.is_null() && let Some(set_fn) = lookup_in_type_where(descr_type, "__set__") && !set_fn.is_null() @@ -11131,8 +11132,8 @@ unsafe fn set( /// /// descroperation.py `space.delete(w_descr, w_obj)` unsafe fn delete(descr: PyObjectRef, obj: PyObjectRef) -> Result<(), crate::PyError> { - // property: call fdel(obj) - if is_property(descr) { + // property: call fdel(obj). Exact type only — see the `__get__` twin. + if is_exact_property(descr) { let fdel = w_property_get_fdel(descr); if fdel.is_null() || is_none(fdel) { return Err(property_no_accessor(descr, obj, "deleter")?); @@ -11177,16 +11178,9 @@ unsafe fn delete(descr: PyObjectRef, obj: PyObjectRef) -> Result<(), crate::PyEr } return Ok(()); } - // General __delete__: look up on descriptor's type MRO — same - // shape as `set` above (resolve type through `r#type` so non- - // INSTANCE_TYPE descriptors like `GetSetProperty` are reached). - let descr_type = if pyre_object::typedef::is_getset_property(descr) { - crate::typedef::r#type(descr).map_or(std::ptr::null_mut(), |p| p.as_ptr()) - } else if is_instance(descr) { - w_instance_get_type(descr) - } else { - std::ptr::null_mut() - }; + // General __delete__: look up on descriptor's type MRO — same shape as + // `set` above. + let descr_type = crate::typedef::r#type(descr).map_or(std::ptr::null_mut(), |p| p.as_ptr()); if !descr_type.is_null() && let Some(del_fn) = lookup_in_type_where(descr_type, "__delete__") && !del_fn.is_null() diff --git a/pyre/pyre-interpreter/src/module/_abc/app_abc.py b/pyre/pyre-interpreter/src/module/_abc/app_abc.py new file mode 100644 index 00000000000..5777fb78989 --- /dev/null +++ b/pyre/pyre-interpreter/src/module/_abc/app_abc.py @@ -0,0 +1,42 @@ +# `app_abc.py:15-44 SimpleWeakSet`. The registry and both caches are +# instances of this, so `_get_dump` can hand out the `data` sets and a +# collected entry drops itself through the callback the set installs. +# +# Held at app level rather than rebuilt over the raw set primitives because +# the callback closes over a weakref to the set: the discard has to run with +# the set still reachable but no longer keeping itself alive through it. +from _weakref import ref + + +class SimpleWeakSet: + def __init__(self, data=None): + self.data = set() + + def _remove(item, selfref=ref(self)): + self = selfref() + if self is not None: + self.data.discard(item) + + self._remove = _remove + + def __iter__(self): + # Weakref callback may remove entry from set. + # So we make a copy first. + copy = list(self.data) + for itemref in copy: + item = itemref() + if item is not None: + yield item + + def __contains__(self, item): + try: + wr = ref(item) + except TypeError: + return False + return wr in self.data + + def add(self, item): + self.data.add(ref(item, self._remove)) + + def clear(self): + self.data.clear() diff --git a/pyre/pyre-interpreter/src/module/_abc/mod.rs b/pyre/pyre-interpreter/src/module/_abc/mod.rs index 847c9353150..a79978cc25b 100644 --- a/pyre/pyre-interpreter/src/module/_abc/mod.rs +++ b/pyre/pyre-interpreter/src/module/_abc/mod.rs @@ -4,29 +4,163 @@ //! `_abc_subclasscheck` walk `__mro__` for direct inheritance and the //! per-class `_abc_registry` list populated by `_abc_register` for //! virtual subclasses. Mirrors `pypy/module/_abc/app_abc.py`'s -//! `_abc_register` / `_abc_subclasscheck` flow (registry-based virtual -//! lookups, no negative cache). +//! `_abc_register` / `_abc_subclasscheck` flow, including its +//! positive/negative caches: without them every check that is not a direct +//! `__mro__` hit re-runs the subclass hook, the registry walk and the +//! `__subclasses__` walk, all recursively, on every single call. use pyre_object::*; use std::sync::atomic::{AtomicU64, Ordering}; -// `abc_invalidation_counter` (`_abcmodule.c`): bumped by every successful -// `_abc_register` and by `_reset_caches`, and read by `get_cache_token`. -// The positive/negative object caches themselves remain omitted as an -// optimisation — only this token is tracked, so a bump makes any cached -// token stale. +// `abc_invalidation_counter` (`app_abc.py:47`): bumped by every successful +// `_abc_register` — and by nothing else — and read by `get_cache_token`. A +// negative cache recorded before a bump no longer describes the registry, so +// `_abc_negative_cache_version` is compared against this on every check. static INVALIDATION_COUNTER: AtomicU64 = AtomicU64::new(0); -// `_py_abc.ABCMeta.__new__` (`_py_abc.py:48`) gives every ABC its OWN -// `_abc_registry`. Create it here as a per-class list so the registry is not -// inherited: without an own entry `register`/`subclass_of` would resolve -// `_abc_registry` up the MRO and share one base class's list across every -// descendant ABC (e.g. Complex/Real/Rational/Integral all collapsing to a -// single registry). +/// The app-level `SimpleWeakSet` (`app_abc.py:15-44`), stashed at module init +/// the way `weakref_type` stashes its own. The registry and both caches are +/// instances of it, so the collection this module installs is the one +/// `_get_dump` describes and a collected member drops itself. +static SIMPLE_WEAK_SET_TYPE: std::sync::OnceLock = std::sync::OnceLock::new(); + +fn simple_weak_set_type() -> PyObjectRef { + *SIMPLE_WEAK_SET_TYPE + .get() + .expect("_abc.SimpleWeakSet must be installed at module init") as PyObjectRef +} + +/// `SimpleWeakSet()` — the empty collection `_abc_init` installs and the +/// invalidation in `subclass_of` rebinds to. +fn new_simple_weak_set() -> Result { + crate::call::call_function_impl_result(simple_weak_set_type(), &[]) +} + +/// Whether `cls` can be weak-referenced at all, which is what decides whether +/// a `SimpleWeakSet` can hold it. +/// +/// `app_abc.py:39-40 add` has no such test — upstream reaches it only with a +/// real class, because `_abc_register` rejects everything else. Pyre admits a +/// callable non-type there (see `register`), so the test lives on this side of +/// the boundary rather than in the app-level source, which stays verbatim. +/// `__contains__` needs none: `app_abc.py:33-38` already reads a referent-less +/// item as absent. +fn can_weakref(cls: PyObjectRef) -> bool { + use crate::module::_weakref::interp__weakref as wr; + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + wr::getlifeline(roots.get(cls_slot)).is_ok() +} + +/// `app_abc.py:33-38 SimpleWeakSet.__contains__` through the membership +/// protocol, which is where the weakref probe and the `TypeError` fallback +/// live. +/// +/// A missing collection reads as "not cached" rather than raising: +/// `_abc_init` installs all three, but an ABC built before this module (a +/// pickled class, a hand-rolled `ABCMeta` subclass that skips `_abc_init`) +/// has none, and such a class must still answer subclass checks. +fn weak_cache_contains( + cls: PyObjectRef, + name: &str, + item: PyObjectRef, +) -> Result { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let item_slot = roots.publish(&[item]); + let cache = cache_attr(roots.get(cls_slot), name)?; + if cache.is_null() { + return Ok(false); + } + let cache_slot = roots.publish(&[cache]); + crate::baseobjspace::contains(roots.get(cache_slot), roots.get(item_slot)) +} + +/// `app_abc.py:39-40 SimpleWeakSet.add` — `self.data.add(ref(item, self._remove))`. +/// Called rather than open-coded so the entry carries the callback that +/// discards it once the referent dies; a bare `ref` would leave a spent one +/// behind for every class the check ever saw. +/// +/// Silently declines a class with no collection, for the same reason +/// [`weak_cache_contains`] reads one as a miss, and one that cannot be +/// weak-referenced, for the reason [`can_weakref`] records. +fn weak_cache_add(cls: PyObjectRef, name: &str, item: PyObjectRef) -> Result<(), crate::PyError> { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let item_slot = roots.publish(&[item]); + if !can_weakref(roots.get(item_slot)) { + return Ok(()); + } + let cache = cache_attr(roots.get(cls_slot), name)?; + if cache.is_null() { + return Ok(()); + } + let cache_slot = roots.publish(&[cache]); + let add = crate::baseobjspace::getattr_str(roots.get(cache_slot), "add")?; + let add_slot = roots.publish(&[add]); + crate::call::call_function_impl_result(roots.get(add_slot), &[roots.get(item_slot)])?; + Ok(()) +} + +/// `SimpleWeakSet.clear` (`app_abc.py:43-44`) on the named collection, in +/// place, so anything already holding it sees the clear. +fn weak_cache_clear(cls: PyObjectRef, name: &str) -> Result<(), crate::PyError> { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let cache = cache_attr(roots.get(cls_slot), name)?; + if cache.is_null() { + return Ok(()); + } + let cache_slot = roots.publish(&[cache]); + let clear = crate::baseobjspace::getattr_str(roots.get(cache_slot), "clear")?; + let clear_slot = roots.publish(&[clear]); + crate::call::call_function_impl_result(roots.get(clear_slot), &[])?; + Ok(()) +} + +/// The named collection attribute of `cls`, or null when it has none. Read +/// fresh at every use: the walks between two reads run arbitrary Python, which +/// can rebind the attribute and can move the object. +/// +/// `app_abc.py:110` reads the slot as a plain attribute, so only its absence +/// is a miss. A metaclass hook that raises something else raises out of the +/// check rather than being read as a class with no cache. +fn cache_attr(cls: PyObjectRef, name: &str) -> Result { + match crate::baseobjspace::getattr_str(cls, name) { + Ok(cache) => Ok(cache), + Err(err) if err.kind == crate::PyErrorKind::AttributeError => Ok(std::ptr::null_mut()), + Err(err) => Err(err), + } +} + +/// The registry generation `cls`'s negative cache was recorded against. A +/// class with no version attribute, or one holding something other than an +/// `int`, reports generation 0, which is below every counter value a +/// registration produces — so its negative cache is discarded rather than +/// trusted. +fn negative_cache_version(cls: PyObjectRef) -> Result { + let version = cache_attr(cls, "_abc_negative_cache_version")?; + if version.is_null() || !unsafe { is_int(version) } { + return Ok(0); + } + Ok(unsafe { w_int_get_value(version) }.max(0) as u64) +} + +/// `app_abc.py _abc_init` — install the three collections and the +/// negative-cache generation, then compute `__abstractmethods__`. fn abc_init(args: &[PyObjectRef]) -> Result { if let Some(&cls) = args.first() { - let fresh = w_list_new(vec![]); - crate::baseobjspace::setattr_str(cls, "_abc_registry", fresh)?; + // `app_abc.py:74-77` — registry and both caches are per-class for the + // same reason: resolved up the MRO, one base's would answer for every + // descendant ABC, and a hit on `Rational` would satisfy `Integral`. + // Each value is built before the call that stores it, so that no + // allocation happens between reading `cls` and using it. + for name in ["_abc_registry", "_abc_cache", "_abc_negative_cache"] { + let fresh = new_simple_weak_set()?; + crate::baseobjspace::setattr_str(cls, name, fresh)?; + } + let version = w_int_new(INVALIDATION_COUNTER.load(Ordering::Relaxed) as i64); + crate::baseobjspace::setattr_str(cls, "_abc_negative_cache_version", version)?; let mut abstract_names = Vec::new(); let bases = unsafe { w_type_get_bases(cls) }; if !bases.is_null() && unsafe { is_tuple(bases) } { @@ -107,18 +241,18 @@ fn register(args: &[PyObjectRef]) -> Result { } else if !crate::baseobjspace::callable_w(subclass) { return Err(crate::PyError::type_error("Can only register classes")); } - let registry = match crate::baseobjspace::getattr_str(cls, "_abc_registry") { - Ok(r) if !unsafe { is_none(r) } => r, - _ => { - let fresh = w_list_new(vec![]); - crate::baseobjspace::setattr_str(cls, "_abc_registry", fresh)?; - fresh - } - }; - unsafe { - w_list_append(registry, subclass); + // `app_abc.py:99 cls._abc_registry.add(subclass)`. An ABC that never ran + // `_abc_init` has no collection to add to; it gets one here rather than + // dropping the registration. + if cache_attr(cls, "_abc_registry")?.is_null() { + let fresh = new_simple_weak_set()?; + crate::baseobjspace::setattr_str(cls, "_abc_registry", fresh)?; } - // Invalidate any outstanding cache token. + weak_cache_add(cls, "_abc_registry", subclass)?; + // `app_abc.py:100-101` — invalidate every negative cache. A class this + // registration now makes a subclass may already be recorded as a non-match + // somewhere, and only the counter can reach those entries: they live on + // arbitrary other ABCs, not on `cls`. INVALIDATION_COUNTER.fetch_add(1, Ordering::Relaxed); // `app_abc.py:102-105` — an ABC that carries a structural-match marker // hands it to the registered class and its descendants @@ -161,11 +295,14 @@ fn set_collection_flag_recursive(w_type: PyObjectRef, flag: u8) { } } -// `_py_abc.ABCMeta.__subclasscheck__` (`_py_abc.py:108-147`): the subclass -// hook first, then a direct `__mro__` test, then the recursive registry and -// subclass walks. The positive/negative caches are a pure optimisation and -// are omitted; `issubclass` re-dispatches through `__subclasscheck__` so a -// registered or descendant ABC applies its own hook in turn. +// `_py_abc.ABCMeta.__subclasscheck__` (`_py_abc.py:108-147`): the caches +// first, then the subclass hook, then a direct `__mro__` test, then the +// recursive registry and subclass walks. `issubclass` re-dispatches through +// `__subclasscheck__` so a registered or descendant ABC applies its own hook +// in turn — which is also why the caches are load-bearing rather than a +// refinement: an uncached miss re-runs all three walks at every level of that +// recursion, so one `isinstance` against a deep ABC costs a walk of the whole +// ABC graph. fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result { // _py_abc.py:110-111 — `if not isinstance(subclass, type): raise // TypeError('issubclass() arg 1 must be a class')`. The `__mro__`/registry @@ -187,44 +324,80 @@ fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result rcls, + Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) => return Err(err), + }; // A registered entry that is not a class cannot be a base // class, so it can never make `subclass` a subclass — skip // it rather than letting `issubclass` raise. `range` is @@ -241,60 +414,132 @@ fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result scls, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, - Err(err) => return Err(err), - }; - let item_roots = pyre_object::gc_roots::push_roots(); - let scls_slot = item_roots.base(); - item_roots.pin_root(scls); - if crate::baseobjspace::issubclass(roots.get(subclass_slot), item_roots.get(scls_slot))? { - return Ok(true); + // _py_abc.py:140-144 — `for scls in cls.__subclasses__():`. This must go + // through normal attribute lookup, call, and iteration. Reading the + // internal type subclass vector directly hides user overrides and their + // TypeError/custom exceptions, which are observable ABCMeta semantics. + let subclasses_method = + crate::baseobjspace::getattr_str(roots.get(cls_slot), "__subclasses__")?; + let walk_roots = pyre_object::gc_roots::push_roots(); + let method_slot = walk_roots.base(); + walk_roots.pin_root(subclasses_method); + let subclasses = crate::call::call_function_impl_result(walk_roots.get(method_slot), &[])?; + let subclasses_slot = method_slot + 1; + walk_roots.pin_root(subclasses); + let iterator = crate::baseobjspace::iter(walk_roots.get(subclasses_slot))?; + let iterator_slot = subclasses_slot + 1; + walk_roots.pin_root(iterator); + loop { + let scls = match crate::baseobjspace::next(walk_roots.get(iterator_slot)) { + Ok(scls) => scls, + Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) => return Err(err), + }; + let item_roots = pyre_object::gc_roots::push_roots(); + let scls_slot = item_roots.base(); + item_roots.pin_root(scls); + if crate::baseobjspace::issubclass(roots.get(subclass_slot), item_roots.get(scls_slot))? + { + break 'decide true; + } } - } - Ok(false) + false + }; + + // `app_abc.py:144-163` records at each of its own arms; one site here covers + // all of them. + let recorded = if verdict { + "_abc_cache" + } else { + "_abc_negative_cache" + }; + weak_cache_add(roots.get(cls_slot), recorded, roots.get(subclass_slot))?; + Ok(verdict) } +/// `_abc_instancecheck` (`app_abc.py:108-121`). +/// +/// The two classes are asked separately because they can differ: `__class__` +/// is an ordinary attribute an object may answer with something other than its +/// real type, and a proxy that does so is meant to pass the check for what it +/// claims to be. Reading only `type(instance)` would ignore the claim; +/// reading only `__class__` would let it deny the real one. A positive cache +/// hit on the claimed class is taken before the real type is even read, which +/// is the whole point of inlining the cache check here rather than leaving it +/// to `__subclasscheck__`. fn instancecheck(args: &[PyObjectRef]) -> Result { if args.len() < 2 { return Ok(w_bool_from(false)); } - let cls = args[0]; - let instance = args[1]; - if unsafe { crate::baseobjspace::isinstance_w(instance, cls) } { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[args[0]]); + let instance_slot = roots.publish(&[args[1]]); + + // `app_abc.py:111 subclass = instance.__class__`. + let subclass = crate::baseobjspace::getattr_str(roots.get(instance_slot), "__class__")?; + let subclass_slot = roots.publish(&[subclass]); + if weak_cache_contains(roots.get(cls_slot), "_abc_cache", roots.get(subclass_slot))? { return Ok(w_bool_from(true)); } - // `type(instance)` — the instance's real class. User-defined instances - // carry the generic layout marker in `ob_type` and the real class in - // `w_class`, so reading `ob_type` directly would resolve to `object`; - // `r#type` returns the class for both builtin and user instances. - let subclass = crate::typedef::r#type(instance).map_or(std::ptr::null_mut(), |p| p.as_ptr()); - if subclass.is_null() { + + // `app_abc.py:113 subtype = type(instance)` — the instance's real class. + // User-defined instances carry the generic layout marker in `ob_type` and + // the real class in `w_class`, so reading `ob_type` directly would resolve + // to `object`; `r#type` returns the class for both builtin and user + // instances. + let subtype = crate::typedef::r#type(roots.get(instance_slot)) + .map_or(std::ptr::null_mut(), |p| p.as_ptr()); + if subtype.is_null() { return Ok(w_bool_from(false)); } - Ok(w_bool_from(subclass_of(cls, subclass)?)) + let subtype_slot = roots.publish(&[subtype]); + + if std::ptr::eq(roots.get(subtype_slot), roots.get(subclass_slot)) { + // `app_abc.py:115-117` — one class, so the negative cache can answer. + // The version test is `==`, not `<`: a cache recorded against a + // *later* counter than the one read here cannot describe this + // registry either. + if negative_cache_version(roots.get(cls_slot))? + == INVALIDATION_COUNTER.load(Ordering::Relaxed) + && weak_cache_contains( + roots.get(cls_slot), + "_abc_negative_cache", + roots.get(subclass_slot), + )? + { + return Ok(w_bool_from(false)); + } + return Ok(w_bool_from(subclasscheck_of( + roots.get(cls_slot), + roots.get(subclass_slot), + )?)); + } + // `app_abc.py:121 any(cls.__subclasscheck__(c) for c in (subclass, subtype))`. + for slot in [subclass_slot, subtype_slot] { + if subclasscheck_of(roots.get(cls_slot), roots.get(slot))? { + return Ok(w_bool_from(true)); + } + } + Ok(w_bool_from(false)) +} + +/// `cls.__subclasscheck__(subclass)` through attribute lookup, the way +/// `app_abc.py:118` and `:121` spell it, so an `ABCMeta` subclass that +/// overrides the hook is the one that answers. +fn subclasscheck_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result { + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + let subclass_slot = roots.publish(&[subclass]); + let check = crate::baseobjspace::getattr_str(roots.get(cls_slot), "__subclasscheck__")?; + let check_slot = roots.publish(&[check]); + let result = + crate::call::call_function_impl_result(roots.get(check_slot), &[roots.get(subclass_slot)])?; + let result_slot = roots.publish(&[result]); + crate::baseobjspace::is_true(roots.get(result_slot)) } fn subclasscheck(args: &[PyObjectRef]) -> Result { @@ -311,11 +556,64 @@ fn subclasscheck(args: &[PyObjectRef]) -> Result { /// an outstanding `get_cache_token` survives a registry reset. fn reset_registry(args: &[PyObjectRef]) -> Result { if let Some(&cls) = args.first() { - crate::baseobjspace::setattr_str(cls, "_abc_registry", w_list_new(vec![]))?; + weak_cache_clear(cls, "_abc_registry")?; } Ok(w_none()) } +/// `_abc._reset_caches(cls)` (`app_abc.py:188-191`): empty both of this ABC's +/// caches, leaving the registry and the invalidation counter untouched — a +/// cleared cache is answered by re-running the walks, which is not a change of +/// answer, so no token needs to expire. +/// +/// Cleared in place rather than rebound, so anything already holding the set +/// sees the clear. +fn reset_caches(args: &[PyObjectRef]) -> Result { + if let Some(&cls) = args.first() { + for name in ["_abc_cache", "_abc_negative_cache"] { + weak_cache_clear(cls, name)?; + } + } + Ok(w_none()) +} + +/// `_abc._get_dump(cls)` (`app_abc.py:165-173`): shallow copies of the +/// registry, both caches, and the negative-cache version. The three sets are +/// the collections' own `data`, which is why they are `SimpleWeakSet`s and not +/// bare sets — `ABC._dump_registry` prints what this returns. +fn get_dump(args: &[PyObjectRef]) -> Result { + let Some(&cls) = args.first() else { + return Ok(w_tuple_new(vec![])); + }; + let roots = pyre_object::gc_roots::push_roots(); + let cls_slot = roots.publish(&[cls]); + // `app_abc.py _get_dump` builds the four-tuple as a single expression, so + // every `.data` it reads stays a live variable that the later reads reload. + // Keep the slots rather than the raw pointers: `data` is whatever + // `cache.data` answers, so it can be a list or a dict — the two kinds a + // minor collection moves — and each further `cache_attr` / `getattr_str` + // runs the descriptor protocol. + let mut data_slots = Vec::with_capacity(3); + for name in ["_abc_registry", "_abc_cache", "_abc_negative_cache"] { + let cache = cache_attr(roots.get(cls_slot), name)?; + // A class that never ran `_abc_init` has nothing to describe; an empty + // set keeps the tuple's shape rather than raising at a debug helper. + let data = if cache.is_null() { + w_set_new() + } else { + let cache_slot = roots.publish(&[cache]); + crate::baseobjspace::getattr_str(roots.get(cache_slot), "data")? + }; + data_slots.push(roots.publish(&[data])); + } + // The last read that can run Python; take it before the reloads below so + // they answer with final addresses. + let version = w_int_new(negative_cache_version(roots.get(cls_slot))? as i64); + let mut items: Vec = data_slots.iter().map(|&slot| roots.get(slot)).collect(); + items.push(version); + Ok(w_tuple_new(items)) +} + crate::py_module! { "_abc", functions: { @@ -324,10 +622,20 @@ crate::py_module! { "_abc_register" / 2 = register, "_abc_instancecheck" / 2 = instancecheck, "_abc_subclasscheck" / 2 = subclasscheck, - "_get_dump" / 1 = |_| Ok(w_tuple_new(vec![])), + "_get_dump" / 1 = get_dump, "_reset_registry" / 1 = reset_registry, - // Pyre keeps no object caches to clear; bumping the token invalidates - // any outstanding `get_cache_token` value. - "_reset_caches" / 1 = |_| { INVALIDATION_COUNTER.fetch_add(1, Ordering::Relaxed); Ok(w_none()) }, + "_reset_caches" / 1 = reset_caches, + }, + extra_init: |ns| { + crate::importing::appleveldef_install_seeded( + ns, + include_str!("app_abc.py"), + "app_abc.py", + &["SimpleWeakSet"], + &[], + ); + let simple_weak_set = crate::module_ns_get(ns, "SimpleWeakSet") + .expect("_abc.SimpleWeakSet must be installed by appleveldefs"); + let _ = SIMPLE_WEAK_SET_TYPE.set(simple_weak_set as usize); }, } diff --git a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs index 2cdda23fa3c..dacec04e004 100644 --- a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs +++ b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs @@ -1760,6 +1760,88 @@ pub unsafe fn load_attr_fast_path( Some((w_type, version_tag, map, p.storageindex)) } +/// The miss twin of [`load_attr_fast_path`]: return the ingredients for +/// inlining the receiver type's `__getattr__` hook when `name` resolves +/// nowhere. +/// +/// `baseobjspace::instance_getattr_hook_or_err` is the tail this stands in for +/// (`descroperation.py:242-245`): once the descriptor protocol has produced an +/// AttributeError, the type's `__getattr__` is looked up and called with the +/// receiver and the name. Reaching that tail is what the two returned pins +/// prove, and both are guards the caller owes: +/// +/// * `version_tag` — the class lookup stays constant, so `name` keeps +/// resolving to nothing on the type and `__getattr__` keeps resolving to +/// the returned hook; +/// * `map` — the instance shape stays constant, so `name` keeps being absent +/// from this receiver's own storage. +/// +/// Together they make the whole `object_getattr_miss` walk a compile-time +/// answer, which is the work the fold removes; the hook itself is what the +/// caller then inlines. +/// +/// Returns `None` for every shape those two guards cannot cover: a non-mapdict +/// receiver, a custom `__getattribute__`, an uncacheable `version_tag`, a name +/// the type or the instance actually owns, or a type with no `__getattr__`. +/// +/// # Safety +/// `w_obj` must be a live object. +pub unsafe fn getattr_hook_fast_path( + w_obj: PyObjectRef, + name: &str, +) -> Option<(PyObjectRef, u64, MapRef, PyObjectRef)> { + // mapdict.py:1495 `if map is not None:` — also filters non-instances. + let map = unsafe { mapdict_map_or_null(w_obj) }; + if map.is_null() { + return None; + } + // mapdict.py:1496 `w_type = map.terminator.w_cls`. + let w_type = unsafe { (*(*map).terminator()).as_terminator() }.w_cls; + if w_type.is_null() { + return None; + } + // mapdict.py:1497-1499 — a custom `__getattribute__` runs its own lookup, + // which neither pin describes. + if unsafe { crate::baseobjspace::getattribute_if_not_from_object(w_type) }.is_some() { + return None; + } + // mapdict.py:1500-1501 `version_tag = w_type.version_tag(); if is not None:`. + let version_tag = unsafe { crate::baseobjspace::w_type_version_tag(w_type) }; + if version_tag == 0 { + return None; + } + // The miss itself. A type-level hit is refused before the map is consulted: + // `classify_attr` reads a `__slots__` member under the `"slot"` name rather + // than its own, so a descriptor found here says nothing about what + // `find_map_attr(name)` below would answer. + if unsafe { crate::baseobjspace::lookup_in_type_where(w_type, name) }.is_some() { + return None; + } + // A devolved instance keeps its attributes in a real dictionary, and + // mapdict.py:1534-1536 states that `find_map_attr` "will always return + // None if attrkind==DICT" for such a map. The hit path reads that call + // for a Some, so a None costs it only the fold; this path reads it for its + // ABSENCE and would take an always-None answer as proof the name is not on + // the instance. `LOAD_METHOD_mapdict_fill_cache_method` — upstream's own + // case of pinning a map to cache a negative instance lookup — refuses the + // shape outright (mapdict.py:1569 `if map is None or + // isinstance(map.terminator, DevolvedDictTerminator): return`), and the + // map pin cannot stand in: the devolved terminator is a per-class + // singleton, so the guarded map word is the same for every devolved + // instance and unchanged by a later `obj. = ...`. + if unsafe { map_is_devolved(map) } { + return None; + } + // `classify_attr(w_type, None, false)` answers `(DICT, false)` — the + // no-descriptor arm (mapdict.py:1509-1510) — so this is the same + // `find_map_attr` call the hit path makes, read for its absence. + if unsafe { find_map_attr(map, Wtf8::new(name), DICT) }.is_some() { + return None; + } + let w_getattr = unsafe { crate::baseobjspace::lookup_in_type_where(w_type, "__getattr__") }?; + Some((w_type, version_tag, map, w_getattr)) +} + /// The [`load_attr_fast_path`] twin for a receiver that keeps its attributes in /// a `newdict(instance=True)` dictionary rather than in header mapdict storage /// (`mapdict.py:1299-1303 make_instance_dict`). It applies the same @@ -1878,25 +1960,34 @@ unsafe fn property_descr_fast_path( return None; } let w_descr = unsafe { crate::baseobjspace::lookup_in_type(w_type, name) }?; - if !unsafe { pyre_object::descriptor::is_property(w_descr) } { + // Exact type: the fold calls `fget`/`fset` directly, which stands in for + // `type(w_descr).__get__` only where that cannot have been overridden + // (`descroperation.py:169-176`). A `property` subclass keeps the base + // layout and retags only `w_class`, so the layout test admits it. + if !unsafe { pyre_object::descriptor::is_exact_property(w_descr) } { return None; } Some((w_type, version_tag, w_descr)) } -/// LOAD_ATTR `property` fast path: return the type, version tag, and Python -/// `fget` when `obj.name` reads a property getter, so the full-body walker can -/// inline `fget(obj)` in place of the opaque `getattr` residual. Returns `None` -/// (leave the residual) for a write-only property or any shape -/// [`property_descr_fast_path`] declines. A custom `__getattribute__` owns the -/// read (mapdict.py:1497-1499), so it declines to the residual. +/// LOAD_ATTR `property` fast path: return the type, version tag, the property +/// object, and its Python `fget` when `obj.name` reads a property getter, so +/// the full-body walker can inline `fget(obj)` in place of the opaque `getattr` +/// residual. Returns `None` (leave the residual) for a write-only property or +/// any shape [`property_descr_fast_path`] declines. A custom +/// `__getattribute__` owns the read (mapdict.py:1497-1499), so it declines to +/// the residual. +/// +/// The property object is part of the answer because `fget` alone cannot be +/// baked: `descriptor.py:175` declares the slot `w_fget?`, so the fold owes it +/// a `QUASIIMMUT_FIELD` marker naming the owner. /// /// # Safety /// `w_obj` must be a live object. pub unsafe fn property_get_fast_path( w_obj: PyObjectRef, name: &str, -) -> Option<(PyObjectRef, u64, PyObjectRef)> { +) -> Option<(PyObjectRef, u64, PyObjectRef, PyObjectRef)> { let (w_type, version_tag, w_descr) = unsafe { property_descr_fast_path(w_obj, name) }?; if unsafe { crate::baseobjspace::getattribute_if_not_from_object(w_type) }.is_some() { return None; @@ -1905,22 +1996,22 @@ pub unsafe fn property_get_fast_path( if fget.is_null() || unsafe { pyre_object::pyobject::is_none(fget) } { return None; } - Some((w_type, version_tag, fget)) + Some((w_type, version_tag, w_descr, fget)) } /// STORE_ATTR `property` fast path: the setter twin of -/// [`property_get_fast_path`], returning the type, version tag, and Python -/// `fset` when `obj.name = value` writes a property setter. Returns `None` -/// (leave the residual) for a read-only property or any shape -/// [`property_descr_fast_path`] declines. A custom `__setattr__` owns the write -/// (mapdict.py:1612-1614), so it declines to the residual. +/// [`property_get_fast_path`], returning the type, version tag, the property +/// object, and its Python `fset` when `obj.name = value` writes a property +/// setter. Returns `None` (leave the residual) for a read-only property or any +/// shape [`property_descr_fast_path`] declines. A custom `__setattr__` owns +/// the write (mapdict.py:1612-1614), so it declines to the residual. /// /// # Safety /// `w_obj` must be a live object. pub unsafe fn property_set_fast_path( w_obj: PyObjectRef, name: &str, -) -> Option<(PyObjectRef, u64, PyObjectRef)> { +) -> Option<(PyObjectRef, u64, PyObjectRef, PyObjectRef)> { let (w_type, version_tag, w_descr) = unsafe { property_descr_fast_path(w_obj, name) }?; if unsafe { crate::baseobjspace::setattr_if_not_from_object(w_type) }.is_some() { return None; @@ -1929,7 +2020,7 @@ pub unsafe fn property_set_fast_path( if fset.is_null() || unsafe { pyre_object::pyobject::is_none(fset) } { return None; } - Some((w_type, version_tag, fset)) + Some((w_type, version_tag, w_descr, fset)) } /// The unboxed counterpart of [`load_attr_fast_path`]. It applies the same diff --git a/pyre/pyre-jit-trace/src/descr.rs b/pyre/pyre-jit-trace/src/descr.rs index c275137ffe2..35eca5ee4df 100644 --- a/pyre/pyre-jit-trace/src/descr.rs +++ b/pyre/pyre-jit-trace/src/descr.rs @@ -69,6 +69,20 @@ const HOLDER_TYP_INDEX: u32 = MAPDICT_DESCR_TAG | 3; // `descr.rs`). const AUDIT_HOLDER_HOOKS_INDEX: u32 = MAPDICT_DESCR_TAG | 4; +// `W_Property.fget` / `.fset` are `PyObjectRef` at the first two offsets past +// the object header — the single most crowded coordinate in the tree, since +// every `W_*` class's first reference field lands there. A +// `stable_field_index(offset, size, type, signed)` would therefore name a +// layout, not an owner, and the index is what selects the pointer cast in +// `install_quasiimmut_field` / `register_quasi_immutable_deps`. Reserved for +// the same reason as the map-node block above, in a tag of its own because the +// owner here IS a `PyObject` and the reasoning that groups those four does not +// apply. Disjoint from FIELD (0x10xx_xxxx), ARRAY, SIZE, CELL, MAPDICT, +// `object.typeptr` (0x6000_0000), the native mapdict block, and the GC tid. +const PROPERTY_DESCR_TAG: u32 = 0x5100_0000; +const PROPERTY_FGET_INDEX: u32 = PROPERTY_DESCR_TAG; +const PROPERTY_FSET_INDEX: u32 = PROPERTY_DESCR_TAG | 1; + // The generated native user layouts append mapdict fields at different base // sizes. HeapCache keys by descriptor index; give each translated STRUCT field // the distinct identity provided by descr.py's per-STRUCT cache. @@ -1536,6 +1550,79 @@ static W_METHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { ) }); +/// `pypy/interpreter/function.py:673` / `:720` +/// `_immutable_fields_ = ['w_function?']` for `StaticMethod` and `ClassMethod`. +/// The `?` is what makes the wrapped callable a constant, and it registers the +/// invalidation an assignment owes; pyre's setters do not force that yet, so +/// the field stays LIVE/MUTABLE here and the read is paired with a +/// `GuardValue`, the same pre-invalidation stand-in +/// [`FUNCTION_DESCR_GROUP`] documents for `code?`. +/// +/// Both censuses are COMPLETE — `w_dict` is listed even though nothing reads +/// it, because a field the struct declares but a group omits has no +/// `index_in_parent` to rederive and the two sides that mint its descr then +/// disagree on the number. `PyObject.w_class` is absent because no emit +/// allocates either wrapper, so the analyzer's count is the whole answer. +static W_STATICMETHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { + use pyre_object::function::{ + STATICMETHOD_W_DICT_OFFSET, STATICMETHOD_W_FUNCTION_OFFSET, W_STATICMETHOD_GC_TYPE_ID, + W_STATICMETHOD_OBJECT_SIZE, + }; + let field = |key, offset| { + ( + key, + offset, + std::mem::size_of::(), + Type::Ref, + false, + false, + false, + ) + }; + build_object_descr_group_with_def_path( + W_STATICMETHOD_OBJECT_SIZE, + W_STATICMETHOD_GC_TYPE_ID, + &pyre_object::function::STATICMETHOD_TYPE as *const _ as usize, + &[ + field("w_function", STATICMETHOD_W_FUNCTION_OFFSET), + field("w_dict", STATICMETHOD_W_DICT_OFFSET), + ], + "StaticMethod", + "function::StaticMethod", + ) +}); + +/// The `classmethod` twin of [`W_STATICMETHOD_DESCR_GROUP`]; see it for why +/// `w_function` is mutable and why `w_dict` is listed. +static W_CLASSMETHOD_DESCR_GROUP: LazyLock = LazyLock::new(|| { + use pyre_object::function::{ + CLASSMETHOD_W_DICT_OFFSET, CLASSMETHOD_W_FUNCTION_OFFSET, W_CLASSMETHOD_GC_TYPE_ID, + W_CLASSMETHOD_OBJECT_SIZE, + }; + let field = |key, offset| { + ( + key, + offset, + std::mem::size_of::(), + Type::Ref, + false, + false, + false, + ) + }; + build_object_descr_group_with_def_path( + W_CLASSMETHOD_OBJECT_SIZE, + W_CLASSMETHOD_GC_TYPE_ID, + &pyre_object::function::CLASSMETHOD_TYPE as *const _ as usize, + &[ + field("w_function", CLASSMETHOD_W_FUNCTION_OFFSET), + field("w_dict", CLASSMETHOD_W_DICT_OFFSET), + ], + "ClassMethod", + "function::ClassMethod", + ) +}); + /// `pypy/objspace/std/typeobject.py:26-34 ObjectMutableCell`. The single /// `w_value` field is read LIVE on the module-global cell fast path: a /// frequently-rewritten global mutates the cell payload in place without @@ -2663,6 +2750,19 @@ pub fn method_w_function_descr() -> DescrRef { field_descr_from_group(&W_METHOD_DESCR_GROUP, 0) } +/// Live `StaticMethod.w_function` — the callable a descriptor fold unwraps in +/// place of invoking `__get__`. Read live and pinned by a `GuardValue`; see +/// [`W_STATICMETHOD_DESCR_GROUP`] for why it is not a constant. +pub fn staticmethod_w_function_descr() -> DescrRef { + field_descr_from_group(&W_STATICMETHOD_DESCR_GROUP, 0) +} + +/// Live `ClassMethod.w_function` — the `classmethod` twin of +/// [`staticmethod_w_function_descr`]. +pub fn classmethod_w_function_descr() -> DescrRef { + field_descr_from_group(&W_CLASSMETHOD_DESCR_GROUP, 0) +} + /// Resolve one [`FUNCTION_DESCR_GROUP`] field by byte offset, so the accessors /// below stay correct however the census is ordered. fn function_field_descr(offset: usize) -> DescrRef { @@ -3204,6 +3304,63 @@ pub fn audit_holder_hooks_descr() -> DescrRef { AUDIT_HOLDER_HOOKS_FIELD_DESCR.clone() } +/// `descriptor.py:175 W_Property._immutable_fields_ = ["w_fget?", "w_fset?", +/// "w_fdel?"]` — the property's getter slot. +/// +/// The LOAD_ATTR property fold inlines `fget(obj)` against a descriptor the +/// receiver's class + `_version_tag?` already pin, which makes the descriptor +/// object constant but says nothing about its accessor slots: `__init__` on an +/// installed property replaces them in place and bumps no type's version. The +/// `?` is what covers the slot, and it costs a `QUASIIMMUT_FIELD` marker plus +/// one `GUARD_NOT_INVALIDATED` per trace rather than a load and a `GUARD_VALUE` +/// per iteration — see [`walker_pin_type_version_tag`](crate::jitcode_dispatch) +/// for why the difference is load-bearing across a residual call. +/// +/// A marker only: the fold never loads through the baked descriptor pointer, +/// which is what keeps it clear of the baked-`ConstPtr` hazard that made the +/// inline-call path skip its `Function.code` reads for a constant callable. +static PROPERTY_FGET_FIELD_DESCR: LazyLock = LazyLock::new(|| { + Arc::new( + majit_ir::descr::SimpleFieldDescr::new_with_name( + PROPERTY_FGET_INDEX, + core::mem::offset_of!(pyre_object::descriptor::W_Property, fget), + std::mem::size_of::(), + Type::Ref, + false, + majit_ir::descr::ArrayFlag::Unsigned, + "W_Property.fget".to_string(), + "fget".to_string(), + ) + .with_quasi_immutable(true), + ) +}); + +pub fn property_fget_descr() -> DescrRef { + PROPERTY_FGET_FIELD_DESCR.clone() +} + +/// The `w_fset?` twin of [`PROPERTY_FGET_FIELD_DESCR`], read by the STORE_ATTR +/// property fold. +static PROPERTY_FSET_FIELD_DESCR: LazyLock = LazyLock::new(|| { + Arc::new( + majit_ir::descr::SimpleFieldDescr::new_with_name( + PROPERTY_FSET_INDEX, + core::mem::offset_of!(pyre_object::descriptor::W_Property, fset), + std::mem::size_of::(), + Type::Ref, + false, + majit_ir::descr::ArrayFlag::Unsigned, + "W_Property.fset".to_string(), + "fset".to_string(), + ) + .with_quasi_immutable(true), + ) +}); + +pub fn property_fset_descr() -> DescrRef { + PROPERTY_FSET_FIELD_DESCR.clone() +} + /// `W_ObjectObject` SizeDescr group (`objectobject.rs:34-46`) — the instance /// layout `[ob_type | w_class | map | storage]`. Built with a parent SizeDescr /// (unlike a bare [`make_field_descr`]) so a `getfield_gc` on `map` / `storage` @@ -6753,6 +6910,8 @@ pub(crate) fn publish_runtime_descr_groups() { &*W_ZIP_DESCR_GROUP, &*RANGE_DESCR_GROUP, &*W_METHOD_DESCR_GROUP, + &*W_STATICMETHOD_DESCR_GROUP, + &*W_CLASSMETHOD_DESCR_GROUP, &*W_OBJECT_MUTABLE_CELL_DESCR_GROUP, &*W_LIST_DESCR_GROUP, &*W_TUPLE_DESCR_GROUP, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs index 53309ccc351..655c75af2c8 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs @@ -2366,6 +2366,41 @@ macro_rules! replay_dirty { }}; } +/// The `pc` a `[replay-dirty]` line names is an offset into the scanned +/// callee's jitcode, and no per-function dump covers a callee — so on its own +/// the number cannot be matched against any op. `PYRE_FBW_REPLAY_DIRTY_BODY=1` +/// lists each body as it is scanned, so the verdict line that follows a listing +/// names an op within it. +fn replay_safety_dump_body(body_code: &[u8], callee_descr_refs: &[DescrRef]) { + if !fbw_inline_diag_enabled() || std::env::var_os("PYRE_FBW_REPLAY_DIRTY_BODY").is_none() { + return; + } + eprintln!( + "[replay-dirty-body] === scanning body len={} ===", + body_code.len() + ); + for d in crate::jitcode_runtime::decoded_ops(body_code) { + // Every residual verdict below turns on the helper kind, and the opname + // alone does not separate a deferred `call_fn` from an untagged helper + // that declines the whole body — so name it. + let helper = if d.opname.starts_with("residual_call") { + residual_call_descr_index_in_body(body_code, &d) + .and_then(|i| callee_descr_refs.get(i)) + .and_then(|descr| descr.as_call_descr()) + .map_or_else( + || " helper=".to_string(), + |cd| format!(" helper={:?}", cd.get_extra_info().pyre_helper), + ) + } else { + String::new() + }; + eprintln!( + "[replay-dirty-body] pc={:>4} {}/{}{}", + d.pc, d.opname, d.argcodes, helper + ); + } +} + pub(crate) fn fbw_callee_body_replay_safety( body_code: &[u8], exact_numeric_args: &[ExactNumericArg], @@ -2376,6 +2411,7 @@ pub(crate) fn fbw_callee_body_replay_safety( callee_descr_refs: &[DescrRef], method_form_deferred_helpers: bool, ) -> CalleeReplaySafety { + replay_safety_dump_body(body_code, callee_descr_refs); let Some(branch_targets) = body_branch_targets(body_code) else { replay_dirty!("BranchTargetsUndecodable", 0, "-"); }; @@ -2547,6 +2583,12 @@ pub(crate) fn fbw_callee_body_replay_safety( // reads the same value again. Its writing twin // `SetCurrentException` is not here — it is journalled, and so // reaches the `deferred_call` arm below instead. + // `load_deref` is that same shape once more, and it is the one every + // closure body carries: `bh_load_deref_value_fn` dereferences a cell + // and returns its contents, writing nothing, so a replay reads the + // same cell again. Its raise on an unbound free variable is no + // barrier — `load_global` above raises `NameError` too, and a replay + // raises the same one. Its writing twin `StoreDeref` is not here. let replay_safe_read = matches!( ei.pyre_helper, majit_ir::PyreHelperKind::LoadConst diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 3059d98e4e8..2c870449348 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -1587,11 +1587,42 @@ pub(crate) fn emit_walker_loop_callee_call_assembler( /// pure-leaf callee resume to the caller's CALL boundary via the inherited /// single-frame snapshot (`entry_py_pc` / `outer_active_boxes`), which is /// sound for side-effect-free leaves (re-execute the whole call on deopt). +/// +/// That layout is a property of a subset of the CALL-family helpers, so the +/// residual this reads from has to be one of them. Every other entry the +/// inline lever serves passes its own receiver-plus-metadata list, not an +/// operand-stack image: `load_attr_fn(obj, code, name_idx)` is +/// `r_args = [obj, code]` and `store_attr_fn` is `[obj, value, code]`, whose +/// `code` operand is a code object the Python stack never held. Publishing it +/// as a stack slot resumes the interpreter with the code object where the +/// receiver belongs — the `__getattr__`/`property` folds returned +/// `AttributeError: 'code' object has no attribute ` for an attribute +/// their own hook answers. Decline for those instead; their operand image +/// comes from the per-slot resume sources +/// ([`reconstructed_call_stack_from_resume_sources`]). +/// +/// `call_kw` is excluded for the same reason one step subtler: its list is a +/// PERMUTATION of the stack rather than a different set of values. The wire +/// order is `(callable, null_or_self, kwnames, arg0..arg{n-1})` +/// (`majit-ir effectinfo.rs` `PyreHelperKind::CallKw`), while `CALL_KW` pops +/// `kwnames` FIRST (`eval.rs call_kw`), so the stack image is +/// `[callable, null_or_self, arg0..arg{n-1}, kwnames]`. A real `CALL_KW` +/// always carries a non-empty kwnames tuple, so `n >= 1` on every reachable +/// path and the two orders never coincide. The flush's only structural check +/// is a depth compare, which a permutation of the right length passes, and the +/// re-executed `CALL_KW` would then pop `arg{n-1}` as its keyword-name tuple. pub(crate) fn reconstructed_all_ref_call_stack( code: &[u8], op: &DecodedOp, ctx: &WalkContext<'_, '_, Sym>, + call_descr: &dyn majit_ir::descr::CallDescr, ) -> Option> { + if !matches!( + call_descr.get_extra_info().pyre_helper, + majit_ir::PyreHelperKind::CallFn | majit_ir::PyreHelperKind::CallFunctionEx + ) { + return None; + } // The Ref list is NOT at a fixed offset: the method-form `CALL` helpers // this leg latches for lower through the mixed `iIRd>r` shape, whose // leading Int list shifts it (`dispatch_residual_call_iIRd_kind` reads it @@ -1629,8 +1660,8 @@ pub(crate) fn reconstructed_all_ref_call_stack( // Only `null_or_self@1` may be null, and the layout above names it by // index, so it is checked by position rather than by admitting a null // anywhere. Everything else here is a Python value the rewound `CALL` - // pops — an argument, or `kwnames` in the `call_kw` layout — and a null in - // one of those slots is an UNRESOLVED register, not a value: the concrete + // pops, and a null in one of those slots is an UNRESOLVED register, not a + // value: the concrete // Ref bank holds `Ref(null)` for a box the walk never materialized, which // is why `concrete_ref_for_color` tests for it and why the prefix loop // above declines on it. Publishing one lets the resumed interpreter pop a @@ -2007,7 +2038,7 @@ pub(crate) fn try_walker_inline_user_call( } if fbw_inline_diag_enabled() { eprintln!( - "[inline-entry] pc={} helper={:?} nrefargs={} subwalk={}", + "[inline-entry] pc={} helper={:?} nrefargs={} subwalk={} dst_bank={dst_bank}", op.pc, pyre_helper, r_args.len(), @@ -3002,6 +3033,7 @@ fn latch_abort_call_resume( code: &[u8], op: &DecodedOp, ctx: &WalkContext<'_, '_, Sym>, + call_descr: &dyn majit_ir::descr::CallDescr, is_top_inline: bool, unjournaled_before_subwalk: bool, executed_effects_before: usize, @@ -3016,7 +3048,7 @@ fn latch_abort_call_resume( let Some((outer_jitcode_index, call_jitcode_pc)) = abort_flush_call_jitcode_coord else { return; }; - if let Some(stack) = reconstructed_all_ref_call_stack(code, op, ctx) { + if let Some(stack) = reconstructed_all_ref_call_stack(code, op, ctx, call_descr) { fbw_set_abort_call_resume(outer_jitcode_index, call_jitcode_pc, stack); } } @@ -3103,6 +3135,19 @@ pub(crate) fn try_walker_inline_resolved_user_call( /// user call nested inside a specialized builtin: it deliberately leaves the /// outer residual's destination untouched, so guards still snapshot the /// caller at the builtin-call boundary. +/// Report which decline in [`try_walker_inline_resolved_user_call_inner`] a +/// call hit. The function has three dozen of them and reaches them from every +/// call shape, so a caller that only learns "declined" has to guess; the +/// `[binop-inline-decline]` and `[type-call-decline]` lines above name the +/// call, and this names the test inside it that refused. +#[inline] +fn resolved_inline_decline(op_pc: usize, line: u32) -> Result, DispatchError> { + if fbw_inline_diag_enabled() { + eprintln!("[resolved-inline-decline] pc={op_pc} inline_call.rs:{line}"); + } + Ok(None) +} + #[allow(clippy::too_many_arguments)] fn try_walker_inline_resolved_user_call_inner( ctx: &mut WalkContext<'_, '_, Sym>, @@ -3140,7 +3185,42 @@ fn try_walker_inline_resolved_user_call_inner( let positional_only = fbw_callee_scope_is_positional_only(w_code); let vararg_slot = fbw_callee_vararg_slot(w_code); if !positional_only && vararg_slot.is_none() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); + } + // Not every caller pins the callee function itself. A specializer that + // resolves an app-level method behind a builtin — `str(e)` reaching an + // exception subclass's `__str__` — passes the CALL's own operand, which is + // the `str` builtin, while `callable` is the resolved `Function`. Reading + // `Function.code` off that operand is a type-confused load: it returns + // whatever sits at the same offset in a `PyCFunction`, so the guard + // compares a value that is not `code` and fails every iteration (99480 + // failures and 497 bridges on `synth/exception_subclass_attrs`, a 31x + // slowdown). Guard the fields only when the pinned object really is the + // function whose code this inline resolved. + let pinned_object_is_the_callee = unsafe { + (*callable_guard_value).ob_type as *const () as usize + == &pyre_interpreter::FUNCTION_TYPE as *const _ as usize + && pyre_interpreter::function_get_code(callable_guard_value) as usize + == w_code as pyre_object::PyObjectRef as usize + }; + // A trace-constant callable is excluded for a second reason: the field + // reads would dereference a baked `ConstPtr`, and loading through one + // dangles as soon as a minor collection moves the object + // (`synth/inline_subwalk_property_mutates` — a property getter that + // allocates on every iteration — segfaulted on cranelift under CI's macOS + // runner with the reads in place). Comparing against such a constant is + // fine; that is why the `code?` marker below covers this arm instead. + let guards_the_callee_function = + !callable_guard_op.is_constant() && pinned_object_is_the_callee; + if !guards_the_callee_function && majit_gc::can_move(majit_ir::GcRef(callable as usize)) { + // The arm below stands the baked code up on `function.py:47`'s `code?` + // instead of a per-iteration guard, and the marker names its owner by + // raw address at both record and compile time. The jitcode + // `MAKE_FUNCTION` lowering allocates its function in the nursery, so + // such a callee can be relocated between those two reads; refuse the + // inline rather than bake a body no invalidation covers. `rgc.can_move` + // parity — false when no moving GC is active. + return resolved_inline_decline(op.pc, line!()); } // `Function.funccall_valuestack` fills every parameter the call left // unbound from `defs_w` before entering the frame @@ -3164,7 +3244,7 @@ fn try_walker_inline_resolved_user_call_inner( let Some(defaults) = (unsafe { positional_defaults_for_inline(callable, &missing, nparams) }) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; callee_args.resize(nparams, OpRef::NONE); callee_arg_concretes.resize(nparams, ConcreteValue::Null); @@ -3186,22 +3266,22 @@ fn try_walker_inline_resolved_user_call_inner( // below; folding the placeholder into the tuple would put the Method // object where the receiver belongs. Decline that one shape. if bound_method.is_some() && nparams == 0 { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } if callee_arg_concretes.len() != callee_args.len() || callee_args.len() <= nparams { // The empty tuple is a runtime singleton (`() is tuple([])`), so a // freshly allocated walker tuple would not be the object the // interpreter installs for a zero-surplus call. - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let surplus_ops: Vec = callee_args[nparams..].to_vec(); let mut surplus_concretes = Vec::with_capacity(surplus_ops.len()); for concrete in &callee_arg_concretes[nparams..] { let ConcreteValue::Ref(obj) = *concrete else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; if obj.is_null() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } surplus_concretes.push(obj); } @@ -3209,7 +3289,7 @@ fn try_walker_inline_resolved_user_call_inner( // same constructor `emit_object_tuple_inline` reproduces. let concrete = pyre_object::w_tuple_new_array_backed(surplus_concretes); if concrete.is_null() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } callee_args.truncate(nparams); callee_arg_concretes.truncate(nparams); @@ -3245,38 +3325,38 @@ fn try_walker_inline_resolved_user_call_inner( // needs fresh cell allocation and stays residual until that constructor // half is ported too. if callee_args.len() != seeded_locals { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let raw_callee_code = unsafe { pyre_interpreter::w_code_get_ptr(w_code as pyre_object::PyObjectRef) as *const pyre_interpreter::CodeObject }; if raw_callee_code.is_null() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let callee_code = unsafe { &*raw_callee_code }; let mut concrete_freevar_cells = Vec::new(); let concrete_closure = if has_closure { if !callee_code.cellvars.is_empty() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let closure = unsafe { pyre_interpreter::function_get_closure(callable) }; if closure.is_null() || !unsafe { pyre_object::is_tuple(closure) } || unsafe { pyre_object::w_tuple_len(closure) } != callee_code.freevars.len() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } for i in 0..callee_code.freevars.len() { let Some(cell) = (unsafe { pyre_object::w_tuple_getitem(closure, i as i64) }) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; concrete_freevar_cells.push(cell); } closure } else { if !callee_code.freevars.is_empty() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } pyre_object::PY_NULL }; @@ -3294,7 +3374,7 @@ fn try_walker_inline_resolved_user_call_inner( .warm_state_mut() .can_inline_callable(callee_green_key) { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } if fbw_inline_recursion_count(ctx, callee_code_key) >= FBW_MAX_INLINE_RECURSION { if let Some((driver, _)) = crate::driver::try_driver_pair() { @@ -3303,13 +3383,13 @@ fn try_walker_inline_resolved_user_call_inner( .warm_state_mut() .disable_noninlinable_function(callee_green_key); } - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let Some(body) = crate::state::sub_jitcode_body_for_code(w_code) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; if nparams > body.num_regs_r { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // Inlining a callee whose body carries an `abort_permanent` marker walks // the sub-walk straight into it. That surfaces as @@ -3336,10 +3416,10 @@ fn try_walker_inline_resolved_user_call_inner( // means no installed body or descr pool, which the pool fetch immediately // below declines on regardless. let Some(body_facts) = sub_jitcode_body_facts_for_code(w_code) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; if body_facts.has_abort_permanent { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // The callee body resolves its `d`/`j` descr operands through its OWN // per-fn pool, not the caller's. Without this the sub-walk reads the @@ -3348,7 +3428,7 @@ fn try_walker_inline_resolved_user_call_inner( let Some((callee_descr_refs, callee_perfn_descrs, callee_lookup)) = crate::state::sub_jitcode_descr_pool_for_code(w_code) else { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); }; // EXACT int/float only. These feed `fbw_callee_body_replay_safety`, whose // question is "will the walker specialize this body's BINARY_OP to a native @@ -3451,7 +3531,7 @@ fn try_walker_inline_resolved_user_call_inner( let subwalk_admit = ctx.fbw_mode.carrier_resume && !ctx.fbw_mode.snapshot_sym.is_null(); let safe_root_bridge = root_bridge || subwalk_admit; if !safe_root_bridge { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } bridge_rec_root_selfrec = unsafe { let raw = pyre_interpreter::w_code_get_ptr(w_code as pyre_object::PyObjectRef) @@ -3472,7 +3552,7 @@ fn try_walker_inline_resolved_user_call_inner( // `SELFREC_CA_FOLD_ACTIVE` exemption from the hazard arm (:2696), so its // recursive residual is not what named the callee here. if !bridge_rec_root_selfrec && fbw_hazardous_inline_denied(callee_code_key) { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // A legacy, unseeded inline sub-walk inside a FOR_ITER body resumes a guard // at the caller's CALL boundary, so deopt re-executes the whole callee. @@ -3571,6 +3651,16 @@ fn try_walker_inline_resolved_user_call_inner( && !pyre_interpreter::code_has_for_iter(callee_code) && !body_facts.has_exception_table && !fbw_foriter_deferred_call_denied(callee_code_key); + if !foriter_deferred_admit && fbw_inline_diag_enabled() { + eprintln!( + "[inline-foriter-deferred] pc={} boundary={entry_is_call_boundary} \ + header={loop_header_admitted} for_iter={} exc_table={} denied={}", + op.pc, + pyre_interpreter::code_has_for_iter(callee_code), + body_facts.has_exception_table, + fbw_foriter_deferred_call_denied(callee_code_key), + ); + } foriter_deferred_admit } CalleeReplaySafety::Dirty => { @@ -3599,7 +3689,7 @@ fn try_walker_inline_resolved_user_call_inner( // source handles it. Stored bound methods instead take the explicit // multi-frame red-frame path above. if !legacy_admit { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } } // A widened method-form body that also raises was declined here until the @@ -3617,7 +3707,7 @@ fn try_walker_inline_resolved_user_call_inner( // 15.3 once admitted, and `self.i >= self.n` 1207 -> 15.9. Swapping that // `raise` for a `return` already measured 17.7, which is what named the // token rather than the branch or the attribute compare. - if std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() { + if fbw_inline_diag_enabled() { let mut pc = 0usize; let mut shown = 0; while pc < body.code.len() && shown < 8 { @@ -3670,7 +3760,7 @@ fn try_walker_inline_resolved_user_call_inner( // and re-runs the instantiation, making the result discard unnecessary to // represent. if constructor_result.is_some() && !strict_inlinable { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // A zero-param callee has no positional argument to seed, so the register // convention above holds vacuously and the strict path serves it like any @@ -3681,7 +3771,7 @@ fn try_walker_inline_resolved_user_call_inner( // body still takes the residual rather than the decline-to-interpretation // below. if nparams == 0 && !strict_inlinable { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // A self-recursive callee unrolls until its own frame count reaches @@ -3751,7 +3841,7 @@ fn try_walker_inline_resolved_user_call_inner( "InlineCallee::BranchyHandlerDirty" }, ); - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // A callee that raises inline needs the cross-frame bridge the carrier // drain builds once a guard inside the compiled chain fails. The drain @@ -3820,7 +3910,7 @@ fn try_walker_inline_resolved_user_call_inner( None }; if foriter_dirty_bound && !try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } if !strict_inlinable && !try_multiframe && !force_caller_boundary_resume { // A non-self-recursive loop/branch callee that neither the strict nor @@ -3833,13 +3923,13 @@ fn try_walker_inline_resolved_user_call_inner( // cache; making an uninlineable method body blacklist the whole outer // loop turns a correct specialization into a compile regression. if method_form { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // Full-portal cutover: instead of poisoning the trace, fall through to // the CALL_ASSEMBLER fold (`try_walker_call_assembler_self_recursive`, // reached next in the residual-call dispatch) so a recursive callee at // the inline cap enters via its own (possibly tmp-callback) loop token. - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } let mut callable_guard_op = callable_guard_op; @@ -3935,35 +4025,6 @@ fn try_walker_inline_resolved_user_call_inner( } } - // Not every caller pins the callee function itself. A specializer that - // resolves an app-level method behind a builtin — `str(e)` reaching an - // exception subclass's `__str__` — passes the CALL's own operand, which is - // the `str` builtin, while `callable` is the resolved `Function`. Reading - // `Function.code` off that operand is a type-confused load: it returns - // whatever sits at the same offset in a `PyCFunction`, so the guard - // compares a value that is not `code` and fails every iteration (99480 - // failures and 497 bridges on `synth/exception_subclass_attrs`, a 31x - // slowdown). Guard the fields only when the pinned object really is the - // function whose code this inline resolved. - // - // A trace-constant callable is excluded for a second reason: the field - // reads would dereference a baked `ConstPtr`, and a baked constant object - // pointer is not GC-forwarded yet (gh #108 gc-table — see the note in - // `synth/exception_subclass_attrs.py`). Comparing against such a constant - // is fine, but loading through one dangles as soon as a minor collection - // moves the object: `synth/inline_subwalk_property_mutates` — a property - // getter that allocates on every iteration — segfaults on cranelift under - // CI's macOS runner with the reads in place. The callable being constant - // means something already pinned the object, so this only gives up the - // `f.__code__ = g.__code__` re-check on that path. - let guards_the_callee_function = !callable_guard_op.is_constant() - && unsafe { - (*callable_guard_value).ob_type as *const () as usize - == &pyre_interpreter::FUNCTION_TYPE as *const _ as usize - && pyre_interpreter::function_get_code(callable_guard_value) as usize - == callee_code_key - }; - // Keep the closure cells as red operands. A MAKE_FUNCTION in the caller's // loop creates a fresh function and fresh enclosing cells on every // iteration; the trace-time cell pointers are only the concrete shadow @@ -3989,6 +4050,16 @@ fn try_walker_inline_resolved_user_call_inner( .record_guard(OpCode::GuardValue, &[callable_guard_op, expected], 0); walker_capture_snapshot_for_last_guard(ctx, op.pc)?; } + // Pinning the operand pins none of the callee's fields, and this inline + // bakes `code` in the strongest form there is — it selects which callee + // body the trace walks into. `function.py:47 _immutable_fields_ = + // ['code?', ...]` is what covers that, and the `?` costs one marker + // plus one `GUARD_NOT_INVALIDATED` per trace instead of a load and a + // `GUARD_VALUE` per iteration. It is also the only form available + // here: the guard arm below reads the field off the pinned operand, + // which this arm either cannot do (the operand is not the callee) or + // must not do (a baked `ConstPtr`). + walker_pin_function_code(ctx, op.pc, callable)?; } else { // `function.py:91-96 getcode()` promotes `self.code`, never `self`. // The code object below and globals namespace in `InlineCalleeConsts` @@ -4003,7 +4074,11 @@ fn try_walker_inline_resolved_user_call_inner( // `opimpl_getfield_gc_r` pairs each read with `record_quasiimmut_field` // and assigning the field invalidates the traces that folded it; the // value guards stay on top as the stricter identity check the reads - // below assume. + // below assume. They also stay because this arm exists for a callee + // whose identity changes every iteration, so the field is re-read + // anyway, and because the marker resolves its owner by raw address — + // the guard is the only answer that keeps working for a callee the + // collector can relocate. // // Guarding the function OBJECT instead pinned its identity, which a // callee built by a `MAKE_FUNCTION` in the caller's own loop body can @@ -4239,7 +4314,7 @@ fn try_walker_inline_resolved_user_call_inner( None => i, }; if reg >= callee_regs_r.len() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } callee_regs_r[reg] = callee_args[i]; callee_concrete_r[reg] = callee_arg_concretes[i]; @@ -4338,7 +4413,7 @@ fn try_walker_inline_resolved_user_call_inner( // `strict_seed` already excludes such a callee, so only the // multiframe path reaches this. if !callee_code.cellvars.is_empty() { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // POP_JUMP_IF_NONE / POP_JUMP_IF_NOT_NONE lower to an `is`/`is_not` // identity residual call whose operands must be Ref (the codewriter @@ -4407,7 +4482,7 @@ fn try_walker_inline_resolved_user_call_inner( }); if has_is_none_branch { if try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } seed_break_reason = "Collapse::IsNoneBranch"; break 'seed; @@ -4421,7 +4496,7 @@ fn try_walker_inline_resolved_user_call_inner( crate::state::ensure_jitcode_index(callee_code_key as *const ()) else { if try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } seed_break_reason = "Collapse::NoCalleeJitcode"; break 'seed; @@ -4433,7 +4508,7 @@ fn try_walker_inline_resolved_user_call_inner( || ec_reg as usize >= callee_regs_r.len() { if try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } seed_break_reason = "Collapse::NoPortalRedRegs"; break 'seed; @@ -4449,7 +4524,7 @@ fn try_walker_inline_resolved_user_call_inner( let sym_ptr = ctx.fbw_mode.snapshot_sym; if sym_ptr.is_null() { if try_multiframe { - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } seed_break_reason = "Collapse::NoSnapshotSym"; break 'seed; @@ -5246,7 +5321,7 @@ fn try_walker_inline_resolved_user_call_inner( let (outcome, _end_pc) = match callee_outcome { Ok(v) => v, Err(e) => { - if std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() { + if fbw_inline_diag_enabled() { eprintln!("[inline-abort] callee sub-walk err: {e:?}"); } // gh#467: a supported abort fired inside this top-level inline @@ -5284,6 +5359,7 @@ fn try_walker_inline_resolved_user_call_inner( code, op, ctx, + call_descr, is_top_inline, unjournaled_before_subwalk, executed_effects_before, @@ -5314,6 +5390,7 @@ fn try_walker_inline_resolved_user_call_inner( code, op, ctx, + call_descr, is_top_inline, unjournaled_before_subwalk, executed_effects_before, @@ -5338,7 +5415,7 @@ fn try_walker_inline_resolved_user_call_inner( // observes and changes nothing (`exc_override_sample_safe`), // and it keeps a legal program from killing the enclosing // loop's trace, which `callee_inline_unsupported` would. - return Ok(None); + return resolved_inline_decline(op.pc, line!()); } // `descr_call` discards `__init__`'s result after checking it is // None and returns the instance instead (`check_init_returned_none`). @@ -5357,6 +5434,7 @@ fn try_walker_inline_resolved_user_call_inner( code, op, ctx, + call_descr, is_top_inline, unjournaled_before_subwalk, executed_effects_before, @@ -5385,6 +5463,7 @@ fn try_walker_inline_resolved_user_call_inner( code, op, ctx, + call_descr, is_top_inline, unjournaled_before_subwalk, executed_effects_before, @@ -5467,8 +5546,8 @@ fn try_walker_inline_resolved_user_call_inner( // pre-subwalk count so it can source the preferred rebuild // while the fallback rewind remains provably disabled. if let Some((outer_jitcode_index, call_jitcode_pc)) = abort_flush_call_jitcode_coord - && let Some(stack) = - reconstructed_all_ref_call_stack(code, op, ctx).or_else(|| { + && let Some(stack) = reconstructed_all_ref_call_stack(code, op, ctx, call_descr) + .or_else(|| { reconstructed_call_stack_from_resume_sources(ctx, call_jitcode_pc) }) { @@ -5500,7 +5579,14 @@ fn try_walker_inline_resolved_user_call_inner( } } -/// Route `str(exc)` / `repr(exc)` through an app-level exception override. +/// Report why the instantiation emit declined, under `PYRE_FBW_INLINE_DIAG`. +fn type_call_decline(reason: &str) -> Result, DispatchError> { + if fbw_inline_diag_enabled() { + eprintln!("[type-call-decline] {reason}"); + } + Ok(None) +} + /// Instantiate a user-defined class inside the trace instead of leaving `P()` /// an opaque `bh_call_fn` residual that re-enters `type_descr_call_impl`, /// `object.__new__` and an interpreted `__init__` frame every iteration. @@ -5518,14 +5604,6 @@ fn try_walker_inline_resolved_user_call_inner( /// `new_with_vtable` is a virtual, so a constructor whose result never escapes /// the loop optimizes away entirely, as it does upstream. #[allow(clippy::too_many_arguments)] -/// Report why the instantiation emit declined, under `PYRE_FBW_INLINE_DIAG`. -fn type_call_decline(reason: &str) -> Result, DispatchError> { - if std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() { - eprintln!("[type-call-decline] {reason}"); - } - Ok(None) -} - pub(crate) fn try_walker_inline_type_call( ctx: &mut WalkContext<'_, '_, Sym>, op: &DecodedOp, @@ -5537,15 +5615,44 @@ pub(crate) fn try_walker_inline_type_call( dst: usize, ) -> Result, DispatchError> { if !ctx.is_authoritative_executor || ctx.fbw_mode.inline_subwalk || dst_bank != 'r' { + // These three reject far more calls than the instantiations this emit is + // about, so name the reason only for a call that does resolve to a + // class, and only while the reasons are being collected — the extra + // resolution below is diagnostic cost, not tracing cost. + if fbw_inline_diag_enabled() + && r_args.len() >= 2 + && walker_concrete_ref_object(ctx, r_args[1]).is_none() + && walker_concrete_ref_object(ctx, r_args[0]) + .is_some_and(|w_type| unsafe { pyre_object::is_type(w_type) }) + { + return type_call_decline(if !ctx.is_authoritative_executor { + "not the authoritative executor" + } else if ctx.fbw_mode.inline_subwalk { + "inline sub-walk" + } else { + "destination is not a ref register" + }); + } return Ok(None); } // `[callable, null_or_self, args...]`. A method-form call (`null_or_self` // populated) never names a class as its callable. - if r_args.len() < 2 || walker_concrete_ref_object(ctx, r_args[1]).is_some() { + if r_args.len() < 2 { return Ok(None); } + // "No receiver" has two spellings in that slot: `call_kw` leaves it with no + // concrete shadow at all, `call_fn` fills it with the checked `PY_NULL` + // sentinel. Reading a present shadow as a receiver rejects the whole + // `call_fn` spelling, which is the one an ordinary `C(...)` lowers to. + if walker_concrete_ref_object(ctx, r_args[1]) + .is_some_and(|null_or_self| !null_or_self.is_null() && null_or_self != pyre_object::PY_NULL) + { + return type_call_decline("receiver slot is populated"); + } let Some(w_type) = walker_concrete_ref_object(ctx, r_args[0]) else { - return Ok(None); + // Whether this even was an instantiation is unknowable without the + // callable, so the reason is reported as the open question it is. + return type_call_decline("callable is not a concrete ref"); }; if !unsafe { pyre_object::is_type(w_type) } { return Ok(None); @@ -5558,9 +5665,33 @@ pub(crate) fn try_walker_inline_type_call( } // What follows is `type.__call__`. A metaclass that overrides `__call__` // runs instead of it and may return anything at all, so it stays residual. - if !std::ptr::eq(unsafe { (*w_type).w_class }, w_metatype) { - return type_call_decline("metaclass overrides __call__"); - } + // + // The question is which `__call__` the metatype resolves to, not whether it + // is `type` itself: `ABCMeta` supplies `__instancecheck__`, + // `__subclasscheck__` and `register` and leaves `__call__` alone, so every + // class that registers with a `numbers` / `collections.abc` ABC — which is + // every `Fraction`, `Decimal` and `deque` construction — resolves to the + // same `type.__call__` a plain class does. Comparing the metatype's + // identity refused all of them. + let w_metaclass = unsafe { (*w_type).w_class }; + let metaclass_to_pin = if std::ptr::eq(w_metaclass, w_metatype) { + None + } else { + let meta_call = + unsafe { pyre_interpreter::baseobjspace::lookup_in_type(w_metaclass, "__call__") }; + let type_call = + unsafe { pyre_interpreter::baseobjspace::lookup_in_type(w_metatype, "__call__") }; + if meta_call != type_call { + return type_call_decline("metaclass overrides __call__"); + } + // The answer above is a dict lookup, so it needs the same pin the + // `__new__` / `__init__` answers get. A metaclass whose dict changes + // are untracked cannot supply one. + if unsafe { pyre_object::typeobject::w_type_get_version_tag(w_metaclass) } == 0 { + return type_call_decline("metaclass has no version tag"); + } + Some(w_metaclass) + }; // A version tag of 0 is a type whose dict changes are not tracked, so the // `__new__` / `__init__` / `__del__` lookups below cannot be pinned. let version_tag = unsafe { pyre_object::typeobject::w_type_get_version_tag(w_type) }; @@ -5586,7 +5717,7 @@ pub(crate) fn try_walker_inline_type_call( } let w_object = pyre_interpreter::typedef::w_object(); if w_object.is_null() { - return Ok(None); + return type_call_decline("object type unavailable"); } // Only `object.__new__` allocates the plain `[ob_type | w_class | map | // storage]` instance this emit builds; any other `__new__` picks its own @@ -5616,9 +5747,9 @@ pub(crate) fn try_walker_inline_type_call( let mut arg_concretes = vec![ConcreteValue::Ref(w_type), ConcreteValue::Null]; let mut callee_arg_concretes = Vec::with_capacity(r_args.len() - 1); - for &arg in &r_args[2..] { + for (i, &arg) in r_args[2..].iter().enumerate() { let Some(concrete) = walker_concrete_ref_object(ctx, arg) else { - return Ok(None); + return type_call_decline(&format!("argument {i} is not a concrete ref")); }; arg_concretes.push(ConcreteValue::Ref(concrete)); callee_arg_concretes.push(ConcreteValue::Ref(concrete)); @@ -5638,6 +5769,12 @@ pub(crate) fn try_walker_inline_type_call( .heap_cache_mut() .replace_box(r_args[0], type_const); walker_pin_type_version_tag(ctx, op.pc, type_const)?; + // A metaclass that does not override `__call__` today can be given one, and + // that changes its own version tag rather than the class's. + if let Some(w_metaclass) = metaclass_to_pin { + let metaclass_const = ctx.trace_ctx.const_ref(w_metaclass as i64); + walker_pin_type_version_tag(ctx, op.pc, metaclass_const)?; + } // The walker is the executor here, so the instance the rest of this walk // reads has to be a real one — the same split `trace_box_int` makes between @@ -5660,7 +5797,7 @@ pub(crate) fn try_walker_inline_type_call( instance, &pyre_object::pyobject::INSTANCE_TYPE as *const _ as i64, ); - if std::env::var("PYRE_FBW_INLINE_DIAG").is_ok() { + if fbw_inline_diag_enabled() { eprintln!( "[type-call-inline] pc={} class={} init={}", op.pc, @@ -5713,12 +5850,27 @@ pub(crate) fn try_walker_inline_type_call( Some((instance, ConcreteValue::Ref(concrete_instance))), )?; if inlined.is_none() { + // The `[type-call-inline]` line above is printed before this sub-walk is + // attempted, because that is where the class and the `init` shape are + // known — so on its own it reports that the fold *began*, not that it + // stood. Say so when it does not: without this line the diagnostic + // reads as a successful fold on a trace that ends up carrying the whole + // instantiation as a residual. + if fbw_inline_diag_enabled() { + eprintln!( + "[type-call-rewind] pc={} class={} why=__init__ sub-walk declined", + op.pc, + unsafe { pyre_object::w_type_get_name(w_type) }, + ); + } ctx.trace_ctx.cut_trace(pre_fold_pos); ctx.trace_ctx.heap_cache_mut().reset(); } Ok(inlined) } +/// Route `str(exc)` / `repr(exc)` through an app-level exception override. +/// /// Pyre's exact `str` type call follows `str_descr_new` → `builtin_str` → /// `exc_user_dunder_obj`; the builtin `repr` follows `builtin_repr` → /// `py_repr_obj`. Both paths look up the receiver dunder before builtin @@ -6126,7 +6278,7 @@ pub(crate) fn try_walker_inline_property_get( let Some(name) = walker_load_name_from_code(w_code_ptr, name_idx) else { return Ok(None); }; - let Some((w_type, version_tag, fget)) = (unsafe { + let Some((w_type, version_tag, w_descr, fget)) = (unsafe { pyre_interpreter::objspace::std::mapdict::property_get_fast_path(concrete_obj, &name) }) else { return Ok(None); @@ -6156,7 +6308,12 @@ pub(crate) fn try_walker_inline_property_get( ConcreteValue::Ref(concrete_obj), ]; let fget_const = ctx.trace_ctx.const_ref(fget as i64); - try_walker_inline_resolved_user_call( + // Everything below emits, and the callee inline has decline paths of its + // own past this point, so keep a rewind point the way the type-call fold + // does. + let pre_fold_pos = ctx.trace_ctx.get_trace_position(); + walker_pin_property_accessor(ctx, op.pc, w_descr, crate::descr::property_fget_descr())?; + let inlined = try_walker_inline_resolved_user_call( ctx, op, code, @@ -6186,7 +6343,227 @@ pub(crate) fn try_walker_inline_property_get( // read (same allowance the exception `__str__`/`__repr__` override uses). false, None, - ) + )?; + if inlined.is_none() { + ctx.trace_ctx.cut_trace(pre_fold_pos); + ctx.trace_ctx.heap_cache_mut().reset(); + } + Ok(inlined) +} + +/// Inline the receiver type's `__getattr__` hook for an attribute the type and +/// the instance both lack — the miss twin of [`try_walker_inline_property_get`]. +/// +/// `descroperation.py:242-245` reaches the hook only after the descriptor +/// protocol has raised, so pyre runs the whole `object_getattr_miss` walk (the +/// `__dict__` / `__doc__` / `__class__` special names, the metaclass loops, the +/// terminal miss) and then a fresh interpreter frame for the hook, on every +/// access, behind one opaque `CALL_MAY_FORCE`. PyPy traces through all of it, +/// so the miss const-folds and only the hook body is left. +/// [`getattr_hook_fast_path`](pyre_interpreter::objspace::std::mapdict::getattr_hook_fast_path) +/// is the oracle for that fold: the version-tag and map pins it asks for are +/// what make the miss a compile-time answer. +/// +/// All three spellings `get_and_call_function` binds are folded, because the +/// version-tag pin makes the binding decision itself constant: a plain +/// `Function` takes `funccall(w_obj, w_name)`, a `classmethod` is entered with +/// the class the descriptor would bind, and a `staticmethod` with the name +/// alone. A custom-descriptor hook stays on the residual. +/// +/// The name argument is an interned immortal block rather than the fresh +/// `w_str_new` the residual path allocates per access, which is the shape +/// `pyopcode.py LOAD_ATTR` passes (`space.getattr(w_obj, w_name)` hands over +/// `co_names_w[oparg]`, one object for the life of the code object). +/// +/// A branching, raising body is admitted: a hook that raises `AttributeError` +/// for an unknown name is the shape worth inlining, not an edge case. Same +/// loop-header and top-frame restrictions as the sibling routes; every other +/// shape declines to the residual (SAFE — no acceleration, unchanged +/// semantics). +/// The leading argument `get_and_call_function` binds ahead of the attribute +/// name, one variant per descriptor spelling of a `__getattr__` hook. +enum HookLeading { + /// Plain `Function`: `funccall(w_obj, w_name)` leads with the receiver. + Receiver, + /// `ClassMethod.__get__` leads with the class. + Class, + /// `StaticMethod.__get__` binds nothing; the name is the only argument. + None, +} + +/// The wrapper slot a descriptor spelling of the hook unwrapped, so the fold +/// can pin the value it read. `function.py:673`/`:720` +/// `_immutable_fields_ = ['w_function?']`. +enum WrapperField { + ClassMethod, + StaticMethod, +} + +impl WrapperField { + fn descr(&self) -> majit_ir::DescrRef { + match self { + Self::ClassMethod => crate::descr::classmethod_w_function_descr(), + Self::StaticMethod => crate::descr::staticmethod_w_function_descr(), + } + } +} + +#[allow(clippy::too_many_arguments)] +pub(crate) fn try_walker_inline_getattr_hook( + ctx: &mut WalkContext<'_, '_, Sym>, + op: &DecodedOp, + code: &[u8], + r_args: &[OpRef], + call_descr: &dyn majit_ir::descr::CallDescr, + obj: OpRef, + w_code_ptr: usize, + name_idx: usize, + dst: usize, + dst_bank: char, +) -> Result, DispatchError> { + if !ctx.is_authoritative_executor || dst_bank != 'r' || ctx.fbw_mode.inline_subwalk { + return Ok(None); + } + let Some(concrete_obj) = walker_concrete_ref_object(ctx, obj) else { + return Ok(None); + }; + let Some(name) = walker_load_name_from_code(w_code_ptr, name_idx) else { + return Ok(None); + }; + let Some((w_type, version_tag, map, w_getattr)) = (unsafe { + pyre_interpreter::objspace::std::mapdict::getattr_hook_fast_path(concrete_obj, &name) + }) else { + return Ok(None); + }; + // `get_and_call_function` (`descroperation.py:169-187`) leads the + // positionals with the receiver only for an exact `Function`; every other + // descriptor goes through `space.get` first and is called with the name + // alone. The version-tag pin makes that binding decision a constant of the + // trace, so each spelling resolves to its own function and leading argument + // here instead of declining. + // + // The type tests are EXACT. Upstream is explicit that they have to be + // ("isinstance(typ, Function) would not be correct here … because a builtin + // function binds differently than a normal function"), and the same holds + // for the two wrappers: a `classmethod` subclass overriding `__get__` binds + // through that override, so unwrapping `w_function` in its place calls the + // wrong callable. `wrapper_field` names the slot that unwrapping read, for + // the guard below; the plain arm reads no field. + let (w_func, leading, wrapper_field) = unsafe { + if pyre_object::function::is_exact_classmethod(w_getattr) { + ( + pyre_object::function::w_classmethod_get_func(w_getattr), + HookLeading::Class, + Some(WrapperField::ClassMethod), + ) + } else if pyre_object::function::is_exact_staticmethod(w_getattr) { + ( + pyre_object::function::w_staticmethod_get_func(w_getattr), + HookLeading::None, + Some(WrapperField::StaticMethod), + ) + } else { + (w_getattr, HookLeading::Receiver, None) + } + }; + if w_func.is_null() { + return Ok(None); + } + let Some((w_code, nparams, has_closure)) = (unsafe { resolve_inlinable_callee(w_func) }) else { + return Ok(None); + }; + // The name, plus the bound leading argument when the descriptor supplies + // one. Any other arity is a shape the call would reject before the body + // runs. + if nparams != usize::from(!matches!(leading, HookLeading::None)) + 1 { + return Ok(None); + } + // Decided once per callee on its jitcode payload; `None` means no body or + // descr pool, which this route declines on either way. + let Some(body_facts) = sub_jitcode_body_facts_for_code(w_code) else { + return Ok(None); + }; + if body_facts.owns_loop_header { + return Ok(None); + } + + // Everything below emits, and the callee inline has decline paths of its + // own past this point, so keep a rewind point the way the property twins + // do. + let pre_fold_pos = ctx.trace_ctx.get_trace_position(); + // Both pins the oracle asked for, plus the layout guard its map read needs. + walker_guard_mapdict_instance_shape(ctx, op.pc, obj, concrete_obj, w_type, version_tag, map)?; + // The pins above make the DESCRIPTOR a constant; they say nothing about the + // callable inside it. Re-initialising an installed wrapper swaps + // `w_function` without touching the owner type's version tag, which is the + // only thing those pins hold, so read the slot live and pin the value this + // fold unwrapped — the stand-in [`walker_guard_function_field`] already + // makes for a quasi-immutable field pyre's setters do not invalidate. + if let Some(field) = wrapper_field { + let wrapper = ctx.trace_ctx.const_ref(w_getattr as i64); + walker_guard_function_field(ctx, op.pc, wrapper, field.descr(), w_func as i64)?; + } + + let name_obj = + pyre_object::unicodeobject::box_str_constant(rustpython_wtf8::Wtf8::new(name.as_str())) + as pyre_object::PyObjectRef; + let name_const = ctx.trace_ctx.const_ref(name_obj as i64); + let leading_arg = match leading { + // The live receiver box: baking it would collapse instances that share + // this shape but not this identity. + HookLeading::Receiver => Some((obj, concrete_obj)), + // The class the `w_class` guard above already pinned. + HookLeading::Class => Some((ctx.trace_ctx.const_ref(w_type as i64), w_type)), + HookLeading::None => None, + }; + // `[__getattr__, , ?, name]`: the method-form + // call header the inline plumbing expects, then the positional args. + let mut arg_concretes = vec![ConcreteValue::Ref(w_func), ConcreteValue::Null]; + let mut callee_args = Vec::with_capacity(2); + let mut callee_arg_concretes = Vec::with_capacity(2); + if let Some((arg, concrete)) = leading_arg { + arg_concretes.push(ConcreteValue::Ref(concrete)); + callee_args.push(arg); + callee_arg_concretes.push(ConcreteValue::Ref(concrete)); + } + arg_concretes.push(ConcreteValue::Ref(name_obj)); + callee_args.push(name_const); + callee_arg_concretes.push(ConcreteValue::Ref(name_obj)); + let getattr_const = ctx.trace_ctx.const_ref(w_func as i64); + let inlined = try_walker_inline_resolved_user_call( + ctx, + op, + code, + getattr_const, + r_args, + call_descr, + 'r', + dst, + w_func, + getattr_const, + w_func, + arg_concretes, + callee_args, + callee_arg_concretes, + true, + None, + w_code, + nparams, + has_closure, + // The class and version pins are already emitted above, alongside the + // map pin this route additionally owes. + None, + None, + // The same LOAD_ATTR entry [`try_walker_inline_property_get`] admits. + true, + false, + None, + )?; + if inlined.is_none() { + ctx.trace_ctx.cut_trace(pre_fold_pos); + ctx.trace_ctx.heap_cache_mut().reset(); + } + Ok(inlined) } /// Inline a `property` setter store (`obj.value = x`) after the plain-attribute @@ -6223,7 +6600,7 @@ pub(crate) fn try_walker_inline_property_set( let Some(name) = walker_load_name_from_code(w_code_ptr, name_idx) else { return Ok(None); }; - let Some((w_type, version_tag, fset)) = (unsafe { + let Some((w_type, version_tag, w_descr, fset)) = (unsafe { pyre_interpreter::objspace::std::mapdict::property_set_fast_path(concrete_obj, &name) }) else { return Ok(None); @@ -6262,7 +6639,10 @@ pub(crate) fn try_walker_inline_property_set( ConcreteValue::Ref(concrete_value), ]; let fset_const = ctx.trace_ctx.const_ref(fset as i64); - try_walker_inline_resolved_user_call( + // Rewind point for the same reason as the getter twin. + let pre_fold_pos = ctx.trace_ctx.get_trace_position(); + walker_pin_property_accessor(ctx, op.pc, w_descr, crate::descr::property_fset_descr())?; + let inlined = try_walker_inline_resolved_user_call( ctx, op, code, @@ -6291,7 +6671,12 @@ pub(crate) fn try_walker_inline_property_set( true, false, None, - ) + )?; + if inlined.is_none() { + ctx.trace_ctx.cut_trace(pre_fold_pos); + ctx.trace_ctx.heap_cache_mut().reset(); + } + Ok(inlined) } /// Whether a concrete object is the canonical machine-word `int` layout that diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index 0b3d04387b5..a5d13978203 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -8626,6 +8626,66 @@ fn walker_pin_type_version_tag( walker_flush_guard_not_invalidated(ctx, op_pc) } +/// The `descriptor.py:175 _immutable_fields_ = ["w_fget?", "w_fset?", +/// "w_fdel?"]` twin of [`walker_pin_type_version_tag`]: pin the accessor slot +/// a property fold is about to bake. +/// +/// The receiver pins the folds already hold — class, `w_class`, and the type's +/// `_version_tag?` — make the DESCRIPTOR a compile-time answer, and stop +/// there: `property.__init__` on an installed descriptor replaces `fget`/ +/// `fset` in place and bumps no type's version, so without this the trace kept +/// calling the previous getter. Upstream covers exactly that gap with the `?` +/// on the slots themselves. +/// +/// A marker, never a load: the descriptor is a baked `ConstPtr`, and reading a +/// field through one is the hazard `try_walker_inline_resolved_user_call`'s +/// `guards_the_callee_function` gate exists to avoid. `record_quasiimmut_field` +/// dereferences the owner only at record and compile time, and a property is +/// allocated non-moving, so both reads see the object where the constant says +/// it is. +fn walker_pin_property_accessor( + ctx: &mut WalkContext<'_, '_, Sym>, + op_pc: usize, + w_descr: pyre_object::PyObjectRef, + field: majit_ir::DescrRef, +) -> Result<(), DispatchError> { + let descr_const = ctx.trace_ctx.const_ref(w_descr as i64); + crate::state::record_quasiimmut_field(ctx.trace_ctx, descr_const, field); + walker_flush_guard_not_invalidated(ctx, op_pc) +} + +/// The `function.py:47 _immutable_fields_ = ['code?', 'w_func_globals?', +/// 'closure?[*]', 'defs_w?[*]']` twin of [`walker_pin_property_accessor`]: pin +/// the callee's code slot when the inline lever cannot re-prove it per +/// iteration. +/// +/// The inline bakes `code` by choosing which callee jitcode to walk into, so +/// the value ends up spread across the inlined body rather than in one box a +/// `GUARD_VALUE` could re-check. Where the caller pins the callee function +/// itself, that guard still runs and is the cheaper answer for a callee whose +/// identity changes every iteration; everywhere else — a constant callable, or +/// a specializer that dispatched on some other object — this marker is what +/// makes `f.__code__ = g.__code__` revoke the loop. +/// +/// A marker, never a load: the owner is dereferenced only at record and +/// compile time, which is why this arm is clear of the baked-`ConstPtr` hazard +/// that keeps the guard arm off a constant callable. Both reads resolve the +/// owner by raw address, so the caller refuses a callee the collector can +/// relocate. +fn walker_pin_function_code( + ctx: &mut WalkContext<'_, '_, Sym>, + op_pc: usize, + callable: pyre_object::PyObjectRef, +) -> Result<(), DispatchError> { + let callable_const = ctx.trace_ctx.const_ref(callable as i64); + crate::state::record_quasiimmut_field( + ctx.trace_ctx, + callable_const, + crate::descr::function_code_descr(), + ); + walker_flush_guard_not_invalidated(ctx, op_pc) +} + /// The `celldict.py:34 _immutable_fields_ = ["version?"]` twin of /// [`walker_pin_type_version_tag`]: pin the module namespace's strategy version /// so the folds that bake a slot's stored cell (or the absence of a name) are diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs index 80c4d809108..5d5f64effff 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -6706,6 +6706,22 @@ pub(crate) fn dispatch_residual_call_iIRd_kind( )? { return Ok(inlined); } + // The name resolves nowhere and the type defines `__getattr__`: + // inline the hook in place of the miss walk plus its frame. + if let Some(inlined) = try_walker_inline_getattr_hook( + ctx, + op, + code, + &r_args, + call_descr, + obj_opref, + w_code_ptr, + namei as usize, + dst, + dst_bank, + )? { + return Ok(inlined); + } // A type receiver whose class-MRO value needs no descriptor // binding folds to that value under receiver + version pins. if spec_gate("load_type_attr", || { diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs index eb5b6f34c65..01edb56704e 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs @@ -299,6 +299,27 @@ fn walker_capture_inline_nonstandard_vable_guard_inner( Ok(()) } +/// Whether a guard emitted at the current inline depth resumes at its own +/// callee coordinate instead of collapsing to the caller's CALL boundary. +/// +/// The multi-frame snapshot below fires only when the paused-caller chain +/// covers the full inline depth — one parent per active inlined callee. A +/// shorter chain falls through to the single-frame collapse, whose resume +/// re-executes the entire call, so a guard emitted under it re-runs every side +/// effect the inline region sequenced before it. A fold that must not be +/// re-run consults this before emitting its guards. +pub(crate) fn walker_inline_guard_resumes_in_callee( + ctx: &WalkContext<'_, '_, Sym>, +) -> bool { + let session = ctx.session.borrow(); + let n_parents = session + .framestack + .iter() + .filter(|frame| frame.parent.is_some()) + .count(); + n_parents > 0 && n_parents == session.framestack.len() +} + pub(crate) fn walker_capture_snapshot_for_last_guard_impl( ctx: &mut WalkContext<'_, '_, Sym>, op_pc: usize, @@ -385,31 +406,22 @@ pub(crate) fn walker_capture_snapshot_for_last_guard_impl( // populates the chain; straight-line callees keep the empty chain + the // single-frame collapse below. if inline_subwalk { - // Fire the multi-frame snapshot only when the paused-caller chain - // covers the FULL current inline depth: framestack levels with parents - // must have one entry per active inlined callee. A nested - // straight-line callee inlined under a multiframe ancestor (e.g. - // `add3` inside a multiframe `mix`) pushes NO parent frame, so its own - // guards see a SHORTER chain than the callee depth — fall through to - // the single-frame collapse (the strict callee's resume-at-CALL - // behavior) rather than emit a chain that skips the intermediate frame. - let (n_parents, n_callees, parent_frames) = { + let parent_frames = { let session = ctx.session.borrow(); - ( - session - .framestack - .iter() - .filter(|frame| frame.parent.is_some()) - .count(), - session.framestack.len(), - session - .framestack - .iter() - .filter_map(|frame| frame.parent.clone()) - .collect::>(), - ) + session + .framestack + .iter() + .filter_map(|frame| frame.parent.clone()) + .collect::>() }; - if n_parents > 0 && n_parents == n_callees { + // Fire the multi-frame snapshot only when the paused-caller chain + // covers the FULL current inline depth. A nested straight-line callee + // inlined under a multiframe ancestor (e.g. `add3` inside a multiframe + // `mix`) pushes NO parent frame, so its own guards see a SHORTER chain + // than the callee depth — fall through to the single-frame collapse + // (the strict callee's resume-at-CALL behavior) rather than emit a + // chain that skips the intermediate frame. + if walker_inline_guard_resumes_in_callee(ctx) { // A STRICT straight-line callee (gh#420) whose own frame is not // MF-snapshot-able (a kept operand-stack temp the sub-walk does not // mirror) propagates the `Unsupported` error the same as the branch diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 7e87180c0cf..c619ee8edc6 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -3299,11 +3299,11 @@ pub(crate) fn try_walker_specialize_load_method_attr( /// type as `cls`, and the following `CALL` inlines `__func__(cls, ...)` — the /// instance-method shape with the class in the receiver slot. /// -/// Restricted to the top full-body frame for the reason -/// [`try_walker_specialize_load_bound_method_attr`] carries: a fold guard -/// inside an inlined callee sub-walk resumes at the caller's CALL, re-running -/// side effects. The `getattr` residual resumes past the call, so declining -/// there re-runs nothing. +/// Carries the inline-depth restriction +/// [`try_walker_specialize_load_bound_method_attr`] documents: under the +/// single-frame collapse a fold guard inside an inlined callee sub-walk +/// resumes at the caller's CALL, re-running side effects. The `getattr` +/// residual resumes past the call, so declining there re-runs nothing. #[allow(clippy::too_many_arguments)] pub(crate) fn try_walker_specialize_load_classmethod_attr( ctx: &mut WalkContext<'_, '_, Sym>, @@ -3317,7 +3317,7 @@ pub(crate) fn try_walker_specialize_load_classmethod_attr( if !ctx.is_authoritative_executor || dst_bank != 'r' { return Ok(None); } - if ctx.fbw_mode.inline_subwalk { + if ctx.fbw_mode.inline_subwalk && !walker_inline_guard_resumes_in_callee(ctx) { return Ok(None); } let Some(concrete_obj) = walker_concrete_ref_object(ctx, obj) else { @@ -3537,12 +3537,14 @@ pub(crate) fn try_walker_specialize_load_type_attr( /// Returns `None` (fall through to the residual, SAFE) for every shape /// [`pyre_interpreter::baseobjspace::bound_method_attr_fast_path`] declines. /// -/// Restricted to the top full-body frame for the reason -/// [`try_walker_orthodox_list_append`] documents: inside an inlined callee -/// sub-walk a fold's guards collapse their resume to the caller's CALL -/// boundary, so a guard failure re-runs the callee from its entry and doubles -/// any side effect it sequenced before this `LOAD_ATTR`. The residual resumes -/// past the call instead, so declining here re-runs nothing extra. +/// Inside an inlined callee sub-walk the fold is restricted to a depth whose +/// guards resume at their own callee coordinate +/// ([`walker_inline_guard_resumes_in_callee`]). Under the single-frame +/// collapse the reason [`try_walker_orthodox_list_append`] documents applies: a +/// guard resumes at the caller's CALL boundary, so a failure re-runs the callee +/// from its entry and doubles any side effect it sequenced before this +/// `LOAD_ATTR`. The residual resumes past the call instead, so declining there +/// re-runs nothing extra. #[allow(clippy::too_many_arguments)] pub(crate) fn try_walker_specialize_load_bound_method_attr( ctx: &mut WalkContext<'_, '_, Sym>, @@ -3553,7 +3555,10 @@ pub(crate) fn try_walker_specialize_load_bound_method_attr( dst: usize, dst_bank: char, ) -> Result, DispatchError> { - if !ctx.is_authoritative_executor || dst_bank != 'r' || ctx.fbw_mode.inline_subwalk { + if !ctx.is_authoritative_executor || dst_bank != 'r' { + return Ok(None); + } + if ctx.fbw_mode.inline_subwalk && !walker_inline_guard_resumes_in_callee(ctx) { return Ok(None); } let Some(concrete_obj) = walker_concrete_ref_object(ctx, obj) else { @@ -3661,10 +3666,10 @@ pub(crate) fn try_walker_fold_load_method_self( let method_type_addr = &pyre_object::function::METHOD_TYPE as *const _ as i64; let class_pinned = attr.is_constant() || ctx.trace_ctx.heap_cache().is_class_known(attr); if !class_pinned { - // A guard here would resume at the caller's CALL inside an inlined - // callee sub-walk, re-running whatever that callee already did; + // Under the single-frame collapse a guard here would resume at the + // caller's CALL, re-running whatever that callee already did; // leave those to the residual (which resumes past the call). - if ctx.fbw_mode.inline_subwalk { + if ctx.fbw_mode.inline_subwalk && !walker_inline_guard_resumes_in_callee(ctx) { return Ok(None); } let type_const = ctx.trace_ctx.const_int(method_type_addr); diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 4a1b7845781..ea68a569948 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -1019,6 +1019,37 @@ pub fn pyjitcode_for_jitcode_index(jitcode_index: i32) -> Option( // carrier, which resumes the OUTER frame at its CALL rather than // inside the discarded callee attempt. The nested-residual variant // marked `blackhole_required: true` owns a complete per-frame image - // and so passes `leaves_complete_image`, but the handoff finishes the - // callee inside the blackhole, which has no counterpart to - // `PyFrame.finish_value`'s `frame_finished_execution` store — the - // walker emits that store itself (`finish_current_frame_execution`) - // and the interpreter performs it on RETURN_VALUE, while the - // blackhole does neither. A frame that outlives the call then reads - // back as still executing, which `parity_tests/` - // `jit_inline_traceback_frame_clear.py` catches on - // `sys._getframe().clear()` once the loop compiles. Restore the - // carrier for it until the blackhole can publish that transition; + // and so passes `leaves_complete_image`, but the image it hands the + // blackhole is not a valid forward resume for every shape that reaches + // it. Two `bench/synth` fixtures are the standing witnesses, both + // wrong-code rather than a decline: `inline_subwalk_user_iterator` + // (the inlined callee's return value comes back as an untyped ref, so + // the caller's `acc += v` raises `TypeError: ... 'int' and 'object'`) + // and `list_append_write_barrier_gc` (`stack underflow during + // interpreter peek` — the resumed frame's operand stack is short). + // `PYRE_WALKABORT_OFF=1` is the control: both pass with the leg + // disabled. The `frame_finished_execution` store the handoff used to + // skip is NO LONGER one of the reasons — the drive now performs it at + // every level it leaves (`state::finish_blackhole_level_frame`, wired + // as `on_leave_level`), which is what + // `parity_tests/jit_inline_traceback_frame_clear.py` needs on + // `sys._getframe().clear()`. Restore the carrier for this variant + // until the two image defects above are closed; // `ForceQuasiImmutable` resumes AT the forcing opcode via // `flush_qmut_abort_state` (arm below), which re-runs the write the // walk stopped in front of instead of finishing the frame past it. diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index fa269a4fe8b..541eaf74f84 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -6377,10 +6377,12 @@ pub(crate) fn register_quasi_immutable_deps(_green_key: u64) { let holder_attr = pyre_jit_trace::descr::holder_attr_descr().index(); let holder_typ = pyre_jit_trace::descr::holder_typ_descr().index(); let audit_holder_hooks = pyre_jit_trace::descr::audit_holder_hooks_descr().index(); + let property_fget = pyre_jit_trace::descr::property_fget_descr().index(); + let property_fset = pyre_jit_trace::descr::property_fset_descr().index(); // Hoisted because each accessor clones a `LazyLock` descr; the index also // decides which type `dep_ptr` is cast to, so the chain below ends in a // fail-loud default rather than reinterpreting a headerless map node as a - // `W_TypeObject`. These seven plus the nine `Function` fields + // `W_TypeObject`. These nine plus the nine `Function` fields // `function_quasi_immut_slot` resolves are every quasi-immutable descr this // binary mints — see the same reasoning on `state.rs // install_quasiimmut_field`. @@ -6421,6 +6423,16 @@ pub(crate) fn register_quasi_immutable_deps(_green_key: u64) { dep_ptr as *const _, &flag, ); + } else if field_index == property_fget { + pyre_object::descriptor::w_property_register_fget_watcher( + dep_ptr as pyre_object::PyObjectRef, + &flag, + ); + } else if field_index == property_fset { + pyre_object::descriptor::w_property_register_fset_watcher( + dep_ptr as pyre_object::PyObjectRef, + &flag, + ); } else if let Some(slot) = pyre_jit_trace::descr::function_quasi_immut_slot(field_index) { pyre_interpreter::function::function_register_quasi_immut_watcher( @@ -7390,38 +7402,89 @@ fn for_iter_bodies_all_jit_safe(code: &pyre_interpreter::CodeObject) -> bool { true } -/// Return the end of the natural loop region whose header is `loop_header_pc`. -/// Out-of-line exception handlers can rejoin the loop through a backward jump -/// to the middle of the body, so grow the region until every such rejoining -/// handler is included. -fn loop_region_end(code: &pyre_interpreter::CodeObject, loop_header_pc: usize) -> Option { +/// Return the pc ranges that make up the natural loop region whose header is +/// `loop_header_pc`: the loop body, plus every out-of-line exception handler +/// that rejoins the body through a backward jump. An empty result means +/// `loop_header_pc` has no backedge and so names no region. +/// +/// The region is a set of ranges rather than one span because a handler is laid +/// out after the code that follows its `try`, not next to the body it protects. +/// Whatever sits between the two — a later comprehension, a disjoint loop — +/// belongs to neither and cannot run in this backedge's trace, so covering the +/// gap would gate the backedge on `FOR_ITER`s it never reaches. The exception +/// table names where the out-of-line code begins, which is what lets a +/// rejoining jump widen the region back to its own handler instead of across +/// the gap. +fn loop_region_ranges( + code: &pyre_interpreter::CodeObject, + loop_header_pc: usize, +) -> Vec> { use pyre_interpreter::Instruction as I; - let mut region_end = None; - loop { - let previous_end = region_end; - let mut arg_state = pyre_interpreter::OpArgState::default(); - for (pc, unit) in code.instructions.iter().copied().enumerate() { - let (instr, op_arg) = arg_state.get(unit); - let target = match instr { - I::JumpBackward { delta } => { - Some(skip_caches(code, pc + 1).saturating_sub(delta.get(op_arg).as_usize())) - } - I::JumpBackwardNoInterrupt { delta } => { - Some((pc + 1).saturating_sub(delta.get(op_arg).as_usize())) - } - _ => None, - }; - let extends_region = match (target, region_end) { - (Some(target), _) if target == loop_header_pc => true, - (Some(target), Some(end)) => pc > end && (loop_header_pc..=end).contains(&target), - _ => false, - }; - if extends_region { - region_end = Some(region_end.map_or(pc, |end: usize| end.max(pc))); + + let mut backward_jumps: Vec<(usize, usize)> = Vec::new(); + let mut arg_state = pyre_interpreter::OpArgState::default(); + for (pc, unit) in code.instructions.iter().copied().enumerate() { + let (instr, op_arg) = arg_state.get(unit); + let target = match instr { + I::JumpBackward { delta } => { + Some(skip_caches(code, pc + 1).saturating_sub(delta.get(op_arg).as_usize())) + } + I::JumpBackwardNoInterrupt { delta } => { + Some((pc + 1).saturating_sub(delta.get(op_arg).as_usize())) } + _ => None, + }; + if let Some(target) = target { + backward_jumps.push((pc, target)); } - if region_end == previous_end { - return region_end; + } + + let Some(body_end) = backward_jumps + .iter() + .filter(|(_, target)| *target == loop_header_pc) + .map(|(pc, _)| *pc) + .max() + else { + return Vec::new(); + }; + + // The exception table is keyed by byte offset; pyre's `pc` is the + // instruction-unit index (two bytes per unit). Only the handlers laid out + // past the body can start an out-of-line block; one inside the body is + // already covered. + let mut handler_starts: Vec = + pyre_interpreter::pycode::decode_exceptiontable(&code.exceptiontable) + .map(|entry| entry.target as usize / 2) + .filter(|start| *start > body_end) + .collect(); + handler_starts.sort_unstable(); + + let mut ranges = vec![loop_header_pc..=body_end]; + loop { + let rejoins: Vec = backward_jumps + .iter() + .filter(|(pc, target)| { + !ranges.iter().any(|range| range.contains(pc)) + && ranges.iter().any(|range| range.contains(target)) + }) + .map(|(pc, _)| *pc) + .collect(); + if rejoins.is_empty() { + return ranges; + } + for pc in rejoins { + // Take the earliest handler that still starts at or before the + // jump: a handler runs on through the ones nested inside it, so + // the block this jump closes begins at the outermost of them. + // Without a handler to name a start the jump is not an out-of-line + // rejoin, and the span back to the body is kept whole rather than + // guessed at. + let start = handler_starts + .iter() + .copied() + .find(|start| *start <= pc) + .unwrap_or(body_end + 1); + ranges.push(start..=pc); } } } @@ -7435,12 +7498,16 @@ fn loop_region_for_iter_bodies_all_jit_safe( loop_header_pc: usize, ) -> bool { use pyre_interpreter::Instruction as I; - let Some(region_end) = loop_region_end(code, loop_header_pc) else { + let ranges = loop_region_ranges(code, loop_header_pc); + if ranges.is_empty() { return true; - }; + } let mut scan_state = pyre_interpreter::OpArgState::default(); - for pc in loop_header_pc..=region_end { - let (instr, _) = scan_state.get(code.instructions[pc]); + for (pc, unit) in code.instructions.iter().copied().enumerate() { + let (instr, _) = scan_state.get(unit); + if !ranges.iter().any(|range| range.contains(&pc)) { + continue; + } if matches!(instr, I::ForIter { .. }) && !for_iter_body_is_jit_safe_at(code, pc) { return false; } @@ -7466,20 +7533,18 @@ fn loop_region_contains_escaping_range_append( AwaitAppendCall, } - let Some(region_end) = loop_region_end(code, loop_header_pc) else { + let ranges = loop_region_ranges(code, loop_header_pc); + if ranges.is_empty() { return false; - }; + } let mut state = State::Searching; let mut decode = pyre_interpreter::OpArgState::default(); for (pc, unit) in code.instructions.iter().copied().enumerate() { let (instr, op_arg) = decode.get(unit); - if pc < loop_header_pc { + if !ranges.iter().any(|range| range.contains(&pc)) { continue; } - if pc > region_end { - break; - } match instr { I::LoadAttr { namei } if code.names[namei.get(op_arg).name_idx() as usize].as_str() == "append" => @@ -9097,7 +9162,7 @@ fn maybe_compile_and_run( let region_safe = cached_loop_region_for_iter_bodies_all_jit_safe(code, loop_header_pc); if !region_safe || (!cached_for_iter_bodies_all_jit_safe(code) - && !cached_loop_region_contains_escaping_range_append(code, loop_header_pc)) + && !frame_has_traceable_escaping_range_loop(code)) { return None; } @@ -13698,8 +13763,8 @@ mod tests { } let direct_end = direct_end.expect("fixture must contain the outer backedge"); - let region_end = loop_region_end(&code, outer_header).expect("loop must have a region"); - assert!(region_end > direct_end); + let ranges = loop_region_ranges(&code, outer_header); + assert!(ranges.iter().any(|range| *range.start() > direct_end)); assert!(!loop_region_for_iter_bodies_all_jit_safe( &code, outer_header diff --git a/pyre/pyre-object/src/descriptor.rs b/pyre/pyre-object/src/descriptor.rs index c85d4523736..2ad2379cdc5 100644 --- a/pyre/pyre-object/src/descriptor.rs +++ b/pyre/pyre-object/src/descriptor.rs @@ -156,9 +156,13 @@ mod super_tests { /// Python property descriptor object. /// -/// Layout: `[ob_type | fget | fset | fdel | w_doc | w_name | getter_doc]` +/// Layout: `[ob_type | fget | fset | fdel | w_doc | w_name | getter_doc | +/// fget_watchers | fset_watchers]` #[pyre_class("property", type_id = 19, static_name = "PROPERTY")] pub struct W_Property { + /// `descriptor.py:175 _immutable_fields_ = ["w_fget?", "w_fset?", + /// "w_fdel?"]` declares all three quasi-immutable; the hidden watcher + /// fields below implement the `?` for the two a fold bakes. pub fget: PyObjectRef, pub fset: PyObjectRef, pub fdel: PyObjectRef, @@ -175,6 +179,23 @@ pub struct W_Property { /// was copied from `fget.__doc__` (descriptor.py:196-204); `_copy` /// uses it to drop the inherited doc when the getter is replaced. pub getter_doc: bool, + /// The hidden `mutate_w_fget` field for `descriptor.py:175 + /// _immutable_fields_ = ["w_fget?", ...]` — see [`crate::quasiimmut`]. + /// + /// Holds no GC pointers, so the derived `PTR_OFFSETS` has nothing to walk + /// here. The allocation is [`crate::gc_hook::try_gc_alloc_stable_raw`], + /// i.e. non-moving, which is [`crate::quasiimmut::QuasiImmutField`]'s + /// stated precondition: the lock cannot be remapped out from under a + /// holder. A property the collector reclaims without a prior invalidation + /// leaks its instance box, the same bounded leak `W_TypeObject` carries, + /// because a GC object's `Drop` never runs. + /// + /// `w_fdel?` is declared upstream on the same line and gets no watcher + /// here: no fold bakes `fdel`, so nothing would ever register on it. A + /// `__delete__` fold must add the third one rather than bake without it. + pub fget_watchers: crate::quasiimmut::QuasiImmutField, + /// The `w_fset?` twin of [`Self::fget_watchers`]. + pub fset_watchers: crate::quasiimmut::QuasiImmutField, } /// Allocate a new property object. @@ -214,6 +235,8 @@ pub fn w_property_new(fget: PyObjectRef, fset: PyObjectRef, fdel: PyObjectRef) - w_doc: PY_NULL, w_name: PY_NULL, getter_doc: false, + fget_watchers: crate::quasiimmut::QuasiImmutField::new(), + fset_watchers: crate::quasiimmut::QuasiImmutField::new(), }, ); } @@ -228,6 +251,8 @@ pub fn w_property_new(fget: PyObjectRef, fset: PyObjectRef, fdel: PyObjectRef) - w_doc: PY_NULL, w_name: PY_NULL, getter_doc: false, + fget_watchers: crate::quasiimmut::QuasiImmutField::new(), + fset_watchers: crate::quasiimmut::QuasiImmutField::new(), }) } @@ -269,6 +294,21 @@ pub unsafe fn w_property_reinit( fdel: PyObjectRef, ) { let prop = obj as *mut W_Property; + // `rclass.py:715-718 hook_setfield` emits `jit_force_quasi_immutable` + // ahead of every store to a `?` field, so the accessors this replaces stop + // being trace constants before they stop being the live values. The hook + // precedes the store and does not consult it, so re-initialising a slot + // with the value it already holds invalidates as well. Nothing else + // revokes them: re-initialising an installed descriptor changes no type's + // version tag, which is the only other pin a fold over `obj.name` holds. + // The `is_installed` test is `pyjitpl.py:1112`'s `mutatebox.nonnull()` — a + // property no loop watches pays one load. + if (*prop).fget_watchers.is_installed() { + crate::quasiimmut::sweep_quasi_immut_field(&(*prop).fget_watchers); + } + if (*prop).fset_watchers.is_installed() { + crate::quasiimmut::sweep_quasi_immut_field(&(*prop).fset_watchers); + } (*prop).fget = fget; (*prop).fset = fset; (*prop).fdel = fdel; @@ -278,6 +318,69 @@ pub unsafe fn w_property_reinit( crate::gc_hook::try_gc_write_barrier(obj as *mut u8); } +/// `quasiimmut.py:116-126 get_current_qmut_instance` for +/// `descriptor.py:175`'s `w_fget?` — install the instance at RECORD time so a +/// write reached later in the same trace sees it. The +/// [`w_type_install_quasi_immut`](crate::typeobject::w_type_install_quasi_immut) +/// shape. +/// +/// # Safety +/// `obj` must point to a live [`W_Property`]. +pub unsafe fn w_property_install_fget_watcher(obj: PyObjectRef) { + if obj.is_null() { + return; + } + (*(obj as *const W_Property)) + .fget_watchers + .ensure_installed(); +} + +/// The `w_fset?` twin of [`w_property_install_fget_watcher`]. +/// +/// # Safety +/// `obj` must point to a live [`W_Property`]. +pub unsafe fn w_property_install_fset_watcher(obj: PyObjectRef) { + if obj.is_null() { + return; + } + (*(obj as *const W_Property)) + .fset_watchers + .ensure_installed(); +} + +/// `quasiimmut.py:72-75 register_loop_token` for `w_fget?` — record a compiled +/// loop's invalidation flag so `property.__init__` revokes it. +/// +/// # Safety +/// `obj` must point to a live [`W_Property`]. +pub unsafe fn w_property_register_fget_watcher( + obj: PyObjectRef, + flag: &std::sync::Arc, +) { + if obj.is_null() { + return; + } + (*(obj as *const W_Property)) + .fget_watchers + .register_loop_token(flag); +} + +/// The `w_fset?` twin of [`w_property_register_fget_watcher`]. +/// +/// # Safety +/// `obj` must point to a live [`W_Property`]. +pub unsafe fn w_property_register_fset_watcher( + obj: PyObjectRef, + flag: &std::sync::Arc, +) { + if obj.is_null() { + return; + } + (*(obj as *const W_Property)) + .fset_watchers + .register_loop_token(flag); +} + /// `descriptor.py:249-250 W_Property.get_doc` — returns the raw slot /// (NULL plays None; the caller wraps). /// # Safety @@ -353,6 +456,29 @@ pub unsafe fn is_property(obj: PyObjectRef) -> bool { py_type_check(obj, &PROPERTY_TYPE) } +/// `type(obj) is property`, as opposed to [`is_property`]'s layout test. +/// +/// `descroperation.py:169-176 get_and_call_function` spells out why the +/// difference decides who may take an accessor shortcut: `typ = type(w_descr)` +/// then `if typ is Function or typ is FunctionWithFixedCode`, with +/// "isinstance(typ, Function) would not be correct here". Everything else +/// reaches its accessor through `space.get`, i.e. `type(w_descr).__get__` off +/// the MRO — so calling `fget` in place of `__get__` is licensed only when the +/// descriptor's type is `property` itself and cannot have overridden it. +/// +/// The two answers really do separate: `property_descr_new` allocates through +/// [`w_property_new`], which sets `w_class` to `property`, and calls +/// `tag_subclass_instance` — the only writer of `w_class` — solely when the +/// requested type is not `property`. `ob_type`, which [`is_property`] reads, +/// stays the shared layout word either way. +/// +/// # Safety +/// The caller must uphold every validity, runtime-type, aliasing, and lifetime +/// invariant required by the object and pointer arguments for the entire call. +pub unsafe fn is_exact_property(obj: PyObjectRef) -> bool { + unsafe { is_property(obj) && std::ptr::eq((*obj).w_class, get_instantiate(&PROPERTY_TYPE)) } +} + #[cfg(test)] mod property_tests { use super::*; diff --git a/pyre/pyre-object/src/function.rs b/pyre/pyre-object/src/function.rs index 8a510ab6de2..747e912a90c 100644 --- a/pyre/pyre-object/src/function.rs +++ b/pyre/pyre-object/src/function.rs @@ -177,6 +177,12 @@ pub struct StaticMethod { pub w_dict: PyObjectRef, } +/// Field offsets of the inline `PyObjectRef` slots within `StaticMethod`, +/// consumed by `pyre-jit-trace/src/descr.rs` on the same footing as the +/// `METHOD_*` consts above. +pub const STATICMETHOD_W_FUNCTION_OFFSET: usize = std::mem::offset_of!(StaticMethod, w_function); +pub const STATICMETHOD_W_DICT_OFFSET: usize = std::mem::offset_of!(StaticMethod, w_dict); + pub fn w_staticmethod_new(func: PyObjectRef) -> PyObjectRef { // `gct_fv_gc_malloc` bracket pattern (`framework.py:853-856`): pin the // wrapped function across the GC malloc and read its relocated address. @@ -271,6 +277,23 @@ pub unsafe fn is_staticmethod(obj: PyObjectRef) -> bool { py_type_check(obj, &STATICMETHOD_TYPE) } +/// An exact `staticmethod`, excluding subclasses — the test a caller needs +/// before it may unwrap `w_function` in place of invoking `__get__`. +/// `descroperation.py:169-187 get_and_call_function` takes its descriptor +/// shortcut only on the exact type and routes every other one through +/// `space.get`, so a subclass that overrides `__get__` binds differently. +/// Compares the user-visible class object, as [`is_exact_tuple`] does, because +/// a subclass instance keeps the base layout in `ob_type` and retags `w_class`. +#[inline] +/// # Safety +/// The caller must uphold every validity, runtime-type, aliasing, and lifetime +/// invariant required by the object and pointer arguments for the entire call. +pub unsafe fn is_exact_staticmethod(obj: PyObjectRef) -> bool { + unsafe { + is_staticmethod(obj) && std::ptr::eq((*obj).w_class, get_instantiate(&STATICMETHOD_TYPE)) + } +} + // ── ClassMethod ────────────────────────────────────────────────────── // PyPy: pypy/interpreter/function.py ClassMethod // @@ -285,6 +308,11 @@ pub struct ClassMethod { pub w_dict: PyObjectRef, } +/// Field offsets of the inline `PyObjectRef` slots within `ClassMethod`, the +/// `classmethod` twin of the `STATICMETHOD_*` consts above. +pub const CLASSMETHOD_W_FUNCTION_OFFSET: usize = std::mem::offset_of!(ClassMethod, w_function); +pub const CLASSMETHOD_W_DICT_OFFSET: usize = std::mem::offset_of!(ClassMethod, w_dict); + pub fn w_classmethod_new(func: PyObjectRef) -> PyObjectRef { // `gct_fv_gc_malloc` bracket pattern (`framework.py:853-856`): pin the // wrapped function across the GC malloc and read its relocated address. @@ -378,6 +406,18 @@ pub unsafe fn is_classmethod(obj: PyObjectRef) -> bool { py_type_check(obj, &CLASSMETHOD_TYPE) } +/// An exact `classmethod`, excluding subclasses — the `classmethod` twin of +/// [`is_exact_staticmethod`], for the same reason. +#[inline] +/// # Safety +/// The caller must uphold every validity, runtime-type, aliasing, and lifetime +/// invariant required by the object and pointer arguments for the entire call. +pub unsafe fn is_exact_classmethod(obj: PyObjectRef) -> bool { + unsafe { + is_classmethod(obj) && std::ptr::eq((*obj).w_class, get_instantiate(&CLASSMETHOD_TYPE)) + } +} + #[cfg(test)] mod tests { use super::*;