diff --git a/crates/vm/src/builtins/builtin_func.rs b/crates/vm/src/builtins/builtin_func.rs index e03999870..ac89021b9 100644 --- a/crates/vm/src/builtins/builtin_func.rs +++ b/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()) diff --git a/crates/vm/src/builtins/descriptor.rs b/crates/vm/src/builtins/descriptor.rs index e8329fbb5..79afc8cb5 100644 --- a/crates/vm/src/builtins/descriptor.rs +++ b/crates/vm/src/builtins/descriptor.rs @@ -155,10 +155,12 @@ impl Py { fn __reduce__( &self, vm: &VirtualMachine, - ) -> (Option, (Option, &'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 { #[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 { #[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 { #[pymethod] fn __reduce__(zelf: PyRef, 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![ diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index b0bd5a010..be4994a62 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -1470,9 +1470,9 @@ impl Py { &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))) } diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 304991f95..0c5f8e2b0 100644 --- a/crates/vm/src/vm/mod.rs +++ b/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::() { + if let Some(dict) = builtins.downcast_ref_if_exact::(self) { match dict.get_item_opt(name, self)? { Some(value) => Ok(value), None => Err(self.new_attribute_error(name.to_string())), diff --git a/extra_tests/snippets/stdlib_pickle.py b/extra_tests/snippets/stdlib_pickle.py index e1872a24b..999106d42 100644 --- a/extra_tests/snippets/stdlib_pickle.py +++ b/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