From 57afe2588050aa41a1ad56a554b74faf20cf2fb4 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 | 200 +++++++++++++++--- .../move-vm/runtime/src/loader/function.rs | 1 - 3 files changed, 184 insertions(+), 27 deletions(-) diff --git a/third_party/move/move-core/types/src/function.rs b/third_party/move/move-core/types/src/function.rs index bd21f8ff7065a..ab869a9be9d46 100644 --- a/third_party/move/move-core/types/src/function.rs +++ b/third_party/move/move-core/types/src/function.rs @@ -40,6 +40,10 @@ impl ClosureMask { /// stack than this number. pub const MAX_ARGS: usize = 64; + pub fn empty() -> Self { + Self(0) + } + pub fn new(mask: u64) -> Self { Self(mask) } @@ -48,6 +52,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 0c5421cc90ff6..b8ee91d767e65 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -8,11 +8,12 @@ use crate::{ frame_type_cache::{ AllRuntimeCaches, FrameTypeCache, NoRuntimeCaches, PerInstructionCache, RuntimeCacheTraits, }, - loader::{LegacyModuleStorageAdapter, Loader, Resolver}, + loader::{LazyLoadedFunction, LegacyModuleStorageAdapter, Loader, Resolver}, module_traversal::TraversalContext, native_extensions::NativeContextExtensions, native_functions::NativeContext, runtime_type_checks::{FullRuntimeTypeCheck, NoRuntimeTypeCheck, RuntimeTypeCheck}, + storage::ty_tag_converter::TypeTagConverter, trace, LoadedFunction, ModuleStorage, }; use fail::fail_point; @@ -20,10 +21,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}, @@ -37,8 +40,8 @@ use move_vm_types::{ }, natives::function::NativeResult, values::{ - self, GlobalValue, IntegerValue, Locals, Reference, SignerRef, Struct, StructRef, - VMValueCast, Value, Vector, VectorRef, + self, AbstractFunction, Closure, GlobalValue, IntegerValue, Locals, Reference, SignerRef, + Struct, StructRef, VMValueCast, Value, Vector, VectorRef, }, views::TypeView, }; @@ -293,7 +296,7 @@ impl InterpreterImpl { }; let mut current_frame = self - .make_new_frame(gas_meter, loader, function, locals, frame_cache) + .make_new_frame(gas_meter, loader, function, locals, frame_cache.clone()) .map_err(|err| self.set_location(err))?; // Access control for the new frame. @@ -419,6 +422,8 @@ impl InterpreterImpl { traversal_context, extensions, &function, + ClosureMask::empty(), + vec![], )?; continue; } @@ -429,6 +434,8 @@ impl InterpreterImpl { loader, function, frame_cache, + ClosureMask::empty(), + vec![], )?; }, ExitCode::CallGeneric(idx) => { @@ -518,6 +525,8 @@ impl InterpreterImpl { traversal_context, extensions, &function, + ClosureMask::empty(), + vec![], )?; continue; } @@ -528,8 +537,73 @@ impl InterpreterImpl { loader, function, frame_cache, + 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, + frame_cache.clone(), + mask, + captured.collect(), + )? + } + }, } } } @@ -541,6 +615,8 @@ impl InterpreterImpl { loader: &Loader, function: Rc, frame_cache: Rc>, + 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 => { @@ -563,7 +639,14 @@ impl InterpreterImpl { } let mut frame = self - .make_call_frame::(gas_meter, loader, function, frame_cache) + .make_call_frame::( + gas_meter, + loader, + function, + frame_cache, + mask, + captured, + ) .map_err(|err| { self.attach_state_if_invariant_violation(self.set_location(err), current_frame) })?; @@ -593,21 +676,29 @@ impl InterpreterImpl { loader: &Loader, function: Rc, frame_cache: Rc>, + 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() @@ -673,6 +764,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::( @@ -683,6 +776,8 @@ impl InterpreterImpl { traversal_context, extensions, function, + mask, + captured, ) .map_err(|e| match function.module_id() { Some(id) => { @@ -711,28 +806,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); } } @@ -890,6 +998,8 @@ impl InterpreterImpl { resolver.loader(), Rc::new(target_func), frame_cache, + ClosureMask::empty(), + vec![], ) .map_err(|err| err.to_partial()) }, @@ -1649,6 +1759,7 @@ enum ExitCode { Return, Call(FunctionHandleIndex), CallGeneric(FunctionInstantiationIndex), + CallClosure(SignatureIndex), } impl AccessSpecifierEnv for Frame { @@ -1859,6 +1970,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, @@ -2199,6 +2311,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( + &TypeTagConverter::new(resolver.module_storage().runtime_environment()), + Rc::new(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( + &TypeTagConverter::new(resolver.module_storage().runtime_environment()), + Rc::new(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 8e7901fc1f884..75c8b48763fcb 100644 --- a/third_party/move/move-vm/runtime/src/loader/function.rs +++ b/third_party/move/move-vm/runtime/src/loader/function.rs @@ -103,7 +103,6 @@ impl LazyLoadedFunction { }))) } - #[allow(unused)] pub(crate) fn new_resolved( converter: &TypeTagConverter, fun: Rc,