Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
76 changes: 60 additions & 16 deletions pyre/pyre-interpreter/src/_pypy_generic_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,21 @@ fn tuple_index(t: PyObjectRef, item: PyObjectRef) -> Result<Option<usize>, crate
/// unpacked `tuple[...]` alias (one exposing `__typing_unpacked_tuple_args__`)
/// into its members, unless those end in `...`. Returns a fresh items tuple.
fn unpack_args(items: PyObjectRef) -> Result<PyObjectRef, crate::PyError> {
let n = unsafe { w_tuple_len(items) };
let mut newargs: Vec<PyObjectRef> = Vec::new();
// The loop body runs Python at every turn, so the accumulator holds slot
// indices rather than values — the same shape `push_newarg` uses above —
// and `items` is read back before each element fetch.
let _roots = pyre_object::gc_roots::push_roots();
let items_slot = pyre_object::gc_roots::shadow_stack_len();
pyre_object::gc_roots::pin_root(items);
let items = || pyre_object::gc_roots::shadow_stack_get(items_slot);
let n = unsafe { w_tuple_len(items()) };
let mut newarg_slots: Vec<usize> = Vec::new();
let mut push_newarg = |value: PyObjectRef, slots: &mut Vec<usize>| {
slots.push(pyre_object::gc_roots::shadow_stack_len());
pyre_object::gc_roots::pin_root(value);
};
for i in 0..n {
let Some(arg) = (unsafe { w_tuple_getitem(items, i as i64) }) else {
let Some(arg) = (unsafe { w_tuple_getitem(items(), i as i64) }) else {
continue;
};
let subargs = match crate::baseobjspace::getattr_str(arg, "__typing_unpacked_tuple_args__")
Expand All @@ -392,13 +403,21 @@ fn unpack_args(items: PyObjectRef) -> Result<PyObjectRef, crate::PyError> {
};
if do_unpack {
// `newargs.extend(subargs)` — any iterable, not just a tuple.
for x in crate::builtins::collect_iterable(subargs)? {
newargs.push(x);
// Publish the collected members in one go: `collect_iterable`'s own
// scope has popped, so its Vec is untraced from here on.
let members = crate::builtins::collect_iterable(subargs)?;
let member_base = pyre_object::gc_roots::pin_roots(&members);
for index in 0..members.len() {
newarg_slots.push(member_base + index);
}
} else {
newargs.push(arg);
push_newarg(arg, &mut newarg_slots);
}
}
let newargs: Vec<PyObjectRef> = newarg_slots
.iter()
.map(|&slot| pyre_object::gc_roots::shadow_stack_get(slot))
.collect();
Ok(w_tuple_new(newargs))
}

Expand Down Expand Up @@ -719,27 +738,46 @@ fn subs_tvars(
return Ok(obj);
}
let nsub = unsafe { w_tuple_len(subparams) };
let mut subargs: Vec<PyObjectRef> = Vec::with_capacity(nsub);
// `tuple_index` runs a user `__eq__` on every turn, so the operands and the
// arguments picked so far are held as shadow-stack slots and read back when
// the substitution tuple is built.
let _roots = pyre_object::gc_roots::push_roots();
let base = pyre_object::gc_roots::pin_roots(&[obj, params, argitems, subparams]);
let obj = || pyre_object::gc_roots::shadow_stack_get(base);
let params = || pyre_object::gc_roots::shadow_stack_get(base + 1);
let argitems = || pyre_object::gc_roots::shadow_stack_get(base + 2);
let subparams = || pyre_object::gc_roots::shadow_stack_get(base + 3);
let mut subarg_slots: Vec<usize> = Vec::with_capacity(nsub);
for i in 0..nsub {
let Some(param) = (unsafe { w_tuple_getitem(subparams, i as i64) }) else {
let Some(param) = (unsafe { w_tuple_getitem(subparams(), i as i64) }) else {
continue;
};
// `try: argitems[params.index(param)] except ValueError: param`.
let arg = match tuple_index(params, param)? {
Some(idx) => unsafe { w_tuple_getitem(argitems, idx as i64) }.unwrap_or(param),
let arg = match tuple_index(params(), param)? {
Some(idx) => unsafe { w_tuple_getitem(argitems(), idx as i64) }.unwrap_or(param),
None => param,
};
// `if isinstance(param, TypeVarTuple): subargs.extend(arg)` — a
// `TypeVarTuple` captures a sequence, so its bound `arg` is spliced.
if is_typevartuple(param) {
for x in crate::builtins::collect_iterable(arg)? {
subargs.push(x);
let members = crate::builtins::collect_iterable(arg)?;
let member_base = pyre_object::gc_roots::pin_roots(&members);
for index in 0..members.len() {
subarg_slots.push(member_base + index);
}
} else {
subargs.push(arg);
subarg_slots.push(pyre_object::gc_roots::shadow_stack_len());
pyre_object::gc_roots::pin_root(arg);
}
}
crate::baseobjspace::getitem(obj, w_tuple_new(subargs))
let subargs: Vec<PyObjectRef> = subarg_slots
.iter()
.map(|&slot| pyre_object::gc_roots::shadow_stack_get(slot))
.collect();
// Build the substitution tuple before reading `obj` back: `w_tuple_new`
// allocates, so a receiver read ahead of it would be the pre-move one.
let subs = w_tuple_new(subargs);
crate::baseobjspace::getitem(obj(), subs)
}

/// `_make_starred(ga)` (`_pypy_generic_alias.py:118`) — a copy of the alias
Expand Down Expand Up @@ -1489,10 +1527,16 @@ fn join_wtf8(parts: &[rustpython_wtf8::Wtf8Buf], sep: &str) -> rustpython_wtf8::
/// after its predecessor's repr so mutation during a callback raises
/// `IndexError` rather than reading stale storage.
unsafe fn repr_items_list(list: PyObjectRef) -> Result<rustpython_wtf8::Wtf8Buf, crate::PyError> {
let n = w_list_len(list);
// An element's repr runs Python, and a `W_ListObject` header moves, so the
// list is re-read from the shadow stack before every fetch.
let _roots = pyre_object::gc_roots::push_roots();
let list_slot = pyre_object::gc_roots::shadow_stack_len();
pyre_object::gc_roots::pin_root(list);
let list = || pyre_object::gc_roots::shadow_stack_get(list_slot);
let n = w_list_len(list());
let mut parts = Vec::with_capacity(n);
for i in 0..n {
let item = w_list_getitem(list, i as i64)
let item = w_list_getitem(list(), i as i64)
.ok_or_else(|| crate::PyError::index_error("list index out of range"))?;
parts.push(repr_item(item)?);
}
Expand Down
54 changes: 44 additions & 10 deletions pyre/pyre-interpreter/src/baseobjspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12724,8 +12724,8 @@ pub fn call_args_and_c_profile(
callable: PyObjectRef,
args: &[PyObjectRef],
) -> PyObjectRef {
let arguments = crate::argument::Arguments::positional_only(args);
call_args_and_c_profile_args(frame, callable, &arguments, args)
let mut arguments = crate::argument::Arguments::positional_only(args);
call_args_and_c_profile_args(frame, callable, &mut arguments, args)
}

/// `baseobjspace.py:1269-1278 call_args_and_c_profile` with a
Expand All @@ -12746,23 +12746,52 @@ pub fn call_args_and_c_profile(
pub fn call_args_and_c_profile_args(
frame: &mut crate::pyframe::PyFrame,
callable: PyObjectRef,
arguments: &crate::argument::Arguments,
arguments: &mut crate::argument::Arguments,
flat_args: &[PyObjectRef],
) -> PyObjectRef {
// Reaching here means a profile function is installed, so the tracer hooks
// below run Python — and the callee runs in between. `callable`, the flat
// slice and the `Arguments` vectors are native storage no root walker
// updates. Root them all, dispatch from the roots, and refresh the
// `Arguments` vectors before the return hook reads them again.
let _roots = pyre_object::gc_roots::push_roots();
let callable_slot = pyre_object::gc_roots::shadow_stack_len();
pyre_object::gc_roots::pin_root(callable);
let flat_base = pyre_object::gc_roots::pin_roots(flat_args);
let positional_base = pyre_object::gc_roots::pin_roots(&arguments.arguments_w);
let keyword_base = arguments
.keywords_w
.as_ref()
.map(|values| pyre_object::gc_roots::pin_roots(values));
let callable = || pyre_object::gc_roots::shadow_stack_get(callable_slot);
let refresh = |arguments: &mut crate::argument::Arguments| {
for (index, slot) in arguments.arguments_w.iter_mut().enumerate() {
*slot = pyre_object::gc_roots::shadow_stack_get(positional_base + index);
}
if let (Some(base), Some(values)) = (keyword_base, arguments.keywords_w.as_mut()) {
for (index, slot) in values.iter_mut().enumerate() {
*slot = pyre_object::gc_roots::shadow_stack_get(base + index);
}
}
};

let ec = crate::call::getexecutioncontext() as *mut crate::PyExecutionContext;
if !ec.is_null()
&& let Err(err) = unsafe {
(*ec).c_call_trace(
frame as *mut crate::pyframe::PyFrame,
callable,
callable(),
Some(arguments),
)
}
{
crate::call::set_call_error(err);
return pyre_object::PY_NULL;
}
let w_res = call_function(callable, flat_args);
let flat_args: Vec<PyObjectRef> = (0..flat_args.len())
.map(|index| pyre_object::gc_roots::shadow_stack_get(flat_base + index))
.collect();
let w_res = call_function(callable(), &flat_args);
if w_res == pyre_object::PY_NULL {
if !ec.is_null() {
// baseobjspace.py:1274-1276 — `except OperationError:
Expand All @@ -12773,27 +12802,32 @@ pub fn call_args_and_c_profile_args(
// stash already holds the original OperationError; if
// c_exception_trace raises, overwrite the stash so the
// tracer error is what propagates.
if let Err(trace_err) =
unsafe { (*ec).c_exception_trace(frame as *mut crate::pyframe::PyFrame, callable) }
{
if let Err(trace_err) = unsafe {
(*ec).c_exception_trace(frame as *mut crate::pyframe::PyFrame, callable())
} {
crate::call::set_call_error(trace_err);
}
}
return pyre_object::PY_NULL;
}
refresh(arguments);
// The return hook runs Python too, so the callee's result is published
// before it and read back afterwards.
let result_slot = pyre_object::gc_roots::shadow_stack_len();
pyre_object::gc_roots::pin_root(w_res);
Comment on lines +12813 to +12817

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Refresh profiler arguments after pinning the result

With an active C profiler and a foreign mutator, pin_root(w_res) can synchronize with a collection after refresh(arguments) has copied the current pointers back into the native vectors. The shadow-stack slots are forwarded, but arguments.arguments_w and keywords_w immediately become stale again, so c_return_trace can read a pre-move list or dict as its firstarg() while rebinding a fixed-code builtin. Pin the result before refreshing, or refresh again afterward.

Useful? React with 👍 / 👎.

if !ec.is_null()
&& let Err(err) = unsafe {
(*ec).c_return_trace(
frame as *mut crate::pyframe::PyFrame,
callable,
callable(),
Some(arguments),
)
}
{
crate::call::set_call_error(err);
return pyre_object::PY_NULL;
}
w_res
pyre_object::gc_roots::shadow_stack_get(result_slot)
}

/// PyPy: baseobjspace.py `call_method`.
Expand Down
20 changes: 16 additions & 4 deletions pyre/pyre-interpreter/src/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7093,6 +7093,11 @@ fn import_error_setstate(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyE
if !require_setstate_dict(w_state)? {
return Ok(pyre_object::w_none());
}
// Each `pop` runs `dict.pop`, and the state proven above is a dict — one of
// the two kinds the collector moves. Root the receiver and the state the
// way `base_exception_setstate` does, and read both back per turn.
let _roots = pyre_object::gc_roots::push_roots();
let base = pyre_object::gc_roots::pin_roots(&[w_self, w_state]);
type ExcSetter = unsafe fn(PyObjectRef, PyObjectRef);
for (key, set) in [
("name", interp_exceptions::w_exception_set_name as ExcSetter),
Expand All @@ -7103,7 +7108,7 @@ fn import_error_setstate(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyE
),
] {
let popped = crate::baseobjspace::call_method(
w_state,
pyre_object::gc_roots::shadow_stack_get(base + 1),
"pop",
&[pyre_object::w_str_new(key), pyre_object::w_none()],
);
Expand All @@ -7112,10 +7117,17 @@ fn import_error_setstate(args: &[PyObjectRef]) -> Result<PyObjectRef, crate::PyE
{
return Err(e);
}
unsafe { set(w_self, popped) };
unsafe { set(pyre_object::gc_roots::shadow_stack_get(base), popped) };
}
let w_olddict = unsafe { interp_exceptions::w_exception_getdict(w_self) };
if crate::baseobjspace::call_method(w_olddict, "update", &[w_state]).is_null()
let w_olddict = unsafe {
interp_exceptions::w_exception_getdict(pyre_object::gc_roots::shadow_stack_get(base))
};
if crate::baseobjspace::call_method(
w_olddict,
"update",
&[pyre_object::gc_roots::shadow_stack_get(base + 1)],
)
.is_null()
&& let Some(e) = crate::call::take_call_error()
{
return Err(e);
Expand Down
Loading
Loading