From 6fd5094c05aa819192df2ed6be9075a5286a9906 Mon Sep 17 00:00:00 2001 From: Jeong Yunwon Date: Sat, 30 Apr 2022 03:28:29 +0900 Subject: [PATCH] Remove Interpreter::default() not to trap users init without stdlib --- benches/execution.rs | 2 +- benches/microbenchmarks.rs | 5 +-- examples/freeze/main.rs | 2 +- examples/hello_embed.rs | 2 +- examples/mini_repl.rs | 2 +- src/lib.rs | 12 +++--- vm/src/builtins/str.rs | 2 +- vm/src/dictdatatype.rs | 4 +- vm/src/eval.rs | 2 +- vm/src/import.rs | 88 +++++++++++++++++++++----------------- vm/src/lib.rs | 2 +- vm/src/macros.rs | 4 +- vm/src/prelude.rs | 2 +- vm/src/vm/interpreter.rs | 37 +++++++++------- vm/src/vm/mod.rs | 15 ++----- vm/src/vm/setting.rs | 4 ++ vm/src/vm/thread.rs | 2 +- wasm/lib/src/vm_class.rs | 8 ++-- 18 files changed, 102 insertions(+), 93 deletions(-) diff --git a/benches/execution.rs b/benches/execution.rs index 9c0bdb024b..d286365b84 100644 --- a/benches/execution.rs +++ b/benches/execution.rs @@ -23,7 +23,7 @@ fn bench_cpython_code(b: &mut Bencher, source: &str) { fn bench_rustpy_code(b: &mut Bencher, name: &str, source: &str) { // NOTE: Take long time. - Interpreter::default().enter(|vm| { + Interpreter::without_stdlib(Default::default()).enter(|vm| { // Note: bench_cpython is both compiling and executing the code. // As such we compile the code in the benchmark loop as well. b.iter(|| { diff --git a/benches/microbenchmarks.rs b/benches/microbenchmarks.rs index 0b3996f3ca..a0e0ba4918 100644 --- a/benches/microbenchmarks.rs +++ b/benches/microbenchmarks.rs @@ -3,7 +3,7 @@ use criterion::{ Criterion, Throughput, }; use rustpython_compiler::Mode; -use rustpython_vm::{common::ascii, InitParameter, Interpreter, PyResult, Settings}; +use rustpython_vm::{common::ascii, Interpreter, PyResult, Settings}; use std::{ ffi, fs, io, path::{Path, PathBuf}, @@ -114,11 +114,10 @@ fn bench_rustpy_code(group: &mut BenchmarkGroup, bench: &MicroBenchmar settings.dont_write_bytecode = true; settings.no_user_site = true; - Interpreter::new_with_init(settings, |vm| { + Interpreter::with_init(settings, |vm| { for (name, init) in rustpython_stdlib::get_module_inits().into_iter() { vm.add_native_module(name, init); } - InitParameter::External }) .enter(|vm| { let setup_code = vm diff --git a/examples/freeze/main.rs b/examples/freeze/main.rs index fec904a948..2395b3301d 100644 --- a/examples/freeze/main.rs +++ b/examples/freeze/main.rs @@ -1,7 +1,7 @@ use rustpython_vm as vm; fn main() -> vm::PyResult<()> { - vm::Interpreter::default().enter(run) + vm::Interpreter::without_stdlib(Default::default()).enter(run) } fn run(vm: &vm::VirtualMachine) -> vm::PyResult<()> { diff --git a/examples/hello_embed.rs b/examples/hello_embed.rs index 9948a84b85..26047d6c84 100644 --- a/examples/hello_embed.rs +++ b/examples/hello_embed.rs @@ -1,7 +1,7 @@ use rustpython_vm as vm; fn main() -> vm::PyResult<()> { - vm::Interpreter::default().enter(|vm| { + vm::Interpreter::without_stdlib(Default::default()).enter(|vm| { let scope = vm.new_scope_with_builtins(); let code_obj = vm diff --git a/examples/mini_repl.rs b/examples/mini_repl.rs index 7e4bf20987..19a4113e50 100644 --- a/examples/mini_repl.rs +++ b/examples/mini_repl.rs @@ -29,7 +29,7 @@ fn on(b: bool) { } fn main() -> vm::PyResult<()> { - vm::Interpreter::default().enter(run) + vm::Interpreter::without_stdlib(Default::default()).enter(run) } fn run(vm: &vm::VirtualMachine) -> vm::PyResult<()> { diff --git a/src/lib.rs b/src/lib.rs index 76c077272e..c3f0e1abd6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -51,7 +51,7 @@ use rustpython_vm::{ match_class, scope::Scope, stdlib::{atexit, sys}, - AsObject, InitParameter, Interpreter, PyResult, Settings, VirtualMachine, + AsObject, Interpreter, PyResult, Settings, VirtualMachine, }; use std::{env, process, str::FromStr}; @@ -83,10 +83,9 @@ where } } - let interp = Interpreter::new_with_init(settings, |vm| { + let interp = Interpreter::with_init(settings, |vm| { add_stdlib(vm); init(vm); - InitParameter::External }); let exitcode = interp.enter(move |vm| { @@ -573,7 +572,7 @@ __import__("io").TextIOWrapper( .downcast() .expect("TextIOWrapper.read() should return str"); eprintln!("running get-pip.py..."); - _run_string(vm, scope, getpip_code.as_str(), "get-pip.py".to_owned()) + vm.run_code_string(scope, getpip_code.as_str(), "get-pip.py".to_owned()) } #[cfg(not(feature = "ssl"))] @@ -599,7 +598,7 @@ fn run_rustpython(vm: &VirtualMachine, matches: &ArgMatches) -> PyResult<()> { // Figure out if a -c option was given: if let Some(command) = matches.value_of("c") { debug!("Running command {}", command); - vm.run_code_string(scope, &command, "".to_owned())?; + vm.run_code_string(scope, command, "".to_owned())?; } else if let Some(module) = matches.value_of("m") { debug!("Running module {}", module); vm.run_module(module)?; @@ -627,9 +626,8 @@ mod tests { use super::*; fn interpreter() -> Interpreter { - Interpreter::new_with_init(Settings::default(), |vm| { + Interpreter::with_init(Settings::default(), |vm| { add_stdlib(vm); - InitParameter::External }) } diff --git a/vm/src/builtins/str.rs b/vm/src/builtins/str.rs index 3eac4ea26d..4d14112484 100644 --- a/vm/src/builtins/str.rs +++ b/vm/src/builtins/str.rs @@ -1559,7 +1559,7 @@ mod tests { #[test] fn str_maketrans_and_translate() { - Interpreter::default().enter(|vm| { + Interpreter::without_stdlib(Default::default()).enter(|vm| { let table = vm.ctx.new_dict(); table .set_item("a", vm.ctx.new_str("🎅").into(), &vm) diff --git a/vm/src/dictdatatype.rs b/vm/src/dictdatatype.rs index fd457a28f5..c94d5cbf23 100644 --- a/vm/src/dictdatatype.rs +++ b/vm/src/dictdatatype.rs @@ -872,7 +872,7 @@ mod tests { #[test] fn test_insert() { - Interpreter::default().enter(|vm| { + Interpreter::without_stdlib(Default::default()).enter(|vm| { let dict = Dict::default(); assert_eq!(0, dict.len()); @@ -921,7 +921,7 @@ mod tests { } fn check_hash_equivalence(text: &str) { - Interpreter::default().enter(|vm| { + Interpreter::without_stdlib(Default::default()).enter(|vm| { let value1 = text; let value2 = vm.new_pyobj(value1.to_owned()); diff --git a/vm/src/eval.rs b/vm/src/eval.rs index 35751e02aa..39637323d3 100644 --- a/vm/src/eval.rs +++ b/vm/src/eval.rs @@ -20,7 +20,7 @@ mod tests { #[test] fn test_print_42() { - Interpreter::default().enter(|vm| { + Interpreter::without_stdlib(Default::default()).enter(|vm| { let source = String::from("print('Hello world')"); let vars = vm.new_scope_with_builtins(); let result = eval(&vm, &source, vars, "").expect("this should pass"); diff --git a/vm/src/import.rs b/vm/src/import.rs index 932acfd6ee..ad68e2a27f 100644 --- a/vm/src/import.rs +++ b/vm/src/import.rs @@ -7,16 +7,23 @@ use crate::{ builtins::{code, code::CodeObject, list, traceback::PyTraceback, PyBaseExceptionRef}, scope::Scope, version::get_git_revision, - vm::{InitParameter, VirtualMachine}, + vm::{thread, VirtualMachine}, AsObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, }; use rand::Rng; pub(crate) fn init_importlib( vm: &mut VirtualMachine, - initialize_parameter: InitParameter, + allow_external_library: bool, ) -> PyResult<()> { - use crate::vm::thread::enter_vm; + let importlib = init_importlib_base(vm)?; + if allow_external_library && cfg!(feature = "rustpython-compiler") { + init_importlib_package(vm, importlib)?; + } + Ok(()) +} + +pub(crate) fn init_importlib_base(vm: &mut VirtualMachine) -> PyResult { flame_guard!("init importlib"); // importlib_bootstrap needs these and it inlines checks to sys.modules before calling into @@ -26,7 +33,7 @@ pub(crate) fn init_importlib( import_builtin(vm, "_warnings")?; import_builtin(vm, "_weakref")?; - let importlib = enter_vm(vm, || { + let importlib = thread::enter_vm(vm, || { let importlib = import_frozen(vm, "_frozen_importlib")?; let impmod = import_builtin(vm, "_imp")?; let install = importlib.get_attr("_install", vm)?; @@ -34,45 +41,48 @@ pub(crate) fn init_importlib( Ok(importlib) })?; vm.import_func = importlib.get_attr("__import__", vm)?; + Ok(importlib) +} - if initialize_parameter == InitParameter::External && cfg!(feature = "rustpython-compiler") { - enter_vm(vm, || { - flame_guard!("install_external"); +pub(crate) fn init_importlib_package( + vm: &mut VirtualMachine, + importlib: PyObjectRef, +) -> PyResult<()> { + thread::enter_vm(vm, || { + flame_guard!("install_external"); - // same deal as imports above - #[cfg(any(not(target_arch = "wasm32"), target_os = "wasi"))] - import_builtin(vm, crate::stdlib::os::MODULE_NAME)?; - #[cfg(windows)] - import_builtin(vm, "winreg")?; - import_builtin(vm, "_io")?; - import_builtin(vm, "marshal")?; + // same deal as imports above + #[cfg(any(not(target_arch = "wasm32"), target_os = "wasi"))] + import_builtin(vm, crate::stdlib::os::MODULE_NAME)?; + #[cfg(windows)] + import_builtin(vm, "winreg")?; + import_builtin(vm, "_io")?; + import_builtin(vm, "marshal")?; - let install_external = importlib.get_attr("_install_external_importers", vm)?; - vm.invoke(&install_external, ())?; - // Set pyc magic number to commit hash. Should be changed when bytecode will be more stable. - let importlib_external = vm.import("_frozen_importlib_external", None, 0)?; - let mut magic = get_git_revision().into_bytes(); - magic.truncate(4); - if magic.len() != 4 { - magic = rand::thread_rng().gen::<[u8; 4]>().to_vec(); - } - let magic: PyObjectRef = vm.ctx.new_bytes(magic).into(); - importlib_external.set_attr("MAGIC_NUMBER", magic, vm)?; - let zipimport_res = (|| -> PyResult<()> { - let zipimport = vm.import("zipimport", None, 0)?; - let zipimporter = zipimport.get_attr("zipimporter", vm)?; - let path_hooks = vm.sys_module.get_attr("path_hooks", vm)?; - let path_hooks = list::PyListRef::try_from_object(vm, path_hooks)?; - path_hooks.insert(0, zipimporter); - Ok(()) - })(); - if zipimport_res.is_err() { - warn!("couldn't init zipimport") - } + let install_external = importlib.get_attr("_install_external_importers", vm)?; + vm.invoke(&install_external, ())?; + // Set pyc magic number to commit hash. Should be changed when bytecode will be more stable. + let importlib_external = vm.import("_frozen_importlib_external", None, 0)?; + let mut magic = get_git_revision().into_bytes(); + magic.truncate(4); + if magic.len() != 4 { + magic = rand::thread_rng().gen::<[u8; 4]>().to_vec(); + } + let magic: PyObjectRef = vm.ctx.new_bytes(magic).into(); + importlib_external.set_attr("MAGIC_NUMBER", magic, vm)?; + let zipimport_res = (|| -> PyResult<()> { + let zipimport = vm.import("zipimport", None, 0)?; + let zipimporter = zipimport.get_attr("zipimporter", vm)?; + let path_hooks = vm.sys_module.get_attr("path_hooks", vm)?; + let path_hooks = list::PyListRef::try_from_object(vm, path_hooks)?; + path_hooks.insert(0, zipimporter); Ok(()) - })? - } - Ok(()) + })(); + if zipimport_res.is_err() { + warn!("couldn't init zipimport") + } + Ok(()) + }) } pub fn import_frozen(vm: &VirtualMachine, module_name: &str) -> PyResult { diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 944ba7cad4..7e9b167e85 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -81,7 +81,7 @@ pub use self::convert::{TryFromBorrowedObject, TryFromObject}; pub use self::object::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult, PyWeakRef, }; -pub use self::vm::{Context, InitParameter, Interpreter, Settings, VirtualMachine}; +pub use self::vm::{Context, Interpreter, Settings, VirtualMachine}; pub use rustpython_bytecode as bytecode; pub use rustpython_common as common; diff --git a/vm/src/macros.rs b/vm/src/macros.rs index a12272c09e..eff8917495 100644 --- a/vm/src/macros.rs +++ b/vm/src/macros.rs @@ -71,7 +71,7 @@ macro_rules! py_namespace { /// use rustpython_vm::builtins::{PyFloat, PyInt}; /// use rustpython_vm::{PyPayload}; /// -/// # rustpython_vm::Interpreter::default().enter(|vm| { +/// # rustpython_vm::Interpreter::without_stdlib(Default::default()).enter(|vm| { /// let obj = PyInt::from(0).into_pyobject(vm); /// assert_eq!( /// "int", @@ -95,7 +95,7 @@ macro_rules! py_namespace { /// use rustpython_vm::builtins::{PyFloat, PyInt}; /// use rustpython_vm::{ PyPayload}; /// -/// # rustpython_vm::Interpreter::default().enter(|vm| { +/// # rustpython_vm::Interpreter::without_stdlib(Default::default()).enter(|vm| { /// let obj = PyInt::from(0).into_pyobject(vm); /// /// let int_value = match_class!(match obj { diff --git a/vm/src/prelude.rs b/vm/src/prelude.rs index f562d0656a..415e7da795 100644 --- a/vm/src/prelude.rs +++ b/vm/src/prelude.rs @@ -2,5 +2,5 @@ pub use crate::{ object::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult, PyWeakRef, }, - vm::{Context, InitParameter, Interpreter, Settings, VirtualMachine}, + vm::{Context, Interpreter, Settings, VirtualMachine}, }; diff --git a/vm/src/vm/interpreter.rs b/vm/src/vm/interpreter.rs index 313c34cef9..45cb23fbff 100644 --- a/vm/src/vm/interpreter.rs +++ b/vm/src/vm/interpreter.rs @@ -1,4 +1,4 @@ -use super::{setting::Settings, thread, InitParameter, VirtualMachine}; +use super::{setting::Settings, thread, VirtualMachine}; /// The general interface for the VM /// @@ -7,7 +7,7 @@ use super::{setting::Settings, thread, InitParameter, VirtualMachine}; /// ``` /// use rustpython_vm::Interpreter; /// use rustpython_vm::compile::Mode; -/// Interpreter::default().enter(|vm| { +/// Interpreter::without_stdlib(Default::default()).enter(|vm| { /// let scope = vm.new_scope_with_builtins(); /// let code_obj = vm.compile(r#"print("Hello World!")"#, /// Mode::Exec, @@ -21,17 +21,28 @@ pub struct Interpreter { } impl Interpreter { - pub fn new(settings: Settings, init: InitParameter) -> Self { - Self::new_with_init(settings, |_| init) + /// To create with stdlib, use `with_init` + pub fn without_stdlib(settings: Settings) -> Self { + Self::with_init(settings, |_| {}) } - pub fn new_with_init(settings: Settings, init: F) -> Self + /// Create with initialize function taking mutable vm reference. + /// ``` + /// use rustpython_vm::Interpreter; + /// Interpreter::with_init(Default::default(), |vm| { + /// // put this line to add stdlib to the vm + /// // vm.add_native_modules(rustpython_stdlib::get_module_inits()); + /// }).enter(|vm| { + /// vm.run_code_string(vm.new_scope_with_builtins(), "print(1)", "<...>".to_owned()); + /// }); + /// ``` + pub fn with_init(settings: Settings, init: F) -> Self where - F: FnOnce(&mut VirtualMachine) -> InitParameter, + F: FnOnce(&mut VirtualMachine), { let mut vm = VirtualMachine::new(settings); - let init = init(&mut vm); - vm.initialize(init); + init(&mut vm); + vm.initialize(); Self { vm } } @@ -54,12 +65,6 @@ impl Interpreter { // pub fn shutdown(self) {} } -impl Default for Interpreter { - fn default() -> Self { - Self::new(Settings::default(), InitParameter::External) - } -} - #[cfg(test)] mod tests { use super::*; @@ -71,7 +76,7 @@ mod tests { #[test] fn test_add_py_integers() { - Interpreter::default().enter(|vm| { + Interpreter::without_stdlib(Default::default()).enter(|vm| { let a: PyObjectRef = vm.ctx.new_int(33_i32).into(); let b: PyObjectRef = vm.ctx.new_int(12_i32).into(); let res = vm._add(&a, &b).unwrap(); @@ -82,7 +87,7 @@ mod tests { #[test] fn test_multiply_str() { - Interpreter::default().enter(|vm| { + Interpreter::without_stdlib(Default::default()).enter(|vm| { let a = vm.new_pyobj(crate::common::ascii!("Hello ")); let b = vm.new_pyobj(4_i32); let res = vm._mul(&a, &b).unwrap(); diff --git a/vm/src/vm/mod.rs b/vm/src/vm/mod.rs index 9ea2c45d74..1ff7d74ca0 100644 --- a/vm/src/vm/mod.rs +++ b/vm/src/vm/mod.rs @@ -88,12 +88,6 @@ pub struct PyGlobalState { pub codec_registry: CodecsRegistry, } -#[derive(Copy, Clone, PartialEq, Eq)] -pub enum InitParameter { - Internal, - External, -} - impl VirtualMachine { /// Create a new `VirtualMachine` structure. fn new(settings: Settings) -> VirtualMachine { @@ -177,7 +171,7 @@ impl VirtualMachine { vm } - fn initialize(&mut self, initialize_parameter: InitParameter) { + fn initialize(&mut self) { flame_guard!("init VirtualMachine"); if self.initialized { @@ -190,8 +184,7 @@ impl VirtualMachine { let mut inner_init = || -> PyResult<()> { #[cfg(not(target_arch = "wasm32"))] import::import_builtin(self, "_signal")?; - - import::init_importlib(self, initialize_parameter)?; + import::init_importlib(self, self.state.settings.allow_external_library)?; // set up the encodings search function self.import("encodings", None, 0).map_err(|import_err| { @@ -249,7 +242,7 @@ impl VirtualMachine { .expect("there should not be multiple threads while a user has a mut ref to a vm") } - /// Can only be used in the initialization closure passed to [`Interpreter::new_with_init`] + /// Can only be used in the initialization closure passed to [`Interpreter::with_init`] pub fn add_native_module(&mut self, name: S, module: stdlib::StdlibInitFunc) where S: Into>, @@ -264,7 +257,7 @@ impl VirtualMachine { self.state_mut().module_inits.extend(iter); } - /// Can only be used in the initialization closure passed to [`Interpreter::new_with_init`] + /// Can only be used in the initialization closure passed to [`Interpreter::with_init`] pub fn add_frozen(&mut self, frozen: I) where I: IntoIterator, diff --git a/vm/src/vm/setting.rs b/vm/src/vm/setting.rs index 9ec22e86f8..2ae33fc5ab 100644 --- a/vm/src/vm/setting.rs +++ b/vm/src/vm/setting.rs @@ -61,6 +61,9 @@ pub struct Settings { /// -u, PYTHONUNBUFFERED=x // TODO: use this; can TextIOWrapper even work with a non-buffered? pub stdio_unbuffered: bool, + + /// false for wasm. Not a command-line option + pub allow_external_library: bool, } /// Sensible default settings. @@ -90,6 +93,7 @@ impl Default for Settings { argv: vec![], hash_seed: None, stdio_unbuffered: false, + allow_external_library: true, } } } diff --git a/vm/src/vm/thread.rs b/vm/src/vm/thread.rs index 1e6987c405..54c72e6976 100644 --- a/vm/src/vm/thread.rs +++ b/vm/src/vm/thread.rs @@ -107,7 +107,7 @@ impl VirtualMachine { /// # Usage /// /// ``` - /// # rustpython_vm::Interpreter::default().enter(|vm| { + /// # rustpython_vm::Interpreter::without_stdlib(Default::default()).enter(|vm| { /// use std::thread::Builder; /// let handle = Builder::new() /// .name("my thread :)".into()) diff --git a/wasm/lib/src/vm_class.rs b/wasm/lib/src/vm_class.rs index a7e972f015..51ea69e2d2 100644 --- a/wasm/lib/src/vm_class.rs +++ b/wasm/lib/src/vm_class.rs @@ -8,7 +8,7 @@ use rustpython_vm::{ builtins::PyWeak, compile::{self, Mode}, scope::Scope, - InitParameter, Interpreter, PyObjectRef, PyPayload, PyRef, PyResult, Settings, VirtualMachine, + Interpreter, PyObjectRef, PyPayload, PyRef, PyResult, Settings, VirtualMachine, }; use std::{ cell::RefCell, @@ -41,7 +41,9 @@ fn init_window_module(vm: &VirtualMachine) -> PyObjectRef { impl StoredVirtualMachine { fn new(id: String, inject_browser_module: bool) -> StoredVirtualMachine { let mut scope = None; - let interp = Interpreter::new_with_init(Settings::default(), |vm| { + let mut settings = Settings::default(); + settings.allow_external_library = false; + let interp = Interpreter::with_init(settings, |vm| { vm.wasm_id = Some(id); js_module::setup_js_module(vm); @@ -57,8 +59,6 @@ impl StoredVirtualMachine { }); scope = Some(vm.new_scope_with_builtins()); - - InitParameter::Internal }); StoredVirtualMachine {