From c7cd374f0023f4c638d6a89284eccea0a72a8361 Mon Sep 17 00:00:00 2001 From: Wolfgang Grieskamp Date: Sun, 3 Nov 2024 07:30:41 -0800 Subject: [PATCH 1/3] 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 | 19 ++ .../move-vm/runtime/src/loader/type_loader.rs | 10 +- .../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 + 39 files changed, 709 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 f45aa01f5fd2b..84a855a7c51d2 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -1604,6 +1604,11 @@ impl Frame { 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(_) => { @@ -1729,6 +1734,12 @@ impl Frame { 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 @@ -2325,6 +2336,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/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 8a6e7c52129c75e0d1a1b60c3e820de552fb0978 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/3] 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 | 21 +- .../move-vm/test-utils/src/gas_schedule.rs | 25 +- .../move-disassembler/src/disassembler.rs | 98 +++++- 25 files changed, 689 insertions(+), 339 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 84a855a7c51d2..407720c217fc4 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -1604,8 +1604,11 @@ impl Frame { 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())) }, @@ -1735,7 +1738,10 @@ impl Frame { 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())) }, @@ -2336,10 +2342,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/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 f49f228d8f04b6dd6fb4bc482e9ed9463572b93c 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/3] 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 | 26 ++++++++++--------- 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 | 12 ++++----- .../move-vm/test-utils/src/gas_schedule.rs | 14 +++++++--- .../move-disassembler/src/disassembler.rs | 8 +++--- 31 files changed, 128 insertions(+), 110 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 b7426dc5f20c9..f83adf74a98b9 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 } //! ``` @@ -297,7 +297,7 @@ impl<'a> LambdaLifter<'a> { None } }, - Invalid(..) | Call(..) | Invoke(..) | Lambda(..) | Quant(..) | Block(..) + Invalid(..) | Call(..) | InvokeFunction(..) | Lambda(..) | Quant(..) | Block(..) | IfElse(..) | Match(..) | Return(..) | Loop(..) | LoopCont(..) | Assign(..) | Mutate(..) | SpecBlock(..) => None, } @@ -340,7 +340,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) }, @@ -360,7 +360,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, } } @@ -416,7 +416,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 @@ -438,7 +438,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(); @@ -497,7 +497,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 @@ -556,7 +556,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(), + ); } } } @@ -798,7 +800,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 407720c217fc4..aa2c5a68a8d70 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -1607,8 +1607,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())) }, @@ -1740,8 +1740,8 @@ impl Frame { // 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())) }, @@ -2345,8 +2345,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/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) => {