Use active builtins in descriptor and method reductions (#9057)

Use frame-aware getattr lookup consistently across descriptor and method reductions. Preserve defining types and exception order, and perform generic builtin-mapping subclass lookup only once.

Assisted-by: OpenAI Codex:model identifier unavailable

crates/vm/src/builtins/builtin_func.rs

@@ -215,7 +215,7 @@ impl PyNativeFunction {
             .as_ref()
             .filter(|bound| !bound.class().is_subtype(vm.ctx.types.module_type))
         {
-            let getattr = vm.builtins.get_attr("getattr", vm)?;
+            let getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
             Ok(vm
                 .new_tuple((getattr, (bound.clone(), zelf.value.name)))
                 .into())

crates/vm/src/builtins/descriptor.rs

@@ -155,10 +155,12 @@ impl Py<PyMethodDescriptor> {
     fn __reduce__(
         &self,
         vm: &VirtualMachine,
-    ) -> (Option<PyObjectRef>, (Option<PyObjectRef>, &'static str)) {
-        let builtins_getattr = vm.builtins.get_attr("getattr", vm).ok();
-        let classname = vm.builtins.get_attr(&self.common.typ.__name__(vm), vm).ok();
-        (builtins_getattr, (classname, self.method.name))
+    ) -> PyResult<(PyObjectRef, (PyTypeRef, &'static str))> {
+        let builtins_getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
+        Ok((
+            builtins_getattr,
+            (self.common.typ.to_owned(), self.method.name),
+        ))
     }
 }
 
@@ -851,7 +853,7 @@ impl Py<PyMemberDescriptor> {
 
     #[pymethod]
     fn __reduce__(&self, vm: &VirtualMachine) -> PyResult {
-        let builtins_getattr = vm.builtins.get_attr("getattr", vm)?;
+        let builtins_getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
         Ok(vm
             .ctx
             .new_tuple(vec![
@@ -2064,7 +2066,7 @@ impl Callable for PyWrapper {
 impl Py<PyWrapper> {
     #[pymethod]
     fn __reduce__(&self, vm: &VirtualMachine) -> PyResult {
-        let getattr = vm.builtins.get_attr("getattr", vm)?;
+        let getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
         let args = vm.ctx.new_tuple(vec![
             self.typ.to_owned().into(),
             self.name.to_owned().into(),
@@ -2181,7 +2183,7 @@ impl Py<PyMethodWrapper> {
 
     #[pymethod]
     fn __reduce__(zelf: PyRef<PyMethodWrapper>, vm: &VirtualMachine) -> PyResult {
-        let builtins_getattr = vm.builtins.get_attr("getattr", vm)?;
+        let builtins_getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
         Ok(vm
             .ctx
             .new_tuple(vec![

crates/vm/src/builtins/function.rs

@@ -1470,9 +1470,9 @@ impl Py<PyBoundMethod> {
         &self,
         vm: &VirtualMachine,
     ) -> PyResult<(PyObjectRef, (PyObjectRef, PyObjectRef))> {
-        let builtins_getattr = vm.builtins.get_attr("getattr", vm)?;
-        let func_self = self.object.clone();
         let func_name = self.function.get_attr("__name__", vm)?;
+        let builtins_getattr = vm.eval_get_builtin(vm.ctx.intern_str("getattr"))?;
+        let func_self = self.object.clone();
         Ok((builtins_getattr, (func_self, func_name)))
     }
 

crates/vm/src/vm/mod.rs

@@ -3170,7 +3170,7 @@ impl VirtualMachine {
     pub fn eval_get_builtin(&self, name: &'static PyStrInterned) -> PyResult {
         let builtins =
             crate::frame::current_builtins().unwrap_or_else(|| self.builtins.dict().into());
-        if let Some(dict) = builtins.downcast_ref::<PyDict>() {
+        if let Some(dict) = builtins.downcast_ref_if_exact::<PyDict>(self) {
             match dict.get_item_opt(name, self)? {
                 Some(value) => Ok(value),
                 None => Err(self.new_attribute_error(name.to_string())),

extra_tests/snippets/stdlib_pickle.py

@@ -1,6 +1,7 @@
 import _pickle
 import io
 import sys
+from types import MappingProxyType, MethodType
 
 from testutils import assert_raises
 
@@ -35,3 +36,83 @@ for opcode, name in (
         else:
             expected = f"{name} exceeds system's maximum size of {sys.maxsize} bytes"
         assert str(caught.exception) == expected
+
+
+# Reduction returns getattr from the active frame's builtins without calling it.
+class ReductionOwner:
+    __slots__ = ("slot",)
+
+    def method(self):
+        pass
+
+
+owner = ReductionOwner()
+items = []
+replacement_getattr = object()
+for descriptor, receiver, name in (
+    (object.__str__, object, "__str__"),
+    (str.join, str, "join"),
+    (io.BytesIO.getvalue, io.BytesIO, "getvalue"),
+    (ReductionOwner.slot, ReductionOwner, "slot"),
+    (owner.__str__, owner, "__str__"),
+    (items.append, items, "append"),
+    (dict.fromkeys, dict, "fromkeys"),
+    (owner.method, owner, "method"),
+):
+    for builtins_type in (dict, MappingProxyType):
+        namespace = {
+            "__builtins__": builtins_type({"getattr": replacement_getattr}),
+            "descriptor": descriptor,
+        }
+        exec("reduced = descriptor.__reduce__()", namespace)
+        assert namespace["reduced"] == (replacement_getattr, (receiver, name))
+
+        namespace["__builtins__"] = builtins_type({})
+        with assert_raises(AttributeError) as caught:
+            exec("descriptor.__reduce__()", namespace)
+        assert caught.exception.args == ("getattr",)
+
+# Module-bound functions reduce to a name without looking up getattr.
+namespace = {"__builtins__": {}, "function": len}
+exec("reduced = function.__reduce__()", namespace)
+assert namespace["reduced"] == "len"
+
+
+class MissingBuiltin(dict):
+    def __getitem__(self, key):
+        raise KeyError(key)
+
+    def __missing__(self, key):
+        raise AssertionError("must not retry __missing__ after custom __getitem__")
+
+
+for descriptor, name in (
+    (object.__str__, "getattr"),
+    (iter([1]), "iter"),
+    (reversed([1]), "reversed"),
+):
+    namespace = {"__builtins__": MissingBuiltin(), "descriptor": descriptor}
+    with assert_raises(AttributeError) as caught:
+        exec("descriptor.__reduce__()", namespace)
+    assert caught.exception.args == (name,)
+
+
+name_error = RuntimeError("function name first")
+
+
+class NamedCallable:
+    def __call__(self, *args):
+        pass
+
+    @property
+    def __name__(self):
+        raise name_error
+
+
+namespace = {
+    "__builtins__": {},
+    "method": MethodType(NamedCallable(), owner),
+}
+with assert_raises(RuntimeError) as caught:
+    exec("method.__reduce__()", namespace)
+assert caught.exception is name_error