diff --git a/pyre/bench/synth/foriter_isinstance_class_property_replay.cranelift.jitstats b/pyre/bench/synth/foriter_isinstance_class_property_replay.cranelift.jitstats new file mode 100644 index 00000000000..910ee008dba --- /dev/null +++ b/pyre/bench/synth/foriter_isinstance_class_property_replay.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=1 +internal_compile_panics=0 +loops_aborted=1 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/foriter_isinstance_class_property_replay.dynasm.jitstats b/pyre/bench/synth/foriter_isinstance_class_property_replay.dynasm.jitstats new file mode 100644 index 00000000000..910ee008dba --- /dev/null +++ b/pyre/bench/synth/foriter_isinstance_class_property_replay.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=1 +internal_compile_panics=0 +loops_aborted=1 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/foriter_isinstance_class_property_replay.py b/pyre/bench/synth/foriter_isinstance_class_property_replay.py new file mode 100644 index 00000000000..0ce911cd3c0 --- /dev/null +++ b/pyre/bench/synth/foriter_isinstance_class_property_replay.py @@ -0,0 +1,38 @@ +# `isinstance(obj, T)` first checks the object's real MRO and, on a miss, +# reads `obj.__class__`. Inheriting `object.__getattribute__` does not make +# that read pure: a class can still override `__class__` with a property. +# +# `helper` is admitted into the surrounding FOR_ITER body. The trailing +# opaque `id` call makes the first inline sub-walk abort and replay `helper`. +# If the `isinstance` call is incorrectly marked replay-safe, its property +# effect is not recorded and that one replay invokes it twice (N + 1 hits). + +N = 5000 +hits = [0] + + +class C: + def __init__(self): + self.x = 1 + + @property + def __class__(self): + hits[0] += 1 + return str + + +def helper(obj): + # Memoize that C inherits object.__getattribute__; this was the incomplete + # proof used by the buggy replay-safe predicate. + obj.x + result = isinstance(obj, int) + id(obj) + return result + + +obj = C() +total = 0 +for _ in range(N): + total += helper(obj) + +print(hits[0], total) diff --git a/pyre/bench/synth/foriter_isinstance_class_property_replay.wasm.jitstats b/pyre/bench/synth/foriter_isinstance_class_property_replay.wasm.jitstats new file mode 100644 index 00000000000..910ee008dba --- /dev/null +++ b/pyre/bench/synth/foriter_isinstance_class_property_replay.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=1 +internal_compile_panics=0 +loops_aborted=1 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/foriter_str_subclass_replay.cranelift.jitstats b/pyre/bench/synth/foriter_str_subclass_replay.cranelift.jitstats new file mode 100644 index 00000000000..910ee008dba --- /dev/null +++ b/pyre/bench/synth/foriter_str_subclass_replay.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=1 +internal_compile_panics=0 +loops_aborted=1 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/foriter_str_subclass_replay.dynasm.jitstats b/pyre/bench/synth/foriter_str_subclass_replay.dynasm.jitstats new file mode 100644 index 00000000000..910ee008dba --- /dev/null +++ b/pyre/bench/synth/foriter_str_subclass_replay.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=1 +internal_compile_panics=0 +loops_aborted=1 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/foriter_str_subclass_replay.py b/pyre/bench/synth/foriter_str_subclass_replay.py new file mode 100644 index 00000000000..686dec09079 --- /dev/null +++ b/pyre/bench/synth/foriter_str_subclass_replay.py @@ -0,0 +1,27 @@ +# The FOR_ITER inline replay exemption for `str(value)` applies only to exact +# immutable builtin scalars. A str subclass can override `__str__` and mutate +# visible state. The trailing opaque `id` call makes the first inline walk +# abort; treating the subclass as exact replays that override once (N + 1). + +N = 5000 +hits = [0] + + +class S(str): + def __str__(self): + hits[0] += 1 + return "x" + + +def helper(value): + result = str(value) + id(value) + return len(result) + + +value = S("abc") +total = 0 +for _ in range(N): + total += helper(value) + +print(hits[0], total) diff --git a/pyre/bench/synth/foriter_str_subclass_replay.wasm.jitstats b/pyre/bench/synth/foriter_str_subclass_replay.wasm.jitstats new file mode 100644 index 00000000000..910ee008dba --- /dev/null +++ b/pyre/bench/synth/foriter_str_subclass_replay.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=1 +internal_compile_panics=0 +loops_aborted=1 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index 84595c59e7d..199becff68f 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -9973,6 +9973,27 @@ pub(crate) unsafe fn has_object_getattribute(w_type: PyObjectRef) -> bool { getattribute_if_not_from_object(w_type).is_none() } +/// Whether an `isinstance` miss can read `obj.__class__` without invoking +/// app-level code. +/// +/// `abstractinst.py:p_recursive_isinstance_type_w` performs that attribute +/// read after the direct MRO test misses. Inheriting +/// `object.__getattribute__` is not sufficient: a subclass may still replace +/// `__class__` with a property. Require both the already-memoized default +/// `__getattribute__` and the canonical `object.__class__` descriptor. +/// `W_TypeObject.mutated` invalidates the memoized flag and the method cache +/// whenever either lookup shape changes. +pub unsafe fn isinstance_miss_class_lookup_is_pure(w_type: PyObjectRef) -> bool { + if !pyre_object::typeobject::w_type_get_uses_object_getattribute(w_type) { + return false; + } + let Some(actual_class_descr) = lookup_in_type_where(w_type, "__class__") else { + return false; + }; + lookup_in_type_where(crate::typedef::w_object(), "__class__") + .is_some_and(|object_class_descr| std::ptr::eq(actual_class_descr, object_class_descr)) +} + /// typeobject.py:328-348 `setattr_if_not_from_object` — the `__setattr__` /// companion of [`getattribute_if_not_from_object`]. pub(crate) unsafe fn setattr_if_not_from_object(w_type: PyObjectRef) -> Option { @@ -19008,6 +19029,24 @@ mod tests { } } + #[test] + fn isinstance_miss_requires_canonical_object_class_descriptor() { + crate::typedef::init_typeobjects(); + let t = crate::typedef::make_builtin_type("IsinstanceClassLookup", |_| {}); + unsafe { + pyre_object::typeobject::w_type_set_uses_object_getattribute(t, true); + assert!(isinstance_miss_class_lookup_is_pure(t)); + + // A `property` is the app-visible hazardous case. Any MRO + // replacement is enough to prove the identity gate itself: only + // the canonical object getset may enter the replay-safe arm. + crate::type_dict_store(t, "__class__", pyre_object::w_int_new(7)); + mutated(t, Some("__class__")); + pyre_object::typeobject::w_type_set_uses_object_getattribute(t, true); + assert!(!isinstance_miss_class_lookup_is_pure(t)); + } + } + /// typeobject.py:303-326 — builtin types and a plain user class that /// inherit object.__getattribute__ answer `None`, so the layout-agnostic /// receiver gate does not route them through an app-level call. 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 563e7014157..1d296ac63dc 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -2830,11 +2830,12 @@ pub(crate) fn try_execute_residual_call_via_executor( && !operand.is_null() && std::ptr::eq(callable, str_type) && unsafe { - pyre_object::is_int_or_long(operand) - || pyre_object::is_float(operand) - || pyre_object::is_complex(operand) - || pyre_object::is_str(operand) - || pyre_object::is_none(operand) + pyre_object::is_exact_builtin_instance(operand) + && (pyre_object::is_int_or_long(operand) + || pyre_object::is_float(operand) + || pyre_object::is_complex(operand) + || pyre_object::is_str(operand) + || pyre_object::is_none(operand)) } }; // PyPy traces `space.iter(w_exact_unicode)` through @@ -2858,7 +2859,10 @@ pub(crate) fn try_execute_residual_call_via_executor( && args.len() == 1 && { let operand = args[0] as pyre_object::PyObjectRef; - !operand.is_null() && unsafe { pyre_object::is_str(operand) } + !operand.is_null() + && unsafe { + pyre_object::is_str(operand) && pyre_object::is_exact_builtin_instance(operand) + } }; // PyPy's `space.ord` reads the immutable unicode payload directly. Match // both canonical builtin-code identity and the observed exact string so a @@ -2876,9 +2880,10 @@ pub(crate) fn try_execute_residual_call_via_executor( // then (only on a miss) `w_inst.__class__` (`abstractinst.py:74-86`). A // hit is replay-safe directly. A miss is replay-safe only when the // instance type has already proven and memoized that it inherits - // `object.__getattribute__`; type mutation invalidates that flag. This - // covers the ordinary `isinstance(C0(), int)` false result without blessing - // an override that can run Python and mutate live heap. + // `object.__getattribute__` AND its MRO resolves `__class__` to the + // canonical `object.__class__` descriptor. The first fact alone is not + // enough: `@property def __class__` retains the default getattribute path + // but runs user code here. Type mutation invalidates both lookup facts. // // Require the canonical callable and an ordinary class whose metaclass is // exactly `type`; tuple/union classinfo and custom `__instancecheck__` @@ -2899,7 +2904,7 @@ pub(crate) fn try_execute_residual_call_via_executor( }) && pyre_interpreter::typedef::r#type(object).is_some_and(|actual| unsafe { pyre_interpreter::baseobjspace::isinstance_w(object, classinfo) - || pyre_object::typeobject::w_type_get_uses_object_getattribute( + || pyre_interpreter::baseobjspace::isinstance_miss_class_lookup_is_pure( actual.as_ptr(), ) })