From 2ea7443781d591714edd939c670ee7c370d77ef6 Mon Sep 17 00:00:00 2001 From: Wolfgang Grieskamp Date: Sun, 5 Jan 2025 14:36:35 -0800 Subject: [PATCH] [move-vm][closures] Interpreter [PR 5/n vm closures] This closes the loop, putting the pieces together to implement the closure logic in the interpreter loop. They are some paranoid checks still missing which are marked in the code. --- .../move/move-core/types/src/function.rs | 10 + .../move/move-vm/runtime/src/interpreter.rs | 197 +++++++++++++++--- .../move-vm/runtime/src/loader/function.rs | 3 +- 3 files changed, 176 insertions(+), 34 deletions(-) diff --git a/third_party/move/move-core/types/src/function.rs b/third_party/move/move-core/types/src/function.rs index b6ed00c8678772..fd6a4b489f501f 100644 --- a/third_party/move/move-core/types/src/function.rs +++ b/third_party/move/move-core/types/src/function.rs @@ -33,6 +33,10 @@ impl fmt::Display for ClosureMask { impl ClosureMask { pub const MAX_ARGS: usize = 64; + pub fn empty() -> Self { + Self(0) + } + pub fn new(mask: u64) -> Self { Self(mask) } @@ -41,6 +45,12 @@ impl ClosureMask { self.0 } + /// Returns true if the i'th argument is captured. If `i` is out of range, false will + /// be returned. + pub fn is_captured(&self, i: usize) -> bool { + i < 64 && self.0 & (1 << i) != 0 + } + /// Apply a closure mask to a list of elements, returning only those /// where position `i` is set in the mask (if `collect_captured` is true) or not /// set (otherwise). diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index a7f59120b09baf..bbc1425b8bd444 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -6,7 +6,7 @@ use crate::{ access_control::AccessControlState, data_cache::TransactionDataCache, frame_type_cache::FrameTypeCache, - loader::{LegacyModuleStorageAdapter, Loader, Resolver}, + loader::{LazyLoadedFunction, LegacyModuleStorageAdapter, Loader, Resolver}, module_traversal::TraversalContext, native_extensions::NativeContextExtensions, native_functions::NativeContext, @@ -18,10 +18,12 @@ use move_binary_format::{ errors::*, file_format::{ AccessKind, Bytecode, FunctionHandleIndex, FunctionInstantiationIndex, LocalIndex, + SignatureIndex, }, }; use move_core_types::{ account_address::AccountAddress, + function::ClosureMask, gas_algebra::{NumArgs, NumBytes, NumTypeNodes}, language_storage::{ModuleId, TypeTag}, vm_status::{StatusCode, StatusType}, @@ -35,8 +37,8 @@ use move_vm_types::{ }, natives::function::NativeResult, values::{ - self, GlobalValue, IntegerValue, Locals, Reference, Struct, StructRef, VMValueCast, Value, - Vector, VectorRef, + self, AbstractFunction, Closure, GlobalValue, IntegerValue, Locals, Reference, Struct, + StructRef, VMValueCast, Value, Vector, VectorRef, }, views::TypeView, }; @@ -288,6 +290,8 @@ impl InterpreterImpl { traversal_context, extensions, &function, + ClosureMask::empty(), + vec![], )?; continue; } @@ -296,6 +300,8 @@ impl InterpreterImpl { gas_meter, loader, function, + ClosureMask::empty(), + vec![], )?; }, ExitCode::CallGeneric(idx) => { @@ -347,6 +353,8 @@ impl InterpreterImpl { traversal_context, extensions, &function, + ClosureMask::empty(), + vec![], )?; continue; } @@ -355,8 +363,72 @@ impl InterpreterImpl { gas_meter, loader, function, + ClosureMask::empty(), + vec![], )?; }, + ExitCode::CallClosure(_sig_idx) => { + // TODO(#15664): runtime check whether closure value on stack is same as sig_idx + let (fun, captured) = self + .operand_stack + .pop_as::() + .map_err(|e| set_err_info!(current_frame, e))? + .unpack(); + let lazy_function = LazyLoadedFunction::expect_this_impl(fun.as_ref()) + .map_err(|e| set_err_info!(current_frame, e))?; + let mask = lazy_function.closure_mask(); + let function = lazy_function + .with_resolved_function(module_storage, |f| Ok(f.clone())) + .map_err(|e| set_err_info!(current_frame, e))?; + + // In difference to regular calls, we skip visibility check. + // It is possible to call a private function of another module via + // a closure. + + // Charge gas + let module_id = function.module_id().ok_or_else(|| { + let err = + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message( + "Failed to get native function module id".to_string(), + ); + set_err_info!(current_frame, err) + })?; + gas_meter + .charge_call( + module_id, + function.name(), + self.operand_stack + .last_n(function.param_tys().len()) + .map_err(|e| set_err_info!(current_frame, e))?, + (function.local_tys().len() as u64).into(), + ) + .map_err(|e| set_err_info!(current_frame, e))?; + + // Call function + if function.is_native() { + self.call_native::( + &mut current_frame, + &resolver, + data_store, + gas_meter, + traversal_context, + extensions, + &function, + mask, + captured.collect(), + )? + } else { + self.set_new_call_frame::( + &mut current_frame, + gas_meter, + loader, + function, + mask, + captured.collect(), + )? + } + }, } } } @@ -367,6 +439,8 @@ impl InterpreterImpl { gas_meter: &mut impl GasMeter, loader: &Loader, function: LoadedFunction, + mask: ClosureMask, + captured: Vec, ) -> VMResult<()> { match (function.module_id(), current_frame.function.module_id()) { (Some(module_id), Some(current_module_id)) if module_id != current_module_id => { @@ -389,7 +463,7 @@ impl InterpreterImpl { } let mut frame = self - .make_call_frame::(gas_meter, loader, function) + .make_call_frame::(gas_meter, loader, function, mask, captured) .map_err(|err| { self.attach_state_if_invariant_violation(self.set_location(err), current_frame) })?; @@ -418,21 +492,29 @@ impl InterpreterImpl { gas_meter: &mut impl GasMeter, loader: &Loader, function: LoadedFunction, + mask: ClosureMask, + mut captured: Vec, ) -> PartialVMResult { let mut locals = Locals::new(function.local_tys().len()); let num_param_tys = function.param_tys().len(); - - for i in 0..num_param_tys { - locals.store_loc( - num_param_tys - i - 1, - self.operand_stack.pop()?, - loader.vm_config().check_invariant_in_swap_loc, - )?; + for i in (0..num_param_tys).rev() { + let is_captured = mask.is_captured(i); + let value = if is_captured { + captured.pop().ok_or_else(|| { + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message("inconsistent closure mask".to_string()) + })? + } else { + self.operand_stack.pop()? + }; + locals.store_loc(i, value, loader.vm_config().check_invariant_in_swap_loc)?; let ty_args = function.ty_args(); - if RTTCheck::should_perform_checks() { + if RTTCheck::should_perform_checks() && is_captured { + // Only perform paranoid type check for actual operands on the stack. + // Captured arguments are already verified against function signature. let ty = self.operand_stack.pop_ty()?; - let expected_ty = &function.local_tys()[num_param_tys - i - 1]; + let expected_ty = &function.local_tys()[i]; if !ty_args.is_empty() { let expected_ty = loader .ty_builder() @@ -497,6 +579,8 @@ impl InterpreterImpl { traversal_context: &mut TraversalContext, extensions: &mut NativeContextExtensions, function: &LoadedFunction, + mask: ClosureMask, + captured: Vec, ) -> VMResult<()> { // Note: refactor if native functions push a frame on the stack self.call_native_impl::( @@ -507,6 +591,8 @@ impl InterpreterImpl { traversal_context, extensions, function, + mask, + captured, ) .map_err(|e| match function.module_id() { Some(id) => { @@ -535,28 +621,41 @@ impl InterpreterImpl { traversal_context: &mut TraversalContext, extensions: &mut NativeContextExtensions, function: &LoadedFunction, + mask: ClosureMask, + mut captured: Vec, ) -> PartialVMResult<()> { let ty_builder = resolver.loader().ty_builder(); - let mut args = VecDeque::new(); let num_param_tys = function.param_tys().len(); - for _ in 0..num_param_tys { - args.push_front(self.operand_stack.pop()?); + let mut args = VecDeque::new(); + for i in (0..num_param_tys).rev() { + if mask.is_captured(i) { + args.push_front(captured.pop().ok_or_else(|| { + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message("inconsistent number of captured arguments".to_string()) + })?) + } else { + args.push_front(self.operand_stack.pop()?) + } } - let mut arg_tys = VecDeque::new(); + let mut arg_tys = VecDeque::new(); let ty_args = function.ty_args(); if RTTCheck::should_perform_checks() { - for i in 0..num_param_tys { - let ty = self.operand_stack.pop_ty()?; - let expected_ty = &function.param_tys()[num_param_tys - i - 1]; - if !ty_args.is_empty() { - let expected_ty = ty_builder.create_ty_with_subst(expected_ty, ty_args)?; - ty.paranoid_check_eq(&expected_ty)?; + for i in (0..num_param_tys).rev() { + let expected_ty = &function.param_tys()[i]; + if !mask.is_captured(i) { + let ty = self.operand_stack.pop_ty()?; + if !ty_args.is_empty() { + let expected_ty = ty_builder.create_ty_with_subst(expected_ty, ty_args)?; + ty.paranoid_check_eq(&expected_ty)?; + } else { + ty.paranoid_check_eq(expected_ty)?; + } + arg_tys.push_front(ty); } else { - ty.paranoid_check_eq(expected_ty)?; + arg_tys.push_front(expected_ty.clone()) } - arg_tys.push_front(ty); } } @@ -707,6 +806,8 @@ impl InterpreterImpl { gas_meter, resolver.loader(), target_func, + ClosureMask::empty(), + vec![], ) .map_err(|err| err.to_partial()) }, @@ -1459,6 +1560,7 @@ enum ExitCode { Return, Call(FunctionHandleIndex), CallGeneric(FunctionInstantiationIndex), + CallClosure(SignatureIndex), } impl AccessSpecifierEnv for Frame { @@ -1545,14 +1647,6 @@ impl Frame { )?; match instruction { - // TODO(#15664): implement closures - Bytecode::PackClosure(..) - | Bytecode::PackClosureGeneric(..) - | Bytecode::CallClosure(..) => { - return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) - .with_message("closure opcodes in interpreter".to_owned())) - }, - Bytecode::Pop => { let popped_val = interpreter.operand_stack.pop()?; gas_meter.charge_pop(popped_val)?; @@ -1667,6 +1761,7 @@ impl Frame { Bytecode::CallGeneric(idx) => { return Ok(ExitCode::CallGeneric(*idx)); }, + Bytecode::CallClosure(idx) => return Ok(ExitCode::CallClosure(*idx)), Bytecode::MutBorrowLoc(idx) | Bytecode::ImmBorrowLoc(idx) => { let instr = match instruction { Bytecode::MutBorrowLoc(_) => S::MutBorrowLoc, @@ -1970,6 +2065,42 @@ impl Frame { .operand_stack .push(reference.test_variant(info.variant)?)?; }, + Bytecode::PackClosure(fh_idx, mask) => { + // TODO(#15664): runtime checks. We should verify that the popped + // argument types match the function. + let function = resolver + .build_loaded_function_from_handle_and_ty_args(*fh_idx, vec![])?; + let captured = interpreter.operand_stack.popn(mask.captured_count())?; + let lazy_function = LazyLoadedFunction::new_resolved( + resolver.module_storage(), + function, + *mask, + )?; + interpreter + .operand_stack + .push(Value::closure(Box::new(lazy_function), captured))? + }, + Bytecode::PackClosureGeneric(fi_idx, mask) => { + // TODO(#15664): runtime checks + let ty_args = resolver.instantiate_generic_function( + Some(gas_meter), + *fi_idx, + self.function.ty_args(), + )?; + let function = resolver + .build_loaded_function_from_instantiation_and_ty_args( + *fi_idx, ty_args, + )?; + let captured = interpreter.operand_stack.popn(mask.captured_count())?; + let lazy_function = LazyLoadedFunction::new_resolved( + resolver.module_storage(), + function, + *mask, + )?; + interpreter + .operand_stack + .push(Value::closure(Box::new(lazy_function), captured))? + }, Bytecode::ReadRef => { let reference = interpreter.operand_stack.pop_as::()?; gas_meter.charge_read_ref(reference.value_view())?; diff --git a/third_party/move/move-vm/runtime/src/loader/function.rs b/third_party/move/move-vm/runtime/src/loader/function.rs index 0294da1a41cc9e..1f07fc8c10c2be 100644 --- a/third_party/move/move-vm/runtime/src/loader/function.rs +++ b/third_party/move/move-vm/runtime/src/loader/function.rs @@ -101,10 +101,11 @@ impl LazyLoadedFunction { #[allow(unused)] pub(crate) fn new_resolved( - converter: &dyn TypeConverter, + module_storage: &dyn ModuleStorage, fun: LoadedFunction, mask: ClosureMask, ) -> PartialVMResult { + let converter = ModuleStorageTypeConverter::new(module_storage); let fun_inst = fun .ty_args .iter()