From 90f0c0853cf9b4bd40600dd82b181ead5037be54 Mon Sep 17 00:00:00 2001 From: Wolfgang Grieskamp Date: Tue, 31 Dec 2024 14:48:20 -0800 Subject: [PATCH] [move-vm][closures] Type and Value representation and serialization [PR 3/n vm closures] This implements types and values for functions and closures, as well as serialization. For a detailed discussion, see the design doc. It does not yet implement the interpreter and runtime verification. Overview: - `MoveValue` and `MoveTypeLayout` are extended to represent closures and functions. A closure carries the function name, the closure mask, the function layout, and the captured arguments. The function layout enables serialization of the captured arguments without any context. - Runtime `Type` is extended by functions. - `ValueImpl` is extended by a closure variant. This representation uses `trait AbstractFunction` to represent the function value and implement core functionality like comparison and printing. The existing trait `FunctionValueExtension` is used to create `AbstracFunction` from the serialized representation. - Similar as with `MoveValue`, the serialized representation of `ValueImpl::Closure` contains the full function layout, enabling to deserialize the captured arguments context independently. This design has been chosen for robustness, avoiding a dependency of serialization from loaded functions, and to enable fully 'lazy' deserialization of closures. The later allows a contract to store hundreds of function values and on loading them from storage, avoiding loading the associated code until the moment the closure is actually executed. - `AbstractFunction` is implemented in the loader such that it can be either in 'unresolved' or in `resolved' state. --- Cargo.lock | 1 + api/types/src/convert.rs | 5 + api/types/src/move_types.rs | 8 + .../src/gas_schedule/misc.rs | 26 ++ aptos-move/aptos-sdk-builder/src/common.rs | 4 +- aptos-move/aptos-sdk-builder/src/golang.rs | 8 +- aptos-move/aptos-sdk-builder/src/rust.rs | 6 +- .../aptos-vm/src/move_vm_ext/session/mod.rs | 1 + .../verifier/transaction_arg_validation.rs | 10 +- .../framework/move-stdlib/src/natives/bcs.rs | 5 +- .../framework/src/natives/string_utils.rs | 19 +- aptos-move/script-composer/src/helpers.rs | 15 +- .../fuzz/fuzz_targets/move/utils/helpers.rs | 8 +- .../move/move-binary-format/src/constant.rs | 1 + .../move/move-binary-format/src/errors.rs | 4 + .../move/move-binary-format/src/normalized.rs | 1 + .../move/move-compiler/src/cfgir/ast.rs | 1 + .../move/move-core/types/src/function.rs | 197 +++++++++++- .../move-core/types/src/language_storage.rs | 50 +++ .../move-core/types/src/safe_serialize.rs | 3 - .../types/src/transaction_argument.rs | 2 +- third_party/move/move-core/types/src/value.rs | 18 ++ third_party/move/move-ir/types/src/ast.rs | 2 +- .../move-model/src/builder/exp_builder.rs | 24 +- third_party/move/move-model/src/ty.rs | 17 +- .../move/move-stdlib/src/natives/debug.rs | 3 + third_party/move/move-vm/runtime/Cargo.toml | 3 +- .../move/move-vm/runtime/src/interpreter.rs | 50 +-- .../move-vm/runtime/src/loader/function.rs | 304 +++++++++++++++++- .../move/move-vm/runtime/src/loader/mod.rs | 65 +++- .../runtime/src/storage/module_storage.rs | 59 +++- .../runtime/src/storage/type_converter.rs | 65 +++- third_party/move/move-vm/types/Cargo.toml | 1 + .../types/src/loaded_data/runtime_types.rs | 174 +++++++++- .../move/move-vm/types/src/value_serde.rs | 27 +- .../move/move-vm/types/src/value_traversal.rs | 8 +- .../types/src/values/function_values_impl.rs | 208 ++++++++++++ .../move/move-vm/types/src/values/mod.rs | 2 + .../move-vm/types/src/values/values_impl.rs | 140 +++++++- third_party/move/move-vm/types/src/views.rs | 8 +- .../tools/move-bytecode-utils/src/layout.rs | 13 + .../move-resource-viewer/src/fat_type.rs | 4 + .../tools/move-resource-viewer/src/lib.rs | 8 + 43 files changed, 1476 insertions(+), 102 deletions(-) create mode 100644 third_party/move/move-vm/types/src/values/function_values_impl.rs diff --git a/Cargo.lock b/Cargo.lock index 0a63506f6e2d9..9c40bb9aff9ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12035,6 +12035,7 @@ version = "0.1.0" dependencies = [ "ambassador", "bcs 0.1.4", + "better_any", "bytes", "claims", "crossbeam", diff --git a/api/types/src/convert.rs b/api/types/src/convert.rs index 7a9d699c20e12..61b9e0f97af22 100644 --- a/api/types/src/convert.rs +++ b/api/types/src/convert.rs @@ -892,6 +892,11 @@ impl<'a, S: StateView> MoveConverter<'a, S> { MoveTypeLayout::Struct(struct_layout) => { self.try_into_vm_value_struct(struct_layout, val)? }, + MoveTypeLayout::Function(..) => { + // TODO(#15664): do we actually need this? It appears the code here is dead and + // nowhere used + bail!("unexpected move type {:?} for value {:?}", layout, val) + }, // Some values, e.g., signer or ones with custom serialization // (native), are not stored to storage and so we do not expect diff --git a/api/types/src/move_types.rs b/api/types/src/move_types.rs index 22342761857e0..b8c995c92fc46 100644 --- a/api/types/src/move_types.rs +++ b/api/types/src/move_types.rs @@ -681,6 +681,10 @@ impl From for MoveType { items: Box::new(MoveType::from(*v)), }, TypeTag::Struct(v) => MoveType::Struct((*v).into()), + TypeTag::Function(..) => { + // TODO(#15664): determine whether functions and closures need to be supported + panic!("functions not supported by API types") + }, } } } @@ -701,6 +705,10 @@ impl From<&TypeTag> for MoveType { items: Box::new(MoveType::from(v.as_ref())), }, TypeTag::Struct(v) => MoveType::Struct((&**v).into()), + TypeTag::Function(..) => { + // TODO(#15664): determine whether functions and closures need to be supported + panic!("functions not supported by API types") + }, } } } diff --git a/aptos-move/aptos-gas-schedule/src/gas_schedule/misc.rs b/aptos-move/aptos-gas-schedule/src/gas_schedule/misc.rs index b9f4a8615c02e..755458b36c24a 100644 --- a/aptos-move/aptos-gas-schedule/src/gas_schedule/misc.rs +++ b/aptos-move/aptos-gas-schedule/src/gas_schedule/misc.rs @@ -125,6 +125,11 @@ where self.offset = 1; true } + + #[inline] + fn visit_closure(&mut self, depth: usize, len: usize) -> bool { + self.inner.visit_closure(depth, len) + } } struct AbstractValueSizeVisitor<'a> { @@ -200,6 +205,13 @@ impl<'a> ValueVisitor for AbstractValueSizeVisitor<'a> { true } + #[inline] + fn visit_closure(&mut self, _depth: usize, _len: usize) -> bool { + // TODO(#15664): introduce a dedicated gas parameter? + self.size += self.params.struct_; + true + } + #[inline] fn visit_vec(&mut self, _depth: usize, _len: usize) -> bool { self.size += self.params.vector; @@ -366,6 +378,13 @@ impl AbstractValueSizeGasParameters { false } + #[inline] + fn visit_closure(&mut self, _depth: usize, _len: usize) -> bool { + // TODO(#15664): independent gas parameter for closures? + self.res = Some(self.params.struct_); + false + } + #[inline] fn visit_vec(&mut self, _depth: usize, _len: usize) -> bool { self.res = Some(self.params.vector); @@ -509,6 +528,13 @@ impl AbstractValueSizeGasParameters { false } + #[inline] + fn visit_closure(&mut self, _depth: usize, _len: usize) -> bool { + // TODO(#15664): independent gas parameter + self.res = Some(self.params.struct_); + false + } + #[inline] fn visit_vec(&mut self, _depth: usize, _len: usize) -> bool { self.res = Some(self.params.vector); diff --git a/aptos-move/aptos-sdk-builder/src/common.rs b/aptos-move/aptos-sdk-builder/src/common.rs index 9914b5ea613cd..1711ecaf3414d 100644 --- a/aptos-move/aptos-sdk-builder/src/common.rs +++ b/aptos-move/aptos-sdk-builder/src/common.rs @@ -45,7 +45,7 @@ fn quote_type_as_format(type_tag: &TypeTag) -> Format { tag if &**tag == Lazy::force(&str_tag) => Format::Seq(Box::new(Format::U8)), _ => type_not_allowed(type_tag), }, - Signer => type_not_allowed(type_tag), + Signer | Function(..) => type_not_allowed(type_tag), } } @@ -122,7 +122,7 @@ pub(crate) fn mangle_type(type_tag: &TypeTag) -> String { tag if &**tag == Lazy::force(&str_tag) => "string".into(), _ => type_not_allowed(type_tag), }, - Signer => type_not_allowed(type_tag), + Signer | Function(..) => type_not_allowed(type_tag), } } diff --git a/aptos-move/aptos-sdk-builder/src/golang.rs b/aptos-move/aptos-sdk-builder/src/golang.rs index dbbbc734d0661..17a847c0a8ceb 100644 --- a/aptos-move/aptos-sdk-builder/src/golang.rs +++ b/aptos-move/aptos-sdk-builder/src/golang.rs @@ -672,7 +672,7 @@ func encode_{}_argument(arg {}) []byte {{ U8 => ("U8Vector", default_stmt), _ => common::type_not_allowed(type_tag), }, - Struct(_) | Signer => common::type_not_allowed(type_tag), + Struct(_) | Signer | Function(..) => common::type_not_allowed(type_tag), }; writeln!( self.out, @@ -793,7 +793,7 @@ func decode_{0}_argument(arg aptostypes.TransactionArgument) (value {1}, err err tag if &**tag == Lazy::force(&str_tag) => "[]uint8".into(), _ => common::type_not_allowed(type_tag), }, - Signer => common::type_not_allowed(type_tag), + Signer | Function(..) => common::type_not_allowed(type_tag), } } @@ -820,7 +820,7 @@ func decode_{0}_argument(arg aptostypes.TransactionArgument) (value {1}, err err U8 => format!("(*aptostypes.TransactionArgument__U8Vector)(&{})", name), _ => common::type_not_allowed(type_tag), }, - Struct(_) | Signer => common::type_not_allowed(type_tag), + Struct(_) | Signer | Function(..) => common::type_not_allowed(type_tag), } } @@ -854,7 +854,7 @@ func decode_{0}_argument(arg aptostypes.TransactionArgument) (value {1}, err err tag if &**tag == Lazy::force(&str_tag) => Some("Bytes"), _ => common::type_not_allowed(type_tag), }, - Signer => common::type_not_allowed(type_tag), + Signer | Function(..) => common::type_not_allowed(type_tag), } } } diff --git a/aptos-move/aptos-sdk-builder/src/rust.rs b/aptos-move/aptos-sdk-builder/src/rust.rs index cc01736cb3748..70980cd5d0a5e 100644 --- a/aptos-move/aptos-sdk-builder/src/rust.rs +++ b/aptos-move/aptos-sdk-builder/src/rust.rs @@ -737,7 +737,7 @@ static SCRIPT_FUNCTION_DECODER_MAP: once_cell::sync::Lazy ("U8Vector", "Some(value)".to_string()), _ => common::type_not_allowed(type_tag), }, - Struct(_) | Signer => common::type_not_allowed(type_tag), + Struct(_) | Signer | Function(..) => common::type_not_allowed(type_tag), }; writeln!( self.out, @@ -880,7 +880,7 @@ fn decode_{}_argument(arg: TransactionArgument) -> Option<{}> {{ tag if &**tag == Lazy::force(&str_tag) => "Vec".into(), _ => common::type_not_allowed(type_tag), }, - Signer => common::type_not_allowed(type_tag), + Signer | Function(..) => common::type_not_allowed(type_tag), } } @@ -909,7 +909,7 @@ fn decode_{}_argument(arg: TransactionArgument) -> Option<{}> {{ _ => common::type_not_allowed(type_tag), }, - Struct(_) | Signer => common::type_not_allowed(type_tag), + Struct(_) | Signer | Function(..) => common::type_not_allowed(type_tag), } } } diff --git a/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs b/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs index f34457f6ee2c0..b07b02cf33fe3 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs @@ -127,6 +127,7 @@ impl<'r, 'l> SessionExt<'r, 'l> { module_storage: &impl ModuleStorage, ) -> VMResult<(VMChangeSet, ModuleWriteSet)> { let move_vm = self.inner.get_move_vm(); + let function_extension = module_storage.as_function_value_extension(); let resource_converter = |value: Value, diff --git a/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs b/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs index 153f26da17ecf..0e9c3e21fd56d 100644 --- a/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs +++ b/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs @@ -212,7 +212,7 @@ pub(crate) fn is_valid_txn_arg( let full_name = format!("{}::{}", st.module.short_str_lossless(), st.name); allowed_structs.contains_key(&full_name) }), - Signer | Reference(_) | MutableReference(_) | TyParam(_) => false, + Signer | Reference(_) | MutableReference(_) | TyParam(_) | Function { .. } => false, } } @@ -304,7 +304,9 @@ fn construct_arg( Err(invalid_signature()) } }, - Reference(_) | MutableReference(_) | TyParam(_) => Err(invalid_signature()), + Reference(_) | MutableReference(_) | TyParam(_) | Function { .. } => { + Err(invalid_signature()) + }, } } @@ -373,7 +375,9 @@ pub(crate) fn recursively_construct_arg( U64 => read_n_bytes(8, cursor, arg)?, U128 => read_n_bytes(16, cursor, arg)?, U256 | Address => read_n_bytes(32, cursor, arg)?, - Signer | Reference(_) | MutableReference(_) | TyParam(_) => return Err(invalid_signature()), + Signer | Reference(_) | MutableReference(_) | TyParam(_) | Function { .. } => { + return Err(invalid_signature()) + }, }; Ok(()) } diff --git a/aptos-move/framework/move-stdlib/src/natives/bcs.rs b/aptos-move/framework/move-stdlib/src/natives/bcs.rs index d7cd0e9d258c4..e64504743bc01 100644 --- a/aptos-move/framework/move-stdlib/src/natives/bcs.rs +++ b/aptos-move/framework/move-stdlib/src/natives/bcs.rs @@ -191,10 +191,11 @@ fn constant_serialized_size(ty_layout: &MoveTypeLayout) -> (u64, PartialVMResult MoveTypeLayout::Signer => Ok(None), // vectors have no constant size MoveTypeLayout::Vector(_) => Ok(None), - // enums have no constant size + // enums and functions have no constant size MoveTypeLayout::Struct( MoveStructLayout::RuntimeVariants(_) | MoveStructLayout::WithVariants(_), - ) => Ok(None), + ) + | MoveTypeLayout::Function(..) => Ok(None), MoveTypeLayout::Struct(MoveStructLayout::Runtime(fields)) => { let mut total = Some(0); for field in fields { diff --git a/aptos-move/framework/src/natives/string_utils.rs b/aptos-move/framework/src/natives/string_utils.rs index 0a4c7c71583f8..a752c9e3e5027 100644 --- a/aptos-move/framework/src/natives/string_utils.rs +++ b/aptos-move/framework/src/natives/string_utils.rs @@ -18,7 +18,7 @@ use move_core_types::{ use move_vm_runtime::native_functions::NativeFunction; use move_vm_types::{ loaded_data::runtime_types::Type, - values::{Reference, Struct, Value, Vector, VectorRef}, + values::{Closure, Reference, Struct, Value, Vector, VectorRef}, }; use smallvec::{smallvec, SmallVec}; use std::{collections::VecDeque, fmt::Write, ops::Deref}; @@ -347,6 +347,23 @@ fn native_format_impl( )?; out.push('}'); }, + MoveTypeLayout::Function(_) => { + let (fun, args) = val.value_as::()?.unpack(); + let data = context + .context + .function_value_extension() + .get_serialization_data(fun.as_ref())?; + out.push_str(&fun.to_stable_string()); + format_vector( + context, + data.captured_layouts.iter(), + args.collect(), + depth, + !context.single_line, + out, + )?; + out.push(')'); + }, // This is unreachable because we check layout at the start. Still, return // an error to be safe. diff --git a/aptos-move/script-composer/src/helpers.rs b/aptos-move/script-composer/src/helpers.rs index 2bbab803fcfdc..adb27fe8bea59 100644 --- a/aptos-move/script-composer/src/helpers.rs +++ b/aptos-move/script-composer/src/helpers.rs @@ -29,6 +29,11 @@ pub(crate) fn import_type_tag( type_tag: &TypeTag, module_resolver: &BTreeMap, ) -> PartialVMResult { + let to_list = |script_builder: &mut CompiledScriptBuilder, ts: &[TypeTag]| { + ts.iter() + .map(|t| import_type_tag(script_builder, t, module_resolver)) + .collect::>>() + }; Ok(match type_tag { TypeTag::Address => SignatureToken::Address, TypeTag::U8 => SignatureToken::U8, @@ -53,13 +58,15 @@ pub(crate) fn import_type_tag( } else { SignatureToken::StructInstantiation( struct_idx, - s.type_args - .iter() - .map(|ty| import_type_tag(script_builder, ty, module_resolver)) - .collect::>>()?, + to_list(script_builder, &s.type_args)?, ) } }, + TypeTag::Function(f) => SignatureToken::Function( + to_list(script_builder, &f.args)?, + to_list(script_builder, &f.results)?, + f.abilities, + ), }) } diff --git a/testsuite/fuzzer/fuzz/fuzz_targets/move/utils/helpers.rs b/testsuite/fuzzer/fuzz/fuzz_targets/move/utils/helpers.rs index 15960f5f920aa..52c3973dec322 100644 --- a/testsuite/fuzzer/fuzz/fuzz_targets/move/utils/helpers.rs +++ b/testsuite/fuzzer/fuzz/fuzz_targets/move/utils/helpers.rs @@ -6,7 +6,10 @@ use aptos_language_e2e_tests::{account::Account, executor::FakeExecutor}; use arbitrary::Arbitrary; use move_binary_format::file_format::CompiledModule; -use move_core_types::value::{MoveStructLayout, MoveTypeLayout}; +use move_core_types::{ + function::MoveFunctionLayout, + value::{MoveStructLayout, MoveTypeLayout}, +}; #[macro_export] macro_rules! tdbg { @@ -82,6 +85,9 @@ pub(crate) fn is_valid_layout(layout: &MoveTypeLayout) -> bool { } fields.iter().all(is_valid_layout) }, + L::Function(MoveFunctionLayout(args, results, _)) => { + args.iter().chain(results).all(is_valid_layout) + }, L::Struct(_) => { // decorated layouts not supported false diff --git a/third_party/move/move-binary-format/src/constant.rs b/third_party/move/move-binary-format/src/constant.rs index 77d995b432349..bd6407714db9d 100644 --- a/third_party/move/move-binary-format/src/constant.rs +++ b/third_party/move/move-binary-format/src/constant.rs @@ -40,6 +40,7 @@ fn construct_ty_for_constant(layout: &MoveTypeLayout) -> Option construct_ty_for_constant(l.as_ref())?, ))), MoveTypeLayout::Struct(_) => None, + MoveTypeLayout::Function(_) => None, MoveTypeLayout::Bool => Some(SignatureToken::Bool), // It is not possible to have native layout for constant values. diff --git a/third_party/move/move-binary-format/src/errors.rs b/third_party/move/move-binary-format/src/errors.rs index a5f5e447786b1..eedbd87ec76ad 100644 --- a/third_party/move/move-binary-format/src/errors.rs +++ b/third_party/move/move-binary-format/src/errors.rs @@ -448,6 +448,10 @@ impl PartialVMError { })) } + pub fn new_invariant_violation(msg: impl ToString) -> PartialVMError { + Self::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR).with_message(msg.to_string()) + } + pub fn major_status(&self) -> StatusCode { self.0.major_status } diff --git a/third_party/move/move-binary-format/src/normalized.rs b/third_party/move/move-binary-format/src/normalized.rs index d938f8fbcebfe..037f6f472f864 100644 --- a/third_party/move/move-binary-format/src/normalized.rs +++ b/third_party/move/move-binary-format/src/normalized.rs @@ -398,6 +398,7 @@ impl From for Type { name: s.name, type_arguments: s.type_args.into_iter().map(|ty| ty.into()).collect(), }, + TypeTag::Function(_) => panic!("function types not supported in normalized types"), } } } diff --git a/third_party/move/move-compiler/src/cfgir/ast.rs b/third_party/move/move-compiler/src/cfgir/ast.rs index 12788db355065..e7e2de089dd85 100644 --- a/third_party/move/move-compiler/src/cfgir/ast.rs +++ b/third_party/move/move-compiler/src/cfgir/ast.rs @@ -323,6 +323,7 @@ impl AstDebug for MoveValue { }, V::Struct(_) => panic!("ICE struct constants not supported"), V::Signer(_) => panic!("ICE signer constants not supported"), + V::Closure(_) => panic!("ICE closures not supported"), } } } diff --git a/third_party/move/move-core/types/src/function.rs b/third_party/move/move-core/types/src/function.rs index aba217289b5c7..b6ed00c867877 100644 --- a/third_party/move/move-core/types/src/function.rs +++ b/third_party/move/move-core/types/src/function.rs @@ -1,7 +1,13 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use serde::{Deserialize, Serialize}; +use crate::{ + ability::AbilitySet, + identifier::Identifier, + language_storage::{FunctionTag, ModuleId, TypeTag}, + value::{MoveTypeLayout, MoveValue}, +}; +use serde::{de::Error, ser::SerializeSeq, Deserialize, Serialize}; use std::fmt; /// A `ClosureMask` is a value which determines how to distinguish those function arguments @@ -94,6 +100,19 @@ impl ClosureMask { i } + /// Return the # of captured arguments in the mask + pub fn captured_count(&self) -> u16 { + let mut i = 0; + let mut mask = self.0; + while mask != 0 { + if mask & 0x1 != 0 { + i += 1 + } + mask >>= 1; + } + i + } + pub fn merge_placeholder_strings( &self, arity: usize, @@ -105,3 +124,179 @@ impl ClosureMask { self.compose(captured, provided) } } + +/// Function type layout, with arguments and result types. +#[derive(Debug, Clone, Hash, Serialize, Deserialize, PartialEq, Eq)] +#[cfg_attr( + any(test, feature = "fuzzing"), + derive(arbitrary::Arbitrary, dearbitrary::Dearbitrary) +)] +pub struct MoveFunctionLayout( + pub Vec, + pub Vec, + pub AbilitySet, +); + +/// A closure (function value). The closure stores the name of the +/// function and it's type instantiation, as well as the closure +/// mask and the captured values together with their layout. The latter +/// allows to deserialize closures context free (without needing +/// to lookup information about the function and its dependencies). +#[derive(Debug, PartialEq, Eq, Clone)] +#[cfg_attr( + any(test, feature = "fuzzing"), + derive(arbitrary::Arbitrary, dearbitrary::Dearbitrary) +)] +pub struct MoveClosure { + pub module_id: ModuleId, + pub fun_id: Identifier, + pub fun_inst: Vec, + pub mask: ClosureMask, + pub captured: Vec<(MoveTypeLayout, MoveValue)>, +} + +#[allow(unused)] // Currently, we do not use the expected function layout +pub(crate) struct ClosureVisitor<'a>(pub(crate) &'a MoveFunctionLayout); + +impl<'d, 'a> serde::de::Visitor<'d> for ClosureVisitor<'a> { + type Value = MoveClosure; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("Closure") + } + + fn visit_seq(self, mut seq: A) -> Result + where + A: serde::de::SeqAccess<'d>, + { + let module_id = read_required_value::<_, ModuleId>(&mut seq)?; + let fun_id = read_required_value::<_, Identifier>(&mut seq)?; + let fun_inst = read_required_value::<_, Vec>(&mut seq)?; + let mask = read_required_value::<_, ClosureMask>(&mut seq)?; + let mut captured = vec![]; + for _ in 0..mask.captured_count() { + let layout = read_required_value::<_, MoveTypeLayout>(&mut seq)?; + match seq.next_element_seed(&layout)? { + Some(v) => captured.push((layout, v)), + None => return Err(A::Error::invalid_length(captured.len(), &self)), + } + } + // If the sequence length is known, check whether there are no extra values + if matches!(seq.size_hint(), Some(remaining) if remaining != 0) { + return Err(A::Error::invalid_length(captured.len(), &self)); + } + Ok(MoveClosure { + module_id, + fun_id, + fun_inst, + mask, + captured, + }) + } +} + +fn read_required_value<'a, A, T>(seq: &mut A) -> Result +where + A: serde::de::SeqAccess<'a>, + T: serde::de::Deserialize<'a>, +{ + match seq.next_element::()? { + Some(x) => Ok(x), + None => Err(A::Error::custom("expected more elements")), + } +} + +impl serde::Serialize for MoveClosure { + fn serialize(&self, serializer: S) -> Result { + let MoveClosure { + module_id, + fun_id, + fun_inst, + mask, + captured, + } = self; + let mut s = serializer.serialize_seq(Some(4 + captured.len()))?; + s.serialize_element(module_id)?; + s.serialize_element(fun_id)?; + s.serialize_element(fun_inst)?; + s.serialize_element(mask)?; + for (l, v) in captured { + s.serialize_element(l)?; + s.serialize_element(v)?; + } + s.end() + } +} + +impl fmt::Display for MoveFunctionLayout { + fn fmt(&self, f: &mut fmt::Formatter) -> std::fmt::Result { + let fmt_list = |l: &[MoveTypeLayout]| { + l.iter() + .map(|t| t.to_string()) + .collect::>() + .join(", ") + }; + let MoveFunctionLayout(args, results, abilities) = self; + write!( + f, + "|{}|{}{}", + fmt_list(args), + fmt_list(results), + abilities.display_postfix() + ) + } +} + +impl TryInto for &MoveFunctionLayout { + type Error = anyhow::Error; + + fn try_into(self) -> Result { + let into_list = |ts: &[MoveTypeLayout]| { + ts.iter() + .map(|t| t.try_into()) + .collect::, _>>() + }; + Ok(FunctionTag { + args: into_list(&self.0)?, + results: into_list(&self.1)?, + abilities: self.2, + }) + } +} + +impl fmt::Display for MoveClosure { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let MoveClosure { + module_id, + fun_id, + fun_inst, + mask, + captured, + } = self; + let captured_str = mask + .merge_placeholder_strings( + mask.max_captured() + 1, + captured.iter().map(|v| v.1.to_string()).collect(), + ) + .unwrap_or_else(|| vec!["*invalid*".to_string()]) + .join(","); + let inst_str = if fun_inst.is_empty() { + "".to_string() + } else { + format!( + "<{}>", + fun_inst + .iter() + .map(|t| t.to_string()) + .collect::>() + .join(",") + ) + }; + write!( + f, + // this will print `a::m::f(a1,_,a2,_)` + "{}::{}{}({})", + module_id, fun_id, inst_str, captured_str + ) + } +} diff --git a/third_party/move/move-core/types/src/language_storage.rs b/third_party/move/move-core/types/src/language_storage.rs index 821b2e22c43d0..010417247d18d 100644 --- a/third_party/move/move-core/types/src/language_storage.rs +++ b/third_party/move/move-core/types/src/language_storage.rs @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ + ability::AbilitySet, account_address::AccountAddress, identifier::{IdentStr, Identifier}, parser::{parse_module_id, parse_struct_tag, parse_type_tag}, @@ -67,6 +68,15 @@ pub enum TypeTag { U32, #[serde(rename = "u256", alias = "U256")] U256, + + // NOTE: added in bytecode version v8 + Function( + #[serde( + serialize_with = "safe_serialize::type_tag_recursive_serialize", + deserialize_with = "safe_serialize::type_tag_recursive_deserialize" + )] + Box, + ), } impl TypeTag { @@ -82,6 +92,7 @@ impl TypeTag { /// to change and should not be used inside stable code. pub fn to_canonical_string(&self) -> String { use TypeTag::*; + match self { Bool => "bool".to_owned(), U8 => "u8".to_owned(), @@ -94,6 +105,25 @@ impl TypeTag { Signer => "signer".to_owned(), Vector(t) => format!("vector<{}>", t.to_canonical_string()), Struct(s) => s.to_canonical_string(), + Function(f) => { + let fmt_list = |l: &[TypeTag]| -> String { + l.iter() + .map(|t| t.to_canonical_string()) + .collect::>() + .join(",") + }; + let FunctionTag { + args, + results, + abilities, + } = f.as_ref(); + format!( + "|{}|{}{}", + fmt_list(args), + fmt_list(results), + abilities.display_postfix() + ) + }, } } } @@ -193,6 +223,19 @@ impl FromStr for StructTag { } } +#[derive(Serialize, Deserialize, Debug, PartialEq, Hash, Eq, Clone, PartialOrd, Ord)] +#[cfg_attr( + feature = "fuzzing", + derive(arbitrary::Arbitrary, dearbitrary::Dearbitrary) +)] +#[cfg_attr(any(test, feature = "fuzzing"), derive(Arbitrary))] +#[cfg_attr(any(test, feature = "fuzzing"), proptest(no_params))] +pub struct FunctionTag { + pub args: Vec, + pub results: Vec, + pub abilities: AbilitySet, +} + /// Represents the initial key into global storage where we first index by the address, and then /// the struct tag #[derive(Serialize, Deserialize, Debug, PartialEq, Hash, Eq, Clone, PartialOrd, Ord)] @@ -320,6 +363,7 @@ impl Display for TypeTag { fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { match self { TypeTag::Struct(s) => write!(f, "{}", s), + TypeTag::Function(_) => write!(f, "{}", self.to_canonical_string()), TypeTag::Vector(ty) => write!(f, "vector<{}>", ty), TypeTag::U8 => write!(f, "u8"), TypeTag::U16 => write!(f, "u16"), @@ -334,6 +378,12 @@ impl Display for TypeTag { } } +impl Display for FunctionTag { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + TypeTag::Function(Box::new(self.clone())).fmt(f) + } +} + impl Display for ResourceKey { fn fmt(&self, f: &mut Formatter) -> std::fmt::Result { write!(f, "0x{}/{}", self.address.short_str_lossless(), self.type_) diff --git a/third_party/move/move-core/types/src/safe_serialize.rs b/third_party/move/move-core/types/src/safe_serialize.rs index 42ab4b4a89708..351e8e42ddfd9 100644 --- a/third_party/move/move-core/types/src/safe_serialize.rs +++ b/third_party/move/move-core/types/src/safe_serialize.rs @@ -4,9 +4,6 @@ //! Custom serializers which track recursion nesting with a thread local, //! and otherwise delegate to the derived serializers. -//! -//! This is currently only implemented for type tags, but can be easily -//! generalized, as the only type-tag specific thing is the allowed nesting. use serde::{Deserialize, Deserializer, Serialize, Serializer}; use std::cell::RefCell; diff --git a/third_party/move/move-core/types/src/transaction_argument.rs b/third_party/move/move-core/types/src/transaction_argument.rs index 701fbce112f48..da85bc7ccabe5 100644 --- a/third_party/move/move-core/types/src/transaction_argument.rs +++ b/third_party/move/move-core/types/src/transaction_argument.rs @@ -82,7 +82,7 @@ impl TryFrom for TransactionArgument { }) .collect::>>()?, ), - MoveValue::Signer(_) | MoveValue::Struct(_) => { + MoveValue::Signer(_) | MoveValue::Struct(_) | MoveValue::Closure(_) => { return Err(anyhow!("invalid transaction argument: {:?}", val)) }, MoveValue::U16(i) => TransactionArgument::U16(i), diff --git a/third_party/move/move-core/types/src/value.rs b/third_party/move/move-core/types/src/value.rs index 0bf4cbcdf8f88..767be547b5a8c 100644 --- a/third_party/move/move-core/types/src/value.rs +++ b/third_party/move/move-core/types/src/value.rs @@ -9,6 +9,7 @@ use crate::{ account_address::AccountAddress, + function::{ClosureVisitor, MoveClosure, MoveFunctionLayout}, ident_str, identifier::Identifier, language_storage::{ModuleId, StructTag, TypeTag}, @@ -120,6 +121,8 @@ pub enum MoveValue { U16(u16), U32(u32), U256(u256::U256), + // Added in bytecode version v8 + Closure(Box), } /// A layout associated with a named field @@ -248,6 +251,10 @@ pub enum MoveTypeLayout { // TODO[agg_v2](?): Do we need a layout here if we have custom serde // implementations available? Native(IdentifierMappingKind, Box), + + // Added in bytecode version v8 + #[serde(rename(serialize = "fun", deserialize = "fun"))] + Function(MoveFunctionLayout), } impl MoveValue { @@ -259,6 +266,10 @@ impl MoveValue { bcs::to_bytes(self).ok() } + pub fn closure(c: MoveClosure) -> MoveValue { + Self::Closure(Box::new(c)) + } + pub fn vector_u8(v: Vec) -> Self { MoveValue::Vector(v.into_iter().map(MoveValue::U8).collect()) } @@ -536,6 +547,9 @@ impl<'d> serde::de::DeserializeSeed<'d> for &MoveTypeLayout { AccountAddress::deserialize(deserializer).map(MoveValue::Signer) }, MoveTypeLayout::Struct(ty) => Ok(MoveValue::Struct(ty.deserialize(deserializer)?)), + MoveTypeLayout::Function(fun) => Ok(MoveValue::Closure(Box::new( + deserializer.deserialize_seq(ClosureVisitor(fun))?, + ))), MoveTypeLayout::Vector(layout) => Ok(MoveValue::Vector( deserializer.deserialize_seq(VectorElementVisitor(layout))?, )), @@ -730,6 +744,7 @@ impl serde::Serialize for MoveValue { fn serialize(&self, serializer: S) -> Result { match self { MoveValue::Struct(s) => s.serialize(serializer), + MoveValue::Closure(c) => c.serialize(serializer), MoveValue::Bool(b) => serializer.serialize_bool(*b), MoveValue::U8(i) => serializer.serialize_u8(*i), MoveValue::U16(i) => serializer.serialize_u16(*i), @@ -849,6 +864,7 @@ impl fmt::Display for MoveTypeLayout { Address => write!(f, "address"), Vector(typ) => write!(f, "vector<{}>", typ), Struct(s) => fmt::Display::fmt(s, f), + Function(fun) => fmt::Display::fmt(fun, f), Signer => write!(f, "signer"), // TODO[agg_v2](cleanup): consider printing the tag as well. Native(_, typ) => write!(f, "native<{}>", typ), @@ -916,6 +932,7 @@ impl TryInto for &MoveTypeLayout { MoveTypeLayout::Signer => TypeTag::Signer, MoveTypeLayout::Vector(v) => TypeTag::Vector(Box::new(v.as_ref().try_into()?)), MoveTypeLayout::Struct(v) => TypeTag::Struct(Box::new(v.try_into()?)), + MoveTypeLayout::Function(f) => TypeTag::Function(Box::new(f.try_into()?)), // Native layout variant is only used by MoveVM, and is irrelevant // for type tags which are used to key resources in the global state. @@ -953,6 +970,7 @@ impl fmt::Display for MoveValue { MoveValue::Signer(a) => write!(f, "signer({})", a.to_hex_literal()), MoveValue::Vector(v) => fmt_list(f, "vector[", v, "]"), MoveValue::Struct(s) => fmt::Display::fmt(s, f), + MoveValue::Closure(c) => fmt::Display::fmt(c, f), } } } diff --git a/third_party/move/move-ir/types/src/ast.rs b/third_party/move/move-ir/types/src/ast.rs index 40fa5695baf43..4718cf9084170 100644 --- a/third_party/move/move-ir/types/src/ast.rs +++ b/third_party/move/move-ir/types/src/ast.rs @@ -1807,7 +1807,7 @@ fn format_move_value(v: &MoveValue) -> String { .join(", "); format!("vector[{}]", items) }, - MoveValue::Struct(_) | MoveValue::Signer(_) => { + MoveValue::Struct(_) | MoveValue::Signer(_) | MoveValue::Closure(_) => { panic!("Should be inexpressible as a constant") }, MoveValue::U16(u) => format!("{}u16", u), diff --git a/third_party/move/move-model/src/builder/exp_builder.rs b/third_party/move/move-model/src/builder/exp_builder.rs index b515e254d0899..3cf66a1ca2f1e 100644 --- a/third_party/move/move-model/src/builder/exp_builder.rs +++ b/third_party/move/move-model/src/builder/exp_builder.rs @@ -5484,30 +5484,10 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo Value::Vector(b) }, }, - (Type::Primitive(_), MoveValue::Vector(_)) - | (Type::Primitive(_), MoveValue::Struct(_)) - | (Type::Tuple(_), MoveValue::Vector(_)) - | (Type::Tuple(_), MoveValue::Struct(_)) - | (Type::Vector(_), MoveValue::Struct(_)) - | (Type::Struct(_, _, _), MoveValue::Vector(_)) - | (Type::Struct(_, _, _), MoveValue::Struct(_)) - | (Type::TypeParameter(_), MoveValue::Vector(_)) - | (Type::TypeParameter(_), MoveValue::Struct(_)) - | (Type::Reference(_, _), MoveValue::Vector(_)) - | (Type::Reference(_, _), MoveValue::Struct(_)) - | (Type::Fun(..), MoveValue::Vector(_)) - | (Type::Fun(..), MoveValue::Struct(_)) - | (Type::TypeDomain(_), MoveValue::Vector(_)) - | (Type::TypeDomain(_), MoveValue::Struct(_)) - | (Type::ResourceDomain(_, _, _), MoveValue::Vector(_)) - | (Type::ResourceDomain(_, _, _), MoveValue::Struct(_)) - | (Type::Error, MoveValue::Vector(_)) - | (Type::Error, MoveValue::Struct(_)) - | (Type::Var(_), MoveValue::Vector(_)) - | (Type::Var(_), MoveValue::Struct(_)) => { + _ => { self.error( loc, - &format!("Not yet supported constant value: {:?}", value), + &format!("Not supported constant value/type combination: {}", value), ); Value::Bool(false) }, diff --git a/third_party/move/move-model/src/ty.rs b/third_party/move/move-model/src/ty.rs index 08079025c9cb1..8ef2891701167 100644 --- a/third_party/move/move-model/src/ty.rs +++ b/third_party/move/move-model/src/ty.rs @@ -24,7 +24,7 @@ use move_binary_format::{ }; use move_core_types::{ ability::{Ability, AbilitySet}, - language_storage::{StructTag, TypeTag}, + language_storage::{FunctionTag, StructTag, TypeTag}, u256::U256, }; use num::BigInt; @@ -1333,6 +1333,21 @@ impl Type { Struct(qid.module_id, qid.id, type_args) }, TypeTag::Vector(type_param) => Vector(Box::new(Self::from_type_tag(type_param, env))), + TypeTag::Function(fun) => { + let FunctionTag { + args, + results, + abilities, + } = fun.as_ref(); + let from_vec = |ts: &[TypeTag]| { + Type::tuple(ts.iter().map(|t| Type::from_type_tag(t, env)).collect_vec()) + }; + Fun( + Box::new(from_vec(args)), + Box::new(from_vec(results)), + *abilities, + ) + }, } } diff --git a/third_party/move/move-stdlib/src/natives/debug.rs b/third_party/move/move-stdlib/src/natives/debug.rs index 38876b7685fdb..408b5a084e899 100644 --- a/third_party/move/move-stdlib/src/natives/debug.rs +++ b/third_party/move/move-stdlib/src/natives/debug.rs @@ -460,6 +460,9 @@ mod testing { )?; } }, + MoveValue::Closure(clos) => { + write!(out, "{}", clos).map_err(fmt_error_to_partial_vm_error)?; + }, MoveValue::Struct(move_struct) => match move_struct { MoveStruct::WithTypes { _type_: type_, diff --git a/third_party/move/move-vm/runtime/Cargo.toml b/third_party/move/move-vm/runtime/Cargo.toml index 4b20b934b638c..6c17403a3acff 100644 --- a/third_party/move/move-vm/runtime/Cargo.toml +++ b/third_party/move/move-vm/runtime/Cargo.toml @@ -12,6 +12,7 @@ edition = "2021" [dependencies] ambassador = { workspace = true } +anyhow = { workspace = true } better_any = { workspace = true } bytes = { workspace = true } claims = { workspace = true } @@ -37,7 +38,7 @@ hex = { workspace = true } move-binary-format = { workspace = true, features = ["fuzzing"] } move-compiler = { workspace = true } move-ir-compiler = { workspace = true } -move-vm-test-utils ={ workspace = true } +move-vm-test-utils = { workspace = true } proptest = { workspace = true } [features] diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index 33802b688be33..a7f59120b09ba 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -263,11 +263,9 @@ impl InterpreterImpl { // 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(), - ); + let err = PartialVMError::new_invariant_violation( + "Failed to get native function module id".to_string(), + ); set_err_info!(current_frame, err) })?; gas_meter @@ -320,8 +318,9 @@ impl InterpreterImpl { let module_id = function .module_id() .ok_or_else(|| { - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message("Failed to get native function module id".to_string()) + PartialVMError::new_invariant_violation( + "Failed to get native function module id", + ) }) .map_err(|e| set_err_info!(current_frame, e))?; gas_meter @@ -589,13 +588,9 @@ impl InterpreterImpl { // Paranoid check to protect us against incorrect native function implementations. A native function that // returns a different number of values than its declared types will trigger this check. if return_values.len() != function.return_tys().len() { - return Err( - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message( - "Arity mismatch: return value count does not match return type count" - .to_string(), - ), - ); + return Err(PartialVMError::new_invariant_violation( + "Arity mismatch: return value count does not match return type count", + )); } // Put return values on the top of the operand stack, where the caller will find them. // This is one of only two times the operand stack is shared across call stack frames; the other is in handling @@ -619,13 +614,14 @@ impl InterpreterImpl { Err(PartialVMError::new(StatusCode::ABORTED).with_sub_status(abort_code)) }, NativeResult::OutOfGas { partial_cost } => { - let err = match gas_meter.charge_native_function( - partial_cost, - Option::>::None, - ) { + let err = match gas_meter + .charge_native_function(partial_cost, Option::>::None) + { Err(err) if err.major_status() == StatusCode::OUT_OF_GAS => err, - Ok(_) | Err(_) => PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR).with_message( - "The partial cost returned by the native function did not cause the gas meter to trigger an OutOfGas error, at least one of them is violating the contract".to_string() + Ok(_) | Err(_) => PartialVMError::new_invariant_violation( + "The partial cost returned by the native function did \ + not cause the gas meter to trigger an OutOfGas error, at least \ + one of them is violating the contract", ), }; @@ -1420,6 +1416,18 @@ fn check_depth_of_type_impl( )?; check_depth!(formula.solve(&ty_arg_depths)) }, + Type::Function { args, results, .. } => { + let mut ty_max = depth; + for ty in args.iter().chain(results.iter()).map(|rc| rc.as_ref()) { + ty_max = ty_max.max(check_depth_of_type_impl( + resolver, + ty, + max_depth, + check_depth!(1), + )?); + } + ty_max + }, Type::TyParam(_) => { return Err( PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) @@ -1537,7 +1545,7 @@ impl Frame { )?; match instruction { - // TODO: implement closures + // TODO(#15664): implement closures Bytecode::PackClosure(..) | Bytecode::PackClosureGeneric(..) | Bytecode::CallClosure(..) => { 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 d4dc9a7b18b7d..0294da1a41cc9 100644 --- a/third_party/move/move-vm/runtime/src/loader/function.rs +++ b/third_party/move/move-vm/runtime/src/loader/function.rs @@ -8,8 +8,9 @@ use crate::{ Resolver, Script, }, native_functions::{NativeFunction, NativeFunctions, UnboxedNativeFunction}, - ModuleStorage, + ModuleStorage, ModuleStorageTypeConverter, TypeConverter, }; +use better_any::{Tid, TidAble, TidExt}; use move_binary_format::{ access::ModuleAccess, binary_views::BinaryIndexedView, @@ -17,13 +18,21 @@ use move_binary_format::{ file_format::{Bytecode, CompiledModule, FunctionDefinitionIndex, Visibility}, }; use move_core_types::{ - ability::AbilitySet, identifier::Identifier, language_storage::ModuleId, vm_status::StatusCode, + ability::{Ability, AbilitySet}, + function::ClosureMask, + identifier::{IdentStr, Identifier}, + language_storage::{ModuleId, TypeTag}, + value::MoveTypeLayout, + vm_status::StatusCode, }; -use move_vm_types::loaded_data::{ - runtime_access_specifier::AccessSpecifier, - runtime_types::{StructIdentifier, Type}, +use move_vm_types::{ + loaded_data::{ + runtime_access_specifier::AccessSpecifier, + runtime_types::{StructIdentifier, Type}, + }, + values::{AbstractFunction, SerializedFunctionData}, }; -use std::{fmt::Debug, sync::Arc}; +use std::{cell::RefCell, cmp::Ordering, fmt::Debug, rc::Rc, sync::Arc}; /// A runtime function definition representation. pub struct Function { @@ -45,12 +54,14 @@ pub struct Function { } /// For loaded function representation, specifies the owner: a script or a module. +#[derive(Clone)] pub(crate) enum LoadedFunctionOwner { Script(Arc