posix, _queue, _types: what the #1359 review threads left, and the frame the method-load opcodes pushed through

pyre/pyre-interpreter/src/cpyext/capsule.rs

@@ -30,8 +30,22 @@ pub(crate) fn capsule_type() -> PyObjectRef {
                 "__repr__",
                 crate::make_builtin_function_with_arity("__repr__", capsule_repr, 1),
             );
+            // `PyCapsule_Type` carries no `tp_new`: every capsule comes from
+            // `PyCapsule_New`, which allocates the carrier directly.
+            pyre_object::dictmultiobject::w_dict_setitem_str_no_proxy(
+                ns,
+                "__new__",
+                crate::typedef::make_new_descr(|_| {
+                    Err(crate::PyError::type_error(
+                        "cannot create 'PyCapsule' instances",
+                    ))
+                }),
+            );
         });
         unsafe { pyre_object::typeobject::w_type_set_hasdict(tp, true) };
+        // No `Py_TPFLAGS_BASETYPE` either -- a subclass would carry the name
+        // without the payload `is_capsule` reads.
+        unsafe { pyre_object::w_type_set_acceptable_as_base_class(tp, false) };
         tp as usize
     }) as PyObjectRef
 }

pyre/pyre-interpreter/src/eval.rs

@@ -4753,21 +4753,35 @@ impl OpcodeStepExecutor for PyFrame {
             self.push(obj);
             return Ok(());
         }
+        // `__getattribute__` allocates: the receiver is popped, so nothing but
+        // this local still reaches it, and the same collection relocates a
+        // JIT-created frame.  Pin the receiver and push onto the forwarded
+        // live frame.
+        let roots = pyre_object::gc_roots::push_roots();
+        let obj_slot = roots.base();
+        roots.pin_root(obj);
+        let anchor = FrameAnchor::new(self);
         let attr = crate::baseobjspace::getattr_str(obj, name)?;
+        let obj = roots.get(obj_slot);
         // LOOKUP_METHOD pushes (attr, null_or_self): the resolved attribute
         // first, then the bound receiver computed by the shared, side-effect
         // free binding decision (NULL when no self should be prepended).
         let bound = compute_load_method_bound(obj, attr, name);
-        self.push(attr);
-        self.push(bound);
+        let live = unsafe { &mut *anchor.live() };
+        live.push(attr);
+        live.push(bound);
         Ok(())
     }
 
     fn load_special(&mut self, name: &str) -> Result<(), PyError> {
         let obj = self.pop();
+        // The descriptor `__get__` allocates and can relocate a JIT-created
+        // frame; push onto the forwarded live frame.
+        let anchor = FrameAnchor::new(self);
         let bound = crate::baseobjspace::load_special_resolve(obj, name)?;
-        self.push(bound);
-        self.push(PY_NULL);
+        let live = unsafe { &mut *anchor.live() };
+        live.push(bound);
+        live.push(PY_NULL);
         Ok(())
     }
 

pyre/pyre-interpreter/src/module/_queue/mod.rs

@@ -42,6 +42,12 @@ fn queue_lock<'a>(
 /// `_queue_SimpleQueue_get_impl` reads `timeout` only on the blocking path:
 /// `block=False` is answered from the queue immediately, so the argument is
 /// neither converted nor range-checked there.
+///
+/// `_PyTime_FromSecondsObject` runs before the sign check, so a value it cannot
+/// represent as a nanosecond timestamp is refused rather than turned into a
+/// wait: an infinity would otherwise block forever and a NaN would poll once.
+/// The conversion is the one `_thread.lock.acquire` already performs
+/// (`module/thread/mod.rs parse_acquire_args`).
 fn parse_timeout(block: bool, timeout: PyObjectRef) -> Result<Option<f64>, crate::PyError> {
     if !block {
         return Ok(None);
@@ -50,6 +56,19 @@ fn parse_timeout(block: bool, timeout: PyObjectRef) -> Result<Option<f64>, crate
         return Ok(None);
     }
     let seconds = crate::baseobjspace::float_w(timeout)?;
+    if seconds.is_nan() {
+        return Err(crate::PyError::value_error(
+            "Invalid value NaN (not a number)",
+        ));
+    }
+    // `rarithmetic.ovfcheck_float_to_longlong` bounds, as in `parse_acquire_args`.
+    const NS_MIN: f64 = -9223372036854776832.0;
+    const NS_MAX: f64 = 9223372036854775296.0;
+    if !(NS_MIN..NS_MAX).contains(&(seconds * 1e9).ceil()) {
+        return Err(crate::PyError::overflow_error(
+            "timestamp out of range for platform time_t",
+        ));
+    }
     if seconds < 0.0 {
         return Err(crate::PyError::value_error(
             "'timeout' must be a non-negative number",
@@ -58,17 +77,10 @@ fn parse_timeout(block: bool, timeout: PyObjectRef) -> Result<Option<f64>, crate
     Ok(Some(seconds))
 }
 
+/// `parse_timeout` accepted only a finite, non-negative number of seconds, so
+/// the deadline is always representable and `None` means "wait forever".
 fn deadline_from_timeout(timeout: Option<f64>) -> Option<Instant> {
-    timeout.and_then(|seconds| {
-        if seconds.is_infinite() && seconds.is_sign_positive() {
-            None
-        } else if seconds <= 0.0 || seconds.is_nan() {
-            Some(Instant::now())
-        } else {
-            let capped = seconds.min((i64::MAX / 1_000_000_000) as f64);
-            Some(Instant::now() + Duration::from_secs_f64(capped))
-        }
-    })
+    timeout.map(|seconds| Instant::now() + Duration::from_secs_f64(seconds))
 }
 
 fn empty_error() -> crate::PyError {

pyre/pyre-interpreter/src/module/_types/mod.rs

@@ -26,10 +26,26 @@ fn capsule_type() -> PyObjectRef {
     not(feature = "sandbox"),
     any(target_os = "macos", target_os = "linux")
 )))]
+/// The build carries no capsules at all, so the name answers with a type that
+/// can produce none: `PyCapsule_Type` has no `tp_new` and no
+/// `Py_TPFLAGS_BASETYPE`, and a capsule only ever comes from `PyCapsule_New`.
 fn capsule_type() -> PyObjectRef {
     static CAPSULE_TYPE: OnceLock<usize> = OnceLock::new();
-    *CAPSULE_TYPE.get_or_init(|| crate::typedef::make_builtin_type("PyCapsule", |_| {}) as usize)
-        as PyObjectRef
+    *CAPSULE_TYPE.get_or_init(|| {
+        let tp = crate::typedef::make_builtin_type("PyCapsule", |ns| unsafe {
+            pyre_object::dictmultiobject::w_dict_setitem_str_no_proxy(
+                ns,
+                "__new__",
+                crate::typedef::make_new_descr(|_| {
+                    Err(crate::PyError::type_error(
+                        "cannot create 'PyCapsule' instances",
+                    ))
+                }),
+            );
+        });
+        unsafe { pyre_object::w_type_set_acceptable_as_base_class(tp, false) };
+        tp as usize
+    }) as PyObjectRef
 }
 
 pub fn init(ns: PyObjectRef) {

pyre/pyre-interpreter/src/module/posix/interp_posix.rs

@@ -6183,8 +6183,16 @@ pub fn register_module(ns: pyre_object::PyObjectRef) {
                     // iterable and each element read with `c_uid_t_w`, which is
                     // what lets -1 name `(gid_t)-1` instead of being refused.
                     let items = crate::builtins::collect_iterable(w_list)?;
+                    // `c_uid_t_w` reaches `__index__` for a non-int entry, so
+                    // converting one entry can collect and move the entries not
+                    // yet converted -- `collect_iterable` hands back a plain
+                    // vector, its own roots already dropped.  Publish the
+                    // sequence once and read each entry back per iteration.
+                    let _seq_roots = pyre_object::gc_roots::push_roots();
+                    let items_base = pyre_object::gc_roots::pin_roots(&items);
                     let mut groups: Vec<libc::gid_t> = Vec::with_capacity(items.len());
-                    for w_gid in items {
+                    for offset in 0..items.len() {
+                        let w_gid = pyre_object::gc_roots::shadow_stack_get(items_base + offset);
                         groups.push(crate::baseobjspace::c_uid_t_w(w_gid)?);
                     }
                     host_setgroups(&groups).map_err(|e| io_err(e, ""))?;
@@ -9017,21 +9025,35 @@ pub fn register_module(ns: pyre_object::PyObjectRef) {
                         "scheduler",
                     ],
                 )?;
-                let positional = [
+                // Every conversion below reaches app-level code -- `__fspath__`,
+                // a mapping's `keys()`, `__index__` -- and each one can collect
+                // and move the arguments that are still unread, including the
+                // keyword dictionary the remaining options are looked up in.
+                // Publish them once and read each back at its use.
+                let _roots = pyre_object::gc_roots::push_roots();
+                let positional_base = pyre_object::gc_roots::pin_roots(&[
                     bound[0].expect("path is required"),
                     bound[1].expect("argv is required"),
                     bound[2].expect("env is required"),
-                ];
-                let path = crate::gateway::fsencode_path_named_w(positional[0], func, "path")?;
+                ]);
+                let kwargs_slot = kwargs.map(|kwargs| {
+                    let slot = pyre_object::gc_roots::shadow_stack_len();
+                    pyre_object::gc_roots::pin_root(kwargs);
+                    slot
+                });
+                let positional =
+                    |index: usize| pyre_object::gc_roots::shadow_stack_get(positional_base + index);
+                let kwargs = || kwargs_slot.map(pyre_object::gc_roots::shadow_stack_get);
+                let path = crate::gateway::fsencode_path_named_w(positional(0), func, "path")?;
                 let c_path = std::ffi::CString::new(path.as_bytes.as_slice()).map_err(|_| {
                     crate::PyError::value_error("posix_spawn: embedded null in path")
                 })?;
-                let argv = collect_cstring_seq(positional[1], func, "argv")?;
+                let argv = collect_cstring_seq(positional(1), func, "argv")?;
                 // posixmodule.c parses `env` through the same keys/values
                 // snapshot used by execve, then filesystem-encodes paired
                 // elements into `key=value`.
-                let env = collect_spawn_env(positional[2])?;
-                let setpgroup = match crate::builtins::kwarg_get(kwargs, "setpgroup") {
+                let env = collect_spawn_env(positional(2))?;
+                let setpgroup = match crate::builtins::kwarg_get(kwargs(), "setpgroup") {
                     Some(value) if !unsafe { pyre_object::is_none(value) } => {
                         let value = crate::baseobjspace::space_index(value)?;
                         let value = crate::baseobjspace::int_w(value)?;
@@ -9043,27 +9065,27 @@ pub fn register_module(ns: pyre_object::PyObjectRef) {
                     }
                     _ => None,
                 };
-                let resetids = crate::builtins::kwarg_get(kwargs, "resetids")
+                let resetids = crate::builtins::kwarg_get(kwargs(), "resetids")
                     .map(crate::baseobjspace::is_true)
                     .transpose()?
                     .unwrap_or(false);
-                let setsid = crate::builtins::kwarg_get(kwargs, "setsid")
+                let setsid = crate::builtins::kwarg_get(kwargs(), "setsid")
                     .map(crate::baseobjspace::is_true)
                     .transpose()?
                     .unwrap_or(false);
                 if setsid && POSIX_SPAWN_SETSID.is_none() {
                     return Err(argument_unavailable(func, "setsid"));
                 }
-                let setsigmask = match crate::builtins::kwarg_get(kwargs, "setsigmask") {
+                let setsigmask = match crate::builtins::kwarg_get(kwargs(), "setsigmask") {
                     Some(value) => Some(sigset_arg(value)?),
                     None => None,
                 };
-                let setsigdef = match crate::builtins::kwarg_get(kwargs, "setsigdef") {
+                let setsigdef = match crate::builtins::kwarg_get(kwargs(), "setsigdef") {
                     Some(value) => Some(sigset_arg(value)?),
                     None => None,
                 };
-                let scheduler = parse_spawn_scheduler(func, kwargs)?;
-                let file_actions_obj = crate::builtins::kwarg_get(kwargs, "file_actions");
+                let scheduler = parse_spawn_scheduler(func, kwargs())?;
+                let file_actions_obj = crate::builtins::kwarg_get(kwargs(), "file_actions");
                 let actions: Vec<rustpython_host_env::posix::PosixSpawnFileAction> =
                     if let Some(fa) = file_actions_obj {
                         if unsafe { pyre_object::is_none(fa) } {
@@ -9276,8 +9298,14 @@ pub fn register_module(ns: pyre_object::PyObjectRef) {
 
             fn sigset_arg(value: PyObjectRef) -> Result<Vec<i32>, crate::PyError> {
                 let items = crate::builtins::collect_iterable(value)?;
+                // `space_index` runs `__index__`, so reading one element can
+                // collect and move the elements not yet read.  Publish the
+                // sequence once and read each element back per iteration.
+                let _seq_roots = pyre_object::gc_roots::push_roots();
+                let items_base = pyre_object::gc_roots::pin_roots(&items);
                 let mut sigs = Vec::with_capacity(items.len());
-                for item in items {
+                for offset in 0..items.len() {
+                    let item = pyre_object::gc_roots::shadow_stack_get(items_base + offset);
                     let item = crate::baseobjspace::space_index(item)?;
                     let signum = crate::baseobjspace::int_w(item)?;
                     if !(1..crate::module::signal::signalstate::NSIG as i64).contains(&signum) {
@@ -9314,9 +9342,17 @@ pub fn register_module(ns: pyre_object::PyObjectRef) {
 
                 #[cfg(all(target_os = "linux", not(target_env = "musl")))]
                 {
-                    let policy_obj = unsafe { pyre_object::w_tuple_getitem(value, 0).unwrap() };
+                    // `sched_priority_w` reaches `__index__`, so the collection
+                    // it can trigger forwards the tuple's own slots while a raw
+                    // element read before it goes stale.  Root the tuple and
+                    // take the policy out of it afterwards.
+                    let _roots = pyre_object::gc_roots::push_roots();
+                    let tuple_slot = pyre_object::gc_roots::shadow_stack_len();
+                    pyre_object::gc_roots::pin_root(value);
                     let param_obj = unsafe { pyre_object::w_tuple_getitem(value, 1).unwrap() };
                     let priority = sched_priority_w(param_obj)?;
+                    let value = pyre_object::gc_roots::shadow_stack_get(tuple_slot);
+                    let policy_obj = unsafe { pyre_object::w_tuple_getitem(value, 0).unwrap() };
                     let mut param: libc::sched_param =
                         unsafe { core::mem::zeroed::<libc::sched_param>() };
                     param.sched_priority = priority;