jit: widen the FOR_ITER callee inline; fix a silent dunder-dispatch abort

majit/majit-metainterp/src/virtualref.rs

@@ -172,9 +172,12 @@ pub use crate::jit::InvalidVirtualRef;
 /// so the address is stable across a minor collection.  The `forced` frame may
 /// be young, hence the creation write barrier.
 ///
-/// The `Box` fallback covers the window before `set_vref_gc_type_id` has run
-/// (the id still reads its unset sentinel) and an old-gen allocation failure;
-/// it is leaked, and reclamation is what it gives up.
+/// The `Box` fallback covers only the window before `set_vref_gc_type_id` has
+/// run (the id still reads its unset sentinel), where there is no registered
+/// type to allocate and nothing has handed a vref to the collector yet; it is
+/// leaked, and reclamation is what it gives up.  Once the id is set the
+/// allocation stays collector-owned or fails loudly — a host box past that
+/// point would silently drop the `forced` edge this object exists to hold.
 fn alloc_virtual_ref(real_object: *mut u8) -> *mut u8 {
     let vref = JitVirtualRef {
         super_: ObjectHeader {
@@ -186,12 +189,14 @@ fn alloc_virtual_ref(real_object: *mut u8) -> *mut u8 {
     let type_id = vref_gc_type_id();
     if type_id != VREF_GC_TYPE_ID_UNSET {
         let gcref = majit_gc::alloc_oldgen_typed(type_id, std::mem::size_of::<JitVirtualRef>());
-        if gcref.0 != 0 {
-            unsafe { std::ptr::write(gcref.0 as *mut JitVirtualRef, vref) };
-            // Creation write barrier: an old-gen vref may point at a young frame.
-            majit_gc::gc_write_barrier(gcref);
-            return gcref.0 as *mut u8;
-        }
+        assert!(
+            gcref.0 != 0,
+            "JitVirtualRef old-gen allocation failed after the type was registered",
+        );
+        unsafe { std::ptr::write(gcref.0 as *mut JitVirtualRef, vref) };
+        // Creation write barrier: an old-gen vref may point at a young frame.
+        majit_gc::gc_write_barrier(gcref);
+        return gcref.0 as *mut u8;
     }
     Box::into_raw(Box::new(vref)) as *mut u8
 }

pyre/bench/synth/type_metatype_method_call.py

@@ -0,0 +1,50 @@
+# A metaclass resolves `Cls.name()` before the class's own MRO does:
+# `type.__getattribute__` lets a metatype DATA descriptor win outright, and a
+# metatype `__getattribute__` override produces the value itself. Either way the
+# call must use what the metaclass returned, not rebind the class onto it.
+
+
+class MetaProp(type):
+    @property
+    def where(cls):
+        return lambda: 'meta-prop'
+
+
+class ByProp(metaclass=MetaProp):
+    @classmethod
+    def where(cls):
+        return 'own-classmethod'
+
+
+class MetaGetattr(type):
+    def __getattribute__(cls, name):
+        if name == 'ping':
+            return lambda: 'meta-getattr'
+        return type.__getattribute__(cls, name)
+
+
+class ByGetattr(metaclass=MetaGetattr):
+    @classmethod
+    def ping(cls):
+        return 'own-classmethod'
+
+
+class Plain:
+    @classmethod
+    def tag(cls):
+        return cls.__name__
+
+
+def main():
+    prop = getattr_ = plain = None
+    for _ in range(20000):
+        prop = ByProp.where()
+        getattr_ = ByGetattr.ping()
+        # an ordinary class still binds its classmethod's cls
+        plain = Plain.tag()
+    print('prop', prop)
+    print('getattr', getattr_)
+    print('plain', plain)
+
+
+main()

pyre/pyre-interpreter/src/baseobjspace.rs

@@ -8905,11 +8905,12 @@ pub unsafe fn load_method_fast_path(
 /// same shape a plain instance method takes, with the class in the receiver
 /// slot instead of an instance.
 ///
-/// `is_type` is exact-metatype, so a custom metaclass declines (its
-/// `__getattribute__` may not follow `type.__getattribute__`).  A name the
-/// metatype itself defines is declined too: a metatype attribute would shadow
-/// the class attribute (data descriptor) or be returned in its place.  An
-/// uncacheable type and any non-`classmethod` descriptor also decline.
+/// The metatype is read off the class (`getclass()`) and must be `type`
+/// itself, so a custom metaclass declines (its `__getattribute__` may not
+/// follow `type.__getattribute__`).  A name the metatype itself defines is
+/// declined too: a metatype attribute would shadow the class attribute (data
+/// descriptor) or be returned in its place.  An uncacheable type and any
+/// non-`classmethod` descriptor also decline.
 ///
 /// # Safety
 /// `w_obj` must be a valid object pointer (null tolerated).
@@ -8921,10 +8922,18 @@ pub unsafe fn classmethod_on_type_fast_path(
         return None;
     }
     let w_type = w_obj;
-    // `is_type` pins the metaclass to exactly `type`, so `type.__getattribute__`
-    // is the resolution path.  A metatype attribute of the same name would win
-    // over the class attribute, so decline any name the metatype defines.
-    let metatype = &pyre_object::pyobject::TYPE_TYPE as *const _ as PyObjectRef;
+    // `is_type` answers for the object's physical layout — every type object
+    // carries the same `ob_type` — so it says nothing about the metaclass.
+    // `getclass()` (baseobjspace.py) reads the metaclass off `w_class`; only
+    // `type` itself resolves the name through `type.__getattribute__`, so any
+    // other metaclass declines rather than have its `__getattribute__`
+    // override bypassed.
+    let metatype = crate::typedef::r#type(w_obj)?.as_ptr();
+    if !std::ptr::eq(metatype, crate::typedef::w_type()) {
+        return None;
+    }
+    // A metatype attribute of the same name would win over the class attribute,
+    // so decline any name the metatype defines.
     if lookup_in_type(metatype, name).is_some() {
         return None;
     }

pyre/pyre-interpreter/src/eval.rs

@@ -2979,6 +2979,20 @@ pub fn compute_load_method_bound(obj: PyObjectRef, attr: PyObjectRef, name: &str
             // METAclass MRO (`space.type(w_obj)`), so a name found in the
             // type's own MRO reaches the call as a plain getattr value
             // with no binding.
+            //
+            // `is_type` reports the physical layout every type object shares,
+            // not the metaclass, so read the metaclass and require it to be
+            // `type`.  The shape inferred below is what
+            // `type.__getattribute__` produces; a custom metaclass can
+            // override `__getattribute__` or define a data descriptor of the
+            // same name, and either one produced `attr` in place of the
+            // class's own MRO entry — binding `cls` onto that value would
+            // pass the class to something that never asked for it.
+            let metatype_is_type = crate::typedef::r#type(obj)
+                .is_some_and(|meta| std::ptr::eq(meta.as_ptr(), crate::typedef::w_type()));
+            if !metatype_is_type {
+                return PY_NULL;
+            }
             let raw = crate::baseobjspace::lookup_in_type(obj, name);
             match raw {
                 Some(d) if pyre_object::is_classmethod(d) => obj,

pyre/pyre-jit-trace/src/descr.rs

@@ -2886,6 +2886,18 @@ pub fn pytraceback_w_next_descr() -> DescrRef {
     field_descr_from_group(&PYTRACEBACK_DESCR_GROUP, index)
 }
 
+/// Field descriptor for `PyTraceback.lineno`, the line `descr_get_tb_lineno`
+/// reports.  Located by offset for the same reason as
+/// [`pytraceback_w_next_descr`].
+pub fn pytraceback_lineno_descr() -> DescrRef {
+    let index = PYTRACEBACK_DESCR_GROUP
+        .field_descrs
+        .iter()
+        .position(|d| d.offset() == pyre_interpreter::pytraceback::PYTRACEBACK_LINENO_OFFSET)
+        .expect("PyTraceback descr group has no lineno field");
+    field_descr_from_group(&PYTRACEBACK_DESCR_GROUP, index)
+}
+
 /// Cached field descriptor for a raw reference slot selected by the
 /// exception attribute fold.  Indices are those of `build_w_exception_group`;
 /// no parallel descriptor is constructed.

pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs

@@ -2148,6 +2148,21 @@ pub(crate) fn fbw_callee_body_replay_safety(
                 // [`fbw_store_journal_rollback`] replays the journal in reverse
                 // on a non-committed exit — so a folded store is undone for the
                 // replay, and an unfolded one never runs.
+                // `binary_op` / `compare_op` over operands this scan could not
+                // prove exact-numeric is the same shape one more time: which
+                // `__add__` / `__lt__` runs is a property of the operand's
+                // runtime class, not of this body.  The proven-operand case was
+                // already accepted above; what is left here is exactly the
+                // operand whose provenance the scan lost — most commonly a
+                // `LOAD_ATTR` result, since that arm is itself deferred and
+                // clears numeric provenance.  Deferring instead of declining is
+                // what lets `self.v + i` inline: at trace time the attribute
+                // read folds to a mapdict slot with a concrete int shadow, so
+                // the walker's numeric specialization erases the residual before
+                // the backstop is reached.  An operand pair that stays opaque
+                // leaves the residual standing, and it reaches
+                // `fbw_abort_nested_unjournaled_residual` like any other — the
+                // helper never runs.
                 if matches!(
                     ei.pyre_helper,
                     majit_ir::PyreHelperKind::CallFn
@@ -2156,6 +2171,8 @@ pub(crate) fn fbw_callee_body_replay_safety(
                         | majit_ir::PyreHelperKind::RaiseVarargs
                         | majit_ir::PyreHelperKind::SetCurrentException
                         | majit_ir::PyreHelperKind::LoadAttr
+                        | majit_ir::PyreHelperKind::BinaryOp
+                        | majit_ir::PyreHelperKind::CompareOp
                 ) {
                     deferred_call = true;
                     // The callee this resolves to is a runtime value, so what it

pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs

@@ -2681,7 +2681,17 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>(
         let legacy_admit = match safety {
             CalleeReplaySafety::Clean => true,
             CalleeReplaySafety::DeferredCall => {
-                foriter_deferred_admit = !fbw_foriter_deferred_call_denied(callee_code_key);
+                // The deferred promise rests on the abort REWINDING to the
+                // enclosing CALL and re-executing it from scratch.  A binop
+                // dunder dispatch (the only entry carrying an
+                // `arg_class_guard`) reaches this lever from a `BINARY_OP`
+                // instead, and that opcode is not a call boundary the rewind
+                // can name: the flush resumes one operand short and the whole
+                // iteration's contribution is dropped, silently.  A `Clean`
+                // body is still admitted from there — it has nothing that can
+                // abort.
+                foriter_deferred_admit =
+                    arg_class_guard.is_none() && !fbw_foriter_deferred_call_denied(callee_code_key);
                 foriter_deferred_admit
             }
             CalleeReplaySafety::Dirty => {
@@ -3196,18 +3206,19 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>(
             // Stored bound methods carry their explicit receiver and callee frame,
             // so their Ref operands remain available to the resume path.
             //
-            // This is the one precondition here that still aborts instead of
-            // returning `Ok(None)`, and deliberately so.  Residualizing it does
-            // work — `bench/synth/_pending/gc_bug_bridge_flavor_traceback_names`
-            // goes from 98 aborts to 2 and
-            // `_pending/exception_nested_exc_info_restore` from 5 aborts to 0,
-            // both compiling loops they never compiled before — but the loops it
-            // newly compiles then print traceback tuples missing their outermost
-            // frame, diverging from the interpreter (that fixture pins its
-            // expected output in its header).  The abort was masking a lost
-            // `PyTraceback` node on the compiled exception path, not preventing
-            // one.  Restore `Ok(None)` here once that node is recorded; it is the
-            // largest single win left in this function.
+            // This precondition used to abort the enclosing trace rather than
+            // decline the inline, because residualizing it let loops compile that
+            // then printed traceback tuples missing their OUTERMOST frame.  That
+            // node is now recorded — the two bridge handler-entry arms attach the
+            // catching frame's own node — so the decline joins every other
+            // precondition here and returns `Ok(None)`.
+            //
+            // The abort was expensive out of all proportion to the inline it was
+            // protecting: a callee that walks a traceback (`while tb is not None`)
+            // lowers to exactly this instruction, so any handler calling such a
+            // helper aborted every retrace of the enclosing loop.  The guard whose
+            // bridge the retrace was building therefore never got one and deopted
+            // on every delivery.
             if bound_method.is_none()
                 && (0..callee_code.instructions.len()).any(|pc| {
                     matches!(
@@ -3221,7 +3232,7 @@ pub(crate) fn try_walker_inline_resolved_user_call<Sym: WalkSym>(
                 })
             {
                 if try_multiframe {
-                    return Err(DispatchError::callee_inline_unsupported(op.pc));
+                    return Ok(None);
                 }
                 break 'seed;
             }