From fc12231651db34bff8446b66a21f80c288ad7ebd Mon Sep 17 00:00:00 2001 From: George Mitenkov Date: Mon, 25 Nov 2024 13:43:26 +0000 Subject: [PATCH] [cleanup] Make Function code nicer --- aptos-move/aptos-gas-meter/src/meter.rs | 4 +- .../aptos-gas-profiling/src/profiler.rs | 8 +- .../aptos-memory-usage-tracker/src/lib.rs | 4 +- .../framework/src/natives/function_info.rs | 2 +- .../move/move-vm/runtime/src/interpreter.rs | 63 ++- .../move-vm/runtime/src/loader/function.rs | 450 +++++++++-------- .../move/move-vm/runtime/src/loader/mod.rs | 4 +- .../move-vm/runtime/src/loader/modules.rs | 464 +++++++++--------- .../move/move-vm/runtime/src/loader/script.rs | 52 +- .../move-vm/test-utils/src/gas_schedule.rs | 4 +- third_party/move/move-vm/types/src/gas.rs | 8 +- 11 files changed, 540 insertions(+), 523 deletions(-) diff --git a/aptos-move/aptos-gas-meter/src/meter.rs b/aptos-move/aptos-gas-meter/src/meter.rs index f17c266e09fef..26fb2b148b097 100644 --- a/aptos-move/aptos-gas-meter/src/meter.rs +++ b/aptos-move/aptos-gas-meter/src/meter.rs @@ -186,7 +186,7 @@ where fn charge_call( &mut self, _module_id: &ModuleId, - _func_name: &str, + _func_name: &IdentStr, args: impl ExactSizeIterator, num_locals: NumArgs, ) -> PartialVMResult<()> { @@ -204,7 +204,7 @@ where fn charge_call_generic( &mut self, _module_id: &ModuleId, - _func_name: &str, + _func_name: &IdentStr, ty_args: impl ExactSizeIterator, args: impl ExactSizeIterator, num_locals: NumArgs, diff --git a/aptos-move/aptos-gas-profiling/src/profiler.rs b/aptos-move/aptos-gas-profiling/src/profiler.rs index c7e94e63997e8..5547ac76393ac 100644 --- a/aptos-move/aptos-gas-profiling/src/profiler.rs +++ b/aptos-move/aptos-gas-profiling/src/profiler.rs @@ -409,7 +409,7 @@ where fn charge_call( &mut self, module_id: &ModuleId, - func_name: &str, + func_name: &IdentStr, args: impl ExactSizeIterator + Clone, num_locals: NumArgs, ) -> PartialVMResult<()> { @@ -419,7 +419,7 @@ where self.record_bytecode(Opcodes::CALL, cost); self.frames.push(CallFrame::new_function( module_id.clone(), - Identifier::new(func_name).unwrap(), + func_name.to_owned(), vec![], )); @@ -429,7 +429,7 @@ where fn charge_call_generic( &mut self, module_id: &ModuleId, - func_name: &str, + func_name: &IdentStr, ty_args: impl ExactSizeIterator + Clone, args: impl ExactSizeIterator + Clone, num_locals: NumArgs, @@ -446,7 +446,7 @@ where self.record_bytecode(Opcodes::CALL_GENERIC, cost); self.frames.push(CallFrame::new_function( module_id.clone(), - Identifier::new(func_name).unwrap(), + func_name.to_owned(), ty_tags, )); diff --git a/aptos-move/aptos-memory-usage-tracker/src/lib.rs b/aptos-move/aptos-memory-usage-tracker/src/lib.rs index 0db7ca683a008..3751f2126d573 100644 --- a/aptos-move/aptos-memory-usage-tracker/src/lib.rs +++ b/aptos-move/aptos-memory-usage-tracker/src/lib.rs @@ -110,7 +110,7 @@ where fn charge_call( &mut self, module_id: &ModuleId, - func_name: &str, + func_name: &IdentStr, args: impl ExactSizeIterator + Clone, num_locals: NumArgs, ) -> PartialVMResult<()>; @@ -177,7 +177,7 @@ where fn charge_call_generic( &mut self, module_id: &ModuleId, - func_name: &str, + func_name: &IdentStr, ty_args: impl ExactSizeIterator + Clone, args: impl ExactSizeIterator + Clone, num_locals: NumArgs, diff --git a/aptos-move/framework/src/natives/function_info.rs b/aptos-move/framework/src/natives/function_info.rs index 2884ad8c82141..fef3fb8f5dfc8 100644 --- a/aptos-move/framework/src/natives/function_info.rs +++ b/aptos-move/framework/src/natives/function_info.rs @@ -114,7 +114,7 @@ fn native_check_dispatch_type_compatibility_impl( Ok(smallvec![Value::bool( rhs.ty_param_abilities() == lhs.ty_param_abilities() && rhs.return_tys() == lhs.return_tys() - && &lhs.param_tys()[0..lhs.param_count() - 1] == rhs.param_tys() + && &lhs.param_tys()[0..lhs.param_tys().len() - 1] == rhs.param_tys() && !rhs.is_friend_or_private() && (!context .get_feature_flags() diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index f45aa01f5fd2b..94761484a4060 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -5,7 +5,7 @@ use crate::{ access_control::AccessControlState, data_cache::TransactionDataCache, - loader::{LegacyModuleStorageAdapter, Loader, Resolver}, + loader::{LegacyModuleStorageAdapter, LoadedFunctionOwner, Loader, Resolver}, module_traversal::TraversalContext, native_extensions::NativeContextExtensions, native_functions::NativeContext, @@ -504,7 +504,7 @@ impl Interpreter { gas_meter.balance_internal(), traversal_context, ); - let native_function = function.get_native()?; + let native_function = function.as_native_function()?; gas_meter.charge_native_function_before_execution( ty_args.iter().map(|ty| TypeWithLoader { ty, resolver }), @@ -1007,19 +1007,20 @@ impl Interpreter { } debug_writeln!(buf)?; - // Print out the current instruction. - debug_writeln!(buf)?; - debug_writeln!(buf, " Code:")?; - let pc = frame.pc as usize; - let code = function.code(); - let before = if pc > 3 { pc - 3 } else { 0 }; - let after = min(code.len(), pc + 4); - for (idx, instr) in code.iter().enumerate().take(pc).skip(before) { - debug_writeln!(buf, " [{}] {:?}", idx, instr)?; - } - debug_writeln!(buf, " > [{}] {:?}", pc, &code[pc])?; - for (idx, instr) in code.iter().enumerate().take(after).skip(pc + 1) { - debug_writeln!(buf, " [{}] {:?}", idx, instr)?; + if let Some(code) = function.code() { + // Print out the current instruction. + debug_writeln!(buf)?; + debug_writeln!(buf, " Code:")?; + let pc = frame.pc as usize; + let before = if pc > 3 { pc - 3 } else { 0 }; + let after = min(code.len(), pc + 4); + for (idx, instr) in code.iter().enumerate().take(pc).skip(before) { + debug_writeln!(buf, " [{}] {:?}", idx, instr)?; + } + debug_writeln!(buf, " > [{}] {:?}", pc, &code[pc])?; + for (idx, instr) in code.iter().enumerate().take(after).skip(pc + 1) { + debug_writeln!(buf, " [{}] {:?}", idx, instr)?; + } } // Print out the locals. @@ -1085,15 +1086,16 @@ impl Interpreter { ) .as_str(), ); - let code = current_frame.function.code(); - let pc = current_frame.pc as usize; - if pc < code.len() { - let mut i = 0; - for bytecode in &code[..pc] { - internal_state.push_str(format!("{}> {:?}\n", i, bytecode).as_str()); - i += 1; + if let Some(code) = current_frame.function.code() { + let pc = current_frame.pc as usize; + if pc < code.len() { + let mut i = 0; + for bytecode in &code[..pc] { + internal_state.push_str(format!("{}> {:?}\n", i, bytecode).as_str()); + i += 1; + } + internal_state.push_str(format!("{}* {:?}\n", i, code[pc]).as_str()); } - internal_state.push_str(format!("{}* {:?}\n", i, code[pc]).as_str()); } internal_state.push_str( format!( @@ -2284,7 +2286,10 @@ impl Frame { }; } - let code = self.function.code(); + let code = self.function.code().ok_or_else(|| { + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message("Native function cannot be an entry point".to_string()) + })?; loop { for instruction in &code[self.pc as usize..] { trace!( @@ -3139,8 +3144,14 @@ impl Frame { module_store: &'a LegacyModuleStorageAdapter, module_storage: &'a impl ModuleStorage, ) -> Resolver<'a> { - self.function - .get_resolver(loader, module_store, module_storage) + match &self.function.owner { + LoadedFunctionOwner::Module(module) => { + Resolver::for_module(loader, module_store, module_storage, module.clone()) + }, + LoadedFunctionOwner::Script(script) => { + Resolver::for_script(loader, module_store, module_storage, script.clone()) + }, + } } fn location(&self) -> Location { 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 6f8672a24c946..3e7068da22f92 100644 --- a/third_party/move/move-vm/runtime/src/loader/function.rs +++ b/third_party/move/move-vm/runtime/src/loader/function.rs @@ -4,42 +4,222 @@ use crate::{ loader::{ - access_specifier_loader::load_access_specifier, LegacyModuleStorageAdapter, Loader, Module, - Resolver, Script, + access_specifier_loader::load_access_specifier, type_loader::intern_type, Module, Script, }, native_functions::{NativeFunction, NativeFunctions, UnboxedNativeFunction}, - ModuleStorage, }; use move_binary_format::{ - access::ModuleAccess, + access::{ModuleAccess, ScriptAccess}, binary_views::BinaryIndexedView, errors::{PartialVMError, PartialVMResult}, - file_format::{AbilitySet, Bytecode, CompiledModule, FunctionDefinitionIndex, Visibility}, + file_format::{ + AbilitySet, Bytecode, CompiledModule, CompiledScript, FunctionDefinitionIndex, Visibility, + }, +}; +use move_core_types::{ + ident_str, + identifier::{IdentStr, Identifier}, + language_storage::ModuleId, + vm_status::StatusCode, }; -use move_core_types::{identifier::Identifier, language_storage::ModuleId, vm_status::StatusCode}; use move_vm_types::loaded_data::{ runtime_access_specifier::AccessSpecifier, - runtime_types::{StructIdentifier, Type}, + runtime_types::{StructIdentifier, StructNameIndex, Type}, }; use std::{fmt::Debug, sync::Arc}; -/// A runtime function definition representation. -pub struct Function { - #[allow(unused)] - pub(crate) file_format_version: u32, - pub(crate) index: FunctionDefinitionIndex, - pub(crate) code: Vec, - pub(crate) ty_param_abilities: Vec, - // TODO: Make `native` and `def_is_native` become an enum. - pub(crate) native: Option, - pub(crate) is_native: bool, - pub(crate) is_friend_or_private: bool, - pub(crate) is_entry: bool, - pub(crate) name: Identifier, - pub(crate) return_tys: Vec, - pub(crate) local_tys: Vec, +/// Type signature of a function, includes: +/// - parameter types, +/// - abilities of type parameters, +/// - access specifier (specifies where the function writes to or reads from in global state), +/// - return types. +pub(crate) struct FunctionSignature { pub(crate) param_tys: Vec, + pub(crate) ty_param_abilities: Vec, pub(crate) access_specifier: AccessSpecifier, + pub(crate) return_tys: Vec, +} + +/// Represents internal details of a function. It can be either a native, or have a concrete +/// implementation with bytecode instructions. +pub(crate) enum FunctionInternals { + Native { + native: NativeFunction, + }, + Bytecode { + local_tys: Vec, + bytecode: Vec, + }, +} + +/// A runtime representation for function definition. +pub struct Function { + /// Index into file format representation corresponding to this definition. + index: FunctionDefinitionIndex, + /// The name of the function. + name: Identifier, + /// If true, this an entry function. + is_entry: bool, + /// If true, this function has private or public(friend) visibility. + is_friend_or_private: bool, + /// Function's type signature, including access specifier. + signature: FunctionSignature, + /// Internal function details, e.g., its bytecode or native implementation. + internals: FunctionInternals, +} + +impl Function { + pub(crate) fn script_function( + script: &CompiledScript, + struct_name_indices: &[StructNameIndex], + ) -> PartialVMResult { + let params = script.signature_at(script.parameters); + let binary_indexed_view = BinaryIndexedView::Script(script); + + let param_tys = params + .0 + .iter() + .map(|tok| intern_type(binary_indexed_view, tok, struct_name_indices)) + .collect::>>()?; + let ty_param_abilities = script.type_parameters.clone(); + + let local_tys = params + .0 + .iter() + .chain(script.signature_at(script.code.locals).0.iter()) + .map(|tok| intern_type(binary_indexed_view, tok, struct_name_indices)) + .collect::>>()?; + + Ok(Function { + index: FunctionDefinitionIndex(0), + // TODO: + // Scripts do not have a name, we use "main" for now. + name: ident_str!("main").to_owned(), + // Script functions are not entry, and do not have visibility. + is_entry: false, + is_friend_or_private: false, + signature: FunctionSignature { + param_tys, + ty_param_abilities, + access_specifier: AccessSpecifier::Any, + // Script must not return values. + return_tys: vec![], + }, + // Script entries cannot be native. + internals: FunctionInternals::Bytecode { + local_tys, + bytecode: script.code.code.clone(), + }, + }) + } + + pub(crate) fn module_function( + natives: &NativeFunctions, + index: FunctionDefinitionIndex, + module: &CompiledModule, + signature_table: &[Vec], + struct_names: &[StructIdentifier], + ) -> PartialVMResult { + let function_def = module.function_def_at(index); + let handle = module.function_handle_at(function_def.function); + + let name = module.identifier_at(handle.name).to_owned(); + let internals = if function_def.is_native() { + assert!(function_def.code.is_none()); + + let native = natives + .resolve( + module.self_addr(), + module.self_name().as_str(), + name.as_str(), + ) + .ok_or_else(|| { + let msg = format!( + "Native function {}::{}::{} does not exist", + module.self_addr(), + module.self_name(), + name + ); + missing_native_function_error(msg) + })?; + FunctionInternals::Native { native } + } else { + let code_unit = function_def + .code + .as_ref() + .expect("Code unit always exist for non-native functions"); + let bytecode = code_unit.code.clone(); + + let mut local_tys = signature_table[handle.parameters.0 as usize].clone(); + local_tys.append(&mut signature_table[code_unit.locals.0 as usize].clone()); + + FunctionInternals::Bytecode { + local_tys, + bytecode, + } + }; + + let is_friend_or_private = match function_def.visibility { + Visibility::Friend | Visibility::Private => true, + Visibility::Public => false, + }; + + let param_tys = signature_table[handle.parameters.0 as usize].clone(); + let ty_param_abilities = handle.type_parameters.clone(); + let access_specifier = load_access_specifier( + BinaryIndexedView::Module(module), + signature_table, + struct_names, + &handle.access_specifiers, + )?; + let return_tys = signature_table[handle.return_.0 as usize].clone(); + + Ok(Self { + index, + name, + signature: FunctionSignature { + param_tys, + ty_param_abilities, + access_specifier, + return_tys, + }, + internals, + is_friend_or_private, + is_entry: function_def.is_entry, + }) + } + + pub fn name(&self) -> &IdentStr { + self.name.as_ident_str() + } + + pub fn param_tys(&self) -> &[Type] { + &self.signature.param_tys + } + + pub fn ty_param_abilities(&self) -> &[AbilitySet] { + &self.signature.ty_param_abilities + } + + pub fn return_tys(&self) -> &[Type] { + &self.signature.return_tys + } + + pub fn is_native(&self) -> bool { + matches!(self.internals, FunctionInternals::Native { .. }) + } + + pub fn is_friend_or_private(&self) -> bool { + self.is_friend_or_private + } +} + +impl Debug for Function { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + f.debug_struct("Function") + .field("name", &self.name) + .finish() + } } /// For loaded function representation, specifies the owner: a script or a module. @@ -48,13 +228,15 @@ pub(crate) enum LoadedFunctionOwner { Module(Arc), } -/// A loaded runtime function representation along with type arguments used to instantiate it. +/// A runtime representation of a "loaded" function. In addition to function's definition, it +/// contains type arguments used to instantiate it and the "owner" of a function (either a script +/// or a module). pub struct LoadedFunction { pub(crate) owner: LoadedFunctionOwner, - // A set of verified type arguments provided for this definition. If - // function is not generic, an empty vector. + /// A set of verified type arguments provided for this definition. If function is not generic, + /// an empty vector. pub(crate) ty_args: Vec, - // Definition of the loaded function. + /// Definition of the loaded function. pub(crate) function: Arc, } @@ -73,18 +255,18 @@ impl LoadedFunction { } /// Returns the name of this function. - pub fn name(&self) -> &str { + pub fn name(&self) -> &IdentStr { self.function.name() } - /// Returns true if the loaded function has friend or private visibility. - pub fn is_friend_or_private(&self) -> bool { - self.function.is_friend_or_private() - } - /// Returns true if the loaded function is an entry function. pub(crate) fn is_entry(&self) -> bool { - self.function.is_entry() + self.function.is_entry + } + + /// Returns true if the loaded function has private or public(friend) visibility. + pub fn is_friend_or_private(&self) -> bool { + self.function.is_friend_or_private() } /// Returns parameter types from the function's definition signature. @@ -92,19 +274,28 @@ impl LoadedFunction { self.function.param_tys() } - /// Returns return types from the function's definition signature. - pub fn return_tys(&self) -> &[Type] { - self.function.return_tys() - } - /// Returns abilities of type parameters from the function's definition signature. pub fn ty_param_abilities(&self) -> &[AbilitySet] { self.function.ty_param_abilities() } - /// Returns types of locals, defined by this function. + /// Returns the access specifier for this function. + pub(crate) fn access_specifier(&self) -> &AccessSpecifier { + &self.function.signature.access_specifier + } + + /// Returns return types from the function's definition signature. + pub fn return_tys(&self) -> &[Type] { + self.function.return_tys() + } + + /// Returns types of locals, defined by this function. If the function is a native, returns an + /// empty slice. pub fn local_tys(&self) -> &[Type] { - self.function.local_tys() + match &self.function.internals { + FunctionInternals::Native { .. } => &[], + FunctionInternals::Bytecode { local_tys, .. } => local_tys, + } } /// Returns true if this function is a native function, i.e. which does not contain @@ -113,20 +304,32 @@ impl LoadedFunction { self.function.is_native() } - pub fn get_native(&self) -> PartialVMResult<&UnboxedNativeFunction> { - self.function.get_native() - } - - pub(crate) fn index(&self) -> FunctionDefinitionIndex { - self.function.index + /// If function is a native, returns its native implementation. Otherwise, returns an error. + pub fn as_native_function(&self) -> PartialVMResult<&UnboxedNativeFunction> { + match &self.function.internals { + FunctionInternals::Native { native } => Ok(native.as_ref()), + FunctionInternals::Bytecode { .. } => { + let msg = format!( + "Native function {} must exist, but it does not", + self.function.name() + ); + Err(missing_native_function_error(msg)) + }, + } } - pub(crate) fn code(&self) -> &[Bytecode] { - &self.function.code + /// If function is a native, returns [None]. Otherwise, returns its bytecode. + pub(crate) fn code(&self) -> Option<&[Bytecode]> { + match &self.function.internals { + FunctionInternals::Native { .. } => None, + FunctionInternals::Bytecode { bytecode, .. } => Some(bytecode), + } } - pub(crate) fn access_specifier(&self) -> &AccessSpecifier { - &self.function.access_specifier + /// Returns the function definition index into the file-format representation of a script or a + /// module. + pub(crate) fn index(&self) -> FunctionDefinitionIndex { + self.function.index } pub(crate) fn name_as_pretty_string(&self) -> String { @@ -140,148 +343,6 @@ impl LoadedFunction { ), } } - - pub(crate) fn get_resolver<'a>( - &self, - loader: &'a Loader, - module_store: &'a LegacyModuleStorageAdapter, - module_storage: &'a impl ModuleStorage, - ) -> Resolver<'a> { - match &self.owner { - LoadedFunctionOwner::Module(module) => { - Resolver::for_module(loader, module_store, module_storage, module.clone()) - }, - LoadedFunctionOwner::Script(script) => { - Resolver::for_script(loader, module_store, module_storage, script.clone()) - }, - } - } -} - -impl Debug for Function { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - f.debug_struct("Function") - .field("name", &self.name) - .finish() - } -} - -impl Function { - pub(crate) fn new( - natives: &NativeFunctions, - index: FunctionDefinitionIndex, - module: &CompiledModule, - signature_table: &[Vec], - struct_names: &[StructIdentifier], - ) -> PartialVMResult { - let def = module.function_def_at(index); - let handle = module.function_handle_at(def.function); - let name = module.identifier_at(handle.name).to_owned(); - let module_id = module.self_id(); - - let is_friend_or_private = match def.visibility { - Visibility::Friend | Visibility::Private => true, - Visibility::Public => false, - }; - let is_entry = def.is_entry; - - let (native, is_native) = if def.is_native() { - let native = natives.resolve( - module_id.address(), - module_id.name().as_str(), - name.as_str(), - ); - (native, true) - } else { - (None, false) - }; - // Native functions do not have a code unit - let code = match &def.code { - Some(code) => code.code.clone(), - None => vec![], - }; - let ty_param_abilities = handle.type_parameters.clone(); - let return_tys = signature_table[handle.return_.0 as usize].clone(); - let local_tys = if let Some(code) = &def.code { - let mut local_tys = signature_table[handle.parameters.0 as usize].clone(); - local_tys.append(&mut signature_table[code.locals.0 as usize].clone()); - local_tys - } else { - vec![] - }; - let param_tys = signature_table[handle.parameters.0 as usize].clone(); - - let access_specifier = load_access_specifier( - BinaryIndexedView::Module(module), - signature_table, - struct_names, - &handle.access_specifiers, - )?; - - Ok(Self { - file_format_version: module.version(), - index, - code, - ty_param_abilities, - native, - is_native, - is_friend_or_private, - is_entry, - name, - local_tys, - return_tys, - param_tys, - access_specifier, - }) - } - - #[allow(unused)] - pub(crate) fn file_format_version(&self) -> u32 { - self.file_format_version - } - - pub fn param_count(&self) -> usize { - self.param_tys.len() - } - - pub(crate) fn name(&self) -> &str { - self.name.as_str() - } - - pub fn ty_param_abilities(&self) -> &[AbilitySet] { - &self.ty_param_abilities - } - - pub(crate) fn local_tys(&self) -> &[Type] { - &self.local_tys - } - - pub fn return_tys(&self) -> &[Type] { - &self.return_tys - } - - pub fn param_tys(&self) -> &[Type] { - &self.param_tys - } - - pub fn is_native(&self) -> bool { - self.is_native - } - - pub fn is_friend_or_private(&self) -> bool { - self.is_friend_or_private - } - - pub(crate) fn is_entry(&self) -> bool { - self.is_entry - } - - pub(crate) fn get_native(&self) -> PartialVMResult<&UnboxedNativeFunction> { - self.native.as_deref().ok_or_else(|| { - PartialVMError::new(StatusCode::MISSING_DEPENDENCY) - .with_message(format!("Missing Native Function `{}`", self.name)) - }) - } } // @@ -295,7 +356,6 @@ impl Function { // A function instantiation. #[derive(Clone, Debug)] pub(crate) struct FunctionInstantiation { - // index to `ModuleCache::functions` global table pub(crate) handle: FunctionHandle, pub(crate) instantiation: Vec, } @@ -305,3 +365,7 @@ pub(crate) enum FunctionHandle { Local(Arc), Remote { module: ModuleId, name: Identifier }, } + +fn missing_native_function_error(msg: String) -> PartialVMError { + PartialVMError::new(StatusCode::MISSING_DEPENDENCY).with_message(msg) +} diff --git a/third_party/move/move-vm/runtime/src/loader/mod.rs b/third_party/move/move-vm/runtime/src/loader/mod.rs index 1777fd5ac9b9b..ae0eeb9fec438 100644 --- a/third_party/move/move-vm/runtime/src/loader/mod.rs +++ b/third_party/move/move-vm/runtime/src/loader/mod.rs @@ -1180,7 +1180,7 @@ pub(crate) struct Resolver<'a> { } impl<'a> Resolver<'a> { - fn for_module( + pub(crate) fn for_module( loader: &'a Loader, module_store: &'a LegacyModuleStorageAdapter, module_storage: &'a dyn ModuleStorage, @@ -1195,7 +1195,7 @@ impl<'a> Resolver<'a> { } } - fn for_script( + pub(crate) fn for_script( loader: &'a Loader, module_store: &'a LegacyModuleStorageAdapter, module_storage: &'a dyn ModuleStorage, diff --git a/third_party/move/move-vm/runtime/src/loader/modules.rs b/third_party/move/move-vm/runtime/src/loader/modules.rs index f7faa99f5416d..9f70f12b055b1 100644 --- a/third_party/move/move-vm/runtime/src/loader/modules.rs +++ b/third_party/move/move-vm/runtime/src/loader/modules.rs @@ -310,262 +310,248 @@ impl Module { let mut single_signature_token_map = BTreeMap::new(); let mut signature_table = vec![]; - let mut create = || { - let mut struct_idxs = vec![]; - let mut struct_names = vec![]; - // validate the correctness of struct handle references. - for struct_handle in module.struct_handles() { - let struct_name = module.identifier_at(struct_handle.name); - let module_handle = module.module_handle_at(struct_handle.module); - let module_id = module.module_id_for_handle(module_handle); - - let struct_name = StructIdentifier { - module: module_id, - name: struct_name.to_owned(), - }; - struct_idxs.push(struct_name_index_map.struct_name_to_idx(struct_name.clone())?); - struct_names.push(struct_name) - } + let mut struct_idxs = vec![]; + let mut struct_names = vec![]; + // validate the correctness of struct handle references. + for struct_handle in module.struct_handles() { + let struct_name = module.identifier_at(struct_handle.name); + let module_handle = module.module_handle_at(struct_handle.module); + let module_id = module.module_id_for_handle(module_handle); + + let struct_name = StructIdentifier { + module: module_id, + name: struct_name.to_owned(), + }; + struct_idxs.push(struct_name_index_map.struct_name_to_idx(struct_name.clone())?); + struct_names.push(struct_name) + } - // Build signature table - for signatures in module.signatures() { - signature_table.push( - signatures - .0 - .iter() - .map(|sig| { - intern_type(BinaryIndexedView::Module(&module), sig, &struct_idxs) - }) - .collect::>>()?, - ) - } + // Build signature table + for signatures in module.signatures() { + signature_table.push( + signatures + .0 + .iter() + .map(|sig| intern_type(BinaryIndexedView::Module(&module), sig, &struct_idxs)) + .collect::>>()?, + ) + } - for (idx, struct_def) in module.struct_defs().iter().enumerate() { - let definition_struct_type = - Arc::new(Self::make_struct_type(&module, struct_def, &struct_idxs)?); - structs.push(StructDef { - field_count: definition_struct_type.field_count(None), - definition_struct_type, - }); - let name = - module.identifier_at(module.struct_handle_at(struct_def.struct_handle).name); - struct_map.insert(name.to_owned(), idx); - } + for (idx, struct_def) in module.struct_defs().iter().enumerate() { + let definition_struct_type = + Arc::new(Self::make_struct_type(&module, struct_def, &struct_idxs)?); + structs.push(StructDef { + field_count: definition_struct_type.field_count(None), + definition_struct_type, + }); + let name = module.identifier_at(module.struct_handle_at(struct_def.struct_handle).name); + struct_map.insert(name.to_owned(), idx); + } - for struct_inst in module.struct_instantiations() { - let def = struct_inst.def.0 as usize; - let struct_def = &structs[def]; - struct_instantiations.push(StructInstantiation { - field_count: struct_def.definition_struct_type.field_count(None), - instantiation: signature_table[struct_inst.type_parameters.0 as usize].clone(), - definition_struct_type: struct_def.definition_struct_type.clone(), - }); - } + for struct_inst in module.struct_instantiations() { + let def = struct_inst.def.0 as usize; + let struct_def = &structs[def]; + struct_instantiations.push(StructInstantiation { + field_count: struct_def.definition_struct_type.field_count(None), + instantiation: signature_table[struct_inst.type_parameters.0 as usize].clone(), + definition_struct_type: struct_def.definition_struct_type.clone(), + }); + } - for struct_variant in module.struct_variant_handles() { - let definition_struct_type = structs[struct_variant.struct_index.0 as usize] - .definition_struct_type - .clone(); - let variant = struct_variant.variant; - struct_variant_infos.push(StructVariantInfo { - field_count: definition_struct_type.field_count(Some(variant)), - variant, - definition_struct_type, - instantiation: vec![], - }) - } + for struct_variant in module.struct_variant_handles() { + let definition_struct_type = structs[struct_variant.struct_index.0 as usize] + .definition_struct_type + .clone(); + let variant = struct_variant.variant; + struct_variant_infos.push(StructVariantInfo { + field_count: definition_struct_type.field_count(Some(variant)), + variant, + definition_struct_type, + instantiation: vec![], + }) + } - for struct_variant_inst in module.struct_variant_instantiations() { - let variant = &struct_variant_infos[struct_variant_inst.handle.0 as usize]; - struct_variant_instantiation_infos.push(StructVariantInfo { - field_count: variant.field_count, - variant: variant.variant, - definition_struct_type: variant.definition_struct_type.clone(), - instantiation: signature_table[struct_variant_inst.type_parameters.0 as usize] - .clone(), - }) - } + for struct_variant_inst in module.struct_variant_instantiations() { + let variant = &struct_variant_infos[struct_variant_inst.handle.0 as usize]; + struct_variant_instantiation_infos.push(StructVariantInfo { + field_count: variant.field_count, + variant: variant.variant, + definition_struct_type: variant.definition_struct_type.clone(), + instantiation: signature_table[struct_variant_inst.type_parameters.0 as usize] + .clone(), + }) + } - for (idx, func) in module.function_defs().iter().enumerate() { - let findex = FunctionDefinitionIndex(idx as TableIndex); - let function = Function::new( - natives, - findex, - &module, - signature_table.as_slice(), - &struct_names, - )?; - - function_map.insert(function.name.to_owned(), idx); - function_defs.push(Arc::new(function)); - - if let Some(code_unit) = &func.code { - for bc in &code_unit.code { - match bc { - Bytecode::VecPack(si, _) - | Bytecode::VecLen(si) - | Bytecode::VecImmBorrow(si) - | Bytecode::VecMutBorrow(si) - | Bytecode::VecPushBack(si) - | Bytecode::VecPopBack(si) - | Bytecode::VecUnpack(si, _) - | Bytecode::VecSwap(si) => { - if !single_signature_token_map.contains_key(si) { - let ty = match module.signature_at(*si).0.first() { - None => { - return Err(PartialVMError::new( - StatusCode::VERIFIER_INVARIANT_VIOLATION, - ) - .with_message( - "the type argument for vector-related bytecode \ + for (idx, func) in module.function_defs().iter().enumerate() { + let findex = FunctionDefinitionIndex(idx as TableIndex); + let function = Function::module_function( + natives, + findex, + &module, + signature_table.as_slice(), + &struct_names, + )?; + + function_map.insert(function.name().to_owned(), idx); + function_defs.push(Arc::new(function)); + + if let Some(code_unit) = &func.code { + for bc in &code_unit.code { + match bc { + Bytecode::VecPack(si, _) + | Bytecode::VecLen(si) + | Bytecode::VecImmBorrow(si) + | Bytecode::VecMutBorrow(si) + | Bytecode::VecPushBack(si) + | Bytecode::VecPopBack(si) + | Bytecode::VecUnpack(si, _) + | Bytecode::VecSwap(si) => { + if !single_signature_token_map.contains_key(si) { + let ty = match module.signature_at(*si).0.first() { + None => { + return Err(PartialVMError::new( + StatusCode::VERIFIER_INVARIANT_VIOLATION, + ) + .with_message( + "the type argument for vector-related bytecode \ expects one and only one signature token" - .to_owned(), - )); - }, - Some(sig_token) => sig_token, - }; - single_signature_token_map.insert( - *si, - intern_type( - BinaryIndexedView::Module(&module), - ty, - &struct_idxs, - )?, - ); - } - }, - _ => {}, - } + .to_owned(), + )); + }, + Some(sig_token) => sig_token, + }; + single_signature_token_map.insert( + *si, + intern_type( + BinaryIndexedView::Module(&module), + ty, + &struct_idxs, + )?, + ); + } + }, + _ => {}, } } } + } - for func_handle in module.function_handles() { - let func_name = module.identifier_at(func_handle.name); - let module_handle = module.module_handle_at(func_handle.module); - let module_id = module.module_id_for_handle(module_handle); - let func_handle = if module_id == id { - FunctionHandle::Local( - function_defs[*function_map.get(func_name).ok_or_else(|| { - PartialVMError::new(StatusCode::TYPE_RESOLUTION_FAILURE).with_message( - "Cannot find function in publishing module".to_string(), - ) - })?] - .clone(), - ) - } else { - FunctionHandle::Remote { - module: module_id, - name: func_name.to_owned(), - } - }; - function_refs.push(func_handle); - } - - for func_inst in module.function_instantiations() { - let handle = function_refs[func_inst.handle.0 as usize].clone(); - function_instantiations.push(FunctionInstantiation { - handle, - instantiation: signature_table[func_inst.type_parameters.0 as usize].clone(), - }); - } - - for field_handle in module.field_handles() { - let def_idx = field_handle.owner; - let definition_struct_type = - structs[def_idx.0 as usize].definition_struct_type.clone(); - let offset = field_handle.field as usize; - let ty = definition_struct_type.field_at(None, offset)?.1.clone(); - field_handles.push(FieldHandle { - offset, - field_ty: ty, - definition_struct_type, - }); - } + for func_handle in module.function_handles() { + let func_name = module.identifier_at(func_handle.name); + let module_handle = module.module_handle_at(func_handle.module); + let module_id = module.module_id_for_handle(module_handle); + let func_handle = if module_id == id { + FunctionHandle::Local( + function_defs[*function_map.get(func_name).ok_or_else(|| { + PartialVMError::new(StatusCode::TYPE_RESOLUTION_FAILURE) + .with_message("Cannot find function in publishing module".to_string()) + })?] + .clone(), + ) + } else { + FunctionHandle::Remote { + module: module_id, + name: func_name.to_owned(), + } + }; + function_refs.push(func_handle); + } - for field_inst in module.field_instantiations() { - let fh_idx = field_inst.handle; - let offset = field_handles[fh_idx.0 as usize].offset; - let owner_struct_def = &structs[module.field_handle_at(fh_idx).owner.0 as usize]; - let uninstantiated_ty = owner_struct_def - .definition_struct_type - .field_at(None, offset)? - .1 - .clone(); - field_instantiations.push(FieldInstantiation { - offset, - uninstantiated_field_ty: uninstantiated_ty, - instantiation: signature_table[field_inst.type_parameters.0 as usize].clone(), - definition_struct_type: owner_struct_def.definition_struct_type.clone(), - }); - } + for func_inst in module.function_instantiations() { + let handle = function_refs[func_inst.handle.0 as usize].clone(); + function_instantiations.push(FunctionInstantiation { + handle, + instantiation: signature_table[func_inst.type_parameters.0 as usize].clone(), + }); + } - for variant_handle in module.variant_field_handles() { - let def_idx = variant_handle.struct_index; - let definition_struct_type = - structs[def_idx.0 as usize].definition_struct_type.clone(); - let offset = variant_handle.field as usize; - let variants = variant_handle.variants.clone(); - let ty = definition_struct_type - .field_at(Some(variants[0]), offset)? - .1 - .clone(); - variant_field_infos.push(VariantFieldInfo { - offset, - variants, - definition_struct_type, - uninstantiated_field_ty: ty, - instantiation: vec![], - }); - } + for field_handle in module.field_handles() { + let def_idx = field_handle.owner; + let definition_struct_type = structs[def_idx.0 as usize].definition_struct_type.clone(); + let offset = field_handle.field as usize; + let ty = definition_struct_type.field_at(None, offset)?.1.clone(); + field_handles.push(FieldHandle { + offset, + field_ty: ty, + definition_struct_type, + }); + } - for variant_inst in module.variant_field_instantiations() { - let variant_info = &variant_field_infos[variant_inst.handle.0 as usize]; - let definition_struct_type = variant_info.definition_struct_type.clone(); - let variants = variant_info.variants.clone(); - let offset = variant_info.offset; - let instantiation = - signature_table[variant_inst.type_parameters.0 as usize].clone(); - // We can select one representative variant for finding the field type, all - // must have the same type as the verifier ensured. - let uninstantiated_ty = definition_struct_type - .field_at(Some(variants[0]), offset)? - .1 - .clone(); - variant_field_instantiation_infos.push(VariantFieldInfo { - offset, - uninstantiated_field_ty: uninstantiated_ty, - variants, - definition_struct_type, - instantiation, - }); - } + for field_inst in module.field_instantiations() { + let fh_idx = field_inst.handle; + let offset = field_handles[fh_idx.0 as usize].offset; + let owner_struct_def = &structs[module.field_handle_at(fh_idx).owner.0 as usize]; + let uninstantiated_ty = owner_struct_def + .definition_struct_type + .field_at(None, offset)? + .1 + .clone(); + field_instantiations.push(FieldInstantiation { + offset, + uninstantiated_field_ty: uninstantiated_ty, + instantiation: signature_table[field_inst.type_parameters.0 as usize].clone(), + definition_struct_type: owner_struct_def.definition_struct_type.clone(), + }); + } - Ok(()) - }; + for variant_handle in module.variant_field_handles() { + let def_idx = variant_handle.struct_index; + let definition_struct_type = structs[def_idx.0 as usize].definition_struct_type.clone(); + let offset = variant_handle.field as usize; + let variants = variant_handle.variants.clone(); + let ty = definition_struct_type + .field_at(Some(variants[0]), offset)? + .1 + .clone(); + variant_field_infos.push(VariantFieldInfo { + offset, + variants, + definition_struct_type, + uninstantiated_field_ty: ty, + instantiation: vec![], + }); + } - match create() { - Ok(_) => Ok(Self { - id, - size, - module, - structs, - struct_instantiations, - struct_variant_infos, - struct_variant_instantiation_infos, - function_refs, - function_defs, - function_instantiations, - field_handles, - field_instantiations, - variant_field_infos, - variant_field_instantiation_infos, - function_map, - struct_map, - single_signature_token_map, - }), - Err(err) => Err(err), + for variant_inst in module.variant_field_instantiations() { + let variant_info = &variant_field_infos[variant_inst.handle.0 as usize]; + let definition_struct_type = variant_info.definition_struct_type.clone(); + let variants = variant_info.variants.clone(); + let offset = variant_info.offset; + let instantiation = signature_table[variant_inst.type_parameters.0 as usize].clone(); + // We can select one representative variant for finding the field type, all + // must have the same type as the verifier ensured. + let uninstantiated_ty = definition_struct_type + .field_at(Some(variants[0]), offset)? + .1 + .clone(); + variant_field_instantiation_infos.push(VariantFieldInfo { + offset, + uninstantiated_field_ty: uninstantiated_ty, + variants, + definition_struct_type, + instantiation, + }); } + + Ok(Self { + id, + size, + module, + structs, + struct_instantiations, + struct_variant_infos, + struct_variant_instantiation_infos, + function_refs, + function_defs, + function_instantiations, + field_handles, + field_instantiations, + variant_field_infos, + variant_field_instantiation_infos, + function_map, + struct_map, + single_signature_token_map, + }) } fn make_struct_type( diff --git a/third_party/move/move-vm/runtime/src/loader/script.rs b/third_party/move/move-vm/runtime/src/loader/script.rs index 2473cd8104abb..026d73a7006e9 100644 --- a/third_party/move/move-vm/runtime/src/loader/script.rs +++ b/third_party/move/move-vm/runtime/src/loader/script.rs @@ -7,13 +7,10 @@ use move_binary_format::{ access::ScriptAccess, binary_views::BinaryIndexedView, errors::{PartialVMError, PartialVMResult}, - file_format::{Bytecode, CompiledScript, FunctionDefinitionIndex, Signature, SignatureIndex}, -}; -use move_core_types::{identifier::Identifier, language_storage::ModuleId, vm_status::StatusCode}; -use move_vm_types::loaded_data::{ - runtime_access_specifier::AccessSpecifier, - runtime_types::{StructIdentifier, Type}, + file_format::{Bytecode, CompiledScript, SignatureIndex}, }; +use move_core_types::{language_storage::ModuleId, vm_status::StatusCode}; +use move_vm_types::loaded_data::runtime_types::{StructIdentifier, Type}; use std::{collections::BTreeMap, ops::Deref, sync::Arc}; // A Script is very similar to a `CompiledScript` but data is "transformed" to a representation @@ -84,48 +81,7 @@ impl Script { }); } - let code: Vec = script.code.code.clone(); - let parameters = script.signature_at(script.parameters).clone(); - - let param_tys = parameters - .0 - .iter() - .map(|tok| intern_type(BinaryIndexedView::Script(&script), tok, &struct_names)) - .collect::>>()?; - let locals = Signature( - parameters - .0 - .iter() - .chain(script.signature_at(script.code.locals).0.iter()) - .cloned() - .collect(), - ); - let local_tys = locals - .0 - .iter() - .map(|tok| intern_type(BinaryIndexedView::Script(&script), tok, &struct_names)) - .collect::>>()?; - let ty_param_abilities = script.type_parameters.clone(); - // TODO: main does not have a name. Revisit. - let name = Identifier::new("main").unwrap(); - let (native, def_is_native) = (None, false); // Script entries cannot be native - let main: Arc = Arc::new(Function { - file_format_version: script.version(), - index: FunctionDefinitionIndex(0), - code, - ty_param_abilities, - native, - is_native: def_is_native, - is_friend_or_private: false, - is_entry: false, - name, - // Script must not return values. - return_tys: vec![], - local_tys, - param_tys, - access_specifier: AccessSpecifier::Any, - }); - + let main = Arc::new(Function::script_function(&script, &struct_names)?); let mut single_signature_token_map = BTreeMap::new(); for bc in &script.code.code { match bc { diff --git a/third_party/move/move-vm/test-utils/src/gas_schedule.rs b/third_party/move/move-vm/test-utils/src/gas_schedule.rs index dc22263926f53..1c72a069a64bc 100644 --- a/third_party/move/move-vm/test-utils/src/gas_schedule.rs +++ b/third_party/move/move-vm/test-utils/src/gas_schedule.rs @@ -242,7 +242,7 @@ impl GasMeter for GasStatus { fn charge_call( &mut self, _module_id: &ModuleId, - _func_name: &str, + _func_name: &IdentStr, args: impl ExactSizeIterator, _num_locals: NumArgs, ) -> PartialVMResult<()> { @@ -252,7 +252,7 @@ impl GasMeter for GasStatus { fn charge_call_generic( &mut self, _module_id: &ModuleId, - _func_name: &str, + _func_name: &IdentStr, ty_args: impl ExactSizeIterator, args: impl ExactSizeIterator, _num_locals: NumArgs, diff --git a/third_party/move/move-vm/types/src/gas.rs b/third_party/move/move-vm/types/src/gas.rs index 5765754854e18..38e0b4b05a6aa 100644 --- a/third_party/move/move-vm/types/src/gas.rs +++ b/third_party/move/move-vm/types/src/gas.rs @@ -157,7 +157,7 @@ pub trait GasMeter { fn charge_call( &mut self, module_id: &ModuleId, - func_name: &str, + func_name: &IdentStr, args: impl ExactSizeIterator + Clone, num_locals: NumArgs, ) -> PartialVMResult<()>; @@ -165,7 +165,7 @@ pub trait GasMeter { fn charge_call_generic( &mut self, module_id: &ModuleId, - func_name: &str, + func_name: &IdentStr, ty_args: impl ExactSizeIterator + Clone, args: impl ExactSizeIterator + Clone, num_locals: NumArgs, @@ -372,7 +372,7 @@ impl GasMeter for UnmeteredGasMeter { fn charge_call( &mut self, _module_id: &ModuleId, - _func_name: &str, + _func_name: &IdentStr, _args: impl IntoIterator, _num_locals: NumArgs, ) -> PartialVMResult<()> { @@ -382,7 +382,7 @@ impl GasMeter for UnmeteredGasMeter { fn charge_call_generic( &mut self, _module_id: &ModuleId, - _func_name: &str, + _func_name: &IdentStr, _ty_args: impl ExactSizeIterator, _args: impl ExactSizeIterator, _num_locals: NumArgs,