Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
283 changes: 241 additions & 42 deletions pyre/pyre-interpreter/src/_pypy_generic_alias.rs

Large diffs are not rendered by default.

167 changes: 146 additions & 21 deletions pyre/pyre-interpreter/src/baseobjspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9280,15 +9280,30 @@ pub(crate) unsafe fn compute_and_set_mro(w_self: PyObjectRef) -> PyResult {
// PyPy's C3 merge raises here when the bases are inconsistent; pyre's
// low-level Vec-returning helper cannot carry that exception, so preserve
// the same ordering through its fallible validation front door.
// The validation and the metaclass `mro()` below both execute Python. A
// type reaching `type.__new__` has no referrer yet beyond this argument,
// and the MRO snapshots are untraced Rust Vecs, so root all of them and
// reread every use that crosses one of those calls.
let _roots = pyre_object::gc_roots::push_roots();
let self_slot = pin_slot(w_self);
let w_bases = pyre_object::typeobject::w_type_get_bases(w_self);
validate_c3_mro(w_bases)?;
validate_c3_mro(w_bases, true)?;
let w_self = pyre_object::gc_roots::shadow_stack_get(self_slot);
let default_mro = compute_default_mro(w_self);
let default_mro_start = pyre_object::gc_roots::shadow_stack_len();
let default_mro_len = default_mro.len();
for w_class in default_mro {
pyre_object::gc_roots::pin_root(w_class);
}
let default_mro =
|index: usize| pyre_object::gc_roots::shadow_stack_get(default_mro_start + index);
if pyre_object::w_type_is_heaptype(w_self) {
let w_metaclass = (*w_self).w_class;
if !w_metaclass.is_null() {
if let Some((w_where, w_mro_func)) = lookup_where_with_method_cache(w_metaclass, "mro")
{
if !std::ptr::eq(w_where, crate::typedef::w_type()) {
let w_self = pyre_object::gc_roots::shadow_stack_get(self_slot);
let w_mro = get_and_call_function(w_mro_func, w_self, w_metaclass, &[])?;
let mro_w = crate::builtins::collect_iterable(w_mro)?;
// `fixedview` keeps PyPy's items GC-visible through the
Expand All @@ -9307,27 +9322,28 @@ pub(crate) unsafe fn compute_and_set_mro(w_self: PyObjectRef) -> PyResult {
return Err(PyError::type_error("mro() returned a non-class"));
}
}
let mro_w: Vec<_> = (0..mro_w.len())
.map(|index| {
pyre_object::gc_roots::shadow_stack_get(mro_root_start + index)
})
.collect();
if !mro_w.iter().any(|&entry| std::ptr::eq(entry, w_self)) {
let mro_len = mro_w.len();
let mro_at = |index: usize| {
pyre_object::gc_roots::shadow_stack_get(mro_root_start + index)
};
let w_self = pyre_object::gc_roots::shadow_stack_get(self_slot);
if !(0..mro_len).any(|index| std::ptr::eq(mro_at(index), w_self)) {
return Err(PyError::type_error(
"mro() returned a result without the new class",
));
}
pyre_object::w_type_set_mro(w_self, mro_w.clone());
pyre_object::w_type_set_mro(w_self, (0..mro_len).map(mro_at).collect());

// typeobject.py `_add_mro_classes_as_subclasses`: custom
// MRO entries outside the default hierarchy participate
// in invalidation just like real bases.
for w_ancestor in mro_w {
if !default_mro
.iter()
.any(|&default| std::ptr::eq(default, w_ancestor))
for index in 0..mro_len {
let w_ancestor = mro_at(index);
if !(0..default_mro_len)
.any(|default| std::ptr::eq(default_mro(default), w_ancestor))
&& pyre_object::is_type(w_ancestor)
{
let w_self = pyre_object::gc_roots::shadow_stack_get(self_slot);
pyre_object::typeobject::w_type_add_subclass(w_ancestor, w_self);
}
}
Expand All @@ -9336,23 +9352,88 @@ pub(crate) unsafe fn compute_and_set_mro(w_self: PyObjectRef) -> PyResult {
}
}
}
pyre_object::w_type_set_mro(w_self, default_mro);
let w_self = pyre_object::gc_roots::shadow_stack_get(self_slot);
pyre_object::w_type_set_mro(w_self, (0..default_mro_len).map(default_mro).collect());
Ok(pyre_object::w_none())
}

/// Pin `w` into the ambient root scope and hand back its slot.
fn pin_slot(w: PyObjectRef) -> usize {
pyre_object::gc_roots::pin_root(w);
pyre_object::gc_roots::shadow_stack_len() - 1
}

/// `typeobject.py:1665-1678 abstract_mro` — the app-level classic-class walk
/// that `get_mro` (typeobject.py:1680-1684) applies to a base which is not a
/// `W_TypeObject`. Reading `__bases__` is what rejects a plain instance
/// handed in as a base.
///
/// Returns shadow-stack slots rather than values: a `__bases__` read can run
/// `__getattr__` and allocate, so the caller's scope owns the walk's roots and
/// the entries stay addressable across the reads that follow.
///
/// `klass not in mro` is answered by pointer identity, matching the C3 merge
/// this feeds instead of running an app-level `__eq__` between the reads.
unsafe fn abstract_mro(w_klass: PyObjectRef) -> Result<Vec<usize>, crate::PyError> {
let mut mro_slots: Vec<usize> = Vec::new();
let mut stack_slots: Vec<usize> = vec![pin_slot(w_klass)];
while let Some(slot) = stack_slots.pop() {
let w_cls = pyre_object::gc_roots::shadow_stack_get(slot);
if mro_slots
.iter()
.any(|&seen| std::ptr::eq(pyre_object::gc_roots::shadow_stack_get(seen), w_cls))
Comment on lines +9382 to +9384

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve Python equality in abstract MRO membership

For a non-type base graph containing distinct nodes that compare equal, this identity test diverges from PyPy's literal klass not in mro, which invokes Python equality. For example, if a child compares equal to an already visited pseudo-class but lacks __bases__, PyPy skips the child while this implementation visits it and raises AttributeError; custom equality side effects are also lost. Use the app-level membership semantics of the upstream walk rather than pointer identity.

AGENTS.md reference: AGENTS.md:L195-L195

Useful? React with 👍 / 👎.

{
continue;
}
mro_slots.push(slot);
let bases_slot = pin_slot(getattr_str(w_cls, "__bases__")?);
if !is_tuple(pyre_object::gc_roots::shadow_stack_get(bases_slot)) {
return Err(crate::PyError::type_error("__bases__ must be a tuple"));
}
// `stack += klass.__bases__[::-1]` — the reversed extend leaves
// `__bases__[0]` on top, so the walk descends in declaration order.
let nbases = w_tuple_len(pyre_object::gc_roots::shadow_stack_get(bases_slot));
for j in (0..nbases).rev() {
if let Some(w_base) = w_tuple_getitem(
pyre_object::gc_roots::shadow_stack_get(bases_slot),
j as i64,
) {
stack_slots.push(pin_slot(w_base));
}
}
}
Ok(mro_slots)
}

/// Reject a base tuple whose C3 merge has no valid next head.
///
/// Type construction calls this before allocating the new type or running
/// descriptor/class-subclass hooks, so an invalid hierarchy cannot escape as
/// a later and unrelated lookup failure.
pub unsafe fn validate_c3_mro(bases: PyObjectRef) -> Result<(), crate::PyError> {
///
/// `walk_classic_bases` selects `get_mro`'s classic branch, which reads a
/// non-type base's `__bases__` and so can run Python. Only the call standing
/// in for `compute_mro` (typeobject.py:1560) passes `true`: that one runs after
/// `check_and_find_best_base` (typeobject.py:1519), so a bad type base is still
/// reported before any classic base's `__bases__` executes. The early
/// pre-flight passes `false` and stays a pure C3 check.
pub unsafe fn validate_c3_mro(
bases: PyObjectRef,
walk_classic_bases: bool,
) -> Result<(), crate::PyError> {
if bases.is_null() || !is_tuple(bases) {
return Ok(());
}
let n = w_tuple_len(bases);
// `is_type_like_w` dispatches through the object space and the classic-base
// walk below runs Python outright, so the tuple is pinned for the whole
// validation and reread after anything that can collect.
let _roots = pyre_object::gc_roots::push_roots();
let bases_slot = pin_slot(bases);
// CPython's typeobject.c reports duplicate direct bases before the C3
// merge. PyPy's mro_error discovers the same case in its final list.
for i in 0..n {
let bases = pyre_object::gc_roots::shadow_stack_get(bases_slot);
let Some(base) = w_tuple_getitem(bases, i as i64) else {
continue;
};
Expand All @@ -9370,23 +9451,67 @@ pub unsafe fn validate_c3_mro(bases: PyObjectRef) -> Result<(), crate::PyError>
)));
}
}
let mut lists: Vec<Vec<PyObjectRef>> = Vec::with_capacity(n + 1);
let mut bases_list = Vec::with_capacity(n);
// typeobject.py:1519,1560 — `setup_user_defined_type` runs
// `check_and_find_best_base` before it reaches `compute_mro`, so the C3
// merge only ever sees a tuple that already holds at least one type. With
// no type among the bases the tuple belongs to `check_and_find_best_base`
// and its own message, not to the classic walk.
let walk_classic_bases = walk_classic_bases
&& (0..n).any(|i| {
w_tuple_getitem(
pyre_object::gc_roots::shadow_stack_get(bases_slot),
i as i64,
)
.is_some_and(|base| is_type_like_w(base))
});

// typeobject.py:1689-1690 `orderlists = [get_mro(space, base) for base in
// cls.bases_w]` then `orderlists.append([cls] + cls.bases_w)`. The
// classic branch of `get_mro` runs Python, so the lists are accumulated as
// shadow-stack slots and only materialized once the build is over — the
// merge below allocates nothing, so reading the values there is safe.
let mut list_slots: Vec<Vec<usize>> = Vec::with_capacity(n + 1);
let mut bases_slots = Vec::with_capacity(n);
for i in 0..n {
let Some(base) = w_tuple_getitem(bases, i as i64) else {
let Some(base) = w_tuple_getitem(
pyre_object::gc_roots::shadow_stack_get(bases_slot),
i as i64,
) else {
continue;
};
// typeobject.py:1680-1684 `get_mro`: a `W_TypeObject` contributes its
// own linearization, anything else is walked as a classic class.
if is_type_like_w(base) {
let mro = w_type_get_mro(base);
lists.push(if mro.is_null() {
let entry = if mro.is_null() {
compute_mro(base)
} else {
(*mro).to_vec()
});
};
list_slots.push(entry.into_iter().map(pin_slot).collect());
} else if walk_classic_bases {
list_slots.push(abstract_mro(base)?);
}
bases_list.push(base);
bases_slots.push(pin_slot(
w_tuple_getitem(
pyre_object::gc_roots::shadow_stack_get(bases_slot),
i as i64,
)
.unwrap_or(base),
));
}
lists.push(bases_list);
list_slots.push(bases_slots);

let bases = pyre_object::gc_roots::shadow_stack_get(bases_slot);
let mut lists: Vec<Vec<PyObjectRef>> = list_slots
.into_iter()
.map(|slots| {
slots
.into_iter()
.map(pyre_object::gc_roots::shadow_stack_get)
.collect()
})
.collect();
Comment thread
coderabbitai[bot] marked this conversation as resolved.

loop {
lists.retain(|list| !list.is_empty());
Expand Down
2 changes: 1 addition & 1 deletion pyre/pyre-interpreter/src/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4820,7 +4820,7 @@ fn type_descr_new_with_metaclass(

// This is type.__new__'s own construction path. A different winning
// metaclass above received the original bases without a C3 pre-check.
unsafe { crate::baseobjspace::validate_c3_mro(w_effective_bases)? };
unsafe { crate::baseobjspace::validate_c3_mro(w_effective_bases, false)? };

let _dict_root = pyre_object::gc_roots::push_roots();
let dict_root = pyre_object::gc_roots::shadow_stack_len();
Expand Down
68 changes: 60 additions & 8 deletions pyre/pyre-interpreter/src/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,10 @@ pub fn call_user_function_resolved(
/// `__origin__`, set `result.__orig_class__ = self`. This is wrapped in
/// `try: ... except (AttributeError, TypeError): pass`, so only those two
/// errors are swallowed; anything else propagates.
fn set_orig_class(result: PyObjectRef, alias: PyObjectRef) -> Result<(), crate::PyError> {
pub(crate) fn set_orig_class(
result: PyObjectRef,
alias: PyObjectRef,
) -> Result<(), crate::PyError> {
match crate::baseobjspace::setattr_str(result, "__orig_class__", alias) {
Ok(_) => Ok(()),
Err(e)
Expand Down Expand Up @@ -3858,10 +3861,16 @@ fn build_class_inner(
} else {
bases
};
// The C3 validations read `__bases__` off classic bases and
// `create_all_slots` unpacks `__slots__`; both execute Python, so the
// tuple cannot stay in an untraced local across them.
let bases_root = pyre_object::gc_roots::shadow_stack_len();
pyre_object::gc_roots::pin_root(w_effective_bases);
// A custom metaclass owns its bases until (and unless) it invokes
// type.__new__; do not perform type's C3 validation before dispatch.
if w_metaclass.is_none() {
unsafe { crate::baseobjspace::validate_c3_mro(w_effective_bases)? };
let w_effective_bases = pyre_object::gc_roots::shadow_stack_get(bases_root);
unsafe { crate::baseobjspace::validate_c3_mro(w_effective_bases, false)? };
}
// Create class via metaclass or default type()
// PyPy: typeobject.py — metaclass(name, bases, dict_w) or type.__new__
Expand Down Expand Up @@ -4024,14 +4033,36 @@ fn build_class_inner(
}
}
let dict_obj = pyre_object::gc_roots::shadow_stack_get(dict_root);
let w = pyre_object::w_type_new(name, w_effective_bases, dict_obj as *mut u8);
// Slot creation, C3 validation and `__set_name__` below all allocate
// and can execute Python; the nascent type has no other referrer
// until its mro is installed, so keep it rooted and reread it after
// every such step.
let w_root = pyre_object::gc_roots::shadow_stack_len();
let w = pyre_object::w_type_new(
name,
pyre_object::gc_roots::shadow_stack_get(bases_root),
dict_obj as *mut u8,
);
pyre_object::gc_roots::pin_root(w);
crate::builtins::type_new_take_qualname(w, dict_obj)?;
// typeobject.py:1143-1204 create_all_slots parity.
unsafe { create_all_slots(w, w_effective_bases)? };
unsafe { create_all_slots(w, pyre_object::gc_roots::shadow_stack_get(bases_root))? };
// baseobjspace.py:76 — set w_class to 'type' (default metaclass)
let w = pyre_object::gc_roots::shadow_stack_get(w_root);
unsafe {
(*w).w_class = crate::typedef::w_type();
}
// typeobject.py:1560 `compute_mro(w_self)`, reached only once
// `check_and_find_best_base` inside `create_all_slots` above accepted
// the tuple. `compute_default_mro` cannot raise, so `get_mro`'s
// classic branch runs through the fallible validation here.
unsafe {
crate::baseobjspace::validate_c3_mro(
pyre_object::gc_roots::shadow_stack_get(bases_root),
true,
)?
};
let w = pyre_object::gc_roots::shadow_stack_get(w_root);
let mro = unsafe { crate::baseobjspace::compute_default_mro(w) };
unsafe { pyre_object::w_type_set_mro(w, mro) };
// typeobject.py:373-377 ready() — register self on each base's
Expand All @@ -4045,6 +4076,7 @@ fn build_class_inner(
// provisional class-body namespace.
if let Some(classdictcell_root) = classdictcell_root {
let classdictcell = pyre_object::gc_roots::shadow_stack_get(classdictcell_root);
let w = pyre_object::gc_roots::shadow_stack_get(w_root);
let type_dict = unsafe { pyre_object::w_type_get_dict_ptr(w) as PyObjectRef };
if !type_dict.is_null() {
unsafe { pyre_object::w_cell_set(classdictcell, type_dict) };
Expand All @@ -4056,16 +4088,29 @@ fn build_class_inner(
// __set_name__. The metaclass path above goes through type.__new__()
// which handles __set_name__ in builtins.rs, so we must NOT call it
// again there to avoid double invocation.
if unsafe { pyre_object::is_type(w) } {
if unsafe { pyre_object::is_type(pyre_object::gc_roots::shadow_stack_get(w_root)) } {
let dict_obj = pyre_object::gc_roots::shadow_stack_get(dict_root);
let entries = unsafe { pyre_object::w_dict_items(dict_obj) };
// Every `__set_name__` runs Python, so the snapshot cannot stay in
// an untraced Vec across the loop.
let _entry_roots = pyre_object::gc_roots::push_roots();
let entries_root = pyre_object::gc_roots::shadow_stack_len();
let mut pinned = 0;
for (w_name, value) in entries {
if !value.is_null() && unsafe { pyre_object::is_str(w_name) } {
unsafe { crate::baseobjspace::set_name(w, w_name, value) }?;
pyre_object::gc_roots::pin_root(w_name);
pyre_object::gc_roots::pin_root(value);
pinned += 1;
}
}
for i in 0..pinned {
let w_name = pyre_object::gc_roots::shadow_stack_get(entries_root + i * 2);
let value = pyre_object::gc_roots::shadow_stack_get(entries_root + i * 2 + 1);
let w = pyre_object::gc_roots::shadow_stack_get(w_root);
unsafe { crate::baseobjspace::set_name(w, w_name, value) }?;
}
}
w
pyre_object::gc_roots::shadow_stack_get(w_root)
};

// `_store_type_in_classcell` runs inside type.__new__, which the
Expand Down Expand Up @@ -4139,7 +4184,11 @@ fn build_class_inner(
},
_ => Vec::new(),
};
call_init_subclass_on_bases(w_type, w_effective_bases, &init_subclass_kwargs)?;
call_init_subclass_on_bases(
w_type,
pyre_object::gc_roots::shadow_stack_get(bases_root),
&init_subclass_kwargs,
)?;
}

Ok(w_type)
Expand Down Expand Up @@ -4705,6 +4754,9 @@ unsafe fn find_best_base(
let mut w_bestbase: pyre_object::PyObjectRef = std::ptr::null_mut();
for i in 0..len {
if let Some(w_candidate) = pyre_object::w_tuple_getitem(w_bases, i as i64) {
// typeobject.py:1341-1342 — a non-type base is skipped here,
// not rejected: it is a classic base, and `get_mro` walks it
// through `abstract_mro` when the C3 merge reaches it.
if !pyre_object::is_type(w_candidate) {
continue;
}
Expand Down
Loading
Loading