From e9a5231bf2ee5c150002c1a4dc3d057eae0ae7f8 Mon Sep 17 00:00:00 2001 From: Wolfgang Grieskamp Date: Sun, 3 Nov 2024 07:30:41 -0800 Subject: [PATCH 1/9] Verbatim cherry-pick of Wolfgang's draft PR 15171 = commit #d85b919 [move-vm] Types and opcodes for closures This PR implements the extensions needed in the file format for representing closures: - The type (SignatureToken) has a new variant `Function(args, result, abilities)` to represent a function type - The opcodes are extendeed by operations `ClosPack`, `ClosPackGeneric`, and `ClosEval` This supports bit masks for specifyinng which arguments of a function are captured when creating a closure. Bytecode verification is extended to support the new types and opcodes. The implementation of consistency, type, and reference safety should be complete. What is missing are: - File format serialization - Interpreter and value representation - Value serialization - Connection of the new types with the various other type representations --- api/types/src/bytecode.rs | 4 + aptos-move/script-composer/src/decompiler.rs | 3 + .../move-binary-format/src/binary_views.rs | 1 + .../move/move-binary-format/src/builders.rs | 16 +- .../move-binary-format/src/check_bounds.rs | 9 +- .../src/check_complexity.rs | 8 +- .../move/move-binary-format/src/constant.rs | 4 + .../move-binary-format/src/file_format.rs | 216 +++++++++++++++++- .../src/file_format_common.rs | 4 + .../move/move-binary-format/src/normalized.rs | 2 + .../src/proptest_types/functions.rs | 2 +- .../src/proptest_types/types.rs | 1 + .../move/move-binary-format/src/serializer.rs | 6 + .../move/move-bytecode-spec/src/lib.rs | 1 + .../invalid-mutations/src/bounds.rs | 2 +- .../invalid-mutations/src/bounds/code_unit.rs | 8 +- .../src/acquires_list_verifier.rs | 5 +- .../src/dependencies.rs | 13 ++ .../src/instantiation_loops.rs | 8 + .../src/instruction_consistency.rs | 39 +++- .../src/locals_safety/mod.rs | 3 + .../src/reference_safety/abstract_state.rs | 23 +- .../src/reference_safety/mod.rs | 69 +++++- .../move-bytecode-verifier/src/signature.rs | 17 ++ .../src/signature_v2.rs | 28 ++- .../src/stack_usage_verifier.rs | 39 +++- .../move-bytecode-verifier/src/struct_defs.rs | 5 + .../move-bytecode-verifier/src/type_safety.rs | 147 +++++++++++- .../move-compiler/src/interface_generator.rs | 16 +- .../move/move-core/types/src/vm_status.rs | 23 +- .../move-ir-to-bytecode/src/context.rs | 3 + .../src/stackless_bytecode_generator.rs | 5 + third_party/move/move-model/src/ty.rs | 4 + .../move/move-vm/runtime/src/interpreter.rs | 8 + .../move-vm/runtime/src/loader/type_loader.rs | 10 +- .../runtime/src/runtime_type_checks.rs | 12 + .../move-vm/types/src/values/values_impl.rs | 2 +- .../tools/move-bytecode-utils/src/layout.rs | 1 + .../move-disassembler/src/disassembler.rs | 6 + .../tools/move-resource-viewer/src/lib.rs | 3 + 40 files changed, 710 insertions(+), 66 deletions(-) diff --git a/api/types/src/bytecode.rs b/api/types/src/bytecode.rs index 0f9a40e243805..f6ed8405471a4 100644 --- a/api/types/src/bytecode.rs +++ b/api/types/src/bytecode.rs @@ -105,6 +105,10 @@ pub trait Bytecode { mutable: true, to: Box::new(self.new_move_type(t.borrow())), }, + SignatureToken::Function(..) => { + // TODO + unimplemented!("signature token function to API MoveType") + }, } } diff --git a/aptos-move/script-composer/src/decompiler.rs b/aptos-move/script-composer/src/decompiler.rs index 8d9ee51ea2203..56806f589ab08 100644 --- a/aptos-move/script-composer/src/decompiler.rs +++ b/aptos-move/script-composer/src/decompiler.rs @@ -208,6 +208,9 @@ impl LocalState { SignatureToken::Vector(s) => { TypeTag::Vector(Box::new(Self::type_tag_from_sig_token(script, s)?)) }, + SignatureToken::Function(..) => { + bail!("function types NYI for script composer") + }, SignatureToken::Struct(s) => { let module_handle = script.module_handle_at(script.struct_handle_at(*s).module); TypeTag::Struct(Box::new(StructTag { diff --git a/third_party/move/move-binary-format/src/binary_views.rs b/third_party/move/move-binary-format/src/binary_views.rs index 840754c3a565f..74c9325c7e2e9 100644 --- a/third_party/move/move-binary-format/src/binary_views.rs +++ b/third_party/move/move-binary-format/src/binary_views.rs @@ -343,6 +343,7 @@ impl<'a> BinaryIndexedView<'a> { Vector(ty) => AbilitySet::polymorphic_abilities(AbilitySet::VECTOR, vec![false], vec![ self.abilities(ty, constraints)?, ]), + Function(_, _, abilities) => Ok(*abilities), Struct(idx) => { let sh = self.struct_handle_at(*idx); Ok(sh.abilities) diff --git a/third_party/move/move-binary-format/src/builders.rs b/third_party/move/move-binary-format/src/builders.rs index 17fae53208aed..b78ba64c23707 100644 --- a/third_party/move/move-binary-format/src/builders.rs +++ b/third_party/move/move-binary-format/src/builders.rs @@ -213,6 +213,12 @@ impl CompiledScriptBuilder { sig: &SignatureToken, ) -> PartialVMResult { use SignatureToken::*; + let import_vec = + |s: &mut Self, v: &[SignatureToken]| -> PartialVMResult> { + v.iter() + .map(|sig| s.import_signature_token(module, sig)) + .collect::>>() + }; Ok(match sig { U8 => U8, U16 => U16, @@ -229,13 +235,15 @@ impl CompiledScriptBuilder { MutableReference(Box::new(self.import_signature_token(module, ty)?)) }, Vector(ty) => Vector(Box::new(self.import_signature_token(module, ty)?)), + Function(args, result, abilities) => Function( + import_vec(self, args)?, + import_vec(self, result)?, + *abilities, + ), Struct(idx) => Struct(self.import_struct(module, *idx)?), StructInstantiation(idx, inst_tys) => StructInstantiation( self.import_struct(module, *idx)?, - inst_tys - .iter() - .map(|sig| self.import_signature_token(module, sig)) - .collect::>>()?, + import_vec(self, inst_tys)?, ), }) } diff --git a/third_party/move/move-binary-format/src/check_bounds.rs b/third_party/move/move-binary-format/src/check_bounds.rs index 91e52ee07fb5e..dca378e0957c3 100644 --- a/third_party/move/move-binary-format/src/check_bounds.rs +++ b/third_party/move/move-binary-format/src/check_bounds.rs @@ -546,12 +546,12 @@ impl<'a> BoundsChecker<'a> { )?; } }, - Call(idx) => self.check_code_unit_bounds_impl( + Call(idx) | ClosPack(idx, _) => self.check_code_unit_bounds_impl( self.view.function_handles(), *idx, bytecode_offset, )?, - CallGeneric(idx) => { + CallGeneric(idx) | ClosPackGeneric(idx, _) => { self.check_code_unit_bounds_impl( self.view.function_instantiations(), *idx, @@ -650,7 +650,8 @@ impl<'a> BoundsChecker<'a> { }, // Instructions that refer to a signature - VecPack(idx, _) + ClosEval(idx) + | VecPack(idx, _) | VecLen(idx) | VecImmBorrow(idx) | VecMutBorrow(idx) @@ -684,7 +685,7 @@ impl<'a> BoundsChecker<'a> { for ty in ty.preorder_traversal() { match ty { Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | TypeParameter(_) - | Reference(_) | MutableReference(_) | Vector(_) => (), + | Reference(_) | MutableReference(_) | Vector(_) | Function(..) => (), Struct(idx) => { check_bounds_impl(self.view.struct_handles(), *idx)?; if let Some(sh) = self.view.struct_handles().get(idx.into_index()) { diff --git a/third_party/move/move-binary-format/src/check_complexity.rs b/third_party/move/move-binary-format/src/check_complexity.rs index 232d530404cc9..dbc8890d4cf3e 100644 --- a/third_party/move/move-binary-format/src/check_complexity.rs +++ b/third_party/move/move-binary-format/src/check_complexity.rs @@ -68,7 +68,7 @@ impl<'a> BinaryComplexityMeter<'a> { cost = cost.saturating_add(moduel_name.len() as u64 * COST_PER_IDENT_BYTE); }, U8 | U16 | U32 | U64 | U128 | U256 | Signer | Address | Bool | Vector(_) - | TypeParameter(_) | Reference(_) | MutableReference(_) => (), + | Function(..) | TypeParameter(_) | Reference(_) | MutableReference(_) => (), } } @@ -262,7 +262,7 @@ impl<'a> BinaryComplexityMeter<'a> { for instr in &code.code { match instr { - CallGeneric(idx) => { + CallGeneric(idx) | ClosPackGeneric(idx, ..) => { self.meter_function_instantiation(*idx)?; }, PackGeneric(idx) | UnpackGeneric(idx) => { @@ -284,7 +284,8 @@ impl<'a> BinaryComplexityMeter<'a> { ImmBorrowVariantFieldGeneric(idx) | MutBorrowVariantFieldGeneric(idx) => { self.meter_variant_field_instantiation(*idx)?; }, - VecPack(idx, _) + ClosEval(idx) + | VecPack(idx, _) | VecLen(idx) | VecImmBorrow(idx) | VecMutBorrow(idx) @@ -323,6 +324,7 @@ impl<'a> BinaryComplexityMeter<'a> { | PackVariant(_) | UnpackVariant(_) | TestVariant(_) + | ClosPack(..) | ReadRef | WriteRef | FreezeRef diff --git a/third_party/move/move-binary-format/src/constant.rs b/third_party/move/move-binary-format/src/constant.rs index 991bee29624f7..6fa8b21fa11fb 100644 --- a/third_party/move/move-binary-format/src/constant.rs +++ b/third_party/move/move-binary-format/src/constant.rs @@ -17,6 +17,10 @@ fn sig_to_ty(sig: &SignatureToken) -> Option { SignatureToken::U128 => Some(MoveTypeLayout::U128), SignatureToken::U256 => Some(MoveTypeLayout::U256), SignatureToken::Vector(v) => Some(MoveTypeLayout::Vector(Box::new(sig_to_ty(v.as_ref())?))), + SignatureToken::Function(..) => { + // TODO: do we need representation in MoveTypeLayout? + None + }, SignatureToken::Reference(_) | SignatureToken::MutableReference(_) | SignatureToken::Struct(_) diff --git a/third_party/move/move-binary-format/src/file_format.rs b/third_party/move/move-binary-format/src/file_format.rs index 468bdcca9fd32..a2058b333dc28 100644 --- a/third_party/move/move-binary-format/src/file_format.rs +++ b/third_party/move/move-binary-format/src/file_format.rs @@ -49,6 +49,7 @@ use proptest::{collection::vec, prelude::*, strategy::BoxedStrategy}; use ref_cast::RefCast; use serde::{Deserialize, Serialize}; use std::{ + collections::BTreeMap, fmt::{self, Formatter}, ops::BitOr, }; @@ -1254,6 +1255,8 @@ pub enum SignatureToken { Signer, /// Vector Vector(Box), + /// Function, with n argument types and m result types, and an associated ability set. + Function(Vec, Vec, AbilitySet), /// User defined type Struct(StructHandleIndex), StructInstantiation(StructHandleIndex, Vec), @@ -1296,6 +1299,11 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIter<'a> { self.stack.extend(inner_toks.iter().rev()) }, + Function(args, result, _) => { + self.stack.extend(args.iter().rev()); + self.stack.extend(result.iter().rev()); + }, + Signer | Bool | Address | U8 | U16 | U32 | U64 | U128 | U256 | Struct(_) | TypeParameter(_) => (), } @@ -1329,6 +1337,13 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIterWithDepth<'a> { .stack .extend(inner_toks.iter().map(|tok| (tok, depth + 1)).rev()), + Function(args, result, _) => { + self.stack + .extend(args.iter().map(|tok| (tok, depth + 1)).rev()); + self.stack + .extend(result.iter().map(|tok| (tok, depth + 1)).rev()); + }, + Signer | Bool | Address | U8 | U16 | U32 | U64 | U128 | U256 | Struct(_) | TypeParameter(_) => (), } @@ -1389,11 +1404,14 @@ impl std::fmt::Debug for SignatureToken { SignatureToken::Address => write!(f, "Address"), SignatureToken::Signer => write!(f, "Signer"), SignatureToken::Vector(boxed) => write!(f, "Vector({:?})", boxed), + SignatureToken::Function(args, result, abilities) => { + write!(f, "Function({:?}, {:?}, {})", args, result, abilities) + }, + SignatureToken::Reference(boxed) => write!(f, "Reference({:?})", boxed), SignatureToken::Struct(idx) => write!(f, "Struct({:?})", idx), SignatureToken::StructInstantiation(idx, types) => { write!(f, "StructInstantiation({:?}, {:?})", idx, types) }, - SignatureToken::Reference(boxed) => write!(f, "Reference({:?})", boxed), SignatureToken::MutableReference(boxed) => write!(f, "MutableReference({:?})", boxed), SignatureToken::TypeParameter(idx) => write!(f, "TypeParameter({:?})", idx), } @@ -1401,7 +1419,7 @@ impl std::fmt::Debug for SignatureToken { } impl SignatureToken { - /// Returns true if the token is an integer type. + // Returns `true` if the `SignatureToken` is an integer type. pub fn is_integer(&self) -> bool { use SignatureToken::*; match self { @@ -1410,6 +1428,7 @@ impl SignatureToken { | Address | Signer | Vector(_) + | Function(..) | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1448,6 +1467,7 @@ impl SignatureToken { Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address => true, Vector(inner) => inner.is_valid_for_constant(), Signer + | Function(..) | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1490,6 +1510,9 @@ impl SignatureToken { pub fn instantiate(&self, subst_mapping: &[SignatureToken]) -> SignatureToken { use SignatureToken::*; + let inst_vec = |v: &[SignatureToken]| -> Vec { + v.iter().map(|ty| ty.instantiate(subst_mapping)).collect() + }; match self { Bool => Bool, U8 => U8, @@ -1501,14 +1524,13 @@ impl SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(ty.instantiate(subst_mapping))), + Function(args, result, abilities) => { + Function(inst_vec(args), inst_vec(result), *abilities) + }, Struct(idx) => Struct(*idx), - StructInstantiation(idx, struct_type_args) => StructInstantiation( - *idx, - struct_type_args - .iter() - .map(|ty| ty.instantiate(subst_mapping)) - .collect(), - ), + StructInstantiation(idx, struct_type_args) => { + StructInstantiation(*idx, inst_vec(struct_type_args)) + }, Reference(ty) => Reference(Box::new(ty.instantiate(subst_mapping))), MutableReference(ty) => MutableReference(Box::new(ty.instantiate(subst_mapping))), TypeParameter(idx) => subst_mapping[*idx as usize].clone(), @@ -1516,6 +1538,101 @@ impl SignatureToken { } } +/// A `ClosureMask` is a value which determines how to distinguish those function arguments +/// which are captured and which are not when a closure is constructed. For instance, +/// with `_` representing an omitted argument, the mask for `f(a,_,b,_)` would have the argument +/// at index 0 and at index 2 captured. The mask can be used to transform lists of types. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +#[cfg_attr(any(test, feature = "fuzzing"), derive(proptest_derive::Arbitrary))] +#[cfg_attr(any(test, feature = "fuzzing"), proptest(no_params))] +#[cfg_attr(feature = "fuzzing", derive(arbitrary::Arbitrary))] +pub struct ClosureMask { + pub mask: u64, +} + +impl fmt::Display for ClosureMask { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:b}", self.mask) + } +} + +impl ClosureMask { + pub fn new(mask: u64) -> Self { + Self { mask } + } + + /// 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). + pub fn extract(&self, tys: &[T], collect_captured: bool) -> Vec { + tys.iter() + .enumerate() + .filter_map(|(pos, x)| { + let set = (1 << pos) & self.mask != 0; + if set && collect_captured || !set && !collect_captured { + Some(x.clone()) + } else { + None + } + }) + .collect() + } + + /// Compose two lists of elements into one based on the given mask such that the + /// following holds: + /// ```ignore + /// mask.compose(mask.extract(v, true), mask.extract(v, false)) == v + /// ``` + /// This returns `None` if the provided lists are inconsistent w.r.t the mask + /// and cannot be composed. This should not happen in verified code, but + /// a caller should decide whether to crash or to error. + pub fn compose(&self, captured: &[T], provided: &[T]) -> Option> { + let mut result = BTreeMap::new(); // expect ordered enumeration + let mut cap_idx = 0; + let mut pro_idx = 0; + for i in 0..64 { + if cap_idx >= captured.len() && pro_idx >= provided.len() { + // all covered + break; + } + if (1 << i) & self.mask != 0 { + if cap_idx >= captured.len() { + // Inconsistency + return None; + } + result.insert(i, captured[cap_idx].clone()); + cap_idx += 1 + } else { + if pro_idx >= provided.len() { + // Inconsistency + return None; + } + result.insert(i, provided[pro_idx].clone()); + pro_idx += 1 + } + } + let map_len = result.len(); + let vec = result.into_values().collect::>(); + if vec.len() != map_len { + // Inconsistency: all indices must be contiguously covered + None + } else { + Some(vec) + } + } + + /// Return the max index of captured arguments + pub fn max_captured(&self) -> usize { + let mut i = 0; + let mut mask = self.mask; + while mask != 0 { + mask >>= 1; + i += 1 + } + i + } +} + /// A `Constant` is a serialized value along with its type. That type will be deserialized by the /// loader/evaluator #[derive(Clone, Debug, Eq, PartialEq, Hash)] @@ -1896,7 +2013,6 @@ pub enum Bytecode { #[gas_type_creation_tier_1 = "field_tys"] PackVariantGeneric(StructVariantInstantiationIndex), - //TODO: Unpack, Test #[group = "struct"] #[static_operands = "[struct_def_idx]"] #[description = "Destroy an instance of a struct and push the values bound to each field onto the stack."] @@ -2934,6 +3050,83 @@ pub enum Bytecode { "#] VecSwap(SignatureIndex), + #[group = "closure"] + #[description = r#" + `ClosPack(fun, mask)` creates a closure for a given function handle as controlled by + the given `mask`. `mask` is a u64 bitset which describes which of the arguments + of `fun` are captured by the closure. + + If the function `fun` has type `|t1..tn|r`, then the following holds: + + - If `m` are the number of bits set in the mask, then `m <= n`, and the stack is + `[vm..v1] + stack`, and if `i` is the `j`th bit set in the mask, + then `vj` has type `ti`. + - type ti is not a reference. + + Thus the values on the stack must match the types in the function + signature which have the bit to be captured set in the mask. + + The type of the resulting value on the stack is derived from the types `|t1..tn|` + for which the bit is not set, which build the arguments of a function type + with `fun`'s result types. + + The `abilities` of this function type are derived from the inputs as follows. + First, take the intersection of the abilities of all captured arguments + with type `t1..tn`. Then intersect this with the abilities derived from the + function: a function handle has `drop` and `copy`, never has `key`, and only + `store` if the underlying function is public, and therefore cannot change + its signature. + + Notice that an implementation can derive the types of the captured arguments + at runtime from a closure value as long as the closure value stores the function + handle (or a derived form of it) and the mask, and the handle allows to lookup the + function's type at runtime. Then the same procedure as outlined above can be used. + "#] + #[static_operands = "[fun, mask]"] + #[semantics = ""] + #[runtime_check_epilogue = ""] + #[gas_type_creation_tier_0 = "closure_ty"] + ClosPack(FunctionHandleIndex, ClosureMask), + + #[group = "closure"] + #[static_operands = "[fun, mask]"] + #[semantics = ""] + #[runtime_check_epilogue = ""] + #[description = r#" + Same as `ClosPack` but for the instantiation of a generic function. + + Notice that an uninstantiated generic function cannot be used to create a closure. + "#] + #[gas_type_creation_tier_0 = "closure_ty"] + ClosPackGeneric(FunctionInstantiationIndex, ClosureMask), + + #[group = "closure"] + #[description = r#" + `ClosEval(|t1..tn|r has a)` evalutes a closure of the given function type, taking + the captured arguments and mixing in the provided ones on the stack. + + On top of the stack is the closure being evaluated, underneath the arguments: + `[c,vn,..,v1] + stack`. The type of the closure must match the type specified in + the instruction, with abilities `a` a subset of the abilities of the closure value. + A value `vi` on the stack must have type `ti`. + + Notice that the type as part of the closure instruction is redundant for + execution semantics. Since the closure is expected to be on top of the stack, + it can decode the arguments underneath without type information. + However, the type is required to do static bytecode verification. + + The semantics of this instruction can be characterized by the following equation: + + ``` + CloseEval(ClosPack(f, mask, c1..cn), a1..am) = f(mask.compose(c1..cn, a1..am)) + ``` + "#] + #[static_operands = "[]"] + #[semantics = ""] + #[runtime_check_epilogue = ""] + #[gas_type_creation_tier_0 = "closure_ty"] + ClosEval(SignatureIndex), + #[group = "stack_and_local"] #[description = "Push a u16 constant onto the stack."] #[static_operands = "[u16_value]"] @@ -3044,6 +3237,9 @@ impl ::std::fmt::Debug for Bytecode { Bytecode::UnpackGeneric(a) => write!(f, "UnpackGeneric({})", a), Bytecode::UnpackVariant(a) => write!(f, "UnpackVariant({})", a), Bytecode::UnpackVariantGeneric(a) => write!(f, "UnpackVariantGeneric({})", a), + Bytecode::ClosPackGeneric(a, mask) => write!(f, "ClosPackGeneric({}, {})", a, mask), + Bytecode::ClosPack(a, mask) => write!(f, "ClosPack({}, {})", a, mask), + Bytecode::ClosEval(a) => write!(f, "ClosEval({})", a), Bytecode::ReadRef => write!(f, "ReadRef"), Bytecode::WriteRef => write!(f, "WriteRef"), Bytecode::FreezeRef => write!(f, "FreezeRef"), diff --git a/third_party/move/move-binary-format/src/file_format_common.rs b/third_party/move/move-binary-format/src/file_format_common.rs index b9790db3d35f8..0201a408262a8 100644 --- a/third_party/move/move-binary-format/src/file_format_common.rs +++ b/third_party/move/move-binary-format/src/file_format_common.rs @@ -789,6 +789,10 @@ pub fn instruction_key(instruction: &Bytecode) -> u8 { UnpackVariantGeneric(_) => Opcodes::UNPACK_VARIANT_GENERIC, TestVariant(_) => Opcodes::TEST_VARIANT, TestVariantGeneric(_) => Opcodes::TEST_VARIANT_GENERIC, + // Since bytecode version 8 + ClosPack(..) | ClosPackGeneric(..) | ClosEval(_) => { + unimplemented!("serialization of closure opcodes") + }, }; opcode as u8 } diff --git a/third_party/move/move-binary-format/src/normalized.rs b/third_party/move/move-binary-format/src/normalized.rs index ef61a63963692..4fe5b3508d0fa 100644 --- a/third_party/move/move-binary-format/src/normalized.rs +++ b/third_party/move/move-binary-format/src/normalized.rs @@ -192,6 +192,8 @@ impl Type { TypeParameter(i) => Type::TypeParameter(*i), Reference(t) => Type::Reference(Box::new(Type::new(m, t))), MutableReference(t) => Type::MutableReference(Box::new(Type::new(m, t))), + + Function(..) => panic!("normalized representation does not support function types"), } } diff --git a/third_party/move/move-binary-format/src/proptest_types/functions.rs b/third_party/move/move-binary-format/src/proptest_types/functions.rs index 806ca57aba9e9..f614ffcfbb8e6 100644 --- a/third_party/move/move-binary-format/src/proptest_types/functions.rs +++ b/third_party/move/move-binary-format/src/proptest_types/functions.rs @@ -1062,7 +1062,7 @@ impl BytecodeGen { use SignatureToken::*; match token { U8 | U16 | U32 | U64 | U128 | U256 | Bool | Address | Signer | Struct(_) - | TypeParameter(_) => true, + | Function(..) | TypeParameter(_) => true, Vector(element_token) => BytecodeGen::check_signature_token(element_token), StructInstantiation(_, type_arguments) => type_arguments .iter() diff --git a/third_party/move/move-binary-format/src/proptest_types/types.rs b/third_party/move/move-binary-format/src/proptest_types/types.rs index 566d45809a735..161586d503804 100644 --- a/third_party/move/move-binary-format/src/proptest_types/types.rs +++ b/third_party/move/move-binary-format/src/proptest_types/types.rs @@ -71,6 +71,7 @@ impl StDefnMaterializeState { let inner = self.potential_abilities(ty); inner.intersect(AbilitySet::VECTOR) }, + Function(_, _, a) => *a, Struct(idx) => { let sh = &self.struct_handles[idx.0 as usize]; sh.abilities diff --git a/third_party/move/move-binary-format/src/serializer.rs b/third_party/move/move-binary-format/src/serializer.rs index 78f98741ad6b5..d3bcbee4885d0 100644 --- a/third_party/move/move-binary-format/src/serializer.rs +++ b/third_party/move/move-binary-format/src/serializer.rs @@ -800,6 +800,9 @@ fn serialize_signature_token_single_node_impl( binary.push(SerializedType::TYPE_PARAMETER as u8)?; serialize_type_parameter_index(binary, *idx)?; }, + SignatureToken::Function(..) => { + unimplemented!("serialization of function types") + }, } Ok(()) } @@ -1092,6 +1095,9 @@ fn serialize_instruction_inner( binary.push(Opcodes::TEST_VARIANT_GENERIC as u8)?; serialize_struct_variant_inst_index(binary, class_idx) }, + Bytecode::ClosPack(..) | Bytecode::ClosPackGeneric(..) | Bytecode::ClosEval(_) => { + unimplemented!("serialization of closure opcodes") + }, Bytecode::ReadRef => binary.push(Opcodes::READ_REF as u8), Bytecode::WriteRef => binary.push(Opcodes::WRITE_REF as u8), Bytecode::Add => binary.push(Opcodes::ADD as u8), diff --git a/third_party/move/move-bytecode-spec/src/lib.rs b/third_party/move/move-bytecode-spec/src/lib.rs index c0e573d29c304..3d8ab4b7164f4 100644 --- a/third_party/move/move-bytecode-spec/src/lib.rs +++ b/third_party/move/move-bytecode-spec/src/lib.rs @@ -129,6 +129,7 @@ static VALID_GROUPS: Lazy> = Lazy::new(|| { "reference", "arithmetic", "casting", + "closure", "bitwise", "comparison", "boolean", diff --git a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs index bda7c8361fd48..cb47be2e8e220 100644 --- a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs +++ b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs @@ -359,6 +359,6 @@ fn struct_handle(token: &SignatureToken) -> Option { StructInstantiation(sh_idx, _) => Some(*sh_idx), Reference(token) | MutableReference(token) => struct_handle(token), Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | Vector(_) - | TypeParameter(_) => None, + | TypeParameter(_) | Function(..) => None, } } diff --git a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs index 0146087e0cbe3..c98e8317ee33f 100644 --- a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs +++ b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs @@ -498,6 +498,9 @@ impl<'a> ApplyCodeUnitBoundsContext<'a> { // TODO(#13806): implement panic!("Enum types bytecode NYI: {:?}", code[bytecode_idx]) }, + ClosPack(..) | ClosPackGeneric(..) | ClosEval(..) => { + panic!("Closure bytecode NYI: {:?}", code[bytecode_idx]) + }, }; code[bytecode_idx] = new_bytecode; @@ -557,7 +560,10 @@ fn is_interesting(bytecode: &Bytecode) -> bool { | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor | Shl | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | Abort | Nop => false, - PackVariant(_) + ClosPack(..) + | ClosPackGeneric(..) + | ClosEval(..) + | PackVariant(_) | PackVariantGeneric(_) | UnpackVariant(_) | UnpackVariantGeneric(_) diff --git a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs index 6b15a043ea97b..0de40723ae24e 100644 --- a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs @@ -102,7 +102,10 @@ impl<'a> AcquiresVerifier<'a> { self.struct_acquire(si.def, offset) }, - Bytecode::Pop + Bytecode::ClosPack(..) + | Bytecode::ClosPackGeneric(..) + | Bytecode::ClosEval(_) + | Bytecode::Pop | Bytecode::BrTrue(_) | Bytecode::BrFalse(_) | Bytecode::Abort diff --git a/third_party/move/move-bytecode-verifier/src/dependencies.rs b/third_party/move/move-bytecode-verifier/src/dependencies.rs index 9ec148e4b43c9..8593100e39746 100644 --- a/third_party/move/move-bytecode-verifier/src/dependencies.rs +++ b/third_party/move/move-bytecode-verifier/src/dependencies.rs @@ -455,6 +455,18 @@ fn compare_types( (SignatureToken::Vector(ty1), SignatureToken::Vector(ty2)) => { compare_types(context, ty1, ty2, def_module) }, + ( + SignatureToken::Function(args1, result1, ab1), + SignatureToken::Function(args2, result2, ab2), + ) => { + compare_cross_module_signatures(context, args1, args2, def_module)?; + compare_cross_module_signatures(context, result1, result2, def_module)?; + if ab1 != ab2 { + Err(PartialVMError::new(StatusCode::TYPE_MISMATCH)) + } else { + Ok(()) + } + }, (SignatureToken::Struct(idx1), SignatureToken::Struct(idx2)) => { compare_structs(context, *idx1, *idx2, def_module) }, @@ -483,6 +495,7 @@ fn compare_types( | (SignatureToken::Address, _) | (SignatureToken::Signer, _) | (SignatureToken::Vector(_), _) + | (SignatureToken::Function(..), _) | (SignatureToken::Struct(_), _) | (SignatureToken::StructInstantiation(_, _), _) | (SignatureToken::Reference(_), _) diff --git a/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs b/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs index 78f7903ff36ca..53896c212ee14 100644 --- a/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs +++ b/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs @@ -148,6 +148,14 @@ impl<'a> InstantiationLoopChecker<'a> { type_params.insert(*idx); }, Vector(ty) => rec(type_params, ty), + Function(args, result, _) => { + for ty in args { + rec(type_params, ty); + } + for ty in result { + rec(type_params, ty); + } + }, Reference(ty) | MutableReference(ty) => rec(type_params, ty), StructInstantiation(_, tys) => { for ty in tys { diff --git a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs index 84abbb3306032..9e170e7cefa63 100644 --- a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs +++ b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs @@ -11,8 +11,8 @@ use move_binary_format::{ binary_views::BinaryIndexedView, errors::{Location, PartialVMError, PartialVMResult, VMResult}, file_format::{ - Bytecode, CodeOffset, CodeUnit, CompiledModule, CompiledScript, FieldHandleIndex, - FunctionDefinitionIndex, FunctionHandleIndex, StructDefinitionIndex, + Bytecode, ClosureMask, CodeOffset, CodeUnit, CompiledModule, CompiledScript, + FieldHandleIndex, FunctionDefinitionIndex, FunctionHandleIndex, StructDefinitionIndex, StructVariantHandleIndex, TableIndex, VariantFieldHandleIndex, }, }; @@ -91,12 +91,22 @@ impl<'a> InstructionConsistency<'a> { )?; }, Call(idx) => { + // Nothing to verify for `_mask`, so merge with Call self.check_function_op(offset, *idx, /* generic */ false)?; }, CallGeneric(idx) => { let func_inst = self.resolver.function_instantiation_at(*idx); self.check_function_op(offset, func_inst.handle, /* generic */ true)?; }, + ClosPack(idx, mask) => { + self.check_function_op(offset, *idx, /* generic */ false)?; + self.check_closure_mask(offset, *idx, *mask)? + }, + ClosPackGeneric(idx, mask) => { + let func_inst = self.resolver.function_instantiation_at(*idx); + self.check_function_op(offset, func_inst.handle, /* generic */ true)?; + self.check_closure_mask(offset, func_inst.handle, *mask)? + }, Pack(idx) | Unpack(idx) => { self.check_struct_op(offset, *idx, /* generic */ false)?; }, @@ -135,11 +145,11 @@ impl<'a> InstructionConsistency<'a> { // List out the other options explicitly so there's a compile error if a new // bytecode gets added. - FreezeRef | Pop | Ret | Branch(_) | BrTrue(_) | BrFalse(_) | LdU8(_) | LdU16(_) - | LdU32(_) | LdU64(_) | LdU128(_) | LdU256(_) | LdConst(_) | CastU8 | CastU16 - | CastU32 | CastU64 | CastU128 | CastU256 | LdTrue | LdFalse | ReadRef - | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor | Shl | Shr - | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | CopyLoc(_) | MoveLoc(_) + ClosEval(_) | FreezeRef | Pop | Ret | Branch(_) | BrTrue(_) | BrFalse(_) + | LdU8(_) | LdU16(_) | LdU32(_) | LdU64(_) | LdU128(_) | LdU256(_) | LdConst(_) + | CastU8 | CastU16 | CastU32 | CastU64 | CastU128 | CastU256 | LdTrue | LdFalse + | ReadRef | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor | Shl + | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | CopyLoc(_) | MoveLoc(_) | StLoc(_) | MutBorrowLoc(_) | ImmBorrowLoc(_) | VecLen(_) | VecImmBorrow(_) | VecMutBorrow(_) | VecPushBack(_) | VecPopBack(_) | VecSwap(_) | Abort | Nop => (), } @@ -227,4 +237,19 @@ impl<'a> InstructionConsistency<'a> { } Ok(()) } + + fn check_closure_mask( + &self, + offset: usize, + func_handle_index: FunctionHandleIndex, + mask: ClosureMask, + ) -> PartialVMResult<()> { + let function_handle = self.resolver.function_handle_at(func_handle_index); + let signature = self.resolver.signature_at(function_handle.parameters); + if mask.max_captured() >= signature.len() { + return Err(PartialVMError::new(StatusCode::INVALID_CLOSURE_MASK) + .at_code_offset(self.current_function(), offset as CodeOffset)); + } + Ok(()) + } } diff --git a/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs index 7d27f91f54e83..f49c37ad812f1 100644 --- a/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs @@ -125,6 +125,9 @@ fn execute_inner( | Bytecode::UnpackVariantGeneric(_) | Bytecode::TestVariant(_) | Bytecode::TestVariantGeneric(_) + | Bytecode::ClosPack(..) + | Bytecode::ClosPackGeneric(..) + | Bytecode::ClosEval(_) | Bytecode::ReadRef | Bytecode::WriteRef | Bytecode::CastU8 diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs index 962487f832a2c..1f8fd1a6d04e9 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs @@ -500,7 +500,17 @@ impl AbstractState { return Err(self.error(StatusCode::GLOBAL_REFERENCE_ERROR, offset)); } } + // Check arguments and return, and abstract value transition + self.core_call(offset, arguments, &return_.0, meter) + } + fn core_call( + &mut self, + offset: CodeOffset, + arguments: Vec, + result_tys: &[SignatureToken], + meter: &mut impl Meter, + ) -> PartialVMResult> { // Check mutable references can be transfered let mut all_references_to_borrow_from = BTreeSet::new(); let mut mutable_references_to_borrow_from = BTreeSet::new(); @@ -518,8 +528,7 @@ impl AbstractState { // Track borrow relationships of return values on inputs let mut returned_refs = 0; - let return_values = return_ - .0 + let return_values = result_tys .iter() .map(|return_type| match return_type { SignatureToken::MutableReference(_) => { @@ -559,6 +568,16 @@ impl AbstractState { Ok(return_values) } + pub fn clos_eval( + &mut self, + offset: CodeOffset, + arguments: Vec, + result_tys: &[SignatureToken], + meter: &mut impl Meter, + ) -> PartialVMResult> { + self.core_call(offset, arguments, result_tys, meter) + } + pub fn ret(&mut self, offset: CodeOffset, values: Vec) -> PartialVMResult<()> { // release all local variables let mut released = BTreeSet::new(); diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs index 2d70d91ae314f..b5708b461e5b8 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs @@ -22,8 +22,9 @@ use move_binary_format::{ binary_views::{BinaryIndexedView, FunctionView}, errors::{PartialVMError, PartialVMResult}, file_format::{ - Bytecode, CodeOffset, FunctionDefinitionIndex, FunctionHandle, IdentifierIndex, - SignatureIndex, SignatureToken, StructDefinition, StructVariantHandle, VariantIndex, + Bytecode, ClosureMask, CodeOffset, FunctionDefinitionIndex, FunctionHandle, + IdentifierIndex, SignatureIndex, SignatureToken, StructDefinition, StructVariantHandle, + VariantIndex, }, safe_assert, safe_unwrap, views::FieldOrVariantIndex, @@ -100,6 +101,43 @@ fn call( Ok(()) } +fn clos_pack( + verifier: &mut ReferenceSafetyAnalysis, + function_handle: &FunctionHandle, + mask: ClosureMask, +) -> PartialVMResult<()> { + let parameters = verifier.resolver.signature_at(function_handle.parameters); + // Extract the captured arguments and pop them from the stack + let argc = mask.extract(¶meters.0, true).len(); + for _ in 0..argc { + // Currently closures require captured arguments to be values. This is verified + // by type safety. + safe_assert!(safe_unwrap!(verifier.stack.pop()).is_value()) + } + verifier.stack.push(AbstractValue::NonReference); + Ok(()) +} + +fn clos_eval( + verifier: &mut ReferenceSafetyAnalysis, + state: &mut AbstractState, + offset: CodeOffset, + arg_tys: Vec, + result_tys: Vec, + meter: &mut impl Meter, +) -> PartialVMResult<()> { + let arguments = arg_tys + .iter() + .map(|_| verifier.stack.pop().unwrap()) + .rev() + .collect(); + let values = state.clos_eval(offset, arguments, &result_tys, meter)?; + for value in values { + verifier.stack.push(value) + } + Ok(()) +} + fn num_fields(struct_def: &StructDefinition) -> usize { struct_def.field_information.field_count(None) } @@ -185,6 +223,18 @@ fn vec_element_type( } } +fn fun_type( + verifier: &mut ReferenceSafetyAnalysis, + idx: SignatureIndex, +) -> PartialVMResult<(Vec, Vec)> { + match verifier.resolver.signature_at(idx).0.first() { + Some(SignatureToken::Function(args, result, _)) => Ok((args.clone(), result.clone())), + _ => Err(PartialVMError::new( + StatusCode::VERIFIER_INVARIANT_VIOLATION, + )), + } +} + fn execute_inner( verifier: &mut ReferenceSafetyAnalysis, state: &mut AbstractState, @@ -497,6 +547,20 @@ fn execute_inner( unpack_variant(verifier, handle)? }, + Bytecode::ClosPack(idx, mask) => { + let function_handle = verifier.resolver.function_handle_at(*idx); + clos_pack(verifier, function_handle, *mask)? + }, + Bytecode::ClosPackGeneric(idx, mask) => { + let func_inst = verifier.resolver.function_instantiation_at(*idx); + let function_handle = verifier.resolver.function_handle_at(func_inst.handle); + clos_pack(verifier, function_handle, *mask)? + }, + Bytecode::ClosEval(idx) => { + let (arg_tys, result_tys) = fun_type(verifier, *idx)?; + clos_eval(verifier, state, offset, arg_tys, result_tys, meter)? + }, + Bytecode::VecPack(idx, num) => { for _ in 0..*num { safe_assert!(safe_unwrap!(verifier.stack.pop()).is_value()) @@ -559,7 +623,6 @@ fn execute_inner( }; Ok(()) } - impl<'a> TransferFunctions for ReferenceSafetyAnalysis<'a> { type State = AbstractState; diff --git a/third_party/move/move-bytecode-verifier/src/signature.rs b/third_party/move/move-bytecode-verifier/src/signature.rs index fdd845ffc871a..08dfe5d31ef73 100644 --- a/third_party/move/move-bytecode-verifier/src/signature.rs +++ b/third_party/move/move-bytecode-verifier/src/signature.rs @@ -259,6 +259,12 @@ impl<'a> SignatureChecker<'a> { self.check_signature_tokens(type_arguments) }, + // Closure operations not supported by legacy signature checker + ClosPack(..) | ClosPackGeneric(..) | ClosEval(_) => { + return Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) + .with_message("closure operations not supported".to_owned())) + }, + // List out the other options explicitly so there's a compile error if a new // bytecode gets added. Pop @@ -363,6 +369,11 @@ impl<'a> SignatureChecker<'a> { } }, + SignatureToken::Function(..) => { + return Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) + .with_message("function types not supported".to_string())); + }, + SignatureToken::Struct(_) | SignatureToken::Reference(_) | SignatureToken::MutableReference(_) @@ -415,6 +426,8 @@ impl<'a> SignatureChecker<'a> { Err(PartialVMError::new(StatusCode::INVALID_SIGNATURE_TOKEN) .with_message("reference not allowed".to_string())) }, + Function(..) => Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) + .with_message("function types not supported".to_string())), Vector(ty) => self.check_signature_token(ty), StructInstantiation(_, type_arguments) => self.check_signature_tokens(type_arguments), } @@ -465,6 +478,10 @@ impl<'a> SignatureChecker<'a> { type_parameters, ) }, + SignatureToken::Function(..) => { + Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) + .with_message("function types not supported".to_string())) + }, SignatureToken::Reference(_) | SignatureToken::MutableReference(_) | SignatureToken::Vector(_) diff --git a/third_party/move/move-bytecode-verifier/src/signature_v2.rs b/third_party/move/move-bytecode-verifier/src/signature_v2.rs index 764a850c72679..c38c9723cfd33 100644 --- a/third_party/move/move-bytecode-verifier/src/signature_v2.rs +++ b/third_party/move/move-bytecode-verifier/src/signature_v2.rs @@ -173,6 +173,18 @@ fn check_ty( param_constraints, )?; }, + Function(args, result, abilities) => { + assert_abilities(*abilities, required_abilities)?; + for ty in args.iter().chain(result.iter()) { + check_ty( + struct_handles, + ty, + false, + required_abilities.requires(), + param_constraints, + )?; + } + }, Struct(sh_idx) => { let handle = &struct_handles[sh_idx.0 as usize]; assert_abilities(handle.abilities, required_abilities)?; @@ -259,6 +271,11 @@ fn check_phantom_params( match ty { Vector(ty) => check_phantom_params(struct_handles, context, false, ty)?, + Function(args, result, _) => { + for ty in args.iter().chain(result) { + check_phantom_params(struct_handles, context, false, ty)? + } + }, StructInstantiation(idx, type_arguments) => { let sh = &struct_handles[idx.0 as usize]; for (i, ty) in type_arguments.iter().enumerate() { @@ -822,7 +839,7 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { }) }; match instr { - CallGeneric(idx) => { + CallGeneric(idx) | ClosPackGeneric(idx, _) => { if let btree_map::Entry::Vacant(entry) = checked_func_insts.entry(*idx) { let constraints = self.verify_function_instantiation_contextless(*idx)?; map_err(constraints.check_in_context(&ability_context))?; @@ -881,6 +898,14 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { entry.insert(()); } }, + ClosEval(idx) => { + let sign = self.resolver.signature_at(*idx); + if sign.len() != 1 || !matches!(&sign.0[0], SignatureToken::Function(..)) { + return map_err(Err(PartialVMError::new( + StatusCode::CLOSURE_EVAL_REQUIRES_FUNCTION, + ))); + } + }, VecPack(idx, _) | VecLen(idx) | VecImmBorrow(idx) @@ -936,6 +961,7 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { | LdTrue | LdFalse | Call(_) + | ClosPack(..) | Pack(_) | Unpack(_) | TestVariant(_) diff --git a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs index 7e43a64fe36d6..427b6d88e01d7 100644 --- a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs @@ -14,7 +14,7 @@ use move_binary_format::{ binary_views::{BinaryIndexedView, FunctionView}, control_flow_graph::{BlockId, ControlFlowGraph}, errors::{PartialVMError, PartialVMResult}, - file_format::{Bytecode, CodeUnit, FunctionDefinitionIndex, Signature}, + file_format::{Bytecode, CodeUnit, FunctionDefinitionIndex, Signature, SignatureToken}, }; use move_core_types::vm_status::StatusCode; @@ -228,6 +228,43 @@ impl<'a> StackUsageVerifier<'a> { (arg_count, return_count) }, + // ClosEval pops the number of arguments and pushes the results of the given function + // type + Bytecode::ClosEval(idx) => { + if let Some(SignatureToken::Function(args, result, _)) = + self.resolver.signature_at(*idx).0.first() + { + ((1 + args.len()) as u64, result.len() as u64) + } else { + // We don't know what it will pop/push, but the signature checker + // ensures we never reach this + (0, 0) + } + }, + + // ClosPack pops the captured arguments and returns 1 value + Bytecode::ClosPack(idx, mask) => { + let function_handle = self.resolver.function_handle_at(*idx); + let arg_count = mask + .extract( + &self.resolver.signature_at(function_handle.parameters).0, + true, + ) + .len() as u64; + (arg_count, 1) + }, + Bytecode::ClosPackGeneric(idx, mask) => { + let func_inst = self.resolver.function_instantiation_at(*idx); + let function_handle = self.resolver.function_handle_at(func_inst.handle); + let arg_count = mask + .extract( + &self.resolver.signature_at(function_handle.parameters).0, + true, + ) + .len() as u64; + (arg_count, 1) + }, + // Pack performs `num_fields` pops and one push Bytecode::Pack(idx) => { let struct_definition = self.resolver.struct_def_at(*idx)?; diff --git a/third_party/move/move-bytecode-verifier/src/struct_defs.rs b/third_party/move/move-bytecode-verifier/src/struct_defs.rs index 6080b0cbb88fb..d78c9e7f58f7d 100644 --- a/third_party/move/move-bytecode-verifier/src/struct_defs.rs +++ b/third_party/move/move-bytecode-verifier/src/struct_defs.rs @@ -123,6 +123,11 @@ impl<'a> StructDefGraphBuilder<'a> { ) }, T::Vector(inner) => self.add_signature_token(neighbors, cur_idx, inner)?, + T::Function(args, result, _) => { + for t in args.iter().chain(result) { + self.add_signature_token(neighbors, cur_idx, t)? + } + }, T::Struct(sh_idx) => { if let Some(struct_def_idx) = self.handle_to_def.get(sh_idx) { neighbors diff --git a/third_party/move/move-bytecode-verifier/src/type_safety.rs b/third_party/move/move-bytecode-verifier/src/type_safety.rs index e50efc0cf23b0..cb6a03aada557 100644 --- a/third_party/move/move-bytecode-verifier/src/type_safety.rs +++ b/third_party/move/move-bytecode-verifier/src/type_safety.rs @@ -11,11 +11,12 @@ use move_binary_format::{ control_flow_graph::ControlFlowGraph, errors::{PartialVMError, PartialVMResult}, file_format::{ - AbilitySet, Bytecode, CodeOffset, FunctionDefinitionIndex, FunctionHandle, LocalIndex, - Signature, SignatureToken, SignatureToken as ST, StructDefinition, StructDefinitionIndex, - StructFieldInformation, StructHandleIndex, VariantIndex, + Ability, AbilitySet, Bytecode, ClosureMask, CodeOffset, FunctionDefinitionIndex, + FunctionHandle, FunctionHandleIndex, LocalIndex, Signature, SignatureToken, + SignatureToken as ST, StructDefinition, StructDefinitionIndex, StructFieldInformation, + StructHandleIndex, VariantIndex, Visibility, }, - safe_unwrap, + safe_assert, safe_unwrap, views::FieldOrVariantIndex, }; use move_core_types::vm_status::StatusCode; @@ -300,6 +301,115 @@ fn call( Ok(()) } +fn clos_eval( + verifier: &mut TypeSafetyChecker, + meter: &mut impl Meter, + offset: CodeOffset, + expected_ty: &SignatureToken, +) -> PartialVMResult<()> { + let SignatureToken::Function(param_tys, ret_tys, abilities) = expected_ty else { + // The signature checker has ensured this is a function + safe_assert!(false); + unreachable!() + }; + // On top of the stack is the closure, pop it. + let closure_ty = safe_unwrap!(verifier.stack.pop()); + // Verify that the closure type matches the expected type + if &closure_ty != expected_ty { + return Err(verifier + .error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset) + .with_message("closure type mismatch".to_owned())); + } + // Verify that the abilities match + let SignatureToken::Function(_, _, closure_abilities) = closure_ty else { + // Ensured above, but never panic + safe_assert!(false); + unreachable!() + }; + if !abilities.is_subset(closure_abilities) { + return Err(verifier + .error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset) + .with_message("closure ability mismatch".to_owned())); + } + // Pop and verify arguments + for param_ty in param_tys.iter().rev() { + let arg_ty = safe_unwrap!(verifier.stack.pop()); + if &arg_ty != param_ty { + return Err(verifier.error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset)); + } + } + for ret_ty in ret_tys { + verifier.push(meter, ret_ty.clone())? + } + Ok(()) +} + +fn clos_pack( + verifier: &mut TypeSafetyChecker, + meter: &mut impl Meter, + offset: CodeOffset, + func_handle_idx: FunctionHandleIndex, + type_actuals: &Signature, + mask: ClosureMask, +) -> PartialVMResult<()> { + let func_handle = verifier.resolver.function_handle_at(func_handle_idx); + // Check the captured arguments on the stack + let param_sign = verifier.resolver.signature_at(func_handle.parameters); + let captured_param_tys = mask.extract(¶m_sign.0, true); + let mut abilities = AbilitySet::ALL; + for ty in captured_param_tys.iter().rev() { + abilities = abilities.intersect(verifier.abilities(ty)?); + let arg = safe_unwrap!(verifier.stack.pop()); + if (type_actuals.is_empty() && &arg != ty) + || (!type_actuals.is_empty() && arg != instantiate(ty, type_actuals)) + { + return Err(verifier + .error(StatusCode::PACK_TYPE_MISMATCH_ERROR, offset) + .with_message("captured argument type mismatch".to_owned())); + } + // A captured argument must not be a reference + if ty.is_reference() { + return Err(verifier + .error(StatusCode::PACK_TYPE_MISMATCH_ERROR, offset) + .with_message("captured argument must not be a reference".to_owned())); + } + } + + // In order to determine whether this closure can be storable, we need to figure whether + // this function is public. + // !!!TODO!!! + // We currently cannot determine for an imported function if it is public or friend. A + // standalone CompiledModule does not give this information. This means that we cannot + // construct storable closures from imported functions for now, which is an + // undesired restriction, so this should be fixed by extending the FunctionHandle data, + // and adding visibility there. + let mut is_storable = false; + for fun_def in verifier.resolver.function_defs().unwrap_or(&[]) { + if fun_def.function == func_handle_idx { + // Function defined in this module, so we can check visibility. + if fun_def.visibility == Visibility::Public { + is_storable = true; + } + break; + } + } + if !is_storable { + abilities.remove(Ability::Store); + } + abilities.remove(Ability::Key); + + // Construct the resulting function type + let not_captured_param_tys = mask.extract(¶m_sign.0, false); + let ret_sign = verifier.resolver.signature_at(func_handle.return_); + verifier.push( + meter, + instantiate( + &SignatureToken::Function(not_captured_param_tys, ret_sign.0.to_vec(), abilities), + type_actuals, + ), + ) +} + fn type_fields_signature( verifier: &mut TypeSafetyChecker, _meter: &mut impl Meter, // TODO: metering @@ -725,6 +835,21 @@ fn verify_instr( call(verifier, meter, offset, func_handle, type_args)? }, + Bytecode::ClosPack(idx, mask) => { + clos_pack(verifier, meter, offset, *idx, &Signature(vec![]), *mask)? + }, + Bytecode::ClosPackGeneric(idx, mask) => { + let func_inst = verifier.resolver.function_instantiation_at(*idx); + let type_args = &verifier.resolver.signature_at(func_inst.type_parameters); + verifier.charge_tys(meter, &type_args.0)?; + clos_pack(verifier, meter, offset, func_inst.handle, type_args, *mask)? + }, + Bytecode::ClosEval(idx) => { + // The signature checker has verified this is a function type. + let expected_ty = safe_unwrap!(verifier.resolver.signature_at(*idx).0.first()); + clos_eval(verifier, meter, offset, expected_ty)? + }, + Bytecode::Pack(idx) => { let struct_definition = verifier.resolver.struct_def_at(*idx)?; pack( @@ -1139,6 +1264,9 @@ fn instantiate(token: &SignatureToken, subst: &Signature) -> SignatureToken { return token.clone(); } + let inst_vec = |v: &[SignatureToken]| -> Vec { + v.iter().map(|ty| instantiate(ty, subst)).collect() + }; match token { Bool => Bool, U8 => U8, @@ -1150,14 +1278,11 @@ fn instantiate(token: &SignatureToken, subst: &Signature) -> SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(instantiate(ty, subst))), + Function(args, result, abilities) => Function(inst_vec(args), inst_vec(result), *abilities), Struct(idx) => Struct(*idx), - StructInstantiation(idx, struct_type_args) => StructInstantiation( - *idx, - struct_type_args - .iter() - .map(|ty| instantiate(ty, subst)) - .collect(), - ), + StructInstantiation(idx, struct_type_args) => { + StructInstantiation(*idx, inst_vec(struct_type_args)) + }, Reference(ty) => Reference(Box::new(instantiate(ty, subst))), MutableReference(ty) => MutableReference(Box::new(instantiate(ty, subst))), TypeParameter(idx) => { diff --git a/third_party/move/move-compiler/src/interface_generator.rs b/third_party/move/move-compiler/src/interface_generator.rs index 4e0e3fd118b07..8a78305aecc9d 100644 --- a/third_party/move/move-compiler/src/interface_generator.rs +++ b/third_party/move/move-compiler/src/interface_generator.rs @@ -348,6 +348,12 @@ fn write_return_type(ctx: &mut Context, tys: &[SignatureToken]) -> String { } fn write_signature_token(ctx: &mut Context, t: &SignatureToken) -> String { + let tok_list = |c: &mut Context, v: &[SignatureToken]| { + v.iter() + .map(|ty| write_signature_token(c, ty)) + .collect::>() + .join(", ") + }; match t { SignatureToken::Bool => "bool".to_string(), SignatureToken::U8 => "u8".to_string(), @@ -359,15 +365,13 @@ fn write_signature_token(ctx: &mut Context, t: &SignatureToken) -> String { SignatureToken::Address => "address".to_string(), SignatureToken::Signer => "signer".to_string(), SignatureToken::Vector(inner) => format!("vector<{}>", write_signature_token(ctx, inner)), + SignatureToken::Function(args, result, _) => { + format!("|{}|{}", tok_list(ctx, args), tok_list(ctx, result)) + }, SignatureToken::Struct(idx) => write_struct_handle_type(ctx, *idx), SignatureToken::StructInstantiation(idx, types) => { let n = write_struct_handle_type(ctx, *idx); - let tys = types - .iter() - .map(|ty| write_signature_token(ctx, ty)) - .collect::>() - .join(", "); - format!("{}<{}>", n, tys) + format!("{}<{}>", n, tok_list(ctx, types)) }, SignatureToken::Reference(inner) => format!("&{}", write_signature_token(ctx, inner)), SignatureToken::MutableReference(inner) => { diff --git a/third_party/move/move-core/types/src/vm_status.rs b/third_party/move/move-core/types/src/vm_status.rs index 0db1e408c3a3b..a33744afd0009 100644 --- a/third_party/move/move-core/types/src/vm_status.rs +++ b/third_party/move/move-core/types/src/vm_status.rs @@ -734,12 +734,16 @@ pub enum StatusCode { ZERO_VARIANTS_ERROR = 1130, // A feature is not enabled. FEATURE_NOT_ENABLED = 1131, + // Closure mask invalid + INVALID_CLOSURE_MASK = 1132, + // Closure eval type is not a function + CLOSURE_EVAL_REQUIRES_FUNCTION = 1133, // Reserved error code for future use - RESERVED_VERIFICATION_ERROR_2 = 1132, - RESERVED_VERIFICATION_ERROR_3 = 1133, - RESERVED_VERIFICATION_ERROR_4 = 1134, - RESERVED_VERIFICATION_ERROR_5 = 1135, + RESERVED_VERIFICATION_ERROR_2 = 1134, + RESERVED_VERIFICATION_ERROR_3 = 1135, + RESERVED_VERIFICATION_ERROR_4 = 1136, + RESERVED_VERIFICATION_ERROR_5 = 1137, // These are errors that the VM might raise if a violation of internal // invariants takes place. @@ -858,11 +862,14 @@ pub enum StatusCode { // Struct variant not matching. This error appears on an attempt to unpack or borrow a // field from a value which is not of the expected variant. STRUCT_VARIANT_MISMATCH = 4038, + // An unimplemented feature in the VM. + UNIMPLEMENTED_FEATURE = 4039, + // Reserved error code for future use. Always keep this buffer of well-defined new codes. - RESERVED_RUNTIME_ERROR_1 = 4039, - RESERVED_RUNTIME_ERROR_2 = 4040, - RESERVED_RUNTIME_ERROR_3 = 4041, - RESERVED_RUNTIME_ERROR_4 = 4042, + RESERVED_RUNTIME_ERROR_1 = 4040, + RESERVED_RUNTIME_ERROR_2 = 4041, + RESERVED_RUNTIME_ERROR_3 = 4042, + RESERVED_RUNTIME_ERROR_4 = 4043, // A reserved status to represent an unknown vm status. // this is std::u64::MAX, but we can't pattern match on that, so put the hardcoded value in diff --git a/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs b/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs index dd996eed4eac6..c29d923504085 100644 --- a/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs +++ b/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs @@ -768,6 +768,9 @@ impl<'a> Context<'a> { let correct_inner = self.reindex_signature_token(dep, *inner)?; SignatureToken::Vector(Box::new(correct_inner)) }, + SignatureToken::Function(..) => { + unimplemented!("function types not supported by MoveIR") + }, SignatureToken::Reference(inner) => { let correct_inner = self.reindex_signature_token(dep, *inner)?; SignatureToken::Reference(Box::new(correct_inner)) diff --git a/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs b/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs index d0b4e19044fcf..0b95bbc1cfd93 100644 --- a/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs +++ b/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs @@ -293,6 +293,11 @@ impl<'a> StacklessBytecodeGenerator<'a> { }; match bytecode { + MoveBytecode::ClosPack(..) + | MoveBytecode::ClosPackGeneric(..) + | MoveBytecode::ClosEval(..) => { + unimplemented!("stackless bytecode generation for closure opcodes") + }, MoveBytecode::Pop => { let temp_index = self.temp_stack.pop().unwrap(); self.code diff --git a/third_party/move/move-model/src/ty.rs b/third_party/move/move-model/src/ty.rs index 1d2c5c74d106a..6da025ea9c482 100644 --- a/third_party/move/move-model/src/ty.rs +++ b/third_party/move/move-model/src/ty.rs @@ -1394,6 +1394,10 @@ impl Type { .collect(), ) }, + SignatureToken::Function(..) => { + // TODO: implement function conversion + unimplemented!("signature token to model type") + }, } } diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index 5950c9a08e5f6..1acba496f4924 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -1537,6 +1537,14 @@ impl Frame { )?; match instruction { + // TODO: implement closures + Bytecode::ClosPack(..) + | Bytecode::ClosPackGeneric(..) + | Bytecode::ClosEval(..) => { + return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) + .with_message("closure opcodes in interpreter".to_owned())) + }, + Bytecode::Pop => { let popped_val = interpreter.operand_stack.pop()?; gas_meter.charge_pop(popped_val)?; diff --git a/third_party/move/move-vm/runtime/src/loader/type_loader.rs b/third_party/move/move-vm/runtime/src/loader/type_loader.rs index b9da4e7348c50..1aa31dcd377ea 100644 --- a/third_party/move/move-vm/runtime/src/loader/type_loader.rs +++ b/third_party/move/move-vm/runtime/src/loader/type_loader.rs @@ -2,8 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 use move_binary_format::{ - binary_views::BinaryIndexedView, errors::PartialVMResult, file_format::SignatureToken, + binary_views::BinaryIndexedView, + errors::{PartialVMError, PartialVMResult}, + file_format::SignatureToken, }; +use move_core_types::vm_status::StatusCode; use move_vm_types::loaded_data::runtime_types::{AbilityInfo, StructNameIndex, Type}; use triomphe::Arc as TriompheArc; @@ -28,6 +31,11 @@ pub fn intern_type( let inner_type = intern_type(module, inner_tok, struct_name_table)?; Type::Vector(TriompheArc::new(inner_type)) }, + SignatureToken::Function(..) => { + // TODO: implement closures + return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) + .with_message("function types in the type loader".to_owned())); + }, SignatureToken::Reference(inner_tok) => { let inner_type = intern_type(module, inner_tok, struct_name_table)?; Type::Reference(Box::new(inner_type)) diff --git a/third_party/move/move-vm/runtime/src/runtime_type_checks.rs b/third_party/move/move-vm/runtime/src/runtime_type_checks.rs index 30590fbfbf41c..21bf2903f7dc0 100644 --- a/third_party/move/move-vm/runtime/src/runtime_type_checks.rs +++ b/third_party/move/move-vm/runtime/src/runtime_type_checks.rs @@ -120,6 +120,12 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck { instruction: &Bytecode, ) -> PartialVMResult<()> { match instruction { + // TODO: implement closures + Bytecode::ClosPack(..) | Bytecode::ClosPackGeneric(..) | Bytecode::ClosEval(..) => { + return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) + .with_message("closure opcodes in interpreter".to_owned())) + }, + // Call instruction will be checked at execute_main. Bytecode::Call(_) | Bytecode::CallGeneric(_) => (), Bytecode::BrFalse(_) | Bytecode::BrTrue(_) => { @@ -247,6 +253,12 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck { let ty_builder = resolver.loader().ty_builder(); match instruction { + // TODO: implement closures + Bytecode::ClosPack(..) | Bytecode::ClosPackGeneric(..) | Bytecode::ClosEval(..) => { + return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) + .with_message("closure opcodes in interpreter".to_owned())) + }, + Bytecode::BrTrue(_) | Bytecode::BrFalse(_) => (), Bytecode::Branch(_) | Bytecode::Ret diff --git a/third_party/move/move-vm/types/src/values/values_impl.rs b/third_party/move/move-vm/types/src/values/values_impl.rs index 42fbe5d9278c3..c3171cd907588 100644 --- a/third_party/move/move-vm/types/src/values/values_impl.rs +++ b/third_party/move/move-vm/types/src/values/values_impl.rs @@ -4154,7 +4154,7 @@ impl Value { S::Signer => return None, S::Vector(inner) => L::Vector(Box::new(Self::constant_sig_token_to_layout(inner)?)), // Not yet supported - S::Struct(_) | S::StructInstantiation(_, _) => return None, + S::Struct(_) | S::StructInstantiation(_, _) | S::Function(..) => return None, // Not allowed/Not meaningful S::TypeParameter(_) | S::Reference(_) | S::MutableReference(_) => return None, }) diff --git a/third_party/move/tools/move-bytecode-utils/src/layout.rs b/third_party/move/tools/move-bytecode-utils/src/layout.rs index ccbda4d4276af..46863644cdc55 100644 --- a/third_party/move/tools/move-bytecode-utils/src/layout.rs +++ b/third_party/move/tools/move-bytecode-utils/src/layout.rs @@ -386,6 +386,7 @@ impl TypeLayoutBuilder { ) -> anyhow::Result { use SignatureToken::*; Ok(match s { + Function(..) => bail!("function types NYI for MoveTypeLayout"), Vector(t) => MoveTypeLayout::Vector(Box::new(Self::build_from_signature_token( m, t, diff --git a/third_party/move/tools/move-disassembler/src/disassembler.rs b/third_party/move/tools/move-disassembler/src/disassembler.rs index 3dedaa3f942b1..e3148cea698cb 100644 --- a/third_party/move/tools/move-disassembler/src/disassembler.rs +++ b/third_party/move/tools/move-disassembler/src/disassembler.rs @@ -570,6 +570,9 @@ impl<'a> Disassembler<'a> { type_param_context: &[SourceName], ) -> Result { Ok(match sig_tok { + // TODO: function types + SignatureToken::Function(..) => unimplemented!("disassembling function sig tokens"), + SignatureToken::Bool => "bool".to_string(), SignatureToken::U8 => "u8".to_string(), SignatureToken::U16 => "u16".to_string(), @@ -641,6 +644,9 @@ impl<'a> Disassembler<'a> { default_location: &Loc, ) -> Result { match instruction { + Bytecode::ClosPack(..) | Bytecode::ClosPackGeneric(..) | Bytecode::ClosEval(..) => { + bail!("closure opcodes not implemented") + }, Bytecode::LdConst(idx) => { let constant = self.source_mapper.bytecode.constant_at(*idx); Ok(format!( diff --git a/third_party/move/tools/move-resource-viewer/src/lib.rs b/third_party/move/tools/move-resource-viewer/src/lib.rs index 4f06ccc18ce22..ba6d7f3eca515 100644 --- a/third_party/move/tools/move-resource-viewer/src/lib.rs +++ b/third_party/move/tools/move-resource-viewer/src/lib.rs @@ -375,6 +375,9 @@ impl MoveValueAnnotator { SignatureToken::Vector(ty) => { FatType::Vector(Box::new(self.resolve_signature(module, ty, limit)?)) }, + SignatureToken::Function(..) => { + bail!("function types NYI by fat types") + }, SignatureToken::Struct(idx) => { FatType::Struct(Box::new(self.resolve_struct_handle(module, *idx, limit)?)) }, From 22a0bdb2d5b541602a540866dfa1d5f5d9ba4ee8 Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Sat, 23 Nov 2024 10:52:41 -0800 Subject: [PATCH 2/9] Rename and rephrase Wolfgang's closure operations to match ongoing parser work: - `LdFunction`, `LdFunctionGeneric` = generate a closure from a defined function - `EarlyBind` = bind more arguments to any closure - `Invoke` = call a closure with final arguments Add some description/semantics to `file_format.rs` to better describe the implementation which will be needed. Get rid of complex Mask calculations in favor of simpler early bninding of `k` initial arguments. Hopefully keep all bytecode verifier operations working the same. --- api/types/src/bytecode.rs | 2 +- aptos-move/script-composer/src/decompiler.rs | 3 +- .../move-binary-format/src/check_bounds.rs | 11 +- .../src/check_complexity.rs | 11 +- .../move/move-binary-format/src/constant.rs | 2 +- .../move-binary-format/src/deserializer.rs | 15 + .../move-binary-format/src/file_format.rs | 285 ++++++++---------- .../src/file_format_common.rs | 12 +- .../move/move-binary-format/src/serializer.rs | 20 +- .../invalid-mutations/src/bounds/code_unit.rs | 27 +- .../src/acquires_list_verifier.rs | 36 ++- .../src/instruction_consistency.rs | 73 +++-- .../src/locals_safety/mod.rs | 7 +- .../src/reference_safety/abstract_state.rs | 17 +- .../src/reference_safety/mod.rs | 64 ++-- .../move-bytecode-verifier/src/signature.rs | 2 +- .../src/signature_v2.rs | 69 ++++- .../src/stack_usage_verifier.rs | 59 ++-- .../move-bytecode-verifier/src/type_safety.rs | 133 +++++--- .../move/move-core/types/src/vm_status.rs | 25 +- .../bytecode/src/function_target_pipeline.rs | 4 +- .../src/stackless_bytecode_generator.rs | 7 +- .../move/move-vm/runtime/src/interpreter.rs | 9 +- .../runtime/src/runtime_type_checks.rs | 14 +- .../move-vm/test-utils/src/gas_schedule.rs | 25 +- .../move-disassembler/src/disassembler.rs | 98 +++++- 26 files changed, 689 insertions(+), 341 deletions(-) diff --git a/api/types/src/bytecode.rs b/api/types/src/bytecode.rs index f6ed8405471a4..ffdea8f6186c6 100644 --- a/api/types/src/bytecode.rs +++ b/api/types/src/bytecode.rs @@ -106,7 +106,7 @@ pub trait Bytecode { to: Box::new(self.new_move_type(t.borrow())), }, SignatureToken::Function(..) => { - // TODO + // TODO(LAMBDA) unimplemented!("signature token function to API MoveType") }, } diff --git a/aptos-move/script-composer/src/decompiler.rs b/aptos-move/script-composer/src/decompiler.rs index 56806f589ab08..147d976b90ccf 100644 --- a/aptos-move/script-composer/src/decompiler.rs +++ b/aptos-move/script-composer/src/decompiler.rs @@ -209,7 +209,8 @@ impl LocalState { TypeTag::Vector(Box::new(Self::type_tag_from_sig_token(script, s)?)) }, SignatureToken::Function(..) => { - bail!("function types NYI for script composer") + // TODO(LAMBDA) + bail!("function types not yet implemented for script composer") }, SignatureToken::Struct(s) => { let module_handle = script.module_handle_at(script.struct_handle_at(*s).module); diff --git a/third_party/move/move-binary-format/src/check_bounds.rs b/third_party/move/move-binary-format/src/check_bounds.rs index dca378e0957c3..2e9f40388d969 100644 --- a/third_party/move/move-binary-format/src/check_bounds.rs +++ b/third_party/move/move-binary-format/src/check_bounds.rs @@ -546,12 +546,12 @@ impl<'a> BoundsChecker<'a> { )?; } }, - Call(idx) | ClosPack(idx, _) => self.check_code_unit_bounds_impl( + Call(idx) | LdFunction(idx) => self.check_code_unit_bounds_impl( self.view.function_handles(), *idx, bytecode_offset, )?, - CallGeneric(idx) | ClosPackGeneric(idx, _) => { + CallGeneric(idx) | LdFunctionGeneric(idx) => { self.check_code_unit_bounds_impl( self.view.function_instantiations(), *idx, @@ -650,15 +650,16 @@ impl<'a> BoundsChecker<'a> { }, // Instructions that refer to a signature - ClosEval(idx) - | VecPack(idx, _) + VecPack(idx, _) | VecLen(idx) | VecImmBorrow(idx) | VecMutBorrow(idx) | VecPushBack(idx) | VecPopBack(idx) | VecUnpack(idx, _) - | VecSwap(idx) => { + | VecSwap(idx) + | Invoke(idx) + | EarlyBind(idx, _) => { self.check_code_unit_bounds_impl( self.view.signatures(), *idx, diff --git a/third_party/move/move-binary-format/src/check_complexity.rs b/third_party/move/move-binary-format/src/check_complexity.rs index dbc8890d4cf3e..4aff6b1bfd7e9 100644 --- a/third_party/move/move-binary-format/src/check_complexity.rs +++ b/third_party/move/move-binary-format/src/check_complexity.rs @@ -262,7 +262,7 @@ impl<'a> BinaryComplexityMeter<'a> { for instr in &code.code { match instr { - CallGeneric(idx) | ClosPackGeneric(idx, ..) => { + CallGeneric(idx) | LdFunctionGeneric(idx, ..) => { self.meter_function_instantiation(*idx)?; }, PackGeneric(idx) | UnpackGeneric(idx) => { @@ -284,15 +284,16 @@ impl<'a> BinaryComplexityMeter<'a> { ImmBorrowVariantFieldGeneric(idx) | MutBorrowVariantFieldGeneric(idx) => { self.meter_variant_field_instantiation(*idx)?; }, - ClosEval(idx) - | VecPack(idx, _) + VecPack(idx, _) | VecLen(idx) | VecImmBorrow(idx) | VecMutBorrow(idx) | VecPushBack(idx) | VecPopBack(idx) | VecUnpack(idx, _) - | VecSwap(idx) => { + | VecSwap(idx) + | Invoke(idx) + | EarlyBind(idx, _) => { self.meter_signature(*idx)?; }, @@ -324,7 +325,7 @@ impl<'a> BinaryComplexityMeter<'a> { | PackVariant(_) | UnpackVariant(_) | TestVariant(_) - | ClosPack(..) + | LdFunction(_) | ReadRef | WriteRef | FreezeRef diff --git a/third_party/move/move-binary-format/src/constant.rs b/third_party/move/move-binary-format/src/constant.rs index 6fa8b21fa11fb..3b8d200787a4e 100644 --- a/third_party/move/move-binary-format/src/constant.rs +++ b/third_party/move/move-binary-format/src/constant.rs @@ -18,7 +18,7 @@ fn sig_to_ty(sig: &SignatureToken) -> Option { SignatureToken::U256 => Some(MoveTypeLayout::U256), SignatureToken::Vector(v) => Some(MoveTypeLayout::Vector(Box::new(sig_to_ty(v.as_ref())?))), SignatureToken::Function(..) => { - // TODO: do we need representation in MoveTypeLayout? + // TODO(LAMBDA): do we need representation in MoveTypeLayout? None }, SignatureToken::Reference(_) diff --git a/third_party/move/move-binary-format/src/deserializer.rs b/third_party/move/move-binary-format/src/deserializer.rs index 89a78a2562c6e..24f096ef482c9 100644 --- a/third_party/move/move-binary-format/src/deserializer.rs +++ b/third_party/move/move-binary-format/src/deserializer.rs @@ -114,6 +114,12 @@ impl Table { } } +fn read_u8_internal(cursor: &mut VersionedCursor) -> BinaryLoaderResult { + cursor.read_u8().map_err(|_| { + PartialVMError::new(StatusCode::MALFORMED).with_message("Unexpected EOF".to_string()) + }) +} + fn read_u16_internal(cursor: &mut VersionedCursor) -> BinaryLoaderResult { let mut u16_bytes = [0; 2]; cursor @@ -1814,6 +1820,15 @@ fn load_code(cursor: &mut VersionedCursor, code: &mut Vec) -> BinaryLo Opcodes::CAST_U16 => Bytecode::CastU16, Opcodes::CAST_U32 => Bytecode::CastU32, Opcodes::CAST_U256 => Bytecode::CastU256, + + Opcodes::LD_FUNCTION => Bytecode::LdFunction(load_function_handle_index(cursor)?), + Opcodes::LD_FUNCTION_GENERIC => { + Bytecode::LdFunctionGeneric(load_function_inst_index(cursor)?) + }, + Opcodes::INVOKE => Bytecode::Invoke(load_signature_index(cursor)?), + Opcodes::EARLY_BIND => { + Bytecode::EarlyBind(load_signature_index(cursor)?, read_u8_internal(cursor)?) + }, }; code.push(bytecode); } diff --git a/third_party/move/move-binary-format/src/file_format.rs b/third_party/move/move-binary-format/src/file_format.rs index a2058b333dc28..4d431f5c319e2 100644 --- a/third_party/move/move-binary-format/src/file_format.rs +++ b/third_party/move/move-binary-format/src/file_format.rs @@ -49,7 +49,6 @@ use proptest::{collection::vec, prelude::*, strategy::BoxedStrategy}; use ref_cast::RefCast; use serde::{Deserialize, Serialize}; use std::{ - collections::BTreeMap, fmt::{self, Formatter}, ops::BitOr, }; @@ -1011,6 +1010,13 @@ impl AbilitySet { pub fn into_u8(self) -> u8 { self.0 } + + pub fn to_string_concise(self) -> String { + self.iter() + .map(|a| a.to_string()) + .collect::>() + .join("+") + } } impl fmt::Display for AbilitySet { @@ -1019,8 +1025,8 @@ impl fmt::Display for AbilitySet { &self .iter() .map(|a| a.to_string()) - .reduce(|l, r| format!("{} + {}", l, r)) - .unwrap_or_default(), + .collect::>() + .join(" + "), ) } } @@ -1476,6 +1482,13 @@ impl SignatureToken { } } + /// Returns true if the `SignatureToken` is a function. + pub fn is_function(&self) -> bool { + use SignatureToken::*; + + matches!(self, Function(..)) + } + /// Set the index to this one. Useful for random testing. /// /// Panics if this token doesn't contain a struct handle. @@ -1538,101 +1551,6 @@ impl SignatureToken { } } -/// A `ClosureMask` is a value which determines how to distinguish those function arguments -/// which are captured and which are not when a closure is constructed. For instance, -/// with `_` representing an omitted argument, the mask for `f(a,_,b,_)` would have the argument -/// at index 0 and at index 2 captured. The mask can be used to transform lists of types. -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] -#[cfg_attr(any(test, feature = "fuzzing"), derive(proptest_derive::Arbitrary))] -#[cfg_attr(any(test, feature = "fuzzing"), proptest(no_params))] -#[cfg_attr(feature = "fuzzing", derive(arbitrary::Arbitrary))] -pub struct ClosureMask { - pub mask: u64, -} - -impl fmt::Display for ClosureMask { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:b}", self.mask) - } -} - -impl ClosureMask { - pub fn new(mask: u64) -> Self { - Self { mask } - } - - /// 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). - pub fn extract(&self, tys: &[T], collect_captured: bool) -> Vec { - tys.iter() - .enumerate() - .filter_map(|(pos, x)| { - let set = (1 << pos) & self.mask != 0; - if set && collect_captured || !set && !collect_captured { - Some(x.clone()) - } else { - None - } - }) - .collect() - } - - /// Compose two lists of elements into one based on the given mask such that the - /// following holds: - /// ```ignore - /// mask.compose(mask.extract(v, true), mask.extract(v, false)) == v - /// ``` - /// This returns `None` if the provided lists are inconsistent w.r.t the mask - /// and cannot be composed. This should not happen in verified code, but - /// a caller should decide whether to crash or to error. - pub fn compose(&self, captured: &[T], provided: &[T]) -> Option> { - let mut result = BTreeMap::new(); // expect ordered enumeration - let mut cap_idx = 0; - let mut pro_idx = 0; - for i in 0..64 { - if cap_idx >= captured.len() && pro_idx >= provided.len() { - // all covered - break; - } - if (1 << i) & self.mask != 0 { - if cap_idx >= captured.len() { - // Inconsistency - return None; - } - result.insert(i, captured[cap_idx].clone()); - cap_idx += 1 - } else { - if pro_idx >= provided.len() { - // Inconsistency - return None; - } - result.insert(i, provided[pro_idx].clone()); - pro_idx += 1 - } - } - let map_len = result.len(); - let vec = result.into_values().collect::>(); - if vec.len() != map_len { - // Inconsistency: all indices must be contiguously covered - None - } else { - Some(vec) - } - } - - /// Return the max index of captured arguments - pub fn max_captured(&self) -> usize { - let mut i = 0; - let mut mask = self.mask; - while mask != 0 { - mask >>= 1; - i += 1 - } - i - } -} - /// A `Constant` is a serialized value along with its type. That type will be deserialized by the /// loader/evaluator #[derive(Clone, Debug, Eq, PartialEq, Hash)] @@ -1786,7 +1704,7 @@ pub enum Bytecode { arithmetic error else: stack << int_val as u8 - "#] + "#] #[runtime_check_epilogue = r#" ty_stack >> _ ty_stack << u8 @@ -1974,6 +1892,7 @@ pub enum Bytecode { ty_stack << struct_ty "#] Pack(StructDefinitionIndex), + #[group = "struct"] #[static_operands = "[struct_inst_idx]"] #[description = "Generic version of `Pack`."] @@ -3051,81 +2970,132 @@ pub enum Bytecode { VecSwap(SignatureIndex), #[group = "closure"] - #[description = r#" - `ClosPack(fun, mask)` creates a closure for a given function handle as controlled by - the given `mask`. `mask` is a u64 bitset which describes which of the arguments - of `fun` are captured by the closure. - - If the function `fun` has type `|t1..tn|r`, then the following holds: - - - If `m` are the number of bits set in the mask, then `m <= n`, and the stack is - `[vm..v1] + stack`, and if `i` is the `j`th bit set in the mask, - then `vj` has type `ti`. - - type ti is not a reference. - - Thus the values on the stack must match the types in the function - signature which have the bit to be captured set in the mask. - - The type of the resulting value on the stack is derived from the types `|t1..tn|` - for which the bit is not set, which build the arguments of a function type - with `fun`'s result types. - - The `abilities` of this function type are derived from the inputs as follows. - First, take the intersection of the abilities of all captured arguments - with type `t1..tn`. Then intersect this with the abilities derived from the - function: a function handle has `drop` and `copy`, never has `key`, and only - `store` if the underlying function is public, and therefore cannot change - its signature. - - Notice that an implementation can derive the types of the captured arguments - at runtime from a closure value as long as the closure value stores the function - handle (or a derived form of it) and the mask, and the handle allows to lookup the - function's type at runtime. Then the same procedure as outlined above can be used. - "#] - #[static_operands = "[fun, mask]"] - #[semantics = ""] - #[runtime_check_epilogue = ""] - #[gas_type_creation_tier_0 = "closure_ty"] - ClosPack(FunctionHandleIndex, ClosureMask), + #[description = "Load a function value onto the stack."] + #[static_operands = "[func_handle_idx]"] + #[semantics = "stack << functions[function_handle_idx]"] + #[runtime_check_epilogue = "ty_stack << func_ty"] + #[gas_type_creation_tier_1 = "func_ty"] + LdFunction(FunctionHandleIndex), + + #[group = "closure"] + #[description = "Generic version of `LdFunction`."] + #[static_operands = "[func_inst_idx]"] + #[semantics = "See `LdFunction`."] + #[runtime_check_epilogue = "See `LdFunction`."] + #[gas_type_creation_tier_0 = "ty_args"] + #[gas_type_creation_tier_1 = "local_tys"] + LdFunctionGeneric(FunctionInstantiationIndex), #[group = "closure"] - #[static_operands = "[fun, mask]"] - #[semantics = ""] - #[runtime_check_epilogue = ""] #[description = r#" - Same as `ClosPack` but for the instantiation of a generic function. + `EarlyBind(|t1..tn|r with a, count)` creates new function value based + on the function value at top of stack by adding `count` arguments + popped from the stack to the closure found on top of stack. + + If the function value's type has at least `count` parameters with types + that match the `count` arguments on the stack, then a function closure + capturing those values with the provided function is pushed on top of + stack. - Notice that an uninstantiated generic function cannot be used to create a closure. + Notice that the type as part of this instruction is redundant for + execution semantics. Since the closure is expected to be on top of the stack, + it can decode the arguments underneath without type information. + However, the type is required for the current implementation of + static bytecode verification. + "#] + #[static_operands = "[u8_value]"] + #[semantics = r#" + stack >> function_handle + + let [func, k, [arg_0, .., arg_{k-1}]] = function_handle + // Information like the function signature are loaded from the file format + i = u8_value + n = func.num_params + if i + k > n then abort + + stack >> arg'_{i-1} + .. + stack >> arg'_0 + new_function_handle = [func, k + i, [arg_0, .., arg_{k-1}, arg'_0, .., arg'_{i-1}]] + stack << new_function_handle + "#] + #[runtime_check_epilogue = r#" + // NOT: assert func visibility rules + func param types match provided parameter types "#] #[gas_type_creation_tier_0 = "closure_ty"] - ClosPackGeneric(FunctionInstantiationIndex, ClosureMask), + EarlyBind(SignatureIndex, u8), #[group = "closure"] #[description = r#" - `ClosEval(|t1..tn|r has a)` evalutes a closure of the given function type, taking - the captured arguments and mixing in the provided ones on the stack. + `Invoke(|t1..tn|r with a)` calls a function value of the specified type, + with `n` argument values from the stack. On top of the stack is the closure being evaluated, underneath the arguments: `[c,vn,..,v1] + stack`. The type of the closure must match the type specified in the instruction, with abilities `a` a subset of the abilities of the closure value. A value `vi` on the stack must have type `ti`. - Notice that the type as part of the closure instruction is redundant for + Notice that the type as part of this instruction is redundant for execution semantics. Since the closure is expected to be on top of the stack, it can decode the arguments underneath without type information. - However, the type is required to do static bytecode verification. + However, the type is required for the current implementation of + static bytecode verification. - The semantics of this instruction can be characterized by the following equation: + The arguments are consumed and pushed to the locals of the function. + + Return values are pushed onto the stack from the first to the last and + available to the caller after returning from the callee. - ``` - CloseEval(ClosPack(f, mask, c1..cn), a1..am) = f(mask.compose(c1..cn, a1..am)) - ``` + During execution of the function conservative_invocation_mode is enabled; + afterwards, it is restored to its original state. "#] - #[static_operands = "[]"] - #[semantics = ""] - #[runtime_check_epilogue = ""] - #[gas_type_creation_tier_0 = "closure_ty"] - ClosEval(SignatureIndex), + #[semantics = r#" + stack >> function_handle + let [func, k, [arg_0, .., arg_{k-1}]] = function_handle + // Information like the function signature are loaded from the file format + // and compared with the signature parameter. + n = func.num_params + ty_args = if func.is_generic then func.ty_args else [] + + n = func.num_params + stack >> arg_{n-1} + .. + stack >> arg_k + + old_conservative_invocation_mode = conservative_invocation_mode + conservative_invocation_mode = true + + let module = func.module + if module is on invocation_stack then + abort + else + invocation_stack << module + + if func.is_native() + call_native(func.name, ty_args, args = [arg_0, .., arg_{n-1}]) + current_frame.pc += 1 + else + call_stack << current_frame + + current_frame = new_frame_from_func( + func, + ty_args, + locals = [arg_0, .., arg_n-1, invalid, ..] + // ^ other locals + ) + + invocation_stack >> _ + conservative_invocation_mode = old_conservative_invocation_mode + "#] + #[runtime_check_epilogue = r#" + // NOT: assert func visibility rules + for i in 0..#args: + ty_stack >> ty + assert ty == locals[#args - i - 1] + "#] + #[gas_type_creation_tier_1 = "closure_ty"] + Invoke(SignatureIndex), #[group = "stack_and_local"] #[description = "Push a u16 constant onto the stack."] @@ -3237,9 +3207,10 @@ impl ::std::fmt::Debug for Bytecode { Bytecode::UnpackGeneric(a) => write!(f, "UnpackGeneric({})", a), Bytecode::UnpackVariant(a) => write!(f, "UnpackVariant({})", a), Bytecode::UnpackVariantGeneric(a) => write!(f, "UnpackVariantGeneric({})", a), - Bytecode::ClosPackGeneric(a, mask) => write!(f, "ClosPackGeneric({}, {})", a, mask), - Bytecode::ClosPack(a, mask) => write!(f, "ClosPack({}, {})", a, mask), - Bytecode::ClosEval(a) => write!(f, "ClosEval({})", a), + Bytecode::LdFunction(a) => write!(f, "LdFunction({})", a), + Bytecode::LdFunctionGeneric(a) => write!(f, "LdFunctionGeneric({})", a), + Bytecode::EarlyBind(sig_idx, a) => write!(f, "EarlyBind({}, {})", sig_idx, a), + Bytecode::Invoke(sig_idx) => write!(f, "Invoke({})", sig_idx), Bytecode::ReadRef => write!(f, "ReadRef"), Bytecode::WriteRef => write!(f, "WriteRef"), Bytecode::FreezeRef => write!(f, "FreezeRef"), diff --git a/third_party/move/move-binary-format/src/file_format_common.rs b/third_party/move/move-binary-format/src/file_format_common.rs index 0201a408262a8..0627507bf1b71 100644 --- a/third_party/move/move-binary-format/src/file_format_common.rs +++ b/third_party/move/move-binary-format/src/file_format_common.rs @@ -299,6 +299,11 @@ pub enum Opcodes { UNPACK_VARIANT_GENERIC = 0x55, TEST_VARIANT = 0x56, TEST_VARIANT_GENERIC = 0x57, + // Closures + LD_FUNCTION = 0x58, + LD_FUNCTION_GENERIC = 0x59, + INVOKE = 0x5A, + EARLY_BIND = 0x5B, } /// Upper limit on the binary size @@ -790,9 +795,10 @@ pub fn instruction_key(instruction: &Bytecode) -> u8 { TestVariant(_) => Opcodes::TEST_VARIANT, TestVariantGeneric(_) => Opcodes::TEST_VARIANT_GENERIC, // Since bytecode version 8 - ClosPack(..) | ClosPackGeneric(..) | ClosEval(_) => { - unimplemented!("serialization of closure opcodes") - }, + LdFunction(_) => Opcodes::LD_FUNCTION, + LdFunctionGeneric(_) => Opcodes::LD_FUNCTION_GENERIC, + Invoke(_) => Opcodes::INVOKE, + EarlyBind(..) => Opcodes::EARLY_BIND, }; opcode as u8 } diff --git a/third_party/move/move-binary-format/src/serializer.rs b/third_party/move/move-binary-format/src/serializer.rs index d3bcbee4885d0..e6e66657c19a7 100644 --- a/third_party/move/move-binary-format/src/serializer.rs +++ b/third_party/move/move-binary-format/src/serializer.rs @@ -1095,9 +1095,25 @@ fn serialize_instruction_inner( binary.push(Opcodes::TEST_VARIANT_GENERIC as u8)?; serialize_struct_variant_inst_index(binary, class_idx) }, - Bytecode::ClosPack(..) | Bytecode::ClosPackGeneric(..) | Bytecode::ClosEval(_) => { - unimplemented!("serialization of closure opcodes") + + Bytecode::LdFunction(method_idx) => { + binary.push(Opcodes::LD_FUNCTION as u8)?; + serialize_function_handle_index(binary, method_idx) + }, + Bytecode::LdFunctionGeneric(method_idx) => { + binary.push(Opcodes::LD_FUNCTION_GENERIC as u8)?; + serialize_function_inst_index(binary, method_idx) + }, + Bytecode::Invoke(sig_idx) => { + binary.push(Opcodes::INVOKE as u8)?; + serialize_signature_index(binary, sig_idx) }, + Bytecode::EarlyBind(sig_idx, value) => { + binary.push(Opcodes::EARLY_BIND as u8)?; + serialize_signature_index(binary, sig_idx)?; + binary.push(*value) + }, + Bytecode::ReadRef => binary.push(Opcodes::READ_REF as u8), Bytecode::WriteRef => binary.push(Opcodes::WRITE_REF as u8), Bytecode::Add => binary.push(Opcodes::ADD as u8), diff --git a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs index c98e8317ee33f..934aca14890b7 100644 --- a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs +++ b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs @@ -498,7 +498,23 @@ impl<'a> ApplyCodeUnitBoundsContext<'a> { // TODO(#13806): implement panic!("Enum types bytecode NYI: {:?}", code[bytecode_idx]) }, - ClosPack(..) | ClosPackGeneric(..) | ClosEval(..) => { + LdFunction(_) => struct_bytecode!( + function_handles_len, + current_fdef, + bytecode_idx, + offset, + FunctionHandleIndex, + LdFunction + ), + LdFunctionGeneric(_) => struct_bytecode!( + function_inst_len, + current_fdef, + bytecode_idx, + offset, + FunctionInstantiationIndex, + LdFunctionGeneric + ), + Invoke(..) | EarlyBind(..) => { panic!("Closure bytecode NYI: {:?}", code[bytecode_idx]) }, }; @@ -560,10 +576,11 @@ fn is_interesting(bytecode: &Bytecode) -> bool { | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor | Shl | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | Abort | Nop => false, - ClosPack(..) - | ClosPackGeneric(..) - | ClosEval(..) - | PackVariant(_) + LdFunction(_) | LdFunctionGeneric(_) | Invoke(_) | EarlyBind(..) => { + // TODO(LAMBDA): implement + false + }, + PackVariant(_) | PackVariantGeneric(_) | UnpackVariant(_) | UnpackVariantGeneric(_) diff --git a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs index 0de40723ae24e..41d5282ac05ea 100644 --- a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs @@ -102,10 +102,15 @@ impl<'a> AcquiresVerifier<'a> { self.struct_acquire(si.def, offset) }, - Bytecode::ClosPack(..) - | Bytecode::ClosPackGeneric(..) - | Bytecode::ClosEval(_) - | Bytecode::Pop + Bytecode::LdFunction(idx) => self.ld_function_acquire(*idx, offset), + Bytecode::LdFunctionGeneric(idx) => { + let fi = self.module.function_instantiation_at(*idx); + self.ld_function_acquire(fi.handle, offset) + }, + Bytecode::EarlyBind(_sig_idx, _count) => Ok(()), + Bytecode::Invoke(_sig_idx) => self.invoke_acquire(offset), + + Bytecode::Pop | Bytecode::BrTrue(_) | Bytecode::BrFalse(_) | Bytecode::Abort @@ -205,6 +210,29 @@ impl<'a> AcquiresVerifier<'a> { Ok(()) } + fn ld_function_acquire( + &mut self, + fh_idx: FunctionHandleIndex, + offset: CodeOffset, + ) -> PartialVMResult<()> { + // Currenty we are disallowing acquires for any function value which + // is created, so Invoke does nothing with acquires. + // TODO(LAMBDA) In the future this may change. + let function_handle = self.module.function_handle_at(fh_idx); + let function_acquired_resources = self.function_acquired_resources(function_handle, fh_idx); + if !function_acquired_resources.is_empty() { + return Err(self.error(StatusCode::LD_FUNCTION_NONEMPTY_ACQUIRES, offset)); + } + Ok(()) + } + + fn invoke_acquire(&mut self, _offset: CodeOffset) -> PartialVMResult<()> { + // Currenty we are disallowing acquires for any function value which + // is created, so Invoke does nothing with acquires. + // TODO(LAMBDA) In the future this may change. + Ok(()) + } + fn struct_acquire( &mut self, sd_idx: StructDefinitionIndex, diff --git a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs index 9e170e7cefa63..fdba5d7033813 100644 --- a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs +++ b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs @@ -11,9 +11,9 @@ use move_binary_format::{ binary_views::BinaryIndexedView, errors::{Location, PartialVMError, PartialVMResult, VMResult}, file_format::{ - Bytecode, ClosureMask, CodeOffset, CodeUnit, CompiledModule, CompiledScript, - FieldHandleIndex, FunctionDefinitionIndex, FunctionHandleIndex, StructDefinitionIndex, - StructVariantHandleIndex, TableIndex, VariantFieldHandleIndex, + Bytecode, CodeOffset, CodeUnit, CompiledModule, CompiledScript, FieldHandleIndex, + FunctionDefinitionIndex, FunctionHandleIndex, SignatureIndex, SignatureToken, + StructDefinitionIndex, StructVariantHandleIndex, TableIndex, VariantFieldHandleIndex, }, }; use move_core_types::vm_status::StatusCode; @@ -91,21 +91,25 @@ impl<'a> InstructionConsistency<'a> { )?; }, Call(idx) => { - // Nothing to verify for `_mask`, so merge with Call self.check_function_op(offset, *idx, /* generic */ false)?; }, CallGeneric(idx) => { let func_inst = self.resolver.function_instantiation_at(*idx); self.check_function_op(offset, func_inst.handle, /* generic */ true)?; }, - ClosPack(idx, mask) => { - self.check_function_op(offset, *idx, /* generic */ false)?; - self.check_closure_mask(offset, *idx, *mask)? + LdFunction(idx) => { + self.check_ld_function_op(offset, *idx, /* generic */ false)?; }, - ClosPackGeneric(idx, mask) => { + LdFunctionGeneric(idx) => { let func_inst = self.resolver.function_instantiation_at(*idx); - self.check_function_op(offset, func_inst.handle, /* generic */ true)?; - self.check_closure_mask(offset, func_inst.handle, *mask)? + self.check_ld_function_op(offset, func_inst.handle, /* generic */ true)?; + }, + Invoke(sig_idx) => { + // reuse code to check for signature issues. + self.check_bind_count(offset, *sig_idx, 0)?; + }, + EarlyBind(sig_idx, count) => { + self.check_bind_count(offset, *sig_idx, *count)?; }, Pack(idx) | Unpack(idx) => { self.check_struct_op(offset, *idx, /* generic */ false)?; @@ -145,11 +149,11 @@ impl<'a> InstructionConsistency<'a> { // List out the other options explicitly so there's a compile error if a new // bytecode gets added. - ClosEval(_) | FreezeRef | Pop | Ret | Branch(_) | BrTrue(_) | BrFalse(_) - | LdU8(_) | LdU16(_) | LdU32(_) | LdU64(_) | LdU128(_) | LdU256(_) | LdConst(_) - | CastU8 | CastU16 | CastU32 | CastU64 | CastU128 | CastU256 | LdTrue | LdFalse - | ReadRef | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor | Shl - | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | CopyLoc(_) | MoveLoc(_) + FreezeRef | Pop | Ret | Branch(_) | BrTrue(_) | BrFalse(_) | LdU8(_) | LdU16(_) + | LdU32(_) | LdU64(_) | LdU128(_) | LdU256(_) | LdConst(_) | CastU8 | CastU16 + | CastU32 | CastU64 | CastU128 | CastU256 | LdTrue | LdFalse | ReadRef + | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor | Shl | Shr + | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | CopyLoc(_) | MoveLoc(_) | StLoc(_) | MutBorrowLoc(_) | ImmBorrowLoc(_) | VecLen(_) | VecImmBorrow(_) | VecMutBorrow(_) | VecPushBack(_) | VecPopBack(_) | VecSwap(_) | Abort | Nop => (), } @@ -222,7 +226,7 @@ impl<'a> InstructionConsistency<'a> { Ok(()) } - fn check_function_op( + fn check_ld_function_op( &self, offset: usize, func_handle_index: FunctionHandleIndex, @@ -238,16 +242,43 @@ impl<'a> InstructionConsistency<'a> { Ok(()) } - fn check_closure_mask( + fn check_function_op( &self, offset: usize, func_handle_index: FunctionHandleIndex, - mask: ClosureMask, + generic: bool, ) -> PartialVMResult<()> { let function_handle = self.resolver.function_handle_at(func_handle_index); - let signature = self.resolver.signature_at(function_handle.parameters); - if mask.max_captured() >= signature.len() { - return Err(PartialVMError::new(StatusCode::INVALID_CLOSURE_MASK) + if function_handle.type_parameters.is_empty() == generic { + return Err( + PartialVMError::new(StatusCode::GENERIC_MEMBER_OPCODE_MISMATCH) + .at_code_offset(self.current_function(), offset as CodeOffset), + ); + } + Ok(()) + } + + fn check_bind_count( + &self, + offset: usize, + sig_index: SignatureIndex, + count: u8, + ) -> PartialVMResult<()> { + let signature = self.resolver.signature_at(sig_index); + if let Some(sig_token) = signature.0.first() { + if let SignatureToken::Function(params, _returns, _abilities) = sig_token { + if count as usize > params.len() { + return Err( + PartialVMError::new(StatusCode::NUMBER_OF_TYPE_ARGUMENTS_MISMATCH) + .at_code_offset(self.current_function(), offset as CodeOffset), + ); + } + } else { + return Err(PartialVMError::new(StatusCode::REQUIRES_FUNCTION) + .at_code_offset(self.current_function(), offset as CodeOffset)); + } + } else { + return Err(PartialVMError::new(StatusCode::UNKNOWN_SIGNATURE_TYPE) .at_code_offset(self.current_function(), offset as CodeOffset)); } Ok(()) diff --git a/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs index f49c37ad812f1..006f102eedbb4 100644 --- a/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs @@ -125,9 +125,10 @@ fn execute_inner( | Bytecode::UnpackVariantGeneric(_) | Bytecode::TestVariant(_) | Bytecode::TestVariantGeneric(_) - | Bytecode::ClosPack(..) - | Bytecode::ClosPackGeneric(..) - | Bytecode::ClosEval(_) + | Bytecode::LdFunction(_) + | Bytecode::LdFunctionGeneric(_) + | Bytecode::Invoke(_) + | Bytecode::EarlyBind(..) | Bytecode::ReadRef | Bytecode::WriteRef | Bytecode::CastU8 diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs index 1f8fd1a6d04e9..e8bfe35827a0f 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs @@ -568,7 +568,22 @@ impl AbstractState { Ok(return_values) } - pub fn clos_eval( + pub fn ld_function( + &mut self, + offset: CodeOffset, + acquired_resources: &BTreeSet, + _meter: &mut impl Meter, + ) -> PartialVMResult { + if !acquired_resources.is_empty() { + // TODO(LAMBDA): Currently acquires must be empty unless we disallow + // Invoke to call to functions defined in the same module. + return Err(self.error(StatusCode::INVALID_ACQUIRES_ANNOTATION, offset)); + } + // TODO(LAMBDA): Double-check that we don't need meter adjustments here. + Ok(AbstractValue::NonReference) + } + + pub fn invoke( &mut self, offset: CodeOffset, arguments: Vec, diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs index b5708b461e5b8..d8426892e30cf 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs @@ -22,9 +22,8 @@ use move_binary_format::{ binary_views::{BinaryIndexedView, FunctionView}, errors::{PartialVMError, PartialVMResult}, file_format::{ - Bytecode, ClosureMask, CodeOffset, FunctionDefinitionIndex, FunctionHandle, - IdentifierIndex, SignatureIndex, SignatureToken, StructDefinition, StructVariantHandle, - VariantIndex, + Bytecode, CodeOffset, FunctionDefinitionIndex, FunctionHandle, IdentifierIndex, + SignatureIndex, SignatureToken, StructDefinition, StructVariantHandle, VariantIndex, }, safe_assert, safe_unwrap, views::FieldOrVariantIndex, @@ -101,24 +100,47 @@ fn call( Ok(()) } -fn clos_pack( +fn ld_function( verifier: &mut ReferenceSafetyAnalysis, + state: &mut AbstractState, + offset: CodeOffset, function_handle: &FunctionHandle, - mask: ClosureMask, + meter: &mut impl Meter, ) -> PartialVMResult<()> { - let parameters = verifier.resolver.signature_at(function_handle.parameters); - // Extract the captured arguments and pop them from the stack - let argc = mask.extract(¶meters.0, true).len(); - for _ in 0..argc { + let _parameters = verifier.resolver.signature_at(function_handle.parameters); + let acquired_resources = match verifier.name_def_map.get(&function_handle.name) { + Some(idx) => { + let func_def = verifier.resolver.function_def_at(*idx)?; + let fh = verifier.resolver.function_handle_at(func_def.function); + if function_handle == fh { + func_def.acquires_global_resources.iter().cloned().collect() + } else { + BTreeSet::new() + } + }, + None => BTreeSet::new(), + }; + let value = state.ld_function(offset, &acquired_resources, meter)?; + verifier.stack.push(value); + Ok(()) +} + +fn early_bind( + verifier: &mut ReferenceSafetyAnalysis, + _arg_tys: Vec, + k: u8, +) -> PartialVMResult<()> { + safe_assert!(safe_unwrap!(verifier.stack.pop()).is_value()); + for _ in 0..k { // Currently closures require captured arguments to be values. This is verified // by type safety. - safe_assert!(safe_unwrap!(verifier.stack.pop()).is_value()) + safe_assert!(safe_unwrap!(verifier.stack.pop()).is_value()); } verifier.stack.push(AbstractValue::NonReference); Ok(()) } -fn clos_eval( +fn invoke( verifier: &mut ReferenceSafetyAnalysis, state: &mut AbstractState, offset: CodeOffset, @@ -131,7 +153,7 @@ fn clos_eval( .map(|_| verifier.stack.pop().unwrap()) .rev() .collect(); - let values = state.clos_eval(offset, arguments, &result_tys, meter)?; + let values = state.invoke(offset, arguments, &result_tys, meter)?; for value in values { verifier.stack.push(value) } @@ -547,18 +569,22 @@ fn execute_inner( unpack_variant(verifier, handle)? }, - Bytecode::ClosPack(idx, mask) => { + Bytecode::LdFunction(idx) => { let function_handle = verifier.resolver.function_handle_at(*idx); - clos_pack(verifier, function_handle, *mask)? + ld_function(verifier, state, offset, function_handle, meter)? }, - Bytecode::ClosPackGeneric(idx, mask) => { + Bytecode::LdFunctionGeneric(idx) => { let func_inst = verifier.resolver.function_instantiation_at(*idx); let function_handle = verifier.resolver.function_handle_at(func_inst.handle); - clos_pack(verifier, function_handle, *mask)? + ld_function(verifier, state, offset, function_handle, meter)? + }, + Bytecode::EarlyBind(sig_idx, k) => { + let (arg_tys, _result_tys) = fun_type(verifier, *sig_idx)?; + early_bind(verifier, arg_tys, *k)? }, - Bytecode::ClosEval(idx) => { - let (arg_tys, result_tys) = fun_type(verifier, *idx)?; - clos_eval(verifier, state, offset, arg_tys, result_tys, meter)? + Bytecode::Invoke(sig_idx) => { + let (arg_tys, result_tys) = fun_type(verifier, *sig_idx)?; + invoke(verifier, state, offset, arg_tys, result_tys, meter)? }, Bytecode::VecPack(idx, num) => { diff --git a/third_party/move/move-bytecode-verifier/src/signature.rs b/third_party/move/move-bytecode-verifier/src/signature.rs index 08dfe5d31ef73..e2c7068d56573 100644 --- a/third_party/move/move-bytecode-verifier/src/signature.rs +++ b/third_party/move/move-bytecode-verifier/src/signature.rs @@ -260,7 +260,7 @@ impl<'a> SignatureChecker<'a> { }, // Closure operations not supported by legacy signature checker - ClosPack(..) | ClosPackGeneric(..) | ClosEval(_) => { + LdFunction(..) | LdFunctionGeneric(..) | Invoke(_) | EarlyBind(..) => { return Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("closure operations not supported".to_owned())) }, diff --git a/third_party/move/move-bytecode-verifier/src/signature_v2.rs b/third_party/move/move-bytecode-verifier/src/signature_v2.rs index c38c9723cfd33..953c26005d921 100644 --- a/third_party/move/move-bytecode-verifier/src/signature_v2.rs +++ b/third_party/move/move-bytecode-verifier/src/signature_v2.rs @@ -820,6 +820,8 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { let mut checked_struct_def_insts = BTreeMap::::new(); let mut checked_struct_def_insts_with_key = BTreeMap::::new(); + let mut checked_fun_insts = BTreeMap::::new(); + let mut checked_early_bind_insts = BTreeMap::<(SignatureIndex, u8), ()>::new(); let mut checked_vec_insts = BTreeMap::::new(); let mut checked_field_insts = BTreeMap::::new(); let mut checked_variant_field_insts = BTreeMap::::new(); @@ -839,7 +841,7 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { }) }; match instr { - CallGeneric(idx) | ClosPackGeneric(idx, _) => { + CallGeneric(idx) | LdFunctionGeneric(idx) => { if let btree_map::Entry::Vacant(entry) = checked_func_insts.entry(*idx) { let constraints = self.verify_function_instantiation_contextless(*idx)?; map_err(constraints.check_in_context(&ability_context))?; @@ -898,12 +900,32 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { entry.insert(()); } }, - ClosEval(idx) => { - let sign = self.resolver.signature_at(*idx); - if sign.len() != 1 || !matches!(&sign.0[0], SignatureToken::Function(..)) { - return map_err(Err(PartialVMError::new( - StatusCode::CLOSURE_EVAL_REQUIRES_FUNCTION, - ))); + Invoke(idx) => map_err(self.verify_fun_sig_idx( + *idx, + &mut checked_fun_insts, + &ability_context, + ))?, + EarlyBind(idx, count) => { + map_err(self.verify_fun_sig_idx( + *idx, + &mut checked_fun_insts, + &ability_context, + ))?; + if let btree_map::Entry::Vacant(entry) = + checked_early_bind_insts.entry((*idx, *count)) + { + // Note non-function case is checked in `verify_fun_sig_idx` above. + if let Some(SignatureToken::Function(params, _results, _abilities)) = + self.resolver.signature_at(*idx).0.first() + { + if *count as usize > params.len() { + return map_err(Err(PartialVMError::new( + StatusCode::NUMBER_OF_ARGUMENTS_MISMATCH, + ) + .with_message("in EarlyBind".to_string()))); + }; + }; + entry.insert(()); } }, VecPack(idx, _) @@ -961,7 +983,7 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { | LdTrue | LdFalse | Call(_) - | ClosPack(..) + | LdFunction(..) | Pack(_) | Unpack(_) | TestVariant(_) @@ -1119,6 +1141,37 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { } Ok(()) } + + // Checks that a `sig_idx` parameter to `Invoke` or `EarlyBind` is well-formed. + fn verify_fun_sig_idx( + &self, + idx: SignatureIndex, + checked_fun_insts: &mut BTreeMap, + ability_context: &BitsetTypeParameterConstraints, + ) -> PartialVMResult<()> { + if let btree_map::Entry::Vacant(entry) = checked_fun_insts.entry(idx) { + let ty_args = &self.resolver.signature_at(idx).0; + if ty_args.len() != 1 { + return Err(PartialVMError::new( + StatusCode::NUMBER_OF_TYPE_ARGUMENTS_MISMATCH, + )) + .map_err(|err| { + err.with_message(format!( + "expected 1 type token for function operations, got {}", + ty_args.len() + )) + }); + } + if !ty_args[0].is_function() { + return Err(PartialVMError::new(StatusCode::INVALID_SIGNATURE_TOKEN)) + .map_err(|err| err.with_message("function required".to_string())); + } + self.verify_signature_in_context(ability_context, idx)?; + + entry.insert(()); + }; + Ok(()) + } } fn verify_module_impl(module: &CompiledModule) -> PartialVMResult<()> { diff --git a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs index 427b6d88e01d7..bb86abdcc346a 100644 --- a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs @@ -228,13 +228,24 @@ impl<'a> StackUsageVerifier<'a> { (arg_count, return_count) }, - // ClosEval pops the number of arguments and pushes the results of the given function - // type - Bytecode::ClosEval(idx) => { - if let Some(SignatureToken::Function(args, result, _)) = + // LdFunction pushes the function handle + Bytecode::LdFunction(idx) => { + let _function_handle = self.resolver.function_handle_at(*idx); + (0, 1) + }, + Bytecode::LdFunctionGeneric(idx) => { + let func_inst = self.resolver.function_instantiation_at(*idx); + let _function_handle = self.resolver.function_handle_at(func_inst.handle); + (0, 1) + }, + + // Invoke pops a function and the number of arguments and pushes the results of the + // given function type + Bytecode::Invoke(idx) => { + if let Some(SignatureToken::Function(args, results, _)) = self.resolver.signature_at(*idx).0.first() { - ((1 + args.len()) as u64, result.len() as u64) + ((1 + args.len()) as u64, results.len() as u64) } else { // We don't know what it will pop/push, but the signature checker // ensures we never reach this @@ -242,27 +253,23 @@ impl<'a> StackUsageVerifier<'a> { } }, - // ClosPack pops the captured arguments and returns 1 value - Bytecode::ClosPack(idx, mask) => { - let function_handle = self.resolver.function_handle_at(*idx); - let arg_count = mask - .extract( - &self.resolver.signature_at(function_handle.parameters).0, - true, - ) - .len() as u64; - (arg_count, 1) - }, - Bytecode::ClosPackGeneric(idx, mask) => { - let func_inst = self.resolver.function_instantiation_at(*idx); - let function_handle = self.resolver.function_handle_at(func_inst.handle); - let arg_count = mask - .extract( - &self.resolver.signature_at(function_handle.parameters).0, - true, - ) - .len() as u64; - (arg_count, 1) + // EarlyBind pops a function value and the captured arguments and returns 1 value + Bytecode::EarlyBind(idx, arg_count) => { + if let Some(SignatureToken::Function(args, _results, _)) = + self.resolver.signature_at(*idx).0.first() + { + if args.len() <= *arg_count as usize { + (1 + *arg_count as u64, 1) + } else { + // We don't know what it will pop/push, but the signature checker + // ensures we never reach this + (0, 0) + } + } else { + // We don't know what it will pop/push, but the signature checker + // ensures we never reach this + (0, 0) + } }, // Pack performs `num_fields` pops and one push diff --git a/third_party/move/move-bytecode-verifier/src/type_safety.rs b/third_party/move/move-bytecode-verifier/src/type_safety.rs index cb6a03aada557..66b8dd4bced7d 100644 --- a/third_party/move/move-bytecode-verifier/src/type_safety.rs +++ b/third_party/move/move-bytecode-verifier/src/type_safety.rs @@ -11,10 +11,10 @@ use move_binary_format::{ control_flow_graph::ControlFlowGraph, errors::{PartialVMError, PartialVMResult}, file_format::{ - Ability, AbilitySet, Bytecode, ClosureMask, CodeOffset, FunctionDefinitionIndex, - FunctionHandle, FunctionHandleIndex, LocalIndex, Signature, SignatureToken, - SignatureToken as ST, StructDefinition, StructDefinitionIndex, StructFieldInformation, - StructHandleIndex, VariantIndex, Visibility, + AbilitySet, Bytecode, CodeOffset, FunctionDefinitionIndex, FunctionHandle, + FunctionHandleIndex, LocalIndex, Signature, SignatureToken, SignatureToken as ST, + StructDefinition, StructDefinitionIndex, StructFieldInformation, StructHandleIndex, + VariantIndex, Visibility, }, safe_assert, safe_unwrap, views::FieldOrVariantIndex, @@ -301,7 +301,7 @@ fn call( Ok(()) } -fn clos_eval( +fn invoke( verifier: &mut TypeSafetyChecker, meter: &mut impl Meter, offset: CodeOffset, @@ -312,6 +312,7 @@ fn clos_eval( safe_assert!(false); unreachable!() }; + // On top of the stack is the closure, pop it. let closure_ty = safe_unwrap!(verifier.stack.pop()); // Verify that the closure type matches the expected type @@ -331,6 +332,7 @@ fn clos_eval( .error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset) .with_message("closure ability mismatch".to_owned())); } + // Pop and verify arguments for param_ty in param_tys.iter().rev() { let arg_ty = safe_unwrap!(verifier.stack.pop()); @@ -344,36 +346,14 @@ fn clos_eval( Ok(()) } -fn clos_pack( +fn ld_function( verifier: &mut TypeSafetyChecker, meter: &mut impl Meter, - offset: CodeOffset, - func_handle_idx: FunctionHandleIndex, + _offset: CodeOffset, + function_handle_idx: &FunctionHandleIndex, type_actuals: &Signature, - mask: ClosureMask, ) -> PartialVMResult<()> { - let func_handle = verifier.resolver.function_handle_at(func_handle_idx); - // Check the captured arguments on the stack - let param_sign = verifier.resolver.signature_at(func_handle.parameters); - let captured_param_tys = mask.extract(¶m_sign.0, true); - let mut abilities = AbilitySet::ALL; - for ty in captured_param_tys.iter().rev() { - abilities = abilities.intersect(verifier.abilities(ty)?); - let arg = safe_unwrap!(verifier.stack.pop()); - if (type_actuals.is_empty() && &arg != ty) - || (!type_actuals.is_empty() && arg != instantiate(ty, type_actuals)) - { - return Err(verifier - .error(StatusCode::PACK_TYPE_MISMATCH_ERROR, offset) - .with_message("captured argument type mismatch".to_owned())); - } - // A captured argument must not be a reference - if ty.is_reference() { - return Err(verifier - .error(StatusCode::PACK_TYPE_MISMATCH_ERROR, offset) - .with_message("captured argument must not be a reference".to_owned())); - } - } + let func_handle = verifier.resolver.function_handle_at(*function_handle_idx); // In order to determine whether this closure can be storable, we need to figure whether // this function is public. @@ -385,7 +365,7 @@ fn clos_pack( // and adding visibility there. let mut is_storable = false; for fun_def in verifier.resolver.function_defs().unwrap_or(&[]) { - if fun_def.function == func_handle_idx { + if fun_def.function == *function_handle_idx { // Function defined in this module, so we can check visibility. if fun_def.visibility == Visibility::Public { is_storable = true; @@ -393,23 +373,80 @@ fn clos_pack( break; } } - if !is_storable { - abilities.remove(Ability::Store); - } - abilities.remove(Ability::Key); + let abilities = if is_storable { + AbilitySet::PUBLIC_FUNCTIONS + } else { + AbilitySet::PRIVATE_FUNCTIONS + }; // Construct the resulting function type - let not_captured_param_tys = mask.extract(¶m_sign.0, false); + let parameters = verifier.resolver.signature_at(func_handle.parameters); let ret_sign = verifier.resolver.signature_at(func_handle.return_); verifier.push( meter, instantiate( - &SignatureToken::Function(not_captured_param_tys, ret_sign.0.to_vec(), abilities), + &SignatureToken::Function(parameters.0.to_vec(), ret_sign.0.to_vec(), abilities), type_actuals, ), ) } +fn early_bind( + verifier: &mut TypeSafetyChecker, + meter: &mut impl Meter, + offset: CodeOffset, + expected_ty: &SignatureToken, + count: u8, +) -> PartialVMResult<()> { + let count = count as usize; + let SignatureToken::Function(param_tys, ret_tys, abilities) = expected_ty else { + // The signature checker has ensured this is a function + safe_assert!(false); + unreachable!() + }; + + // On top of the stack is the closure, pop it. + let closure_ty = safe_unwrap!(verifier.stack.pop()); + // Verify that the closure type matches the expected type + if &closure_ty != expected_ty { + return Err(verifier + .error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset) + .with_message("closure type mismatch".to_owned())); + } + // Verify that the abilities match + let SignatureToken::Function(_, _, closure_abilities) = closure_ty else { + // Ensured above, but never panic + safe_assert!(false); + unreachable!() + }; + if !abilities.is_subset(closure_abilities) { + return Err(verifier + .error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset) + .with_message("closure ability mismatch".to_owned())); + } + + if param_tys.len() < count { + return Err(verifier.error(StatusCode::NUMBER_OF_TYPE_ARGUMENTS_MISMATCH, offset)); + } + + let binding_param_tys = ¶m_tys[0..count]; + let remaining_param_tys = ¶m_tys[count..]; + + // Pop and verify arguments + for param_ty in binding_param_tys.iter().rev() { + let arg_ty = safe_unwrap!(verifier.stack.pop()); + if &arg_ty != param_ty { + return Err(verifier.error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset)); + } + } + let result_ty = SignatureToken::Function( + (*remaining_param_tys).to_vec(), + ret_tys.to_vec(), + *abilities, + ); + verifier.push(meter, result_ty) +} + fn type_fields_signature( verifier: &mut TypeSafetyChecker, _meter: &mut impl Meter, // TODO: metering @@ -835,19 +872,25 @@ fn verify_instr( call(verifier, meter, offset, func_handle, type_args)? }, - Bytecode::ClosPack(idx, mask) => { - clos_pack(verifier, meter, offset, *idx, &Signature(vec![]), *mask)? - }, - Bytecode::ClosPackGeneric(idx, mask) => { + Bytecode::LdFunction(idx) => ld_function(verifier, meter, offset, idx, &Signature(vec![]))?, + + Bytecode::LdFunctionGeneric(idx) => { let func_inst = verifier.resolver.function_instantiation_at(*idx); let type_args = &verifier.resolver.signature_at(func_inst.type_parameters); verifier.charge_tys(meter, &type_args.0)?; - clos_pack(verifier, meter, offset, func_inst.handle, type_args, *mask)? + ld_function(verifier, meter, offset, &func_inst.handle, type_args)? }, - Bytecode::ClosEval(idx) => { + + Bytecode::EarlyBind(idx, count) => { + // The signature checker has verified this is a function type. + let expected_ty = safe_unwrap!(verifier.resolver.signature_at(*idx).0.first()); + early_bind(verifier, meter, offset, expected_ty, *count)? + }, + + Bytecode::Invoke(idx) => { // The signature checker has verified this is a function type. let expected_ty = safe_unwrap!(verifier.resolver.signature_at(*idx).0.first()); - clos_eval(verifier, meter, offset, expected_ty)? + invoke(verifier, meter, offset, expected_ty)? }, Bytecode::Pack(idx) => { diff --git a/third_party/move/move-core/types/src/vm_status.rs b/third_party/move/move-core/types/src/vm_status.rs index a33744afd0009..9aaccff1feecd 100644 --- a/third_party/move/move-core/types/src/vm_status.rs +++ b/third_party/move/move-core/types/src/vm_status.rs @@ -734,16 +734,14 @@ pub enum StatusCode { ZERO_VARIANTS_ERROR = 1130, // A feature is not enabled. FEATURE_NOT_ENABLED = 1131, - // Closure mask invalid - INVALID_CLOSURE_MASK = 1132, - // Closure eval type is not a function - CLOSURE_EVAL_REQUIRES_FUNCTION = 1133, + // Invoke or EarlyBind parameter type is not a function + REQUIRES_FUNCTION = 1132, // Reserved error code for future use - RESERVED_VERIFICATION_ERROR_2 = 1134, - RESERVED_VERIFICATION_ERROR_3 = 1135, - RESERVED_VERIFICATION_ERROR_4 = 1136, - RESERVED_VERIFICATION_ERROR_5 = 1137, + RESERVED_VERIFICATION_ERROR_2 = 1133, + RESERVED_VERIFICATION_ERROR_3 = 1134, + RESERVED_VERIFICATION_ERROR_4 = 1135, + RESERVED_VERIFICATION_ERROR_5 = 1136, // These are errors that the VM might raise if a violation of internal // invariants takes place. @@ -784,13 +782,14 @@ pub enum StatusCode { // Should never be committed on chain SPECULATIVE_EXECUTION_ABORT_ERROR = 2024, ACCESS_CONTROL_INVARIANT_VIOLATION = 2025, + LD_FUNCTION_NONEMPTY_ACQUIRES = 2026, // Reserved error code for future use - RESERVED_INVARIANT_VIOLATION_ERROR_1 = 2026, - RESERVED_INVARIANT_VIOLATION_ERROR_2 = 2027, - RESERVED_INVARIANT_VIOLATION_ERROR_3 = 2028, - RESERVED_INVARIANT_VIOLATION_ERROR_4 = 2039, - RESERVED_INVARIANT_VIOLATION_ERROR_5 = 2040, + RESERVED_INVARIANT_VIOLATION_ERROR_1 = 2027, + RESERVED_INVARIANT_VIOLATION_ERROR_2 = 2028, + RESERVED_INVARIANT_VIOLATION_ERROR_3 = 2039, + RESERVED_INVARIANT_VIOLATION_ERROR_4 = 2040, + RESERVED_INVARIANT_VIOLATION_ERROR_5 = 2041, // Errors that can arise from binary decoding (deserialization) // Deserialization Errors: 3000-3999 diff --git a/third_party/move/move-model/bytecode/src/function_target_pipeline.rs b/third_party/move/move-model/bytecode/src/function_target_pipeline.rs index 4fd4d5b0e0485..187a92e907d1f 100644 --- a/third_party/move/move-model/bytecode/src/function_target_pipeline.rs +++ b/third_party/move/move-model/bytecode/src/function_target_pipeline.rs @@ -353,7 +353,7 @@ impl FunctionTargetPipeline { /// Build the call graph. /// Nodes of this call graph are qualified function ids. - /// An edge A -> B in the call graph means that function A calls function B. + /// An edge A -> B in the call graph means that function A calls function B or refers to function B. fn build_call_graph( env: &GlobalEnv, targets: &FunctionTargetsHolder, @@ -369,7 +369,7 @@ impl FunctionTargetPipeline { let fun_env = env.get_function(fun_id); for callee in fun_env .get_used_functions() - .expect("called functions must be computed") + .expect("used functions must be computed") { let dst_idx = nodes .get(callee) diff --git a/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs b/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs index 0b95bbc1cfd93..f83916b3b4e87 100644 --- a/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs +++ b/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs @@ -293,9 +293,10 @@ impl<'a> StacklessBytecodeGenerator<'a> { }; match bytecode { - MoveBytecode::ClosPack(..) - | MoveBytecode::ClosPackGeneric(..) - | MoveBytecode::ClosEval(..) => { + MoveBytecode::Invoke(..) + | MoveBytecode::LdFunction(..) + | MoveBytecode::LdFunctionGeneric(..) + | MoveBytecode::EarlyBind(..) => { unimplemented!("stackless bytecode generation for closure opcodes") }, MoveBytecode::Pop => { diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index 1acba496f4924..817deae344dce 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -1537,10 +1537,11 @@ impl Frame { )?; match instruction { - // TODO: implement closures - Bytecode::ClosPack(..) - | Bytecode::ClosPackGeneric(..) - | Bytecode::ClosEval(..) => { + // TODO(LAMBDA): implement closures + Bytecode::LdFunction(..) + | Bytecode::LdFunctionGeneric(..) + | Bytecode::Invoke(..) + | Bytecode::EarlyBind(..) => { return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("closure opcodes in interpreter".to_owned())) }, diff --git a/third_party/move/move-vm/runtime/src/runtime_type_checks.rs b/third_party/move/move-vm/runtime/src/runtime_type_checks.rs index 21bf2903f7dc0..a8a87dbc26136 100644 --- a/third_party/move/move-vm/runtime/src/runtime_type_checks.rs +++ b/third_party/move/move-vm/runtime/src/runtime_type_checks.rs @@ -120,12 +120,14 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck { instruction: &Bytecode, ) -> PartialVMResult<()> { match instruction { - // TODO: implement closures - Bytecode::ClosPack(..) | Bytecode::ClosPackGeneric(..) | Bytecode::ClosEval(..) => { + // TODO(LAMBDA): implement closures + Bytecode::LdFunction(..) + | Bytecode::LdFunctionGeneric(..) + | Bytecode::Invoke(..) + | Bytecode::EarlyBind(..) => { return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("closure opcodes in interpreter".to_owned())) }, - // Call instruction will be checked at execute_main. Bytecode::Call(_) | Bytecode::CallGeneric(_) => (), Bytecode::BrFalse(_) | Bytecode::BrTrue(_) => { @@ -254,11 +256,13 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck { match instruction { // TODO: implement closures - Bytecode::ClosPack(..) | Bytecode::ClosPackGeneric(..) | Bytecode::ClosEval(..) => { + Bytecode::LdFunction(..) + | Bytecode::LdFunctionGeneric(..) + | Bytecode::Invoke(..) + | Bytecode::EarlyBind(..) => { return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("closure opcodes in interpreter".to_owned())) }, - Bytecode::BrTrue(_) | Bytecode::BrFalse(_) => (), Bytecode::Branch(_) | Bytecode::Ret 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..be4683e92f7d1 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 @@ -703,6 +703,13 @@ pub fn zero_cost_instruction_table() -> Vec<(Bytecode, GasCost)> { (CastU16, GasCost::new(0, 0)), (CastU32, GasCost::new(0, 0)), (CastU256, GasCost::new(0, 0)), + (LdFunction(FunctionHandleIndex::new(0)), GasCost::new(0, 0)), + ( + LdFunctionGeneric(FunctionInstantiationIndex::new(0)), + GasCost::new(0, 0), + ), + (Invoke(SignatureIndex::new(0)), GasCost::new(0, 0)), + (EarlyBind(SignatureIndex::new(0), 0u8), GasCost::new(0, 0)), ] } @@ -876,13 +883,23 @@ pub fn bytecode_instruction_costs() -> Vec<(Bytecode, GasCost)> { (CastU16, GasCost::new(2, 1)), (CastU32, GasCost::new(2, 1)), (CastU256, GasCost::new(2, 1)), + ( + LdFunction(FunctionHandleIndex::new(0)), + GasCost::new(1132, 1), + ), + ( + LdFunctionGeneric(FunctionInstantiationIndex::new(0)), + GasCost::new(1132, 1), + ), + (Invoke(SignatureIndex::new(0)), GasCost::new(1132, 1)), + ( + EarlyBind(SignatureIndex::new(0), 0u8), + GasCost::new(1132, 1), + ), ] } pub static INITIAL_COST_SCHEDULE: Lazy = Lazy::new(|| { - let mut instrs = bytecode_instruction_costs(); - // Note that the DiemVM is expecting the table sorted by instruction order. - instrs.sort_by_key(|cost| instruction_key(&cost.0)); - + let instrs = bytecode_instruction_costs(); new_from_instructions(instrs) }); diff --git a/third_party/move/tools/move-disassembler/src/disassembler.rs b/third_party/move/tools/move-disassembler/src/disassembler.rs index e3148cea698cb..a0d9e15417156 100644 --- a/third_party/move/tools/move-disassembler/src/disassembler.rs +++ b/third_party/move/tools/move-disassembler/src/disassembler.rs @@ -644,9 +644,103 @@ impl<'a> Disassembler<'a> { default_location: &Loc, ) -> Result { match instruction { - Bytecode::ClosPack(..) | Bytecode::ClosPackGeneric(..) | Bytecode::ClosEval(..) => { - bail!("closure opcodes not implemented") + Bytecode::LdFunction(method_idx) => { + let function_handle = self.source_mapper.bytecode.function_handle_at(*method_idx); + let module_handle = self + .source_mapper + .bytecode + .module_handle_at(function_handle.module); + let fcall_name = self.get_function_string(module_handle, function_handle); + let type_arguments = self + .source_mapper + .bytecode + .signature_at(function_handle.parameters) + .0 + .iter() + .map(|sig_tok| self.disassemble_sig_tok(sig_tok.clone(), &[])) + .collect::>>()? + .join(", "); + let type_rets = self + .source_mapper + .bytecode + .signature_at(function_handle.return_) + .0 + .iter() + .map(|sig_tok| self.disassemble_sig_tok(sig_tok.clone(), &[])) + .collect::>>()?; + Ok(format!( + "LdFunction {}({}){}", + fcall_name, + type_arguments, + Self::format_ret_type(&type_rets) + )) + }, + Bytecode::LdFunctionGeneric(method_idx) => { + let func_inst = self + .source_mapper + .bytecode + .function_instantiation_at(*method_idx); + let function_handle = self + .source_mapper + .bytecode + .function_handle_at(func_inst.handle); + let module_handle = self + .source_mapper + .bytecode + .module_handle_at(function_handle.module); + let fcall_name = self.get_function_string(module_handle, function_handle); + let ty_params = self + .source_mapper + .bytecode + .signature_at(func_inst.type_parameters) + .0 + .iter() + .map(|sig_tok| { + Ok(( + self.disassemble_sig_tok( + sig_tok.clone(), + &function_source_map.type_parameters, + )?, + *default_location, + )) + }) + .collect::>>()?; + let type_arguments = self + .source_mapper + .bytecode + .signature_at(function_handle.parameters) + .0 + .iter() + .map(|sig_tok| self.disassemble_sig_tok(sig_tok.clone(), &ty_params)) + .collect::>>()? + .join(", "); + let type_rets = self + .source_mapper + .bytecode + .signature_at(function_handle.return_) + .0 + .iter() + .map(|sig_tok| self.disassemble_sig_tok(sig_tok.clone(), &ty_params)) + .collect::>>()?; + Ok(format!( + "LdFunctionGeneric {}{}({}){}", + fcall_name, + Self::format_type_params( + &ty_params.into_iter().map(|(s, _)| s).collect::>() + ), + type_arguments, + Self::format_ret_type(&type_rets) + )) + }, + Bytecode::Invoke(idx) => { + let _signature = self.source_mapper.bytecode.signature_at(*idx); + Ok(format!("Invoke({})", idx)) }, + Bytecode::EarlyBind(idx, count) => { + let _signature = self.source_mapper.bytecode.signature_at(*idx); + Ok(format!("EarlyBind({}, {})", idx, count)) + }, + Bytecode::LdConst(idx) => { let constant = self.source_mapper.bytecode.constant_at(*idx); Ok(format!( From bfc73ac28425231d9370872385e9fe52d5d744fb Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Wed, 27 Nov 2024 02:48:51 -0800 Subject: [PATCH 3/9] rename EarlyBind and Invoke operations to EarlyBindFunction and InvokeFunction based on George's suggestion --- .../move-binary-format/src/check_bounds.rs | 4 ++-- .../src/check_complexity.rs | 4 ++-- .../move-binary-format/src/deserializer.rs | 9 +++---- .../move-binary-format/src/file_format.rs | 14 ++++++----- .../src/file_format_common.rs | 8 +++---- .../move/move-binary-format/src/serializer.rs | 8 +++---- .../invalid-mutations/src/bounds/code_unit.rs | 4 ++-- .../src/acquires_list_verifier.rs | 8 +++---- .../src/instruction_consistency.rs | 4 ++-- .../src/locals_safety/mod.rs | 4 ++-- .../src/reference_safety/abstract_state.rs | 4 ++-- .../src/reference_safety/mod.rs | 14 +++++------ .../move-bytecode-verifier/src/signature.rs | 5 +++- .../src/signature_v2.rs | 8 +++---- .../src/stack_usage_verifier.rs | 8 +++---- .../move-bytecode-verifier/src/type_safety.rs | 12 +++++----- .../src/bytecode_generator.rs | 4 ++-- .../src/env_pipeline/ast_simplifier.rs | 2 +- .../src/env_pipeline/lambda_lifter.rs | 24 ++++++++++--------- third_party/move/move-compiler-v2/src/lib.rs | 3 ++- .../move/move-core/types/src/vm_status.rs | 2 +- .../src/stackless_bytecode_generator.rs | 4 ++-- third_party/move/move-model/src/ast.rs | 20 ++++++++-------- .../move-model/src/builder/exp_builder.rs | 6 ++--- .../move/move-model/src/exp_rewriter.rs | 4 ++-- .../move/move-model/src/pureness_checker.rs | 2 +- third_party/move/move-model/src/sourcifier.rs | 4 ++-- .../boogie-backend/src/spec_translator.rs | 9 ++++--- .../move/move-vm/runtime/src/interpreter.rs | 4 ++-- .../runtime/src/runtime_type_checks.rs | 8 +++---- .../move-vm/test-utils/src/gas_schedule.rs | 14 +++++++---- .../move-disassembler/src/disassembler.rs | 8 +++---- 32 files changed, 127 insertions(+), 109 deletions(-) diff --git a/third_party/move/move-binary-format/src/check_bounds.rs b/third_party/move/move-binary-format/src/check_bounds.rs index 2e9f40388d969..1992a22e47968 100644 --- a/third_party/move/move-binary-format/src/check_bounds.rs +++ b/third_party/move/move-binary-format/src/check_bounds.rs @@ -658,8 +658,8 @@ impl<'a> BoundsChecker<'a> { | VecPopBack(idx) | VecUnpack(idx, _) | VecSwap(idx) - | Invoke(idx) - | EarlyBind(idx, _) => { + | InvokeFunction(idx) + | EarlyBindFunction(idx, _) => { self.check_code_unit_bounds_impl( self.view.signatures(), *idx, diff --git a/third_party/move/move-binary-format/src/check_complexity.rs b/third_party/move/move-binary-format/src/check_complexity.rs index 4aff6b1bfd7e9..78310559d7b96 100644 --- a/third_party/move/move-binary-format/src/check_complexity.rs +++ b/third_party/move/move-binary-format/src/check_complexity.rs @@ -292,8 +292,8 @@ impl<'a> BinaryComplexityMeter<'a> { | VecPopBack(idx) | VecUnpack(idx, _) | VecSwap(idx) - | Invoke(idx) - | EarlyBind(idx, _) => { + | InvokeFunction(idx) + | EarlyBindFunction(idx, _) => { self.meter_signature(*idx)?; }, diff --git a/third_party/move/move-binary-format/src/deserializer.rs b/third_party/move/move-binary-format/src/deserializer.rs index 24f096ef482c9..fcfcd33b192db 100644 --- a/third_party/move/move-binary-format/src/deserializer.rs +++ b/third_party/move/move-binary-format/src/deserializer.rs @@ -1825,10 +1825,11 @@ fn load_code(cursor: &mut VersionedCursor, code: &mut Vec) -> BinaryLo Opcodes::LD_FUNCTION_GENERIC => { Bytecode::LdFunctionGeneric(load_function_inst_index(cursor)?) }, - Opcodes::INVOKE => Bytecode::Invoke(load_signature_index(cursor)?), - Opcodes::EARLY_BIND => { - Bytecode::EarlyBind(load_signature_index(cursor)?, read_u8_internal(cursor)?) - }, + Opcodes::INVOKE_FUNCTION => Bytecode::InvokeFunction(load_signature_index(cursor)?), + Opcodes::EARLY_BIND_FUNCTION => Bytecode::EarlyBindFunction( + load_signature_index(cursor)?, + read_u8_internal(cursor)?, + ), }; code.push(bytecode); } diff --git a/third_party/move/move-binary-format/src/file_format.rs b/third_party/move/move-binary-format/src/file_format.rs index 4d431f5c319e2..a2137420e3226 100644 --- a/third_party/move/move-binary-format/src/file_format.rs +++ b/third_party/move/move-binary-format/src/file_format.rs @@ -2988,7 +2988,7 @@ pub enum Bytecode { #[group = "closure"] #[description = r#" - `EarlyBind(|t1..tn|r with a, count)` creates new function value based + `EarlyBindFunction(|t1..tn|r with a, count)` creates new function value based on the function value at top of stack by adding `count` arguments popped from the stack to the closure found on top of stack. @@ -3024,11 +3024,11 @@ pub enum Bytecode { func param types match provided parameter types "#] #[gas_type_creation_tier_0 = "closure_ty"] - EarlyBind(SignatureIndex, u8), + EarlyBindFunction(SignatureIndex, u8), #[group = "closure"] #[description = r#" - `Invoke(|t1..tn|r with a)` calls a function value of the specified type, + `InvokeFunction(|t1..tn|r with a)` calls a function value of the specified type, with `n` argument values from the stack. On top of the stack is the closure being evaluated, underneath the arguments: @@ -3095,7 +3095,7 @@ pub enum Bytecode { assert ty == locals[#args - i - 1] "#] #[gas_type_creation_tier_1 = "closure_ty"] - Invoke(SignatureIndex), + InvokeFunction(SignatureIndex), #[group = "stack_and_local"] #[description = "Push a u16 constant onto the stack."] @@ -3209,8 +3209,10 @@ impl ::std::fmt::Debug for Bytecode { Bytecode::UnpackVariantGeneric(a) => write!(f, "UnpackVariantGeneric({})", a), Bytecode::LdFunction(a) => write!(f, "LdFunction({})", a), Bytecode::LdFunctionGeneric(a) => write!(f, "LdFunctionGeneric({})", a), - Bytecode::EarlyBind(sig_idx, a) => write!(f, "EarlyBind({}, {})", sig_idx, a), - Bytecode::Invoke(sig_idx) => write!(f, "Invoke({})", sig_idx), + Bytecode::EarlyBindFunction(sig_idx, a) => { + write!(f, "EarlyBindFunction({}, {})", sig_idx, a) + }, + Bytecode::InvokeFunction(sig_idx) => write!(f, "InvokeFunction({})", sig_idx), Bytecode::ReadRef => write!(f, "ReadRef"), Bytecode::WriteRef => write!(f, "WriteRef"), Bytecode::FreezeRef => write!(f, "FreezeRef"), diff --git a/third_party/move/move-binary-format/src/file_format_common.rs b/third_party/move/move-binary-format/src/file_format_common.rs index 0627507bf1b71..3fa19710d9176 100644 --- a/third_party/move/move-binary-format/src/file_format_common.rs +++ b/third_party/move/move-binary-format/src/file_format_common.rs @@ -302,8 +302,8 @@ pub enum Opcodes { // Closures LD_FUNCTION = 0x58, LD_FUNCTION_GENERIC = 0x59, - INVOKE = 0x5A, - EARLY_BIND = 0x5B, + INVOKE_FUNCTION = 0x5A, + EARLY_BIND_FUNCTION = 0x5B, } /// Upper limit on the binary size @@ -797,8 +797,8 @@ pub fn instruction_key(instruction: &Bytecode) -> u8 { // Since bytecode version 8 LdFunction(_) => Opcodes::LD_FUNCTION, LdFunctionGeneric(_) => Opcodes::LD_FUNCTION_GENERIC, - Invoke(_) => Opcodes::INVOKE, - EarlyBind(..) => Opcodes::EARLY_BIND, + InvokeFunction(_) => Opcodes::INVOKE_FUNCTION, + EarlyBindFunction(..) => Opcodes::EARLY_BIND_FUNCTION, }; opcode as u8 } diff --git a/third_party/move/move-binary-format/src/serializer.rs b/third_party/move/move-binary-format/src/serializer.rs index e6e66657c19a7..686df2927b1fa 100644 --- a/third_party/move/move-binary-format/src/serializer.rs +++ b/third_party/move/move-binary-format/src/serializer.rs @@ -1104,12 +1104,12 @@ fn serialize_instruction_inner( binary.push(Opcodes::LD_FUNCTION_GENERIC as u8)?; serialize_function_inst_index(binary, method_idx) }, - Bytecode::Invoke(sig_idx) => { - binary.push(Opcodes::INVOKE as u8)?; + Bytecode::InvokeFunction(sig_idx) => { + binary.push(Opcodes::INVOKE_FUNCTION as u8)?; serialize_signature_index(binary, sig_idx) }, - Bytecode::EarlyBind(sig_idx, value) => { - binary.push(Opcodes::EARLY_BIND as u8)?; + Bytecode::EarlyBindFunction(sig_idx, value) => { + binary.push(Opcodes::EARLY_BIND_FUNCTION as u8)?; serialize_signature_index(binary, sig_idx)?; binary.push(*value) }, diff --git a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs index 934aca14890b7..ce9a0a537a8f3 100644 --- a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs +++ b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds/code_unit.rs @@ -514,7 +514,7 @@ impl<'a> ApplyCodeUnitBoundsContext<'a> { FunctionInstantiationIndex, LdFunctionGeneric ), - Invoke(..) | EarlyBind(..) => { + InvokeFunction(..) | EarlyBindFunction(..) => { panic!("Closure bytecode NYI: {:?}", code[bytecode_idx]) }, }; @@ -576,7 +576,7 @@ fn is_interesting(bytecode: &Bytecode) -> bool { | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor | Shl | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | Abort | Nop => false, - LdFunction(_) | LdFunctionGeneric(_) | Invoke(_) | EarlyBind(..) => { + LdFunction(_) | LdFunctionGeneric(_) | InvokeFunction(_) | EarlyBindFunction(..) => { // TODO(LAMBDA): implement false }, diff --git a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs index 41d5282ac05ea..f7c3de60ff1d1 100644 --- a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs @@ -107,8 +107,8 @@ impl<'a> AcquiresVerifier<'a> { let fi = self.module.function_instantiation_at(*idx); self.ld_function_acquire(fi.handle, offset) }, - Bytecode::EarlyBind(_sig_idx, _count) => Ok(()), - Bytecode::Invoke(_sig_idx) => self.invoke_acquire(offset), + Bytecode::EarlyBindFunction(_sig_idx, _count) => Ok(()), + Bytecode::InvokeFunction(_sig_idx) => self.invoke_acquire(offset), Bytecode::Pop | Bytecode::BrTrue(_) @@ -216,7 +216,7 @@ impl<'a> AcquiresVerifier<'a> { offset: CodeOffset, ) -> PartialVMResult<()> { // Currenty we are disallowing acquires for any function value which - // is created, so Invoke does nothing with acquires. + // is created, so InvokeFunction does nothing with acquires. // TODO(LAMBDA) In the future this may change. let function_handle = self.module.function_handle_at(fh_idx); let function_acquired_resources = self.function_acquired_resources(function_handle, fh_idx); @@ -228,7 +228,7 @@ impl<'a> AcquiresVerifier<'a> { fn invoke_acquire(&mut self, _offset: CodeOffset) -> PartialVMResult<()> { // Currenty we are disallowing acquires for any function value which - // is created, so Invoke does nothing with acquires. + // is created, so InvokeFunction does nothing with acquires. // TODO(LAMBDA) In the future this may change. Ok(()) } diff --git a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs index fdba5d7033813..6a00182700d90 100644 --- a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs +++ b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs @@ -104,11 +104,11 @@ impl<'a> InstructionConsistency<'a> { let func_inst = self.resolver.function_instantiation_at(*idx); self.check_ld_function_op(offset, func_inst.handle, /* generic */ true)?; }, - Invoke(sig_idx) => { + InvokeFunction(sig_idx) => { // reuse code to check for signature issues. self.check_bind_count(offset, *sig_idx, 0)?; }, - EarlyBind(sig_idx, count) => { + EarlyBindFunction(sig_idx, count) => { self.check_bind_count(offset, *sig_idx, *count)?; }, Pack(idx) | Unpack(idx) => { diff --git a/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs index 006f102eedbb4..cf6bc86a7a2d4 100644 --- a/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/locals_safety/mod.rs @@ -127,8 +127,8 @@ fn execute_inner( | Bytecode::TestVariantGeneric(_) | Bytecode::LdFunction(_) | Bytecode::LdFunctionGeneric(_) - | Bytecode::Invoke(_) - | Bytecode::EarlyBind(..) + | Bytecode::InvokeFunction(_) + | Bytecode::EarlyBindFunction(..) | Bytecode::ReadRef | Bytecode::WriteRef | Bytecode::CastU8 diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs index e8bfe35827a0f..23e94aa17cf1d 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/abstract_state.rs @@ -576,14 +576,14 @@ impl AbstractState { ) -> PartialVMResult { if !acquired_resources.is_empty() { // TODO(LAMBDA): Currently acquires must be empty unless we disallow - // Invoke to call to functions defined in the same module. + // InvokeFunction to call to functions defined in the same module. return Err(self.error(StatusCode::INVALID_ACQUIRES_ANNOTATION, offset)); } // TODO(LAMBDA): Double-check that we don't need meter adjustments here. Ok(AbstractValue::NonReference) } - pub fn invoke( + pub fn invoke_function( &mut self, offset: CodeOffset, arguments: Vec, diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs index d8426892e30cf..80f3127a0b8a2 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs @@ -125,7 +125,7 @@ fn ld_function( Ok(()) } -fn early_bind( +fn early_bind_function( verifier: &mut ReferenceSafetyAnalysis, _arg_tys: Vec, k: u8, @@ -140,7 +140,7 @@ fn early_bind( Ok(()) } -fn invoke( +fn invoke_function( verifier: &mut ReferenceSafetyAnalysis, state: &mut AbstractState, offset: CodeOffset, @@ -153,7 +153,7 @@ fn invoke( .map(|_| verifier.stack.pop().unwrap()) .rev() .collect(); - let values = state.invoke(offset, arguments, &result_tys, meter)?; + let values = state.invoke_function(offset, arguments, &result_tys, meter)?; for value in values { verifier.stack.push(value) } @@ -578,13 +578,13 @@ fn execute_inner( let function_handle = verifier.resolver.function_handle_at(func_inst.handle); ld_function(verifier, state, offset, function_handle, meter)? }, - Bytecode::EarlyBind(sig_idx, k) => { + Bytecode::EarlyBindFunction(sig_idx, k) => { let (arg_tys, _result_tys) = fun_type(verifier, *sig_idx)?; - early_bind(verifier, arg_tys, *k)? + early_bind_function(verifier, arg_tys, *k)? }, - Bytecode::Invoke(sig_idx) => { + Bytecode::InvokeFunction(sig_idx) => { let (arg_tys, result_tys) = fun_type(verifier, *sig_idx)?; - invoke(verifier, state, offset, arg_tys, result_tys, meter)? + invoke_function(verifier, state, offset, arg_tys, result_tys, meter)? }, Bytecode::VecPack(idx, num) => { diff --git a/third_party/move/move-bytecode-verifier/src/signature.rs b/third_party/move/move-bytecode-verifier/src/signature.rs index e2c7068d56573..a7b2d4372c9ab 100644 --- a/third_party/move/move-bytecode-verifier/src/signature.rs +++ b/third_party/move/move-bytecode-verifier/src/signature.rs @@ -260,7 +260,10 @@ impl<'a> SignatureChecker<'a> { }, // Closure operations not supported by legacy signature checker - LdFunction(..) | LdFunctionGeneric(..) | Invoke(_) | EarlyBind(..) => { + LdFunction(..) + | LdFunctionGeneric(..) + | InvokeFunction(_) + | EarlyBindFunction(..) => { return Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("closure operations not supported".to_owned())) }, diff --git a/third_party/move/move-bytecode-verifier/src/signature_v2.rs b/third_party/move/move-bytecode-verifier/src/signature_v2.rs index 953c26005d921..6a9eccf8ce9a5 100644 --- a/third_party/move/move-bytecode-verifier/src/signature_v2.rs +++ b/third_party/move/move-bytecode-verifier/src/signature_v2.rs @@ -900,12 +900,12 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { entry.insert(()); } }, - Invoke(idx) => map_err(self.verify_fun_sig_idx( + InvokeFunction(idx) => map_err(self.verify_fun_sig_idx( *idx, &mut checked_fun_insts, &ability_context, ))?, - EarlyBind(idx, count) => { + EarlyBindFunction(idx, count) => { map_err(self.verify_fun_sig_idx( *idx, &mut checked_fun_insts, @@ -922,7 +922,7 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { return map_err(Err(PartialVMError::new( StatusCode::NUMBER_OF_ARGUMENTS_MISMATCH, ) - .with_message("in EarlyBind".to_string()))); + .with_message("in EarlyBindFunction".to_string()))); }; }; entry.insert(()); @@ -1142,7 +1142,7 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { Ok(()) } - // Checks that a `sig_idx` parameter to `Invoke` or `EarlyBind` is well-formed. + // Checks that a `sig_idx` parameter to `InvokeFunction` or `EarlyBindFunction` is well-formed. fn verify_fun_sig_idx( &self, idx: SignatureIndex, diff --git a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs index bb86abdcc346a..a76f2aab3d2af 100644 --- a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs @@ -239,9 +239,9 @@ impl<'a> StackUsageVerifier<'a> { (0, 1) }, - // Invoke pops a function and the number of arguments and pushes the results of the + // InvokeFunction pops a function and the number of arguments and pushes the results of the // given function type - Bytecode::Invoke(idx) => { + Bytecode::InvokeFunction(idx) => { if let Some(SignatureToken::Function(args, results, _)) = self.resolver.signature_at(*idx).0.first() { @@ -253,8 +253,8 @@ impl<'a> StackUsageVerifier<'a> { } }, - // EarlyBind pops a function value and the captured arguments and returns 1 value - Bytecode::EarlyBind(idx, arg_count) => { + // EarlyBindFunction pops a function value and the captured arguments and returns 1 value + Bytecode::EarlyBindFunction(idx, arg_count) => { if let Some(SignatureToken::Function(args, _results, _)) = self.resolver.signature_at(*idx).0.first() { diff --git a/third_party/move/move-bytecode-verifier/src/type_safety.rs b/third_party/move/move-bytecode-verifier/src/type_safety.rs index 66b8dd4bced7d..bc1bb05d03336 100644 --- a/third_party/move/move-bytecode-verifier/src/type_safety.rs +++ b/third_party/move/move-bytecode-verifier/src/type_safety.rs @@ -301,7 +301,7 @@ fn call( Ok(()) } -fn invoke( +fn invoke_function( verifier: &mut TypeSafetyChecker, meter: &mut impl Meter, offset: CodeOffset, @@ -391,7 +391,7 @@ fn ld_function( ) } -fn early_bind( +fn early_bind_function( verifier: &mut TypeSafetyChecker, meter: &mut impl Meter, offset: CodeOffset, @@ -881,16 +881,16 @@ fn verify_instr( ld_function(verifier, meter, offset, &func_inst.handle, type_args)? }, - Bytecode::EarlyBind(idx, count) => { + Bytecode::EarlyBindFunction(idx, count) => { // The signature checker has verified this is a function type. let expected_ty = safe_unwrap!(verifier.resolver.signature_at(*idx).0.first()); - early_bind(verifier, meter, offset, expected_ty, *count)? + early_bind_function(verifier, meter, offset, expected_ty, *count)? }, - Bytecode::Invoke(idx) => { + Bytecode::InvokeFunction(idx) => { // The signature checker has verified this is a function type. let expected_ty = safe_unwrap!(verifier.resolver.signature_at(*idx).0.first()); - invoke(verifier, meter, offset, expected_ty)? + invoke_function(verifier, meter, offset, expected_ty)? }, Bytecode::Pack(idx) => { diff --git a/third_party/move/move-compiler-v2/src/bytecode_generator.rs b/third_party/move/move-compiler-v2/src/bytecode_generator.rs index e6be8ca0e80c5..e93c5ab2b5a7f 100644 --- a/third_party/move/move-compiler-v2/src/bytecode_generator.rs +++ b/third_party/move/move-compiler-v2/src/bytecode_generator.rs @@ -499,7 +499,7 @@ impl<'env> Generator<'env> { } ), // TODO(LAMBDA) - ExpData::Invoke(id, _exp, _) => self.error( + ExpData::InvokeFunction(id, _exp, _) => self.error( *id, if self.check_if_lambdas_enabled() { "Calls to function values other than inline function parameters not yet implemented" @@ -816,7 +816,7 @@ impl<'env> Generator<'env> { self.gen_function_call(targets, id, m.qualified(*f), args) }, // TODO(LAMBDA) - Operation::EarlyBind => self.error( + Operation::EarlyBindFunction => self.error( id, if self.check_if_lambdas_enabled() { "Function-typed values not yet implemented except as parameters to calls to inline functions" diff --git a/third_party/move/move-compiler-v2/src/env_pipeline/ast_simplifier.rs b/third_party/move/move-compiler-v2/src/env_pipeline/ast_simplifier.rs index 62c9a6cf63d12..2beaf57e7156d 100644 --- a/third_party/move/move-compiler-v2/src/env_pipeline/ast_simplifier.rs +++ b/third_party/move/move-compiler-v2/src/env_pipeline/ast_simplifier.rs @@ -346,7 +346,7 @@ fn find_possibly_modified_vars( }, }; }, - Invoke(..) | Return(..) | Quant(..) | Loop(..) | Mutate(..) | SpecBlock(..) => { + InvokeFunction(..) | Return(..) | Quant(..) | Loop(..) | Mutate(..) | SpecBlock(..) => { // We don't modify top-level variables here. match pos { VisitorPosition::Pre => { diff --git a/third_party/move/move-compiler-v2/src/env_pipeline/lambda_lifter.rs b/third_party/move/move-compiler-v2/src/env_pipeline/lambda_lifter.rs index f3f661ba11f30..31352fdeeeda6 100644 --- a/third_party/move/move-compiler-v2/src/env_pipeline/lambda_lifter.rs +++ b/third_party/move/move-compiler-v2/src/env_pipeline/lambda_lifter.rs @@ -13,9 +13,9 @@ //! not modify free variables. //! //! Lambda lifting rewrites lambda expressions into construction -//! of *closures* using the `EarlyBind` operation. A closure refers to a function and contains a list +//! of *closures* using the `EarlyBindFunction` operation. A closure refers to a function and contains a list //! of "early bound" leading arguments for that function, essentially currying it. We use the -//! `EarlyBind` operation to construct a closure from a function and set of arguments, +//! `EarlyBindFunction` operation to construct a closure from a function and set of arguments, //! which must be the first `k` arguments to the function argument list. //! //! ```ignore @@ -23,7 +23,7 @@ //! vec.any(|x| x > c) //! ==> //! let c = 1; -//! vec.any(EarlyBind(lifted, c)) +//! vec.any(EarlyBindFunction(lifted, c)) //! where //! fun lifted(c: u64, x: u64): bool { x > c } //! ``` @@ -36,7 +36,7 @@ //! vec.any(|S{x}| x > c) //! ==> //! let c = 1; -//! vec.any(EarlyBind(lifted, c)) +//! vec.any(EarlyBindFunction(lifted, c)) //! where //! fun lifted(c: u64, arg$2: S): bool { let S{x} = arg$2; x > y } //! ``` @@ -322,7 +322,7 @@ impl<'a> LambdaLifter<'a> { fn exp_is_simple(exp: &Exp) -> bool { use ExpData::*; match exp.as_ref() { - Call(_, Operation::EarlyBind, args) => args.iter().all(Self::exp_is_simple), + Call(_, Operation::EarlyBindFunction, args) => args.iter().all(Self::exp_is_simple), Call(_, op, args) => { op.is_ok_to_remove_from_code() && args.iter().all(Self::exp_is_simple) }, @@ -342,7 +342,7 @@ impl<'a> LambdaLifter<'a> { false }, LocalVar(..) | Temporary(..) | Value(..) => true, - Invalid(..) | Invoke(..) | Quant(..) | Block(..) | Match(..) | Return(..) + Invalid(..) | InvokeFunction(..) | Quant(..) | Block(..) | Match(..) | Return(..) | Loop(..) | LoopCont(..) | Assign(..) | Mutate(..) | SpecBlock(..) => false, } } @@ -398,7 +398,7 @@ impl<'a> LambdaLifter<'a> { match body.as_ref() { Call(id, oper, args) => { match oper { - Operation::EarlyBind => { + Operation::EarlyBindFunction => { // TODO(LAMBDA): We might be able to to do something with this, // but skip for now because it will be complicated. None @@ -420,7 +420,7 @@ impl<'a> LambdaLifter<'a> { _ => None, } }, - Invoke(_id, fn_exp, args) => { + InvokeFunction(_id, fn_exp, args) => { Self::get_args_if_simple(lambda_params, args).and_then(|args| { // Function expression may not contain lambda params let free_vars = fn_exp.as_ref().free_vars(); @@ -476,7 +476,7 @@ impl<'a> LambdaLifter<'a> { let fn_id = fn_exp.node_id(); let fn_type = env.get_node_type(fn_id); if let Type::Fun(_fn_param_type, _fn_result_type, fun_abilities) = &fn_type { - // First param to EarlyBind is the function expr + // First param to EarlyBindFunction is the function expr new_args.insert(0, fn_exp); let ty_params = self.fun_env.get_type_parameters_ref(); // Check bound value abilities @@ -535,7 +535,9 @@ impl<'a> LambdaLifter<'a> { // We have no parameters, just use the function directly. return Some(new_args.pop().unwrap()); } else { - return Some(ExpData::Call(id, Operation::EarlyBind, new_args).into_exp()); + return Some( + ExpData::Call(id, Operation::EarlyBindFunction, new_args).into_exp(), + ); } } } @@ -777,7 +779,7 @@ impl<'a> ExpRewriterFunctions for LambdaLifter<'a> { env.set_node_instantiation(id, inst); } closure_args.insert(0, fn_exp); - Some(ExpData::Call(id, Operation::EarlyBind, closure_args).into_exp()) + Some(ExpData::Call(id, Operation::EarlyBindFunction, closure_args).into_exp()) } } } diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index 84eda127b9f03..5ff3a597d509a 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -244,8 +244,9 @@ pub fn run_bytecode_gen(env: &GlobalEnv) -> FunctionTargetsHolder { if module.is_target() { for fun in module.get_functions() { let id = fun.get_qualified_id(); - // Skip inline functions because invoke and lambda are not supported in the current code generator + // Skip inline functions because invoke_function and lambda are not supported in the current code generator if !fun.is_inline() { + // TODO(LAMBDA) todo.insert(id); } } diff --git a/third_party/move/move-core/types/src/vm_status.rs b/third_party/move/move-core/types/src/vm_status.rs index 9aaccff1feecd..0776f84d760b3 100644 --- a/third_party/move/move-core/types/src/vm_status.rs +++ b/third_party/move/move-core/types/src/vm_status.rs @@ -734,7 +734,7 @@ pub enum StatusCode { ZERO_VARIANTS_ERROR = 1130, // A feature is not enabled. FEATURE_NOT_ENABLED = 1131, - // Invoke or EarlyBind parameter type is not a function + // InvokeFunction or EarlyBindFunction parameter type is not a function REQUIRES_FUNCTION = 1132, // Reserved error code for future use diff --git a/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs b/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs index f83916b3b4e87..9e498ea434537 100644 --- a/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs +++ b/third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs @@ -293,10 +293,10 @@ impl<'a> StacklessBytecodeGenerator<'a> { }; match bytecode { - MoveBytecode::Invoke(..) + MoveBytecode::InvokeFunction(..) | MoveBytecode::LdFunction(..) | MoveBytecode::LdFunctionGeneric(..) - | MoveBytecode::EarlyBind(..) => { + | MoveBytecode::EarlyBindFunction(..) => { unimplemented!("stackless bytecode generation for closure opcodes") }, MoveBytecode::Pop => { diff --git a/third_party/move/move-model/src/ast.rs b/third_party/move/move-model/src/ast.rs index 7c5e120fd9de7..4f05b8cfb474f 100644 --- a/third_party/move/move-model/src/ast.rs +++ b/third_party/move/move-model/src/ast.rs @@ -639,7 +639,7 @@ pub enum ExpData { /// (including operators, constants, ...) as well as user functions. Call(NodeId, Operation, Vec), /// Represents an invocation of a function value, as a lambda. - Invoke(NodeId, Exp, Vec), + InvokeFunction(NodeId, Exp, Vec), /// Represents a lambda. Lambda(NodeId, Pattern, Exp, LambdaCaptureKind, AbilitySet), /// Represents a quantified formula over multiple variables and ranges. @@ -820,7 +820,7 @@ impl ExpData { | LocalVar(node_id, ..) | Temporary(node_id, ..) | Call(node_id, ..) - | Invoke(node_id, ..) + | InvokeFunction(node_id, ..) | Lambda(node_id, ..) | Quant(node_id, ..) | Block(node_id, ..) @@ -1435,7 +1435,7 @@ impl ExpData { exp.visit_positions_impl(visitor)?; } }, - Invoke(_, target, args) => { + InvokeFunction(_, target, args) => { target.visit_positions_impl(visitor)?; for exp in args { exp.visit_positions_impl(visitor)?; @@ -1819,9 +1819,9 @@ pub enum Operation { /// arguments will be bound, in order, to the leading parameters of that function, /// generating a function which takes the remaining parameters and then calls /// the function with the complete set of parameters. - /// (move |x, y| f(z, x, y)) === ExpData::Call(_, EarlyBind, vec![f, z]) - /// (move || f(z, x, y)) === ExpData::Call(_, EarlyBind, vec![f, z, x, y]) - EarlyBind, + /// (move |x, y| f(z, x, y)) === ExpData::Call(_, EarlyBindFunction, vec![f, z]) + /// (move || f(z, x, y)) === ExpData::Call(_, EarlyBindFunction, vec![f, z, x, y]) + EarlyBindFunction, Pack(ModuleId, StructId, /*variant*/ Option), Tuple, Select(ModuleId, StructId, FieldId), @@ -2674,7 +2674,7 @@ impl Operation { Select(..) => false, // Move-related SelectVariants(..) => false, // Move-related UpdateField(..) => false, // Move-related - EarlyBind => true, + EarlyBindFunction => true, // Specification specific Result(..) => false, // Spec @@ -2888,7 +2888,7 @@ impl ExpData { is_pure = false; } }, - Invoke(..) => { + InvokeFunction(..) => { // Leave it alone for now, but with more analysis maybe we can do something. is_pure = false; }, @@ -3340,7 +3340,7 @@ impl<'a> fmt::Display for ExpDisplay<'a> { body.display_cont(self) ) }, - Invoke(_, fun, args) => { + InvokeFunction(_, fun, args) => { write!(f, "({})({})", fun.display_cont(self), self.fmt_exps(args)) }, IfElse(_, cond, if_exp, else_exp) => { @@ -3571,7 +3571,7 @@ impl<'a> fmt::Display for OperationDisplay<'a> { .unwrap_or_else(|| "".to_string()) ) }, - EarlyBind => { + EarlyBindFunction => { write!(f, "earlybind") }, Global(label_opt) => { 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 f91e4dd28bd9f..98892bab334b6 100644 --- a/third_party/move/move-model/src/builder/exp_builder.rs +++ b/third_party/move/move-model/src/builder/exp_builder.rs @@ -1552,7 +1552,7 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo ); let fexp = self.translate_exp(efexp, &fun_t); let id = self.new_node_id_with_type_loc(expected_type, &loc); - ExpData::Invoke(id, fexp.into_exp(), args) + ExpData::InvokeFunction(id, fexp.into_exp(), args) }, EA::Exp_::Pack(maccess, generics, fields) => self .translate_pack( @@ -3226,7 +3226,7 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo if let EA::ModuleAccess_::Name(n) = &maccess.value { let sym = self.symbol_pool().make(&n.value); - // Check whether this is an Invoke on a function value. + // Check whether this is an InvokeFunction on a function value. if let Some(entry) = self.lookup_local(sym, false) { // Check whether the local has the expected function type. let sym_ty = entry.type_.clone(); @@ -3246,7 +3246,7 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo let local_id = self.new_node_id_with_type_loc(&sym_ty, &self.to_loc(&n.loc)); let local_var = ExpData::LocalVar(local_id, sym); let id = self.new_node_id_with_type_loc(expected_type, loc); - return Some(ExpData::Invoke(id, local_var.into_exp(), args)); + return Some(ExpData::InvokeFunction(id, local_var.into_exp(), args)); } if self.is_spec_mode() { diff --git a/third_party/move/move-model/src/exp_rewriter.rs b/third_party/move/move-model/src/exp_rewriter.rs index af93e0e72553f..d7b0138b9de41 100644 --- a/third_party/move/move-model/src/exp_rewriter.rs +++ b/third_party/move/move-model/src/exp_rewriter.rs @@ -341,7 +341,7 @@ pub trait ExpRewriterFunctions { exp } }, - Invoke(id, target, args) => { + InvokeFunction(id, target, args) => { let (id_changed, new_id) = self.internal_rewrite_id(*id); let (target_changed, new_target) = self.internal_rewrite_exp(target); let new_args_opt = self.internal_rewrite_vec(args); @@ -358,7 +358,7 @@ pub trait ExpRewriterFunctions { } else { args.to_owned() }; - Invoke(new_id, new_target, args_owned).into_exp() + InvokeFunction(new_id, new_target, args_owned).into_exp() } else { exp } diff --git a/third_party/move/move-model/src/pureness_checker.rs b/third_party/move/move-model/src/pureness_checker.rs index 402090f9da8fc..6ed7011ec1923 100644 --- a/third_party/move/move-model/src/pureness_checker.rs +++ b/third_party/move/move-model/src/pureness_checker.rs @@ -56,7 +56,7 @@ impl FunctionPurenessChecker where F: FnMut(NodeId, &str, &[(QualifiedId, NodeId)]), { - /// Creates a new checker. The given function is invoke with diagnostic information + /// Creates a new checker. The given function is invoked with diagnostic information /// if impurity is detected. It is up to this function whether an actual error is /// reported. pub fn new(mode: FunctionPurenessCheckerMode, impure_action: F) -> Self { diff --git a/third_party/move/move-model/src/sourcifier.rs b/third_party/move/move-model/src/sourcifier.rs index fb5e2754c9d7c..a4ab01420eb66 100644 --- a/third_party/move/move-model/src/sourcifier.rs +++ b/third_party/move/move-model/src/sourcifier.rs @@ -758,7 +758,7 @@ impl<'a> ExpSourcifier<'a> { emit!(self.wr(), " '{}", self.sym(*label)) } }), - Invoke(_, fun, args) => self.parenthesize(context_prio, Prio::Postfix, || { + InvokeFunction(_, fun, args) => self.parenthesize(context_prio, Prio::Postfix, || { self.print_exp(Prio::Postfix, false, fun); self.print_exp_list("(", ")", args); }), @@ -823,7 +823,7 @@ impl<'a> ExpSourcifier<'a> { self.print_exp_list("(", ")", args) }) }, - Operation::EarlyBind => self.parenthesize(context_prio, Prio::Postfix, || { + Operation::EarlyBindFunction => self.parenthesize(context_prio, Prio::Postfix, || { emit!(self.wr(), "earlybind"); self.print_node_inst(id); self.print_exp_list("(", ")", args) diff --git a/third_party/move/move-prover/boogie-backend/src/spec_translator.rs b/third_party/move/move-prover/boogie-backend/src/spec_translator.rs index d3157c01a7161..a20a942be52b9 100644 --- a/third_party/move/move-prover/boogie-backend/src/spec_translator.rs +++ b/third_party/move/move-prover/boogie-backend/src/spec_translator.rs @@ -686,8 +686,11 @@ impl<'env> SpecTranslator<'env> { self.set_writer_location(*node_id); self.translate_call(*node_id, oper, args); }, - ExpData::Invoke(node_id, ..) => { - self.error(&self.env.get_node_loc(*node_id), "Invoke not yet supported"); + ExpData::InvokeFunction(node_id, ..) => { + self.error( + &self.env.get_node_loc(*node_id), + "InvokeFunction not yet supported", + ); // TODO(LAMBDA) }, ExpData::Lambda(node_id, ..) => self.error( @@ -1020,7 +1023,7 @@ impl<'env> SpecTranslator<'env> { | Operation::Deref | Operation::MoveTo | Operation::MoveFrom - | Operation::EarlyBind + | Operation::EarlyBindFunction | Operation::Old => { self.env.error( &self.env.get_node_loc(node_id), diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index 817deae344dce..e14e0ce08c1e7 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -1540,8 +1540,8 @@ impl Frame { // TODO(LAMBDA): implement closures Bytecode::LdFunction(..) | Bytecode::LdFunctionGeneric(..) - | Bytecode::Invoke(..) - | Bytecode::EarlyBind(..) => { + | Bytecode::InvokeFunction(..) + | Bytecode::EarlyBindFunction(..) => { return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("closure opcodes in interpreter".to_owned())) }, diff --git a/third_party/move/move-vm/runtime/src/runtime_type_checks.rs b/third_party/move/move-vm/runtime/src/runtime_type_checks.rs index a8a87dbc26136..4c9bc24124551 100644 --- a/third_party/move/move-vm/runtime/src/runtime_type_checks.rs +++ b/third_party/move/move-vm/runtime/src/runtime_type_checks.rs @@ -123,8 +123,8 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck { // TODO(LAMBDA): implement closures Bytecode::LdFunction(..) | Bytecode::LdFunctionGeneric(..) - | Bytecode::Invoke(..) - | Bytecode::EarlyBind(..) => { + | Bytecode::InvokeFunction(..) + | Bytecode::EarlyBindFunction(..) => { return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("closure opcodes in interpreter".to_owned())) }, @@ -258,8 +258,8 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck { // TODO: implement closures Bytecode::LdFunction(..) | Bytecode::LdFunctionGeneric(..) - | Bytecode::Invoke(..) - | Bytecode::EarlyBind(..) => { + | Bytecode::InvokeFunction(..) + | Bytecode::EarlyBindFunction(..) => { return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("closure opcodes in interpreter".to_owned())) }, 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 be4683e92f7d1..a73b39a26b060 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 @@ -708,8 +708,11 @@ pub fn zero_cost_instruction_table() -> Vec<(Bytecode, GasCost)> { LdFunctionGeneric(FunctionInstantiationIndex::new(0)), GasCost::new(0, 0), ), - (Invoke(SignatureIndex::new(0)), GasCost::new(0, 0)), - (EarlyBind(SignatureIndex::new(0), 0u8), GasCost::new(0, 0)), + (InvokeFunction(SignatureIndex::new(0)), GasCost::new(0, 0)), + ( + EarlyBindFunction(SignatureIndex::new(0), 0u8), + GasCost::new(0, 0), + ), ] } @@ -891,9 +894,12 @@ pub fn bytecode_instruction_costs() -> Vec<(Bytecode, GasCost)> { LdFunctionGeneric(FunctionInstantiationIndex::new(0)), GasCost::new(1132, 1), ), - (Invoke(SignatureIndex::new(0)), GasCost::new(1132, 1)), ( - EarlyBind(SignatureIndex::new(0), 0u8), + InvokeFunction(SignatureIndex::new(0)), + GasCost::new(1132, 1), + ), + ( + EarlyBindFunction(SignatureIndex::new(0), 0u8), GasCost::new(1132, 1), ), ] diff --git a/third_party/move/tools/move-disassembler/src/disassembler.rs b/third_party/move/tools/move-disassembler/src/disassembler.rs index a0d9e15417156..f368e269e5d77 100644 --- a/third_party/move/tools/move-disassembler/src/disassembler.rs +++ b/third_party/move/tools/move-disassembler/src/disassembler.rs @@ -732,13 +732,13 @@ impl<'a> Disassembler<'a> { Self::format_ret_type(&type_rets) )) }, - Bytecode::Invoke(idx) => { + Bytecode::InvokeFunction(idx) => { let _signature = self.source_mapper.bytecode.signature_at(*idx); - Ok(format!("Invoke({})", idx)) + Ok(format!("InvokeFunction({})", idx)) }, - Bytecode::EarlyBind(idx, count) => { + Bytecode::EarlyBindFunction(idx, count) => { let _signature = self.source_mapper.bytecode.signature_at(*idx); - Ok(format!("EarlyBind({}, {})", idx, count)) + Ok(format!("EarlyBindFunction({}, {})", idx, count)) }, Bytecode::LdConst(idx) => { From d3cd45743aedb8b271d16aa76f20f84c5c8078f0 Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Tue, 3 Dec 2024 21:51:01 -0800 Subject: [PATCH 4/9] Address review comments: - use struct for SignatureToken::Function - simplify AcuiresVerifier::ld_function_acquire - comments cleanup, especially file_format semantics for new bytecodes --- api/types/src/bytecode.rs | 2 +- aptos-move/script-composer/src/decompiler.rs | 2 +- .../move-binary-format/src/binary_views.rs | 2 +- .../move/move-binary-format/src/builders.rs | 14 +++-- .../move-binary-format/src/check_bounds.rs | 16 ++++- .../src/check_complexity.rs | 16 ++++- .../move/move-binary-format/src/constant.rs | 2 +- .../move-binary-format/src/file_format.rs | 58 ++++++++++++++----- .../move/move-binary-format/src/normalized.rs | 2 +- .../src/proptest_types/functions.rs | 14 ++++- .../src/proptest_types/types.rs | 2 +- .../move/move-binary-format/src/serializer.rs | 2 +- .../invalid-mutations/src/bounds.rs | 14 ++++- .../src/acquires_list_verifier.rs | 19 +++--- .../src/dependencies.rs | 19 ++++-- .../src/instantiation_loops.rs | 4 +- .../src/instruction_consistency.rs | 2 +- .../src/reference_safety/mod.rs | 2 +- .../move-bytecode-verifier/src/signature.rs | 6 +- .../src/signature_v2.rs | 16 +++-- .../src/stack_usage_verifier.rs | 4 +- .../move-bytecode-verifier/src/struct_defs.rs | 4 +- .../move-bytecode-verifier/src/type_safety.rs | 52 +++++++++++++---- .../move-compiler/src/interface_generator.rs | 4 +- .../move-ir-to-bytecode/src/context.rs | 2 +- third_party/move/move-model/src/ty.rs | 2 +- .../move-vm/runtime/src/loader/type_loader.rs | 2 +- .../move-vm/types/src/values/values_impl.rs | 2 +- .../tools/move-bytecode-utils/src/layout.rs | 2 +- .../move-disassembler/src/disassembler.rs | 2 +- .../tools/move-resource-viewer/src/lib.rs | 2 +- 31 files changed, 204 insertions(+), 88 deletions(-) diff --git a/api/types/src/bytecode.rs b/api/types/src/bytecode.rs index ffdea8f6186c6..45908b2e2b712 100644 --- a/api/types/src/bytecode.rs +++ b/api/types/src/bytecode.rs @@ -105,7 +105,7 @@ pub trait Bytecode { mutable: true, to: Box::new(self.new_move_type(t.borrow())), }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO(LAMBDA) unimplemented!("signature token function to API MoveType") }, diff --git a/aptos-move/script-composer/src/decompiler.rs b/aptos-move/script-composer/src/decompiler.rs index 147d976b90ccf..861bbe990a6b1 100644 --- a/aptos-move/script-composer/src/decompiler.rs +++ b/aptos-move/script-composer/src/decompiler.rs @@ -208,7 +208,7 @@ impl LocalState { SignatureToken::Vector(s) => { TypeTag::Vector(Box::new(Self::type_tag_from_sig_token(script, s)?)) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO(LAMBDA) bail!("function types not yet implemented for script composer") }, diff --git a/third_party/move/move-binary-format/src/binary_views.rs b/third_party/move/move-binary-format/src/binary_views.rs index 74c9325c7e2e9..f1093d22c45e7 100644 --- a/third_party/move/move-binary-format/src/binary_views.rs +++ b/third_party/move/move-binary-format/src/binary_views.rs @@ -343,7 +343,7 @@ impl<'a> BinaryIndexedView<'a> { Vector(ty) => AbilitySet::polymorphic_abilities(AbilitySet::VECTOR, vec![false], vec![ self.abilities(ty, constraints)?, ]), - Function(_, _, abilities) => Ok(*abilities), + Function { abilities, .. } => Ok(*abilities), Struct(idx) => { let sh = self.struct_handle_at(*idx); Ok(sh.abilities) diff --git a/third_party/move/move-binary-format/src/builders.rs b/third_party/move/move-binary-format/src/builders.rs index b78ba64c23707..5cacb8e3e62b2 100644 --- a/third_party/move/move-binary-format/src/builders.rs +++ b/third_party/move/move-binary-format/src/builders.rs @@ -235,11 +235,15 @@ impl CompiledScriptBuilder { MutableReference(Box::new(self.import_signature_token(module, ty)?)) }, Vector(ty) => Vector(Box::new(self.import_signature_token(module, ty)?)), - Function(args, result, abilities) => Function( - import_vec(self, args)?, - import_vec(self, result)?, - *abilities, - ), + Function { + args, + results, + abilities, + } => Function { + args: import_vec(self, args)?, + results: import_vec(self, results)?, + abilities: *abilities, + }, Struct(idx) => Struct(self.import_struct(module, *idx)?), StructInstantiation(idx, inst_tys) => StructInstantiation( self.import_struct(module, *idx)?, diff --git a/third_party/move/move-binary-format/src/check_bounds.rs b/third_party/move/move-binary-format/src/check_bounds.rs index 1992a22e47968..6560f376d1d72 100644 --- a/third_party/move/move-binary-format/src/check_bounds.rs +++ b/third_party/move/move-binary-format/src/check_bounds.rs @@ -685,8 +685,20 @@ impl<'a> BoundsChecker<'a> { for ty in ty.preorder_traversal() { match ty { - Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | TypeParameter(_) - | Reference(_) | MutableReference(_) | Vector(_) | Function(..) => (), + Bool + | U8 + | U16 + | U32 + | U64 + | U128 + | U256 + | Address + | Signer + | TypeParameter(_) + | Reference(_) + | MutableReference(_) + | Vector(_) + | Function { .. } => (), Struct(idx) => { check_bounds_impl(self.view.struct_handles(), *idx)?; if let Some(sh) = self.view.struct_handles().get(idx.into_index()) { diff --git a/third_party/move/move-binary-format/src/check_complexity.rs b/third_party/move/move-binary-format/src/check_complexity.rs index 78310559d7b96..1aa4b3d949994 100644 --- a/third_party/move/move-binary-format/src/check_complexity.rs +++ b/third_party/move/move-binary-format/src/check_complexity.rs @@ -67,8 +67,20 @@ impl<'a> BinaryComplexityMeter<'a> { cost = cost.saturating_add(struct_name.len() as u64 * COST_PER_IDENT_BYTE); cost = cost.saturating_add(moduel_name.len() as u64 * COST_PER_IDENT_BYTE); }, - U8 | U16 | U32 | U64 | U128 | U256 | Signer | Address | Bool | Vector(_) - | Function(..) | TypeParameter(_) | Reference(_) | MutableReference(_) => (), + U8 + | U16 + | U32 + | U64 + | U128 + | U256 + | Signer + | Address + | Bool + | Vector(_) + | Function { .. } + | TypeParameter(_) + | Reference(_) + | MutableReference(_) => (), } } diff --git a/third_party/move/move-binary-format/src/constant.rs b/third_party/move/move-binary-format/src/constant.rs index 3b8d200787a4e..1f27d9f259fe3 100644 --- a/third_party/move/move-binary-format/src/constant.rs +++ b/third_party/move/move-binary-format/src/constant.rs @@ -17,7 +17,7 @@ fn sig_to_ty(sig: &SignatureToken) -> Option { SignatureToken::U128 => Some(MoveTypeLayout::U128), SignatureToken::U256 => Some(MoveTypeLayout::U256), SignatureToken::Vector(v) => Some(MoveTypeLayout::Vector(Box::new(sig_to_ty(v.as_ref())?))), - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO(LAMBDA): do we need representation in MoveTypeLayout? None }, diff --git a/third_party/move/move-binary-format/src/file_format.rs b/third_party/move/move-binary-format/src/file_format.rs index a2137420e3226..7afb189156026 100644 --- a/third_party/move/move-binary-format/src/file_format.rs +++ b/third_party/move/move-binary-format/src/file_format.rs @@ -1262,7 +1262,11 @@ pub enum SignatureToken { /// Vector Vector(Box), /// Function, with n argument types and m result types, and an associated ability set. - Function(Vec, Vec, AbilitySet), + Function { + args: Vec, + results: Vec, + abilities: AbilitySet, + }, /// User defined type Struct(StructHandleIndex), StructInstantiation(StructHandleIndex, Vec), @@ -1305,9 +1309,9 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIter<'a> { self.stack.extend(inner_toks.iter().rev()) }, - Function(args, result, _) => { + Function { args, results, .. } => { self.stack.extend(args.iter().rev()); - self.stack.extend(result.iter().rev()); + self.stack.extend(results.iter().rev()); }, Signer | Bool | Address | U8 | U16 | U32 | U64 | U128 | U256 | Struct(_) @@ -1343,11 +1347,11 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIterWithDepth<'a> { .stack .extend(inner_toks.iter().map(|tok| (tok, depth + 1)).rev()), - Function(args, result, _) => { + Function { args, results, .. } => { self.stack .extend(args.iter().map(|tok| (tok, depth + 1)).rev()); self.stack - .extend(result.iter().map(|tok| (tok, depth + 1)).rev()); + .extend(results.iter().map(|tok| (tok, depth + 1)).rev()); }, Signer | Bool | Address | U8 | U16 | U32 | U64 | U128 | U256 | Struct(_) @@ -1410,8 +1414,12 @@ impl std::fmt::Debug for SignatureToken { SignatureToken::Address => write!(f, "Address"), SignatureToken::Signer => write!(f, "Signer"), SignatureToken::Vector(boxed) => write!(f, "Vector({:?})", boxed), - SignatureToken::Function(args, result, abilities) => { - write!(f, "Function({:?}, {:?}, {})", args, result, abilities) + SignatureToken::Function { + args, + results, + abilities, + } => { + write!(f, "Function({:?}, {:?}, {})", args, results, abilities) }, SignatureToken::Reference(boxed) => write!(f, "Reference({:?})", boxed), SignatureToken::Struct(idx) => write!(f, "Struct({:?})", idx), @@ -1425,7 +1433,7 @@ impl std::fmt::Debug for SignatureToken { } impl SignatureToken { - // Returns `true` if the `SignatureToken` is an integer type. + /// Returns `true` if the `SignatureToken` is an integer type. pub fn is_integer(&self) -> bool { use SignatureToken::*; match self { @@ -1434,7 +1442,7 @@ impl SignatureToken { | Address | Signer | Vector(_) - | Function(..) + | Function { .. } | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1473,7 +1481,7 @@ impl SignatureToken { Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address => true, Vector(inner) => inner.is_valid_for_constant(), Signer - | Function(..) + | Function { .. } | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1486,7 +1494,7 @@ impl SignatureToken { pub fn is_function(&self) -> bool { use SignatureToken::*; - matches!(self, Function(..)) + matches!(self, Function { .. }) } /// Set the index to this one. Useful for random testing. @@ -1537,8 +1545,14 @@ impl SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(ty.instantiate(subst_mapping))), - Function(args, result, abilities) => { - Function(inst_vec(args), inst_vec(result), *abilities) + Function { + args, + results, + abilities, + } => Function { + args: inst_vec(args), + results: inst_vec(results), + abilities: *abilities, }, Struct(idx) => Struct(*idx), StructInstantiation(idx, struct_type_args) => { @@ -1834,6 +1848,11 @@ pub enum Bytecode { // Here `func` is loaded from the file format, containing information like the // the function signature, the locals, and the body. + // TODO(LAMBDA) - try to loosen this restriction after MVP if we can + // prove safety using access specifiers or other mechanisms. + if conservative_invocation_mode then + if stack already contains a call to the same module as func, then abort + ty_args = if func.is_generic then func.ty_args else [] n = func.num_params @@ -3047,8 +3066,13 @@ pub enum Bytecode { Return values are pushed onto the stack from the first to the last and available to the caller after returning from the callee. - During execution of the function conservative_invocation_mode is enabled; - afterwards, it is restored to its original state. + For MVP, we dynamically disallow all module reentrancy starting with + any dynamic call. This means that normal calls need a dynamic stack check + if there is a dynamic call frame on the stack. We denote this here + using a "conservative_invocation_mode" state variable. + + During execution of the dynamic function conservative_invocation_mode + is enabled; afterwards, it is restored to its original state. "#] #[semantics = r#" stack >> function_handle @@ -3058,6 +3082,10 @@ pub enum Bytecode { n = func.num_params ty_args = if func.is_generic then func.ty_args else [] + // TODO(LAMBDA) - try to loosen this restriction after MVP if we can + // prove safety using access specifiers or other mechanisms. + if stack already contains a call to the same module as func, then abort + n = func.num_params stack >> arg_{n-1} .. diff --git a/third_party/move/move-binary-format/src/normalized.rs b/third_party/move/move-binary-format/src/normalized.rs index 4fe5b3508d0fa..8d13b0d015478 100644 --- a/third_party/move/move-binary-format/src/normalized.rs +++ b/third_party/move/move-binary-format/src/normalized.rs @@ -193,7 +193,7 @@ impl Type { Reference(t) => Type::Reference(Box::new(Type::new(m, t))), MutableReference(t) => Type::MutableReference(Box::new(Type::new(m, t))), - Function(..) => panic!("normalized representation does not support function types"), + Function { .. } => panic!("normalized representation does not support function types"), } } diff --git a/third_party/move/move-binary-format/src/proptest_types/functions.rs b/third_party/move/move-binary-format/src/proptest_types/functions.rs index f614ffcfbb8e6..77dbd31092cf5 100644 --- a/third_party/move/move-binary-format/src/proptest_types/functions.rs +++ b/third_party/move/move-binary-format/src/proptest_types/functions.rs @@ -1061,8 +1061,18 @@ impl BytecodeGen { fn check_signature_token(token: &SignatureToken) -> bool { use SignatureToken::*; match token { - U8 | U16 | U32 | U64 | U128 | U256 | Bool | Address | Signer | Struct(_) - | Function(..) | TypeParameter(_) => true, + U8 + | U16 + | U32 + | U64 + | U128 + | U256 + | Bool + | Address + | Signer + | Struct(_) + | Function { .. } + | TypeParameter(_) => true, Vector(element_token) => BytecodeGen::check_signature_token(element_token), StructInstantiation(_, type_arguments) => type_arguments .iter() diff --git a/third_party/move/move-binary-format/src/proptest_types/types.rs b/third_party/move/move-binary-format/src/proptest_types/types.rs index 161586d503804..830dd5c564298 100644 --- a/third_party/move/move-binary-format/src/proptest_types/types.rs +++ b/third_party/move/move-binary-format/src/proptest_types/types.rs @@ -71,7 +71,7 @@ impl StDefnMaterializeState { let inner = self.potential_abilities(ty); inner.intersect(AbilitySet::VECTOR) }, - Function(_, _, a) => *a, + Function { abilities, .. } => *abilities, Struct(idx) => { let sh = &self.struct_handles[idx.0 as usize]; sh.abilities diff --git a/third_party/move/move-binary-format/src/serializer.rs b/third_party/move/move-binary-format/src/serializer.rs index 686df2927b1fa..f9c9e51917482 100644 --- a/third_party/move/move-binary-format/src/serializer.rs +++ b/third_party/move/move-binary-format/src/serializer.rs @@ -800,7 +800,7 @@ fn serialize_signature_token_single_node_impl( binary.push(SerializedType::TYPE_PARAMETER as u8)?; serialize_type_parameter_index(binary, *idx)?; }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { unimplemented!("serialization of function types") }, } diff --git a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs index cb47be2e8e220..0d7858b658c7d 100644 --- a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs +++ b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs @@ -358,7 +358,17 @@ fn struct_handle(token: &SignatureToken) -> Option { Struct(sh_idx) => Some(*sh_idx), StructInstantiation(sh_idx, _) => Some(*sh_idx), Reference(token) | MutableReference(token) => struct_handle(token), - Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | Vector(_) - | TypeParameter(_) | Function(..) => None, + Bool + | U8 + | U16 + | U32 + | U64 + | U128 + | U256 + | Address + | Signer + | Vector(_) + | TypeParameter(_) + | Function { .. } => None, } } diff --git a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs index f7c3de60ff1d1..e18baabbb313b 100644 --- a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs @@ -212,23 +212,20 @@ impl<'a> AcquiresVerifier<'a> { fn ld_function_acquire( &mut self, - fh_idx: FunctionHandleIndex, - offset: CodeOffset, + _fh_idx: FunctionHandleIndex, + _offset: CodeOffset, ) -> PartialVMResult<()> { - // Currenty we are disallowing acquires for any function value which - // is created, so InvokeFunction does nothing with acquires. + // Note that function values are dynamically disallowed to be called from the module hosting + // their implementation function, and to be called while their host module is on the stack, + // so we don't need to do anything about acquires at this point. // TODO(LAMBDA) In the future this may change. - let function_handle = self.module.function_handle_at(fh_idx); - let function_acquired_resources = self.function_acquired_resources(function_handle, fh_idx); - if !function_acquired_resources.is_empty() { - return Err(self.error(StatusCode::LD_FUNCTION_NONEMPTY_ACQUIRES, offset)); - } Ok(()) } fn invoke_acquire(&mut self, _offset: CodeOffset) -> PartialVMResult<()> { - // Currenty we are disallowing acquires for any function value which - // is created, so InvokeFunction does nothing with acquires. + // Note that function values are dynamically disallowed to be called from their + // hosting module, or if that module appears on the stack at all. This means + // that a dynamic call cannot affect local resources in the caller. // TODO(LAMBDA) In the future this may change. Ok(()) } diff --git a/third_party/move/move-bytecode-verifier/src/dependencies.rs b/third_party/move/move-bytecode-verifier/src/dependencies.rs index 8593100e39746..74d86a40a5783 100644 --- a/third_party/move/move-bytecode-verifier/src/dependencies.rs +++ b/third_party/move/move-bytecode-verifier/src/dependencies.rs @@ -456,13 +456,22 @@ fn compare_types( compare_types(context, ty1, ty2, def_module) }, ( - SignatureToken::Function(args1, result1, ab1), - SignatureToken::Function(args2, result2, ab2), + SignatureToken::Function { + args: args1, + results: result1, + abilities: abilities1, + }, + SignatureToken::Function { + args: args2, + results: result2, + abilities: abilities2, + }, ) => { compare_cross_module_signatures(context, args1, args2, def_module)?; compare_cross_module_signatures(context, result1, result2, def_module)?; - if ab1 != ab2 { - Err(PartialVMError::new(StatusCode::TYPE_MISMATCH)) + if abilities1 != abilities2 { + Err(PartialVMError::new(StatusCode::TYPE_MISMATCH) + .with_message("Function type ability constraints mismatch".to_string())) } else { Ok(()) } @@ -495,7 +504,7 @@ fn compare_types( | (SignatureToken::Address, _) | (SignatureToken::Signer, _) | (SignatureToken::Vector(_), _) - | (SignatureToken::Function(..), _) + | (SignatureToken::Function { .. }, _) | (SignatureToken::Struct(_), _) | (SignatureToken::StructInstantiation(_, _), _) | (SignatureToken::Reference(_), _) diff --git a/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs b/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs index 53896c212ee14..d1a1501d41430 100644 --- a/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs +++ b/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs @@ -148,11 +148,11 @@ impl<'a> InstantiationLoopChecker<'a> { type_params.insert(*idx); }, Vector(ty) => rec(type_params, ty), - Function(args, result, _) => { + Function { args, results, .. } => { for ty in args { rec(type_params, ty); } - for ty in result { + for ty in results { rec(type_params, ty); } }, diff --git a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs index 6a00182700d90..ba4e7d9e5c14c 100644 --- a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs +++ b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs @@ -266,7 +266,7 @@ impl<'a> InstructionConsistency<'a> { ) -> PartialVMResult<()> { let signature = self.resolver.signature_at(sig_index); if let Some(sig_token) = signature.0.first() { - if let SignatureToken::Function(params, _returns, _abilities) = sig_token { + if let SignatureToken::Function { args: params, .. } = sig_token { if count as usize > params.len() { return Err( PartialVMError::new(StatusCode::NUMBER_OF_TYPE_ARGUMENTS_MISMATCH) diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs index 80f3127a0b8a2..651f54078d784 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs @@ -250,7 +250,7 @@ fn fun_type( idx: SignatureIndex, ) -> PartialVMResult<(Vec, Vec)> { match verifier.resolver.signature_at(idx).0.first() { - Some(SignatureToken::Function(args, result, _)) => Ok((args.clone(), result.clone())), + Some(SignatureToken::Function { args, results, .. }) => Ok((args.clone(), results.clone())), _ => Err(PartialVMError::new( StatusCode::VERIFIER_INVARIANT_VIOLATION, )), diff --git a/third_party/move/move-bytecode-verifier/src/signature.rs b/third_party/move/move-bytecode-verifier/src/signature.rs index a7b2d4372c9ab..a89dc52c8d783 100644 --- a/third_party/move/move-bytecode-verifier/src/signature.rs +++ b/third_party/move/move-bytecode-verifier/src/signature.rs @@ -372,7 +372,7 @@ impl<'a> SignatureChecker<'a> { } }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { return Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())); }, @@ -429,7 +429,7 @@ impl<'a> SignatureChecker<'a> { Err(PartialVMError::new(StatusCode::INVALID_SIGNATURE_TOKEN) .with_message("reference not allowed".to_string())) }, - Function(..) => Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) + Function { .. } => Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())), Vector(ty) => self.check_signature_token(ty), StructInstantiation(_, type_arguments) => self.check_signature_tokens(type_arguments), @@ -481,7 +481,7 @@ impl<'a> SignatureChecker<'a> { type_parameters, ) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())) }, diff --git a/third_party/move/move-bytecode-verifier/src/signature_v2.rs b/third_party/move/move-bytecode-verifier/src/signature_v2.rs index 6a9eccf8ce9a5..f352aee8fe922 100644 --- a/third_party/move/move-bytecode-verifier/src/signature_v2.rs +++ b/third_party/move/move-bytecode-verifier/src/signature_v2.rs @@ -173,9 +173,13 @@ fn check_ty( param_constraints, )?; }, - Function(args, result, abilities) => { + Function { + args, + results, + abilities, + } => { assert_abilities(*abilities, required_abilities)?; - for ty in args.iter().chain(result.iter()) { + for ty in args.iter().chain(results.iter()) { check_ty( struct_handles, ty, @@ -271,8 +275,8 @@ fn check_phantom_params( match ty { Vector(ty) => check_phantom_params(struct_handles, context, false, ty)?, - Function(args, result, _) => { - for ty in args.iter().chain(result) { + Function { args, results, .. } => { + for ty in args.iter().chain(results) { check_phantom_params(struct_handles, context, false, ty)? } }, @@ -915,10 +919,10 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { checked_early_bind_insts.entry((*idx, *count)) { // Note non-function case is checked in `verify_fun_sig_idx` above. - if let Some(SignatureToken::Function(params, _results, _abilities)) = + if let Some(SignatureToken::Function { args, .. }) = self.resolver.signature_at(*idx).0.first() { - if *count as usize > params.len() { + if *count as usize > args.len() { return map_err(Err(PartialVMError::new( StatusCode::NUMBER_OF_ARGUMENTS_MISMATCH, ) diff --git a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs index a76f2aab3d2af..4fb7856670a42 100644 --- a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs @@ -242,7 +242,7 @@ impl<'a> StackUsageVerifier<'a> { // InvokeFunction pops a function and the number of arguments and pushes the results of the // given function type Bytecode::InvokeFunction(idx) => { - if let Some(SignatureToken::Function(args, results, _)) = + if let Some(SignatureToken::Function { args, results, .. }) = self.resolver.signature_at(*idx).0.first() { ((1 + args.len()) as u64, results.len() as u64) @@ -255,7 +255,7 @@ impl<'a> StackUsageVerifier<'a> { // EarlyBindFunction pops a function value and the captured arguments and returns 1 value Bytecode::EarlyBindFunction(idx, arg_count) => { - if let Some(SignatureToken::Function(args, _results, _)) = + if let Some(SignatureToken::Function { args, .. }) = self.resolver.signature_at(*idx).0.first() { if args.len() <= *arg_count as usize { diff --git a/third_party/move/move-bytecode-verifier/src/struct_defs.rs b/third_party/move/move-bytecode-verifier/src/struct_defs.rs index d78c9e7f58f7d..a3ce1f606d77e 100644 --- a/third_party/move/move-bytecode-verifier/src/struct_defs.rs +++ b/third_party/move/move-bytecode-verifier/src/struct_defs.rs @@ -123,8 +123,8 @@ impl<'a> StructDefGraphBuilder<'a> { ) }, T::Vector(inner) => self.add_signature_token(neighbors, cur_idx, inner)?, - T::Function(args, result, _) => { - for t in args.iter().chain(result) { + T::Function { args, results, .. } => { + for t in args.iter().chain(results) { self.add_signature_token(neighbors, cur_idx, t)? } }, diff --git a/third_party/move/move-bytecode-verifier/src/type_safety.rs b/third_party/move/move-bytecode-verifier/src/type_safety.rs index bc1bb05d03336..bf9897f47ea05 100644 --- a/third_party/move/move-bytecode-verifier/src/type_safety.rs +++ b/third_party/move/move-bytecode-verifier/src/type_safety.rs @@ -307,7 +307,12 @@ fn invoke_function( offset: CodeOffset, expected_ty: &SignatureToken, ) -> PartialVMResult<()> { - let SignatureToken::Function(param_tys, ret_tys, abilities) = expected_ty else { + let SignatureToken::Function { + args: param_tys, + results: ret_tys, + abilities, + } = expected_ty + else { // The signature checker has ensured this is a function safe_assert!(false); unreachable!() @@ -322,7 +327,11 @@ fn invoke_function( .with_message("closure type mismatch".to_owned())); } // Verify that the abilities match - let SignatureToken::Function(_, _, closure_abilities) = closure_ty else { + let SignatureToken::Function { + abilities: closure_abilities, + .. + } = closure_ty + else { // Ensured above, but never panic safe_assert!(false); unreachable!() @@ -385,7 +394,11 @@ fn ld_function( verifier.push( meter, instantiate( - &SignatureToken::Function(parameters.0.to_vec(), ret_sign.0.to_vec(), abilities), + &SignatureToken::Function { + args: parameters.0.to_vec(), + results: ret_sign.0.to_vec(), + abilities, + }, type_actuals, ), ) @@ -399,7 +412,12 @@ fn early_bind_function( count: u8, ) -> PartialVMResult<()> { let count = count as usize; - let SignatureToken::Function(param_tys, ret_tys, abilities) = expected_ty else { + let SignatureToken::Function { + args: param_tys, + results: ret_tys, + abilities, + } = expected_ty + else { // The signature checker has ensured this is a function safe_assert!(false); unreachable!() @@ -414,7 +432,11 @@ fn early_bind_function( .with_message("closure type mismatch".to_owned())); } // Verify that the abilities match - let SignatureToken::Function(_, _, closure_abilities) = closure_ty else { + let SignatureToken::Function { + abilities: closure_abilities, + .. + } = closure_ty + else { // Ensured above, but never panic safe_assert!(false); unreachable!() @@ -439,11 +461,11 @@ fn early_bind_function( return Err(verifier.error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset)); } } - let result_ty = SignatureToken::Function( - (*remaining_param_tys).to_vec(), - ret_tys.to_vec(), - *abilities, - ); + let result_ty = SignatureToken::Function { + args: (*remaining_param_tys).to_vec(), + results: ret_tys.to_vec(), + abilities: *abilities, + }; verifier.push(meter, result_ty) } @@ -1321,7 +1343,15 @@ fn instantiate(token: &SignatureToken, subst: &Signature) -> SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(instantiate(ty, subst))), - Function(args, result, abilities) => Function(inst_vec(args), inst_vec(result), *abilities), + Function { + args, + results, + abilities, + } => Function { + args: inst_vec(args), + results: inst_vec(results), + abilities: *abilities, + }, Struct(idx) => Struct(*idx), StructInstantiation(idx, struct_type_args) => { StructInstantiation(*idx, inst_vec(struct_type_args)) diff --git a/third_party/move/move-compiler/src/interface_generator.rs b/third_party/move/move-compiler/src/interface_generator.rs index 8a78305aecc9d..09c4c8379eb98 100644 --- a/third_party/move/move-compiler/src/interface_generator.rs +++ b/third_party/move/move-compiler/src/interface_generator.rs @@ -365,8 +365,8 @@ fn write_signature_token(ctx: &mut Context, t: &SignatureToken) -> String { SignatureToken::Address => "address".to_string(), SignatureToken::Signer => "signer".to_string(), SignatureToken::Vector(inner) => format!("vector<{}>", write_signature_token(ctx, inner)), - SignatureToken::Function(args, result, _) => { - format!("|{}|{}", tok_list(ctx, args), tok_list(ctx, result)) + SignatureToken::Function { args, results, .. } => { + format!("|{}|{}", tok_list(ctx, args), tok_list(ctx, results)) }, SignatureToken::Struct(idx) => write_struct_handle_type(ctx, *idx), SignatureToken::StructInstantiation(idx, types) => { diff --git a/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs b/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs index c29d923504085..3da165cdb01e1 100644 --- a/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs +++ b/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs @@ -768,7 +768,7 @@ impl<'a> Context<'a> { let correct_inner = self.reindex_signature_token(dep, *inner)?; SignatureToken::Vector(Box::new(correct_inner)) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { unimplemented!("function types not supported by MoveIR") }, SignatureToken::Reference(inner) => { diff --git a/third_party/move/move-model/src/ty.rs b/third_party/move/move-model/src/ty.rs index 6da025ea9c482..d1eedf5ed561c 100644 --- a/third_party/move/move-model/src/ty.rs +++ b/third_party/move/move-model/src/ty.rs @@ -1394,7 +1394,7 @@ impl Type { .collect(), ) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO: implement function conversion unimplemented!("signature token to model type") }, diff --git a/third_party/move/move-vm/runtime/src/loader/type_loader.rs b/third_party/move/move-vm/runtime/src/loader/type_loader.rs index 1aa31dcd377ea..19099c68c5c4c 100644 --- a/third_party/move/move-vm/runtime/src/loader/type_loader.rs +++ b/third_party/move/move-vm/runtime/src/loader/type_loader.rs @@ -31,7 +31,7 @@ pub fn intern_type( let inner_type = intern_type(module, inner_tok, struct_name_table)?; Type::Vector(TriompheArc::new(inner_type)) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO: implement closures return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("function types in the type loader".to_owned())); diff --git a/third_party/move/move-vm/types/src/values/values_impl.rs b/third_party/move/move-vm/types/src/values/values_impl.rs index c3171cd907588..ffdc8634d2cf1 100644 --- a/third_party/move/move-vm/types/src/values/values_impl.rs +++ b/third_party/move/move-vm/types/src/values/values_impl.rs @@ -4154,7 +4154,7 @@ impl Value { S::Signer => return None, S::Vector(inner) => L::Vector(Box::new(Self::constant_sig_token_to_layout(inner)?)), // Not yet supported - S::Struct(_) | S::StructInstantiation(_, _) | S::Function(..) => return None, + S::Struct(_) | S::StructInstantiation(_, _) | S::Function { .. } => return None, // Not allowed/Not meaningful S::TypeParameter(_) | S::Reference(_) | S::MutableReference(_) => return None, }) diff --git a/third_party/move/tools/move-bytecode-utils/src/layout.rs b/third_party/move/tools/move-bytecode-utils/src/layout.rs index 46863644cdc55..cfb305dd6b408 100644 --- a/third_party/move/tools/move-bytecode-utils/src/layout.rs +++ b/third_party/move/tools/move-bytecode-utils/src/layout.rs @@ -386,7 +386,7 @@ impl TypeLayoutBuilder { ) -> anyhow::Result { use SignatureToken::*; Ok(match s { - Function(..) => bail!("function types NYI for MoveTypeLayout"), + Function { .. } => bail!("function types NYI for MoveTypeLayout"), Vector(t) => MoveTypeLayout::Vector(Box::new(Self::build_from_signature_token( m, t, diff --git a/third_party/move/tools/move-disassembler/src/disassembler.rs b/third_party/move/tools/move-disassembler/src/disassembler.rs index f368e269e5d77..1e4bcc1e3608f 100644 --- a/third_party/move/tools/move-disassembler/src/disassembler.rs +++ b/third_party/move/tools/move-disassembler/src/disassembler.rs @@ -571,7 +571,7 @@ impl<'a> Disassembler<'a> { ) -> Result { Ok(match sig_tok { // TODO: function types - SignatureToken::Function(..) => unimplemented!("disassembling function sig tokens"), + SignatureToken::Function { .. } => unimplemented!("disassembling function sig tokens"), SignatureToken::Bool => "bool".to_string(), SignatureToken::U8 => "u8".to_string(), diff --git a/third_party/move/tools/move-resource-viewer/src/lib.rs b/third_party/move/tools/move-resource-viewer/src/lib.rs index ba6d7f3eca515..3e81f50c13076 100644 --- a/third_party/move/tools/move-resource-viewer/src/lib.rs +++ b/third_party/move/tools/move-resource-viewer/src/lib.rs @@ -375,7 +375,7 @@ impl MoveValueAnnotator { SignatureToken::Vector(ty) => { FatType::Vector(Box::new(self.resolve_signature(module, ty, limit)?)) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { bail!("function types NYI by fat types") }, SignatureToken::Struct(idx) => { From fd9e1e9fd7cd54018a8fb688a1ab921cc483730b Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Thu, 5 Dec 2024 19:37:33 -0800 Subject: [PATCH 5/9] add ENABLE_FUNCTION_VALUES feature flag and tests to show it is blocked by verifier. adding tests required serialization/desrialization of SignatureToken::Function. --- .../src/components/feature_flags.rs | 2 + .../aptos-vm-environment/src/prod_configs.rs | 2 + aptos-move/framework/src/built_package.rs | 9 + .../move-binary-format/src/deserializer.rs | 64 +++- .../move-binary-format/src/file_format.rs | 1 + .../src/file_format_common.rs | 1 + .../move/move-binary-format/src/serializer.rs | 11 +- .../feature_function_values_tests.rs | 302 ++++++++++++++++++ .../src/unit_tests/generic_ops_tests.rs | 42 +-- .../src/unit_tests/mod.rs | 1 + .../src/unit_tests/multi_pass_tests.rs | 2 +- .../move-bytecode-verifier/src/features.rs | 18 +- .../src/instruction_consistency.rs | 41 ++- .../move-bytecode-verifier/src/verifier.rs | 8 +- .../tests/sources/functional/restrictions.exp | 2 +- .../sources/functional/restrictions.v2_exp | 2 +- types/src/on_chain_config/aptos_features.rs | 6 + 17 files changed, 478 insertions(+), 36 deletions(-) create mode 100644 third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs diff --git a/aptos-move/aptos-release-builder/src/components/feature_flags.rs b/aptos-move/aptos-release-builder/src/components/feature_flags.rs index dc71091298b85..1f5270592048e 100644 --- a/aptos-move/aptos-release-builder/src/components/feature_flags.rs +++ b/aptos-move/aptos-release-builder/src/components/feature_flags.rs @@ -353,6 +353,7 @@ impl From for AptosFeatureFlag { FeatureFlag::CollectionOwner => AptosFeatureFlag::COLLECTION_OWNER, FeatureFlag::NativeMemoryOperations => AptosFeatureFlag::NATIVE_MEMORY_OPERATIONS, FeatureFlag::EnableLoaderV2 => AptosFeatureFlag::ENABLE_LOADER_V2, + FeatureFlag::EnableFunctionValues => AptosFeatureFlag::ENABLE_FUNCTION_VALUES, } } } @@ -500,6 +501,7 @@ impl From for FeatureFlag { AptosFeatureFlag::COLLECTION_OWNER => FeatureFlag::CollectionOwner, AptosFeatureFlag::NATIVE_MEMORY_OPERATIONS => FeatureFlag::NativeMemoryOperations, AptosFeatureFlag::ENABLE_LOADER_V2 => FeatureFlag::EnableLoaderV2, + AptosFeatureFlag::ENABLE_FUNCTION_VALUES => FeatureFlag::EnableFunctionValues, } } } diff --git a/aptos-move/aptos-vm-environment/src/prod_configs.rs b/aptos-move/aptos-vm-environment/src/prod_configs.rs index 1696776b739a7..c9e97346252b2 100644 --- a/aptos-move/aptos-vm-environment/src/prod_configs.rs +++ b/aptos-move/aptos-vm-environment/src/prod_configs.rs @@ -76,6 +76,7 @@ pub fn aptos_prod_verifier_config(features: &Features) -> VerifierConfig { let enable_enum_types = features.is_enabled(FeatureFlag::ENABLE_ENUM_TYPES); let enable_resource_access_control = features.is_enabled(FeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL); + let enable_function_values = features.is_enabled(FeatureFlag::ENABLE_FUNCTION_VALUES); VerifierConfig { max_loop_depth: Some(5), @@ -98,6 +99,7 @@ pub fn aptos_prod_verifier_config(features: &Features) -> VerifierConfig { sig_checker_v2_fix_script_ty_param_count, enable_enum_types, enable_resource_access_control, + enable_function_values, } } diff --git a/aptos-move/framework/src/built_package.rs b/aptos-move/framework/src/built_package.rs index 1a7d5edfb3d9f..640314def0e15 100644 --- a/aptos-move/framework/src/built_package.rs +++ b/aptos-move/framework/src/built_package.rs @@ -149,6 +149,15 @@ impl BuildOptions { } } + pub fn move_2_2() -> Self { + BuildOptions { + bytecode_version: Some(VERSION_7), + language_version: Some(LanguageVersion::V2_2), + compiler_version: Some(CompilerVersion::latest_stable()), + ..Self::default() + } + } + pub fn inferred_bytecode_version(&self) -> u32 { self.language_version .unwrap_or_default() diff --git a/third_party/move/move-binary-format/src/deserializer.rs b/third_party/move/move-binary-format/src/deserializer.rs index fcfcd33b192db..df8236d78eeef 100644 --- a/third_party/move/move-binary-format/src/deserializer.rs +++ b/third_party/move/move-binary-format/src/deserializer.rs @@ -1137,6 +1137,13 @@ fn load_signature_token(cursor: &mut VersionedCursor) -> BinaryLoaderResult, }, + Function { + args_arity: u64, + res_arity: u64, + args: Vec, + results: Vec, + abilities: AbilitySet, + }, } impl TypeBuilder { @@ -1163,6 +1170,41 @@ fn load_signature_token(cursor: &mut VersionedCursor) -> BinaryLoaderResult { + if args.len() >= args_arity as usize { + results.push(tok); + if results.len() >= res_arity as usize { + T::Saturated(SignatureToken::Function { + args, + results, + abilities, + }) + } else { + T::Function { + args_arity, + res_arity, + args, + results, + abilities, + } + } + } else { + args.push(tok); + T::Function { + args_arity, + res_arity, + args, + results, + abilities, + } + } + }, _ => unreachable!("invalid type constructor application"), } } @@ -1229,6 +1271,19 @@ fn load_signature_token(cursor: &mut VersionedCursor) -> BinaryLoaderResult { + let args_arity = load_signature_size(cursor)?; + let res_arity = load_signature_size(cursor)?; + let abilities = + load_ability_set(cursor, AbilitySetPosition::FunctionValueType)?; + T::Function { + args_arity, + res_arity, + abilities, + args: vec![], + results: vec![], + } + }, }) } else { Err(PartialVMError::new(StatusCode::MALFORMED) @@ -1263,6 +1318,7 @@ enum AbilitySetPosition { FunctionTypeParameters, StructTypeParameters, StructHandle, + FunctionValueType, } fn load_ability_set( @@ -1312,11 +1368,17 @@ fn load_ability_set( DeprecatedKind::RESOURCE => AbilitySet::EMPTY | Ability::Key, }; Ok(match pos { - AbilitySetPosition::StructHandle => unreachable!(), + AbilitySetPosition::StructHandle | AbilitySetPosition::FunctionValueType => { + unreachable!() + }, AbilitySetPosition::FunctionTypeParameters => set | Ability::Store, AbilitySetPosition::StructTypeParameters => set, }) }, + AbilitySetPosition::FunctionValueType => { + // This is a new type, shouldn't show up here. + Err(PartialVMError::new(StatusCode::UNKNOWN_ABILITY)) + }, } } else { // The uleb here doesn't really do anything as it is bounded currently to 0xF, but the diff --git a/third_party/move/move-binary-format/src/file_format.rs b/third_party/move/move-binary-format/src/file_format.rs index 7afb189156026..2c1791b853f01 100644 --- a/third_party/move/move-binary-format/src/file_format.rs +++ b/third_party/move/move-binary-format/src/file_format.rs @@ -1246,6 +1246,7 @@ pub enum AddressSpecifier { feature = "fuzzing", derive(arbitrary::Arbitrary, dearbitrary::Dearbitrary) )] +#[allow(unused_variables)] pub enum SignatureToken { /// Boolean, `true` or `false`. Bool, diff --git a/third_party/move/move-binary-format/src/file_format_common.rs b/third_party/move/move-binary-format/src/file_format_common.rs index 3fa19710d9176..4ba46291a8994 100644 --- a/third_party/move/move-binary-format/src/file_format_common.rs +++ b/third_party/move/move-binary-format/src/file_format_common.rs @@ -140,6 +140,7 @@ pub enum SerializedType { U16 = 0xD, U32 = 0xE, U256 = 0xF, + FUNCTION = 0x10, } /// A marker for an option in the serialized output. diff --git a/third_party/move/move-binary-format/src/serializer.rs b/third_party/move/move-binary-format/src/serializer.rs index f9c9e51917482..376ff1dceecc8 100644 --- a/third_party/move/move-binary-format/src/serializer.rs +++ b/third_party/move/move-binary-format/src/serializer.rs @@ -800,8 +800,15 @@ fn serialize_signature_token_single_node_impl( binary.push(SerializedType::TYPE_PARAMETER as u8)?; serialize_type_parameter_index(binary, *idx)?; }, - SignatureToken::Function { .. } => { - unimplemented!("serialization of function types") + SignatureToken::Function { + args, + results, + abilities, + } => { + binary.push(SerializedType::FUNCTION as u8)?; + serialize_signature_size(binary, args.len())?; + serialize_signature_size(binary, results.len())?; + serialize_ability_set(binary, *abilities)?; }, } Ok(()) diff --git a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs new file mode 100644 index 0000000000000..9ff7d9693edf0 --- /dev/null +++ b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs @@ -0,0 +1,302 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +use move_binary_format::file_format::{ + empty_module, AbilitySet, Bytecode, CodeUnit, FunctionDefinition, FunctionHandle, + FunctionHandleIndex, IdentifierIndex, ModuleHandleIndex, Signature, SignatureIndex, + SignatureToken, Visibility::Public, +}; +use move_bytecode_verifier::VerifierConfig; +use move_core_types::{identifier::Identifier, vm_status::StatusCode}; + +fn get_fun_type_bool_to_bool() -> SignatureToken { + let bool_token = SignatureToken::Bool; + let abilities = AbilitySet::PUBLIC_FUNCTIONS; + SignatureToken::Function { + args: vec![bool_token.clone()], + results: vec![bool_token.clone()], + abilities, + } +} + +fn get_fun_type_nothing_to_bool() -> SignatureToken { + let bool_token = SignatureToken::Bool; + let abilities = AbilitySet::PUBLIC_FUNCTIONS; + SignatureToken::Function { + args: vec![], + results: vec![bool_token.clone()], + abilities, + } +} + +#[test] +fn test_function_value_type() { + let mut m = empty_module(); + + // 0 == no values + m.signatures.push(Signature(vec![])); + // 1 == function bool->bool + m.signatures + .push(Signature(vec![get_fun_type_bool_to_bool()])); + + // fun f0(x: |bool|bool): |bool|bool { x } + m.identifiers + .push(Identifier::new(format!("f{}", 0)).unwrap()); + m.function_handles.push(FunctionHandle { + module: ModuleHandleIndex(0), + name: IdentifierIndex(0), + parameters: SignatureIndex(1), + return_: SignatureIndex(1), + type_parameters: vec![], + access_specifiers: None, + }); + m.function_defs.push(FunctionDefinition { + function: FunctionHandleIndex(0), + visibility: Public, + is_entry: false, + acquires_global_resources: vec![], + code: Some(CodeUnit { + // No locals + locals: SignatureIndex(0), + // Just pass through the single function value parameter + code: vec![Bytecode::Ret], + }), + }); + + let result = move_bytecode_verifier::verify_module_with_config_for_test( + "test_function_value_type", + &VerifierConfig::production(), + &m, + ); + assert_eq!( + result.unwrap_err().major_status(), + StatusCode::FEATURE_NOT_ENABLED + ); +} + +#[test] +fn test_function_ld_function() { + let mut m = empty_module(); + + // 0 == no values + m.signatures.push(Signature(vec![])); + // 1 == function bool->bool + m.signatures + .push(Signature(vec![get_fun_type_bool_to_bool()])); + // 2 == bool + m.signatures.push(Signature(vec![SignatureToken::Bool])); + + // fun f0(x; bool): bool { x } + m.identifiers + .push(Identifier::new(format!("f{}", 0)).unwrap()); + m.function_handles.push(FunctionHandle { + module: ModuleHandleIndex(0), + name: IdentifierIndex(0), + parameters: SignatureIndex(1), + return_: SignatureIndex(1), + type_parameters: vec![], + access_specifiers: None, + }); + m.function_defs.push(FunctionDefinition { + function: FunctionHandleIndex(0), + visibility: Public, + is_entry: false, + acquires_global_resources: vec![], + code: Some(CodeUnit { + // No locals + locals: SignatureIndex(0), + // Just pass through the single function value parameter + code: vec![Bytecode::Ret], + }), + }); + + // fun f1(): |bool|bool { f0 } + m.identifiers + .push(Identifier::new(format!("f{}", 1)).unwrap()); + m.function_handles.push(FunctionHandle { + module: ModuleHandleIndex(0), + name: IdentifierIndex(1), + parameters: SignatureIndex(0), + return_: SignatureIndex(1), + type_parameters: vec![], + access_specifiers: None, + }); + m.function_defs.push(FunctionDefinition { + function: FunctionHandleIndex(1), + visibility: Public, + is_entry: false, + acquires_global_resources: vec![], + code: Some(CodeUnit { + // No locals + locals: SignatureIndex(0), + // Return the function value + code: vec![Bytecode::LdFunction(FunctionHandleIndex(1)), Bytecode::Ret], + }), + }); + + let result = move_bytecode_verifier::verify_module_with_config_for_test( + "test_function_ld_function", + &VerifierConfig::production(), + &m, + ); + assert_eq!( + result.unwrap_err().major_status(), + StatusCode::FEATURE_NOT_ENABLED, + ); +} + +#[test] +fn test_function_early_bind() { + let mut m = empty_module(); + + // 0 == no values + m.signatures.push(Signature(vec![])); + // 1 == function bool->bool + m.signatures + .push(Signature(vec![get_fun_type_bool_to_bool()])); + // 2 == bool + m.signatures.push(Signature(vec![SignatureToken::Bool])); + // 3 == function ()->bool + m.signatures + .push(Signature(vec![get_fun_type_nothing_to_bool()])); + + // fun f0(x; bool): bool { x } + m.identifiers + .push(Identifier::new(format!("f{}", 0)).unwrap()); + m.function_handles.push(FunctionHandle { + module: ModuleHandleIndex(0), + name: IdentifierIndex(0), + parameters: SignatureIndex(1), + return_: SignatureIndex(1), + type_parameters: vec![], + access_specifiers: None, + }); + m.function_defs.push(FunctionDefinition { + function: FunctionHandleIndex(0), + visibility: Public, + is_entry: false, + acquires_global_resources: vec![], + code: Some(CodeUnit { + // No locals + locals: SignatureIndex(0), + // Just pass through the single function value parameter + code: vec![Bytecode::Ret], + }), + }); + + // fun f1(x: bool): ||bool { || f0(x) } + m.identifiers + .push(Identifier::new(format!("f{}", 1)).unwrap()); + m.function_handles.push(FunctionHandle { + module: ModuleHandleIndex(0), + name: IdentifierIndex(1), + parameters: SignatureIndex(1), + return_: SignatureIndex(2), + type_parameters: vec![], + access_specifiers: None, + }); + m.function_defs.push(FunctionDefinition { + function: FunctionHandleIndex(1), + visibility: Public, + is_entry: false, + acquires_global_resources: vec![], + code: Some(CodeUnit { + // No locals + locals: SignatureIndex(0), + // Bool is on stack, load the function, early bind 1 param, return result + code: vec![ + Bytecode::LdFunction(FunctionHandleIndex(0)), + Bytecode::EarlyBindFunction(SignatureIndex(1), 1u8), + Bytecode::Ret, + ], + }), + }); + + let result = move_bytecode_verifier::verify_module_with_config_for_test( + "test_function_ld_function", + &VerifierConfig::production(), + &m, + ); + assert_eq!( + result.unwrap_err().major_status(), + StatusCode::FEATURE_NOT_ENABLED + ); +} + +#[test] +fn test_function_value_call() { + let mut m = empty_module(); + + // 0 == no values + m.signatures.push(Signature(vec![])); + // 1 == function bool->bool + m.signatures + .push(Signature(vec![get_fun_type_bool_to_bool()])); + // 2 == bool + m.signatures.push(Signature(vec![SignatureToken::Bool])); + // 3 == function ()->bool + m.signatures + .push(Signature(vec![get_fun_type_nothing_to_bool()])); + + // fun f0(x; bool): bool { x } + m.identifiers + .push(Identifier::new(format!("f{}", 0)).unwrap()); + m.function_handles.push(FunctionHandle { + module: ModuleHandleIndex(0), + name: IdentifierIndex(0), + parameters: SignatureIndex(1), + return_: SignatureIndex(1), + type_parameters: vec![], + access_specifiers: None, + }); + m.function_defs.push(FunctionDefinition { + function: FunctionHandleIndex(0), + visibility: Public, + is_entry: false, + acquires_global_resources: vec![], + code: Some(CodeUnit { + // No locals + locals: SignatureIndex(0), + // Just pass through the single function value parameter + code: vec![Bytecode::Ret], + }), + }); + + // fun f1(x: bool): ||bool { (f0)(x) } + m.identifiers + .push(Identifier::new(format!("f{}", 1)).unwrap()); + m.function_handles.push(FunctionHandle { + module: ModuleHandleIndex(0), + name: IdentifierIndex(1), + parameters: SignatureIndex(1), + return_: SignatureIndex(2), + type_parameters: vec![], + access_specifiers: None, + }); + m.function_defs.push(FunctionDefinition { + function: FunctionHandleIndex(1), + visibility: Public, + is_entry: false, + acquires_global_resources: vec![], + code: Some(CodeUnit { + // No locals + locals: SignatureIndex(0), + // Bool is on stack, load the function value, Invoke + code: vec![ + Bytecode::LdFunction(FunctionHandleIndex(0)), + Bytecode::InvokeFunction(SignatureIndex(1)), + Bytecode::Ret, + ], + }), + }); + + let result = move_bytecode_verifier::verify_module_with_config_for_test( + "test_function_ld_function", + &VerifierConfig::production(), + &m, + ); + assert_eq!( + result.unwrap_err().major_status(), + StatusCode::FEATURE_NOT_ENABLED + ); +} diff --git a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/generic_ops_tests.rs b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/generic_ops_tests.rs index 7808e41642981..18c03fd82f5d2 100644 --- a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/generic_ops_tests.rs +++ b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/generic_ops_tests.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 use move_binary_format::file_format::*; -use move_bytecode_verifier::InstructionConsistency; +use move_bytecode_verifier::{InstructionConsistency, VerifierConfig}; use move_core_types::{ account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, }; @@ -206,7 +206,7 @@ fn generic_call_to_non_generic_func() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("CallGeneric to non generic function must fail"); assert_eq!( err.major_status(), @@ -222,7 +222,7 @@ fn non_generic_call_to_generic_func() { locals: SignatureIndex(0), code: vec![Bytecode::Call(FunctionHandleIndex(1)), Bytecode::Ret], }); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("Call to generic function must fail"); assert_eq!( err.major_status(), @@ -250,7 +250,7 @@ fn generic_pack_on_non_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("PackGeneric to non generic struct must fail"); assert_eq!( err.major_status(), @@ -271,7 +271,7 @@ fn non_generic_pack_on_generic_struct() { Bytecode::Ret, ], }); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("Pack to generic struct must fail"); assert_eq!( err.major_status(), @@ -300,7 +300,7 @@ fn generic_unpack_on_non_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("UnpackGeneric to non generic struct must fail"); assert_eq!( err.major_status(), @@ -329,7 +329,7 @@ fn non_generic_unpack_on_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("Unpack to generic struct must fail"); assert_eq!( err.major_status(), @@ -360,7 +360,7 @@ fn generic_mut_borrow_field_on_non_generic_struct() { field: 0, }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("MutBorrowFieldGeneric to non generic struct must fail"); assert_eq!( err.major_status(), @@ -393,7 +393,7 @@ fn non_generic_mut_borrow_field_on_generic_struct() { field: 0, }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("MutBorrowField to generic struct must fail"); assert_eq!( err.major_status(), @@ -424,7 +424,7 @@ fn generic_borrow_field_on_non_generic_struct() { field: 0, }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("ImmBorrowFieldGeneric to non generic struct must fail"); assert_eq!( err.major_status(), @@ -457,7 +457,7 @@ fn non_generic_borrow_field_on_generic_struct() { field: 0, }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("ImmBorrowField to generic struct must fail"); assert_eq!( err.major_status(), @@ -488,7 +488,7 @@ fn generic_mut_borrow_global_to_non_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("MutBorrowGlobalGeneric to non generic function must fail"); assert_eq!( err.major_status(), @@ -512,7 +512,7 @@ fn non_generic_mut_borrow_global_to_generic_struct() { Bytecode::Ret, ], }); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("MutBorrowGlobal to generic function must fail"); assert_eq!( err.major_status(), @@ -543,7 +543,7 @@ fn generic_immut_borrow_global_to_non_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("ImmBorrowGlobalGeneric to non generic function must fail"); assert_eq!( err.major_status(), @@ -567,7 +567,7 @@ fn non_generic_immut_borrow_global_to_generic_struct() { Bytecode::Ret, ], }); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("ImmBorrowGlobal to generic function must fail"); assert_eq!( err.major_status(), @@ -595,7 +595,7 @@ fn generic_exists_to_non_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("ExistsGeneric to non generic function must fail"); assert_eq!( err.major_status(), @@ -616,7 +616,7 @@ fn non_generic_exists_to_generic_struct() { Bytecode::Ret, ], }); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("Exists to generic function must fail"); assert_eq!( err.major_status(), @@ -648,7 +648,7 @@ fn generic_move_from_to_non_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("MoveFromGeneric to non generic function must fail"); assert_eq!( err.major_status(), @@ -680,7 +680,7 @@ fn non_generic_move_from_to_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("MoveFrom to generic function must fail"); assert_eq!( err.major_status(), @@ -709,7 +709,7 @@ fn generic_move_to_on_non_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("MoveToGeneric to non generic struct must fail"); assert_eq!( err.major_status(), @@ -738,7 +738,7 @@ fn non_generic_move_to_on_generic_struct() { type_parameters: SignatureIndex(2), }); module.signatures.push(Signature(vec![SignatureToken::U64])); - let err = InstructionConsistency::verify_module(&module) + let err = InstructionConsistency::verify_module(&VerifierConfig::default(), &module) .expect_err("MoveTo to generic struct must fail"); assert_eq!( err.major_status(), diff --git a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/mod.rs b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/mod.rs index 0540045fb8b43..a59f60b4e9955 100644 --- a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/mod.rs +++ b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/mod.rs @@ -11,6 +11,7 @@ pub mod constants_tests; pub mod control_flow_tests; pub mod dependencies_tests; pub mod duplication_tests; +pub mod feature_function_values_tests; pub mod generic_ops_tests; pub mod large_type_test; pub mod limit_tests; diff --git a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs index d244f6dd37e5a..7d05ec4e188d9 100644 --- a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs +++ b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs @@ -14,7 +14,7 @@ proptest! { fn check_verifier_passes(module in CompiledModule::valid_strategy(20)) { DuplicationChecker::verify_module(&module).expect("DuplicationChecker failure"); SignatureChecker::verify_module(&module).expect("SignatureChecker failure"); - InstructionConsistency::verify_module(&module).expect("InstructionConsistency failure"); + InstructionConsistency::verify_module(&Default::default(), &module).expect("InstructionConsistency failure"); constants::verify_module(&module).expect("constants failure"); ability_field_requirements::verify_module(&module).expect("ability_field_requirements failure"); RecursiveStructDefChecker::verify_module(&module).expect("RecursiveStructDefChecker failure"); diff --git a/third_party/move/move-bytecode-verifier/src/features.rs b/third_party/move/move-bytecode-verifier/src/features.rs index 9ea211b09f8ab..e2f898d66d75d 100644 --- a/third_party/move/move-bytecode-verifier/src/features.rs +++ b/third_party/move/move-bytecode-verifier/src/features.rs @@ -8,7 +8,7 @@ use crate::VerifierConfig; use move_binary_format::{ binary_views::BinaryIndexedView, errors::{Location, PartialVMError, PartialVMResult, VMResult}, - file_format::{CompiledModule, CompiledScript, StructFieldInformation}, + file_format::{CompiledModule, CompiledScript, SignatureToken, StructFieldInformation}, IndexKind, }; use move_core_types::vm_status::StatusCode; @@ -33,6 +33,7 @@ impl<'a> FeatureVerifier<'a> { code: BinaryIndexedView::Module(module), }; verifier.verify_function_handles()?; + verifier.verify_function_value_types()?; verifier.verify_struct_defs() } @@ -81,4 +82,19 @@ impl<'a> FeatureVerifier<'a> { } Ok(()) } + + fn verify_function_value_types(&self) -> PartialVMResult<()> { + if !self.config.enable_function_values { + for (idx, signature) in self.code.signatures().iter().enumerate() { + for signature_token in signature.0.iter() { + if matches!(signature_token, SignatureToken::Function { .. }) { + return Err(PartialVMError::new(StatusCode::FEATURE_NOT_ENABLED) + .at_index(IndexKind::Signature, idx as u16) + .with_message("function values feature not enabled".to_string())); + } + } + } + } + Ok(()) + } } diff --git a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs index ba4e7d9e5c14c..752489ff69a89 100644 --- a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs +++ b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs @@ -6,6 +6,7 @@ //! instruction, in particular, for the bytecode instructions that come in both generic and //! non-generic flavors. It also checks constraints on instructions like VecPack/VecUnpack. +use crate::VerifierConfig; use move_binary_format::{ access::ModuleAccess, binary_views::BinaryIndexedView, @@ -19,16 +20,21 @@ use move_binary_format::{ use move_core_types::vm_status::StatusCode; pub struct InstructionConsistency<'a> { + config: &'a VerifierConfig, resolver: BinaryIndexedView<'a>, current_function: Option, } impl<'a> InstructionConsistency<'a> { - pub fn verify_module(module: &'a CompiledModule) -> VMResult<()> { - Self::verify_module_impl(module).map_err(|e| e.finish(Location::Module(module.self_id()))) + pub fn verify_module(config: &'a VerifierConfig, module: &'a CompiledModule) -> VMResult<()> { + Self::verify_module_impl(config, module) + .map_err(|e| e.finish(Location::Module(module.self_id()))) } - fn verify_module_impl(module: &'a CompiledModule) -> PartialVMResult<()> { + fn verify_module_impl( + config: &'a VerifierConfig, + module: &'a CompiledModule, + ) -> PartialVMResult<()> { let resolver = BinaryIndexedView::Module(module); for (idx, func_def) in module.function_defs().iter().enumerate() { @@ -36,6 +42,7 @@ impl<'a> InstructionConsistency<'a> { None => (), Some(code) => { let checker = Self { + config, resolver, current_function: Some(FunctionDefinitionIndex(idx as TableIndex)), }; @@ -46,12 +53,16 @@ impl<'a> InstructionConsistency<'a> { Ok(()) } - pub fn verify_script(module: &'a CompiledScript) -> VMResult<()> { - Self::verify_script_impl(module).map_err(|e| e.finish(Location::Script)) + pub fn verify_script(config: &'a VerifierConfig, module: &'a CompiledScript) -> VMResult<()> { + Self::verify_script_impl(config, module).map_err(|e| e.finish(Location::Script)) } - pub fn verify_script_impl(script: &'a CompiledScript) -> PartialVMResult<()> { + pub fn verify_script_impl( + config: &'a VerifierConfig, + script: &'a CompiledScript, + ) -> PartialVMResult<()> { let checker = Self { + config, resolver: BinaryIndexedView::Script(script), current_function: None, }; @@ -98,17 +109,29 @@ impl<'a> InstructionConsistency<'a> { self.check_function_op(offset, func_inst.handle, /* generic */ true)?; }, LdFunction(idx) => { + if !self.config.enable_function_values { + return self.function_value_disabled_error(offset); + }; self.check_ld_function_op(offset, *idx, /* generic */ false)?; }, LdFunctionGeneric(idx) => { + if !self.config.enable_function_values { + return self.function_value_disabled_error(offset); + }; let func_inst = self.resolver.function_instantiation_at(*idx); self.check_ld_function_op(offset, func_inst.handle, /* generic */ true)?; }, InvokeFunction(sig_idx) => { + if !self.config.enable_function_values { + return self.function_value_disabled_error(offset); + }; // reuse code to check for signature issues. self.check_bind_count(offset, *sig_idx, 0)?; }, EarlyBindFunction(sig_idx, count) => { + if !self.config.enable_function_values { + return self.function_value_disabled_error(offset); + }; self.check_bind_count(offset, *sig_idx, *count)?; }, Pack(idx) | Unpack(idx) => { @@ -283,4 +306,10 @@ impl<'a> InstructionConsistency<'a> { } Ok(()) } + + fn function_value_disabled_error(&self, offset: usize) -> PartialVMResult<()> { + return Err(PartialVMError::new(StatusCode::FEATURE_NOT_ENABLED) + .at_code_offset(self.current_function(), offset as CodeOffset) + .with_message("function values feature not enabled".to_string())); + } } diff --git a/third_party/move/move-bytecode-verifier/src/verifier.rs b/third_party/move/move-bytecode-verifier/src/verifier.rs index bd5feb8942664..77cc265998c1e 100644 --- a/third_party/move/move-bytecode-verifier/src/verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/verifier.rs @@ -42,6 +42,7 @@ pub struct VerifierConfig { pub sig_checker_v2_fix_script_ty_param_count: bool, pub enable_enum_types: bool, pub enable_resource_access_control: bool, + pub enable_function_values: bool, } /// Helper for a "canonical" verification of a module. @@ -121,7 +122,7 @@ pub fn verify_module_with_config(config: &VerifierConfig, module: &CompiledModul SignatureChecker::verify_module(module)?; } - InstructionConsistency::verify_module(module)?; + InstructionConsistency::verify_module(config, module)?; constants::verify_module(module)?; friends::verify_module(module)?; if !config.use_signature_checker_v2 { @@ -182,7 +183,7 @@ pub fn verify_script_with_config(config: &VerifierConfig, script: &CompiledScrip SignatureChecker::verify_script(script)?; } - InstructionConsistency::verify_script(script)?; + InstructionConsistency::verify_script(config, script)?; constants::verify_script(script)?; CodeUnitVerifier::verify_script(config, script)?; script_signature::verify_script(script, no_additional_script_signature_checks) @@ -240,6 +241,7 @@ impl Default for VerifierConfig { enable_enum_types: true, enable_resource_access_control: true, + enable_function_values: true, } } } @@ -284,6 +286,8 @@ impl VerifierConfig { enable_enum_types: true, enable_resource_access_control: true, + + enable_function_values: false, } } } diff --git a/third_party/move/move-prover/tests/sources/functional/restrictions.exp b/third_party/move/move-prover/tests/sources/functional/restrictions.exp index bcc3d281dd599..acbf297be4b0e 100644 --- a/third_party/move/move-prover/tests/sources/functional/restrictions.exp +++ b/third_party/move/move-prover/tests/sources/functional/restrictions.exp @@ -11,7 +11,7 @@ error: [boogie translator] function result type not yet supported 12 │ fun f2(): | |num { | | 1 } │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: [boogie translator] Invoke not yet supported +error: [boogie translator] InvokeFunction not yet supported ┌─ tests/sources/functional/restrictions.move:16:13 │ 16 │ f(1u64) diff --git a/third_party/move/move-prover/tests/sources/functional/restrictions.v2_exp b/third_party/move/move-prover/tests/sources/functional/restrictions.v2_exp index bcc3d281dd599..acbf297be4b0e 100644 --- a/third_party/move/move-prover/tests/sources/functional/restrictions.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/restrictions.v2_exp @@ -11,7 +11,7 @@ error: [boogie translator] function result type not yet supported 12 │ fun f2(): | |num { | | 1 } │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: [boogie translator] Invoke not yet supported +error: [boogie translator] InvokeFunction not yet supported ┌─ tests/sources/functional/restrictions.move:16:13 │ 16 │ f(1u64) diff --git a/types/src/on_chain_config/aptos_features.rs b/types/src/on_chain_config/aptos_features.rs index 70bc4f18aa546..fddcf1c486723 100644 --- a/types/src/on_chain_config/aptos_features.rs +++ b/types/src/on_chain_config/aptos_features.rs @@ -100,6 +100,7 @@ pub enum FeatureFlag { /// AIP-105 (https://github.com/aptos-foundation/AIPs/blob/main/aips/aip-105.md) NATIVE_MEMORY_OPERATIONS = 80, ENABLE_LOADER_V2 = 81, + ENABLE_FUNCTION_VALUES = 82, } impl FeatureFlag { @@ -178,6 +179,7 @@ impl FeatureFlag { FeatureFlag::NATIVE_MEMORY_OPERATIONS, FeatureFlag::COLLECTION_OWNER, FeatureFlag::ENABLE_LOADER_V2, + FeatureFlag::ENABLE_FUNCTION_VALUES, ] } } @@ -326,6 +328,10 @@ impl Features { self.is_enabled(FeatureFlag::ENABLE_LOADER_V2) } + pub fn is_function_values_enabled(&self) -> bool { + self.is_enabled(FeatureFlag::ENABLE_FUNCTION_VALUES) + } + pub fn get_max_identifier_size(&self) -> u64 { if self.is_enabled(FeatureFlag::LIMIT_MAX_IDENTIFIER_LENGTH) { IDENTIFIER_SIZE_MAX From e8107d70b1ffb9f8433e9ac2e19ccb806d32efa1 Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Sat, 7 Dec 2024 02:26:32 -0800 Subject: [PATCH 6/9] don't use named fields for enum SignatureVariant variant Function, since that yields Clippy warnings, apparently due to a hidden bug in one of the derived trait macros invoked here (dearbitrary) --- api/types/src/bytecode.rs | 2 +- aptos-move/script-composer/src/decompiler.rs | 2 +- .../move-binary-format/src/binary_views.rs | 10 +-- .../move/move-binary-format/src/builders.rs | 14 ++--- .../move/move-binary-format/src/constant.rs | 2 +- .../move-binary-format/src/deserializer.rs | 4 +- .../move-binary-format/src/file_format.rs | 36 +++++------ .../src/proptest_types/types.rs | 2 +- .../move/move-binary-format/src/serializer.rs | 4 +- .../feature_function_values_tests.rs | 16 ++--- .../invalid-mutations/src/bounds.rs | 2 +- .../src/dependencies.rs | 22 +++---- .../move-bytecode-verifier/src/features.rs | 2 +- .../src/instantiation_loops.rs | 2 +- .../src/instruction_consistency.rs | 2 +- .../src/reference_safety/mod.rs | 2 +- .../move-bytecode-verifier/src/signature.rs | 6 +- .../src/signature_v2.rs | 8 +-- .../src/stack_usage_verifier.rs | 4 +- .../move-bytecode-verifier/src/struct_defs.rs | 2 +- .../move-bytecode-verifier/src/type_safety.rs | 62 +++++++++---------- .../move-compiler/src/interface_generator.rs | 23 ++++++- .../move-ir-to-bytecode/src/context.rs | 3 +- third_party/move/move-model/src/ty.rs | 2 +- .../move-vm/runtime/src/loader/type_loader.rs | 4 +- .../move-vm/types/src/values/values_impl.rs | 2 +- .../tools/move-bytecode-utils/src/layout.rs | 5 +- .../move-disassembler/src/disassembler.rs | 26 +++++++- .../tools/move-resource-viewer/src/lib.rs | 2 +- 29 files changed, 157 insertions(+), 116 deletions(-) diff --git a/api/types/src/bytecode.rs b/api/types/src/bytecode.rs index 45908b2e2b712..f51a8ccfb16e1 100644 --- a/api/types/src/bytecode.rs +++ b/api/types/src/bytecode.rs @@ -105,7 +105,7 @@ pub trait Bytecode { mutable: true, to: Box::new(self.new_move_type(t.borrow())), }, - SignatureToken::Function { .. } => { + SignatureToken::Function( .. ) => { // TODO(LAMBDA) unimplemented!("signature token function to API MoveType") }, diff --git a/aptos-move/script-composer/src/decompiler.rs b/aptos-move/script-composer/src/decompiler.rs index 861bbe990a6b1..147d976b90ccf 100644 --- a/aptos-move/script-composer/src/decompiler.rs +++ b/aptos-move/script-composer/src/decompiler.rs @@ -208,7 +208,7 @@ impl LocalState { SignatureToken::Vector(s) => { TypeTag::Vector(Box::new(Self::type_tag_from_sig_token(script, s)?)) }, - SignatureToken::Function { .. } => { + SignatureToken::Function(..) => { // TODO(LAMBDA) bail!("function types not yet implemented for script composer") }, diff --git a/third_party/move/move-binary-format/src/binary_views.rs b/third_party/move/move-binary-format/src/binary_views.rs index f1093d22c45e7..a7c4a8f6600d2 100644 --- a/third_party/move/move-binary-format/src/binary_views.rs +++ b/third_party/move/move-binary-format/src/binary_views.rs @@ -340,10 +340,12 @@ impl<'a> BinaryIndexedView<'a> { Reference(_) | MutableReference(_) => Ok(AbilitySet::REFERENCES), Signer => Ok(AbilitySet::SIGNER), TypeParameter(idx) => Ok(constraints[*idx as usize]), - Vector(ty) => AbilitySet::polymorphic_abilities(AbilitySet::VECTOR, vec![false], vec![ - self.abilities(ty, constraints)?, - ]), - Function { abilities, .. } => Ok(*abilities), + Vector(ty) => AbilitySet::polymorphic_abilities( + AbilitySet::VECTOR, + vec![false], + vec![self.abilities(ty, constraints)?], + ), + Function(_, _, abilities) => Ok(*abilities), Struct(idx) => { let sh = self.struct_handle_at(*idx); Ok(sh.abilities) diff --git a/third_party/move/move-binary-format/src/builders.rs b/third_party/move/move-binary-format/src/builders.rs index 5cacb8e3e62b2..69c6d7889d60a 100644 --- a/third_party/move/move-binary-format/src/builders.rs +++ b/third_party/move/move-binary-format/src/builders.rs @@ -235,15 +235,11 @@ impl CompiledScriptBuilder { MutableReference(Box::new(self.import_signature_token(module, ty)?)) }, Vector(ty) => Vector(Box::new(self.import_signature_token(module, ty)?)), - Function { - args, - results, - abilities, - } => Function { - args: import_vec(self, args)?, - results: import_vec(self, results)?, - abilities: *abilities, - }, + Function(args, results, abilities) => Function( + import_vec(self, args)?, + import_vec(self, results)?, + *abilities, + ), Struct(idx) => Struct(self.import_struct(module, *idx)?), StructInstantiation(idx, inst_tys) => StructInstantiation( self.import_struct(module, *idx)?, diff --git a/third_party/move/move-binary-format/src/constant.rs b/third_party/move/move-binary-format/src/constant.rs index 1f27d9f259fe3..3b8d200787a4e 100644 --- a/third_party/move/move-binary-format/src/constant.rs +++ b/third_party/move/move-binary-format/src/constant.rs @@ -17,7 +17,7 @@ fn sig_to_ty(sig: &SignatureToken) -> Option { SignatureToken::U128 => Some(MoveTypeLayout::U128), SignatureToken::U256 => Some(MoveTypeLayout::U256), SignatureToken::Vector(v) => Some(MoveTypeLayout::Vector(Box::new(sig_to_ty(v.as_ref())?))), - SignatureToken::Function { .. } => { + SignatureToken::Function(..) => { // TODO(LAMBDA): do we need representation in MoveTypeLayout? None }, diff --git a/third_party/move/move-binary-format/src/deserializer.rs b/third_party/move/move-binary-format/src/deserializer.rs index df8236d78eeef..b5418d12dd469 100644 --- a/third_party/move/move-binary-format/src/deserializer.rs +++ b/third_party/move/move-binary-format/src/deserializer.rs @@ -1180,11 +1180,11 @@ fn load_signature_token(cursor: &mut VersionedCursor) -> BinaryLoaderResult= args_arity as usize { results.push(tok); if results.len() >= res_arity as usize { - T::Saturated(SignatureToken::Function { + T::Saturated(SignatureToken::Function( args, results, abilities, - }) + )) } else { T::Function { args_arity, diff --git a/third_party/move/move-binary-format/src/file_format.rs b/third_party/move/move-binary-format/src/file_format.rs index 2c1791b853f01..927fa7b9f6e5b 100644 --- a/third_party/move/move-binary-format/src/file_format.rs +++ b/third_party/move/move-binary-format/src/file_format.rs @@ -1263,11 +1263,11 @@ pub enum SignatureToken { /// Vector Vector(Box), /// Function, with n argument types and m result types, and an associated ability set. - Function { - args: Vec, - results: Vec, - abilities: AbilitySet, - }, + Function( + Vec, // args + Vec, // results + AbilitySet, // abilities + ), /// User defined type Struct(StructHandleIndex), StructInstantiation(StructHandleIndex, Vec), @@ -1310,7 +1310,7 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIter<'a> { self.stack.extend(inner_toks.iter().rev()) }, - Function { args, results, .. } => { + Function( args, results, .. ) => { self.stack.extend(args.iter().rev()); self.stack.extend(results.iter().rev()); }, @@ -1348,7 +1348,7 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIterWithDepth<'a> { .stack .extend(inner_toks.iter().map(|tok| (tok, depth + 1)).rev()), - Function { args, results, .. } => { + Function( args, results, .. ) => { self.stack .extend(args.iter().map(|tok| (tok, depth + 1)).rev()); self.stack @@ -1415,11 +1415,11 @@ impl std::fmt::Debug for SignatureToken { SignatureToken::Address => write!(f, "Address"), SignatureToken::Signer => write!(f, "Signer"), SignatureToken::Vector(boxed) => write!(f, "Vector({:?})", boxed), - SignatureToken::Function { + SignatureToken::Function ( args, results, abilities, - } => { + ) => { write!(f, "Function({:?}, {:?}, {})", args, results, abilities) }, SignatureToken::Reference(boxed) => write!(f, "Reference({:?})", boxed), @@ -1443,7 +1443,7 @@ impl SignatureToken { | Address | Signer | Vector(_) - | Function { .. } + | Function ( .. ) | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1482,7 +1482,7 @@ impl SignatureToken { Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address => true, Vector(inner) => inner.is_valid_for_constant(), Signer - | Function { .. } + | Function ( .. ) | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1495,7 +1495,7 @@ impl SignatureToken { pub fn is_function(&self) -> bool { use SignatureToken::*; - matches!(self, Function { .. }) + matches!(self, Function ( .. )) } /// Set the index to this one. Useful for random testing. @@ -1546,15 +1546,15 @@ impl SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(ty.instantiate(subst_mapping))), - Function { + Function ( args, results, abilities, - } => Function { - args: inst_vec(args), - results: inst_vec(results), - abilities: *abilities, - }, + ) => Function ( + inst_vec(args), + inst_vec(results), + *abilities, + ), Struct(idx) => Struct(*idx), StructInstantiation(idx, struct_type_args) => { StructInstantiation(*idx, inst_vec(struct_type_args)) diff --git a/third_party/move/move-binary-format/src/proptest_types/types.rs b/third_party/move/move-binary-format/src/proptest_types/types.rs index 830dd5c564298..17e599f3aacb6 100644 --- a/third_party/move/move-binary-format/src/proptest_types/types.rs +++ b/third_party/move/move-binary-format/src/proptest_types/types.rs @@ -71,7 +71,7 @@ impl StDefnMaterializeState { let inner = self.potential_abilities(ty); inner.intersect(AbilitySet::VECTOR) }, - Function { abilities, .. } => *abilities, + Function(_, _, abilities) => *abilities, Struct(idx) => { let sh = &self.struct_handles[idx.0 as usize]; sh.abilities diff --git a/third_party/move/move-binary-format/src/serializer.rs b/third_party/move/move-binary-format/src/serializer.rs index 376ff1dceecc8..b64cf045ec2ff 100644 --- a/third_party/move/move-binary-format/src/serializer.rs +++ b/third_party/move/move-binary-format/src/serializer.rs @@ -800,11 +800,11 @@ fn serialize_signature_token_single_node_impl( binary.push(SerializedType::TYPE_PARAMETER as u8)?; serialize_type_parameter_index(binary, *idx)?; }, - SignatureToken::Function { + SignatureToken::Function( args, results, abilities, - } => { + ) => { binary.push(SerializedType::FUNCTION as u8)?; serialize_signature_size(binary, args.len())?; serialize_signature_size(binary, results.len())?; diff --git a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs index 9ff7d9693edf0..31ed738c6c4a7 100644 --- a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs +++ b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs @@ -12,21 +12,21 @@ use move_core_types::{identifier::Identifier, vm_status::StatusCode}; fn get_fun_type_bool_to_bool() -> SignatureToken { let bool_token = SignatureToken::Bool; let abilities = AbilitySet::PUBLIC_FUNCTIONS; - SignatureToken::Function { - args: vec![bool_token.clone()], - results: vec![bool_token.clone()], + SignatureToken::Function( + vec![bool_token.clone()], + vec![bool_token.clone()], abilities, - } + ) } fn get_fun_type_nothing_to_bool() -> SignatureToken { let bool_token = SignatureToken::Bool; let abilities = AbilitySet::PUBLIC_FUNCTIONS; - SignatureToken::Function { - args: vec![], - results: vec![bool_token.clone()], + SignatureToken::Function( + vec![], + vec![bool_token.clone()], abilities, - } + ) } #[test] diff --git a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs index 0d7858b658c7d..9470a6cbf3485 100644 --- a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs +++ b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs @@ -369,6 +369,6 @@ fn struct_handle(token: &SignatureToken) -> Option { | Signer | Vector(_) | TypeParameter(_) - | Function { .. } => None, + | Function(..) => None, } } diff --git a/third_party/move/move-bytecode-verifier/src/dependencies.rs b/third_party/move/move-bytecode-verifier/src/dependencies.rs index 74d86a40a5783..1ce3789fa6452 100644 --- a/third_party/move/move-bytecode-verifier/src/dependencies.rs +++ b/third_party/move/move-bytecode-verifier/src/dependencies.rs @@ -456,16 +456,16 @@ fn compare_types( compare_types(context, ty1, ty2, def_module) }, ( - SignatureToken::Function { - args: args1, - results: result1, - abilities: abilities1, - }, - SignatureToken::Function { - args: args2, - results: result2, - abilities: abilities2, - }, + SignatureToken::Function( + args1, + result1, + abilities1, + ), + SignatureToken::Function( + args2, + result2, + abilities2, + ), ) => { compare_cross_module_signatures(context, args1, args2, def_module)?; compare_cross_module_signatures(context, result1, result2, def_module)?; @@ -504,7 +504,7 @@ fn compare_types( | (SignatureToken::Address, _) | (SignatureToken::Signer, _) | (SignatureToken::Vector(_), _) - | (SignatureToken::Function { .. }, _) + | (SignatureToken::Function(..), _) | (SignatureToken::Struct(_), _) | (SignatureToken::StructInstantiation(_, _), _) | (SignatureToken::Reference(_), _) diff --git a/third_party/move/move-bytecode-verifier/src/features.rs b/third_party/move/move-bytecode-verifier/src/features.rs index e2f898d66d75d..b2de902b7959d 100644 --- a/third_party/move/move-bytecode-verifier/src/features.rs +++ b/third_party/move/move-bytecode-verifier/src/features.rs @@ -87,7 +87,7 @@ impl<'a> FeatureVerifier<'a> { if !self.config.enable_function_values { for (idx, signature) in self.code.signatures().iter().enumerate() { for signature_token in signature.0.iter() { - if matches!(signature_token, SignatureToken::Function { .. }) { + if matches!(signature_token, SignatureToken::Function(..)) { return Err(PartialVMError::new(StatusCode::FEATURE_NOT_ENABLED) .at_index(IndexKind::Signature, idx as u16) .with_message("function values feature not enabled".to_string())); diff --git a/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs b/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs index d1a1501d41430..00979d9826e2c 100644 --- a/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs +++ b/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs @@ -148,7 +148,7 @@ impl<'a> InstantiationLoopChecker<'a> { type_params.insert(*idx); }, Vector(ty) => rec(type_params, ty), - Function { args, results, .. } => { + Function(args, results, ..) => { for ty in args { rec(type_params, ty); } diff --git a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs index 752489ff69a89..17c6bf4e6bf64 100644 --- a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs +++ b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs @@ -289,7 +289,7 @@ impl<'a> InstructionConsistency<'a> { ) -> PartialVMResult<()> { let signature = self.resolver.signature_at(sig_index); if let Some(sig_token) = signature.0.first() { - if let SignatureToken::Function { args: params, .. } = sig_token { + if let SignatureToken::Function(params, ..) = sig_token { if count as usize > params.len() { return Err( PartialVMError::new(StatusCode::NUMBER_OF_TYPE_ARGUMENTS_MISMATCH) diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs index 651f54078d784..276e3bae106f0 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs @@ -250,7 +250,7 @@ fn fun_type( idx: SignatureIndex, ) -> PartialVMResult<(Vec, Vec)> { match verifier.resolver.signature_at(idx).0.first() { - Some(SignatureToken::Function { args, results, .. }) => Ok((args.clone(), results.clone())), + Some(SignatureToken::Function(args, results, ..)) => Ok((args.clone(), results.clone())), _ => Err(PartialVMError::new( StatusCode::VERIFIER_INVARIANT_VIOLATION, )), diff --git a/third_party/move/move-bytecode-verifier/src/signature.rs b/third_party/move/move-bytecode-verifier/src/signature.rs index a89dc52c8d783..a7b2d4372c9ab 100644 --- a/third_party/move/move-bytecode-verifier/src/signature.rs +++ b/third_party/move/move-bytecode-verifier/src/signature.rs @@ -372,7 +372,7 @@ impl<'a> SignatureChecker<'a> { } }, - SignatureToken::Function { .. } => { + SignatureToken::Function(..) => { return Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())); }, @@ -429,7 +429,7 @@ impl<'a> SignatureChecker<'a> { Err(PartialVMError::new(StatusCode::INVALID_SIGNATURE_TOKEN) .with_message("reference not allowed".to_string())) }, - Function { .. } => Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) + Function(..) => Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())), Vector(ty) => self.check_signature_token(ty), StructInstantiation(_, type_arguments) => self.check_signature_tokens(type_arguments), @@ -481,7 +481,7 @@ impl<'a> SignatureChecker<'a> { type_parameters, ) }, - SignatureToken::Function { .. } => { + SignatureToken::Function(..) => { Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())) }, diff --git a/third_party/move/move-bytecode-verifier/src/signature_v2.rs b/third_party/move/move-bytecode-verifier/src/signature_v2.rs index f352aee8fe922..0c4598963f2ad 100644 --- a/third_party/move/move-bytecode-verifier/src/signature_v2.rs +++ b/third_party/move/move-bytecode-verifier/src/signature_v2.rs @@ -173,11 +173,11 @@ fn check_ty( param_constraints, )?; }, - Function { + Function( args, results, abilities, - } => { + ) => { assert_abilities(*abilities, required_abilities)?; for ty in args.iter().chain(results.iter()) { check_ty( @@ -275,7 +275,7 @@ fn check_phantom_params( match ty { Vector(ty) => check_phantom_params(struct_handles, context, false, ty)?, - Function { args, results, .. } => { + Function(args, results, ..) => { for ty in args.iter().chain(results) { check_phantom_params(struct_handles, context, false, ty)? } @@ -919,7 +919,7 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { checked_early_bind_insts.entry((*idx, *count)) { // Note non-function case is checked in `verify_fun_sig_idx` above. - if let Some(SignatureToken::Function { args, .. }) = + if let Some(SignatureToken::Function(args, ..)) = self.resolver.signature_at(*idx).0.first() { if *count as usize > args.len() { diff --git a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs index 4fb7856670a42..d60b3bf3c8691 100644 --- a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs @@ -242,7 +242,7 @@ impl<'a> StackUsageVerifier<'a> { // InvokeFunction pops a function and the number of arguments and pushes the results of the // given function type Bytecode::InvokeFunction(idx) => { - if let Some(SignatureToken::Function { args, results, .. }) = + if let Some(SignatureToken::Function(args, results, ..)) = self.resolver.signature_at(*idx).0.first() { ((1 + args.len()) as u64, results.len() as u64) @@ -255,7 +255,7 @@ impl<'a> StackUsageVerifier<'a> { // EarlyBindFunction pops a function value and the captured arguments and returns 1 value Bytecode::EarlyBindFunction(idx, arg_count) => { - if let Some(SignatureToken::Function { args, .. }) = + if let Some(SignatureToken::Function(args, ..)) = self.resolver.signature_at(*idx).0.first() { if args.len() <= *arg_count as usize { diff --git a/third_party/move/move-bytecode-verifier/src/struct_defs.rs b/third_party/move/move-bytecode-verifier/src/struct_defs.rs index a3ce1f606d77e..3d9fec2e6a62d 100644 --- a/third_party/move/move-bytecode-verifier/src/struct_defs.rs +++ b/third_party/move/move-bytecode-verifier/src/struct_defs.rs @@ -123,7 +123,7 @@ impl<'a> StructDefGraphBuilder<'a> { ) }, T::Vector(inner) => self.add_signature_token(neighbors, cur_idx, inner)?, - T::Function { args, results, .. } => { + T::Function(args, results, ..) => { for t in args.iter().chain(results) { self.add_signature_token(neighbors, cur_idx, t)? } diff --git a/third_party/move/move-bytecode-verifier/src/type_safety.rs b/third_party/move/move-bytecode-verifier/src/type_safety.rs index bf9897f47ea05..ee451e9388feb 100644 --- a/third_party/move/move-bytecode-verifier/src/type_safety.rs +++ b/third_party/move/move-bytecode-verifier/src/type_safety.rs @@ -307,11 +307,11 @@ fn invoke_function( offset: CodeOffset, expected_ty: &SignatureToken, ) -> PartialVMResult<()> { - let SignatureToken::Function { - args: param_tys, - results: ret_tys, + let SignatureToken::Function( + param_tys, + ret_tys, abilities, - } = expected_ty + ) = expected_ty else { // The signature checker has ensured this is a function safe_assert!(false); @@ -327,10 +327,10 @@ fn invoke_function( .with_message("closure type mismatch".to_owned())); } // Verify that the abilities match - let SignatureToken::Function { - abilities: closure_abilities, - .. - } = closure_ty + let SignatureToken::Function( + _, _, + closure_abilities, + ) = closure_ty else { // Ensured above, but never panic safe_assert!(false); @@ -394,11 +394,11 @@ fn ld_function( verifier.push( meter, instantiate( - &SignatureToken::Function { - args: parameters.0.to_vec(), - results: ret_sign.0.to_vec(), + &SignatureToken::Function( + parameters.0.to_vec(), + ret_sign.0.to_vec(), abilities, - }, + ), type_actuals, ), ) @@ -412,11 +412,11 @@ fn early_bind_function( count: u8, ) -> PartialVMResult<()> { let count = count as usize; - let SignatureToken::Function { - args: param_tys, - results: ret_tys, + let SignatureToken::Function( + param_tys, + ret_tys, abilities, - } = expected_ty + ) = expected_ty else { // The signature checker has ensured this is a function safe_assert!(false); @@ -432,10 +432,10 @@ fn early_bind_function( .with_message("closure type mismatch".to_owned())); } // Verify that the abilities match - let SignatureToken::Function { - abilities: closure_abilities, - .. - } = closure_ty + let SignatureToken::Function( + _, _, + closure_abilities + ) = closure_ty else { // Ensured above, but never panic safe_assert!(false); @@ -461,11 +461,11 @@ fn early_bind_function( return Err(verifier.error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset)); } } - let result_ty = SignatureToken::Function { - args: (*remaining_param_tys).to_vec(), - results: ret_tys.to_vec(), - abilities: *abilities, - }; + let result_ty = SignatureToken::Function( + (*remaining_param_tys).to_vec(), + ret_tys.to_vec(), + *abilities, + ); verifier.push(meter, result_ty) } @@ -1343,15 +1343,15 @@ fn instantiate(token: &SignatureToken, subst: &Signature) -> SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(instantiate(ty, subst))), - Function { + Function ( args, results, abilities, - } => Function { - args: inst_vec(args), - results: inst_vec(results), - abilities: *abilities, - }, + ) => Function ( + inst_vec(args), + inst_vec(results), + *abilities, + ), Struct(idx) => Struct(*idx), StructInstantiation(idx, struct_type_args) => { StructInstantiation(*idx, inst_vec(struct_type_args)) diff --git a/third_party/move/move-compiler/src/interface_generator.rs b/third_party/move/move-compiler/src/interface_generator.rs index 09c4c8379eb98..db4798d321dcb 100644 --- a/third_party/move/move-compiler/src/interface_generator.rs +++ b/third_party/move/move-compiler/src/interface_generator.rs @@ -272,6 +272,20 @@ fn write_ability_constraint(abs: AbilitySet) -> String { ) } +fn function_value_abilities_to_str(abs: AbilitySet) -> String { + if abs.is_subset(AbilitySet::FUNCTIONS) { + return "".to_string(); + } + format!( + ": {}", + abs.setminus(AbilitySet::FUNCTIONS) + .into_iter() + .map(write_ability) + .collect::>() + .join("+") + ) +} + fn write_ability(ab: Ability) -> String { use crate::parser::ast::Ability_ as A_; match ab { @@ -365,8 +379,13 @@ fn write_signature_token(ctx: &mut Context, t: &SignatureToken) -> String { SignatureToken::Address => "address".to_string(), SignatureToken::Signer => "signer".to_string(), SignatureToken::Vector(inner) => format!("vector<{}>", write_signature_token(ctx, inner)), - SignatureToken::Function { args, results, .. } => { - format!("|{}|{}", tok_list(ctx, args), tok_list(ctx, results)) + SignatureToken::Function(args, results, abilities) => { + format!( + "|{}|{}{}", + tok_list(ctx, args), + tok_list(ctx, results), + function_value_abilities_to_str(*abilities) + ) }, SignatureToken::Struct(idx) => write_struct_handle_type(ctx, *idx), SignatureToken::StructInstantiation(idx, types) => { diff --git a/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs b/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs index 3da165cdb01e1..cd2e1560e9caf 100644 --- a/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs +++ b/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs @@ -768,7 +768,8 @@ impl<'a> Context<'a> { let correct_inner = self.reindex_signature_token(dep, *inner)?; SignatureToken::Vector(Box::new(correct_inner)) }, - SignatureToken::Function { .. } => { + SignatureToken::Function(..) => { + // TODO(LAMBDA) unimplemented!("function types not supported by MoveIR") }, SignatureToken::Reference(inner) => { diff --git a/third_party/move/move-model/src/ty.rs b/third_party/move/move-model/src/ty.rs index d1eedf5ed561c..5dc9c97a67fd8 100644 --- a/third_party/move/move-model/src/ty.rs +++ b/third_party/move/move-model/src/ty.rs @@ -1394,7 +1394,7 @@ impl Type { .collect(), ) }, - SignatureToken::Function { .. } => { + SignatureToken::Function (..) => { // TODO: implement function conversion unimplemented!("signature token to model type") }, diff --git a/third_party/move/move-vm/runtime/src/loader/type_loader.rs b/third_party/move/move-vm/runtime/src/loader/type_loader.rs index 19099c68c5c4c..90de57faa2769 100644 --- a/third_party/move/move-vm/runtime/src/loader/type_loader.rs +++ b/third_party/move/move-vm/runtime/src/loader/type_loader.rs @@ -31,8 +31,8 @@ pub fn intern_type( let inner_type = intern_type(module, inner_tok, struct_name_table)?; Type::Vector(TriompheArc::new(inner_type)) }, - SignatureToken::Function { .. } => { - // TODO: implement closures + SignatureToken::Function(..) => { + // TODO(LAMBDA): implement closures return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("function types in the type loader".to_owned())); }, diff --git a/third_party/move/move-vm/types/src/values/values_impl.rs b/third_party/move/move-vm/types/src/values/values_impl.rs index ffdc8634d2cf1..c3171cd907588 100644 --- a/third_party/move/move-vm/types/src/values/values_impl.rs +++ b/third_party/move/move-vm/types/src/values/values_impl.rs @@ -4154,7 +4154,7 @@ impl Value { S::Signer => return None, S::Vector(inner) => L::Vector(Box::new(Self::constant_sig_token_to_layout(inner)?)), // Not yet supported - S::Struct(_) | S::StructInstantiation(_, _) | S::Function { .. } => return None, + S::Struct(_) | S::StructInstantiation(_, _) | S::Function(..) => return None, // Not allowed/Not meaningful S::TypeParameter(_) | S::Reference(_) | S::MutableReference(_) => return None, }) diff --git a/third_party/move/tools/move-bytecode-utils/src/layout.rs b/third_party/move/tools/move-bytecode-utils/src/layout.rs index cfb305dd6b408..635b154e61378 100644 --- a/third_party/move/tools/move-bytecode-utils/src/layout.rs +++ b/third_party/move/tools/move-bytecode-utils/src/layout.rs @@ -386,7 +386,10 @@ impl TypeLayoutBuilder { ) -> anyhow::Result { use SignatureToken::*; Ok(match s { - Function { .. } => bail!("function types NYI for MoveTypeLayout"), + Function(..) => { + // TODO(LAMBDA) + bail!("function types NYI for MoveTypeLayout") + }, Vector(t) => MoveTypeLayout::Vector(Box::new(Self::build_from_signature_token( m, t, diff --git a/third_party/move/tools/move-disassembler/src/disassembler.rs b/third_party/move/tools/move-disassembler/src/disassembler.rs index 1e4bcc1e3608f..dcb747e390373 100644 --- a/third_party/move/tools/move-disassembler/src/disassembler.rs +++ b/third_party/move/tools/move-disassembler/src/disassembler.rs @@ -570,9 +570,6 @@ impl<'a> Disassembler<'a> { type_param_context: &[SourceName], ) -> Result { Ok(match sig_tok { - // TODO: function types - SignatureToken::Function { .. } => unimplemented!("disassembling function sig tokens"), - SignatureToken::Bool => "bool".to_string(), SignatureToken::U8 => "u8".to_string(), SignatureToken::U16 => "u16".to_string(), @@ -632,6 +629,29 @@ impl<'a> Disassembler<'a> { })? .0 .to_string(), + SignatureToken::Function(args, results, abilities) => { + let args_str = args + .into_iter() + .map(|tok| self.disassemble_sig_tok(tok, type_param_context)) + .collect::>>()? + .join(","); + let results_str = results + .into_iter() + .map(|tok| self.disassemble_sig_tok(tok, type_param_context)) + .collect::>>()? + .join(","); + let abilities_str = if abilities.is_subset(AbilitySet::FUNCTIONS) { + "".to_string() + } else { + let ability_vec: Vec<_> = abilities + .setminus(AbilitySet::FUNCTIONS) + .into_iter() + .map(Self::format_ability) + .collect(); + format!(" with {}", ability_vec.join("+")) + }; + format!("|{}|{}{}", args_str, results_str, abilities_str) + }, }) } diff --git a/third_party/move/tools/move-resource-viewer/src/lib.rs b/third_party/move/tools/move-resource-viewer/src/lib.rs index 3e81f50c13076..ba6d7f3eca515 100644 --- a/third_party/move/tools/move-resource-viewer/src/lib.rs +++ b/third_party/move/tools/move-resource-viewer/src/lib.rs @@ -375,7 +375,7 @@ impl MoveValueAnnotator { SignatureToken::Vector(ty) => { FatType::Vector(Box::new(self.resolve_signature(module, ty, limit)?)) }, - SignatureToken::Function { .. } => { + SignatureToken::Function(..) => { bail!("function types NYI by fat types") }, SignatureToken::Struct(idx) => { From be210b79fbe43a4a802a8b796e4c784953d8bac5 Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Sat, 7 Dec 2024 14:14:40 -0800 Subject: [PATCH 7/9] fix clippy warning in last commit --- .../move-bytecode-verifier/src/instruction_consistency.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs index 17c6bf4e6bf64..4dd405d3815d6 100644 --- a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs +++ b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs @@ -308,8 +308,8 @@ impl<'a> InstructionConsistency<'a> { } fn function_value_disabled_error(&self, offset: usize) -> PartialVMResult<()> { - return Err(PartialVMError::new(StatusCode::FEATURE_NOT_ENABLED) + Err(PartialVMError::new(StatusCode::FEATURE_NOT_ENABLED) .at_code_offset(self.current_function(), offset as CodeOffset) - .with_message("function values feature not enabled".to_string())); + .with_message("function values feature not enabled".to_string())) } } From 76b304207a0a730586ea8f5fe1e87243719441f3 Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Sun, 8 Dec 2024 20:13:05 -0800 Subject: [PATCH 8/9] add feature flag which was somehow dropped --- aptos-move/aptos-release-builder/src/components/feature_flags.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/aptos-move/aptos-release-builder/src/components/feature_flags.rs b/aptos-move/aptos-release-builder/src/components/feature_flags.rs index 1f5270592048e..6b08b5a874b53 100644 --- a/aptos-move/aptos-release-builder/src/components/feature_flags.rs +++ b/aptos-move/aptos-release-builder/src/components/feature_flags.rs @@ -133,6 +133,7 @@ pub enum FeatureFlag { CollectionOwner, NativeMemoryOperations, EnableLoaderV2, + EnableFunctionValues, } fn generate_features_blob(writer: &CodeWriter, data: &[u64]) { From 4f10a35f64c82beac8aae82ad428da385bfd170e Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Sun, 8 Dec 2024 20:24:10 -0800 Subject: [PATCH 9/9] fix more lints again --- api/types/src/bytecode.rs | 2 +- .../move-binary-format/src/binary_views.rs | 8 ++-- .../move-binary-format/src/deserializer.rs | 6 +-- .../move-binary-format/src/file_format.rs | 30 +++++-------- .../move/move-binary-format/src/serializer.rs | 6 +-- .../feature_function_values_tests.rs | 6 +-- .../invalid-mutations/src/bounds.rs | 14 +----- .../src/dependencies.rs | 12 +---- .../src/signature_v2.rs | 6 +-- .../move-bytecode-verifier/src/type_safety.rs | 44 ++++--------------- third_party/move/move-model/src/ty.rs | 2 +- 11 files changed, 31 insertions(+), 105 deletions(-) diff --git a/api/types/src/bytecode.rs b/api/types/src/bytecode.rs index f51a8ccfb16e1..ffdea8f6186c6 100644 --- a/api/types/src/bytecode.rs +++ b/api/types/src/bytecode.rs @@ -105,7 +105,7 @@ pub trait Bytecode { mutable: true, to: Box::new(self.new_move_type(t.borrow())), }, - SignatureToken::Function( .. ) => { + SignatureToken::Function(..) => { // TODO(LAMBDA) unimplemented!("signature token function to API MoveType") }, diff --git a/third_party/move/move-binary-format/src/binary_views.rs b/third_party/move/move-binary-format/src/binary_views.rs index a7c4a8f6600d2..74c9325c7e2e9 100644 --- a/third_party/move/move-binary-format/src/binary_views.rs +++ b/third_party/move/move-binary-format/src/binary_views.rs @@ -340,11 +340,9 @@ impl<'a> BinaryIndexedView<'a> { Reference(_) | MutableReference(_) => Ok(AbilitySet::REFERENCES), Signer => Ok(AbilitySet::SIGNER), TypeParameter(idx) => Ok(constraints[*idx as usize]), - Vector(ty) => AbilitySet::polymorphic_abilities( - AbilitySet::VECTOR, - vec![false], - vec![self.abilities(ty, constraints)?], - ), + Vector(ty) => AbilitySet::polymorphic_abilities(AbilitySet::VECTOR, vec![false], vec![ + self.abilities(ty, constraints)?, + ]), Function(_, _, abilities) => Ok(*abilities), Struct(idx) => { let sh = self.struct_handle_at(*idx); diff --git a/third_party/move/move-binary-format/src/deserializer.rs b/third_party/move/move-binary-format/src/deserializer.rs index b5418d12dd469..297d69132958d 100644 --- a/third_party/move/move-binary-format/src/deserializer.rs +++ b/third_party/move/move-binary-format/src/deserializer.rs @@ -1180,11 +1180,7 @@ fn load_signature_token(cursor: &mut VersionedCursor) -> BinaryLoaderResult= args_arity as usize { results.push(tok); if results.len() >= res_arity as usize { - T::Saturated(SignatureToken::Function( - args, - results, - abilities, - )) + T::Saturated(SignatureToken::Function(args, results, abilities)) } else { T::Function { args_arity, diff --git a/third_party/move/move-binary-format/src/file_format.rs b/third_party/move/move-binary-format/src/file_format.rs index 927fa7b9f6e5b..6784ef7bf232b 100644 --- a/third_party/move/move-binary-format/src/file_format.rs +++ b/third_party/move/move-binary-format/src/file_format.rs @@ -1266,7 +1266,7 @@ pub enum SignatureToken { Function( Vec, // args Vec, // results - AbilitySet, // abilities + AbilitySet, // abilities ), /// User defined type Struct(StructHandleIndex), @@ -1310,7 +1310,7 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIter<'a> { self.stack.extend(inner_toks.iter().rev()) }, - Function( args, results, .. ) => { + Function(args, results, ..) => { self.stack.extend(args.iter().rev()); self.stack.extend(results.iter().rev()); }, @@ -1348,7 +1348,7 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIterWithDepth<'a> { .stack .extend(inner_toks.iter().map(|tok| (tok, depth + 1)).rev()), - Function( args, results, .. ) => { + Function(args, results, ..) => { self.stack .extend(args.iter().map(|tok| (tok, depth + 1)).rev()); self.stack @@ -1415,11 +1415,7 @@ impl std::fmt::Debug for SignatureToken { SignatureToken::Address => write!(f, "Address"), SignatureToken::Signer => write!(f, "Signer"), SignatureToken::Vector(boxed) => write!(f, "Vector({:?})", boxed), - SignatureToken::Function ( - args, - results, - abilities, - ) => { + SignatureToken::Function(args, results, abilities) => { write!(f, "Function({:?}, {:?}, {})", args, results, abilities) }, SignatureToken::Reference(boxed) => write!(f, "Reference({:?})", boxed), @@ -1443,7 +1439,7 @@ impl SignatureToken { | Address | Signer | Vector(_) - | Function ( .. ) + | Function(..) | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1482,7 +1478,7 @@ impl SignatureToken { Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address => true, Vector(inner) => inner.is_valid_for_constant(), Signer - | Function ( .. ) + | Function(..) | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1495,7 +1491,7 @@ impl SignatureToken { pub fn is_function(&self) -> bool { use SignatureToken::*; - matches!(self, Function ( .. )) + matches!(self, Function(..)) } /// Set the index to this one. Useful for random testing. @@ -1546,15 +1542,9 @@ impl SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(ty.instantiate(subst_mapping))), - Function ( - args, - results, - abilities, - ) => Function ( - inst_vec(args), - inst_vec(results), - *abilities, - ), + Function(args, results, abilities) => { + Function(inst_vec(args), inst_vec(results), *abilities) + }, Struct(idx) => Struct(*idx), StructInstantiation(idx, struct_type_args) => { StructInstantiation(*idx, inst_vec(struct_type_args)) diff --git a/third_party/move/move-binary-format/src/serializer.rs b/third_party/move/move-binary-format/src/serializer.rs index b64cf045ec2ff..e6694989113e4 100644 --- a/third_party/move/move-binary-format/src/serializer.rs +++ b/third_party/move/move-binary-format/src/serializer.rs @@ -800,11 +800,7 @@ fn serialize_signature_token_single_node_impl( binary.push(SerializedType::TYPE_PARAMETER as u8)?; serialize_type_parameter_index(binary, *idx)?; }, - SignatureToken::Function( - args, - results, - abilities, - ) => { + SignatureToken::Function(args, results, abilities) => { binary.push(SerializedType::FUNCTION as u8)?; serialize_signature_size(binary, args.len())?; serialize_signature_size(binary, results.len())?; diff --git a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs index 31ed738c6c4a7..1cd6c4cb329f7 100644 --- a/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs +++ b/third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/feature_function_values_tests.rs @@ -22,11 +22,7 @@ fn get_fun_type_bool_to_bool() -> SignatureToken { fn get_fun_type_nothing_to_bool() -> SignatureToken { let bool_token = SignatureToken::Bool; let abilities = AbilitySet::PUBLIC_FUNCTIONS; - SignatureToken::Function( - vec![], - vec![bool_token.clone()], - abilities, - ) + SignatureToken::Function(vec![], vec![bool_token.clone()], abilities) } #[test] diff --git a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs index 9470a6cbf3485..cb47be2e8e220 100644 --- a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs +++ b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs @@ -358,17 +358,7 @@ fn struct_handle(token: &SignatureToken) -> Option { Struct(sh_idx) => Some(*sh_idx), StructInstantiation(sh_idx, _) => Some(*sh_idx), Reference(token) | MutableReference(token) => struct_handle(token), - Bool - | U8 - | U16 - | U32 - | U64 - | U128 - | U256 - | Address - | Signer - | Vector(_) - | TypeParameter(_) - | Function(..) => None, + Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | Vector(_) + | TypeParameter(_) | Function(..) => None, } } diff --git a/third_party/move/move-bytecode-verifier/src/dependencies.rs b/third_party/move/move-bytecode-verifier/src/dependencies.rs index 1ce3789fa6452..89bab2eaf6cac 100644 --- a/third_party/move/move-bytecode-verifier/src/dependencies.rs +++ b/third_party/move/move-bytecode-verifier/src/dependencies.rs @@ -456,16 +456,8 @@ fn compare_types( compare_types(context, ty1, ty2, def_module) }, ( - SignatureToken::Function( - args1, - result1, - abilities1, - ), - SignatureToken::Function( - args2, - result2, - abilities2, - ), + SignatureToken::Function(args1, result1, abilities1), + SignatureToken::Function(args2, result2, abilities2), ) => { compare_cross_module_signatures(context, args1, args2, def_module)?; compare_cross_module_signatures(context, result1, result2, def_module)?; diff --git a/third_party/move/move-bytecode-verifier/src/signature_v2.rs b/third_party/move/move-bytecode-verifier/src/signature_v2.rs index 0c4598963f2ad..be0d075f6b9ef 100644 --- a/third_party/move/move-bytecode-verifier/src/signature_v2.rs +++ b/third_party/move/move-bytecode-verifier/src/signature_v2.rs @@ -173,11 +173,7 @@ fn check_ty( param_constraints, )?; }, - Function( - args, - results, - abilities, - ) => { + Function(args, results, abilities) => { assert_abilities(*abilities, required_abilities)?; for ty in args.iter().chain(results.iter()) { check_ty( diff --git a/third_party/move/move-bytecode-verifier/src/type_safety.rs b/third_party/move/move-bytecode-verifier/src/type_safety.rs index ee451e9388feb..7cec22aa33ac8 100644 --- a/third_party/move/move-bytecode-verifier/src/type_safety.rs +++ b/third_party/move/move-bytecode-verifier/src/type_safety.rs @@ -307,12 +307,7 @@ fn invoke_function( offset: CodeOffset, expected_ty: &SignatureToken, ) -> PartialVMResult<()> { - let SignatureToken::Function( - param_tys, - ret_tys, - abilities, - ) = expected_ty - else { + let SignatureToken::Function(param_tys, ret_tys, abilities) = expected_ty else { // The signature checker has ensured this is a function safe_assert!(false); unreachable!() @@ -327,11 +322,7 @@ fn invoke_function( .with_message("closure type mismatch".to_owned())); } // Verify that the abilities match - let SignatureToken::Function( - _, _, - closure_abilities, - ) = closure_ty - else { + let SignatureToken::Function(_, _, closure_abilities) = closure_ty else { // Ensured above, but never panic safe_assert!(false); unreachable!() @@ -394,11 +385,7 @@ fn ld_function( verifier.push( meter, instantiate( - &SignatureToken::Function( - parameters.0.to_vec(), - ret_sign.0.to_vec(), - abilities, - ), + &SignatureToken::Function(parameters.0.to_vec(), ret_sign.0.to_vec(), abilities), type_actuals, ), ) @@ -412,12 +399,7 @@ fn early_bind_function( count: u8, ) -> PartialVMResult<()> { let count = count as usize; - let SignatureToken::Function( - param_tys, - ret_tys, - abilities, - ) = expected_ty - else { + let SignatureToken::Function(param_tys, ret_tys, abilities) = expected_ty else { // The signature checker has ensured this is a function safe_assert!(false); unreachable!() @@ -432,11 +414,7 @@ fn early_bind_function( .with_message("closure type mismatch".to_owned())); } // Verify that the abilities match - let SignatureToken::Function( - _, _, - closure_abilities - ) = closure_ty - else { + let SignatureToken::Function(_, _, closure_abilities) = closure_ty else { // Ensured above, but never panic safe_assert!(false); unreachable!() @@ -1343,15 +1321,9 @@ fn instantiate(token: &SignatureToken, subst: &Signature) -> SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(instantiate(ty, subst))), - Function ( - args, - results, - abilities, - ) => Function ( - inst_vec(args), - inst_vec(results), - *abilities, - ), + Function(args, results, abilities) => { + Function(inst_vec(args), inst_vec(results), *abilities) + }, Struct(idx) => Struct(*idx), StructInstantiation(idx, struct_type_args) => { StructInstantiation(*idx, inst_vec(struct_type_args)) diff --git a/third_party/move/move-model/src/ty.rs b/third_party/move/move-model/src/ty.rs index 5dc9c97a67fd8..6da025ea9c482 100644 --- a/third_party/move/move-model/src/ty.rs +++ b/third_party/move/move-model/src/ty.rs @@ -1394,7 +1394,7 @@ impl Type { .collect(), ) }, - SignatureToken::Function (..) => { + SignatureToken::Function(..) => { // TODO: implement function conversion unimplemented!("signature token to model type") },