From d382ca0bd5ba3f86cdf8c652ca95c43e7d605d40 Mon Sep 17 00:00:00 2001 From: "Brian R. Murphy" <132495859+brmataptos@users.noreply.github.com> Date: Tue, 3 Dec 2024 21:51:01 -0800 Subject: [PATCH] Address review comments: - use struct for SignatureToken::Function - simplify AcuiresVerifier::ld_function_acquire - comments cleanup, especially file_format semantics for new bytecodes --- api/types/src/bytecode.rs | 2 +- aptos-move/script-composer/src/decompiler.rs | 2 +- .../move-binary-format/src/binary_views.rs | 2 +- .../move/move-binary-format/src/builders.rs | 14 +++-- .../move-binary-format/src/check_bounds.rs | 16 ++++- .../src/check_complexity.rs | 16 ++++- .../move/move-binary-format/src/constant.rs | 2 +- .../move-binary-format/src/file_format.rs | 58 ++++++++++++++----- .../move/move-binary-format/src/normalized.rs | 2 +- .../src/proptest_types/functions.rs | 14 ++++- .../src/proptest_types/types.rs | 2 +- .../move/move-binary-format/src/serializer.rs | 2 +- .../invalid-mutations/src/bounds.rs | 14 ++++- .../src/acquires_list_verifier.rs | 19 +++--- .../src/dependencies.rs | 19 ++++-- .../src/instantiation_loops.rs | 4 +- .../src/instruction_consistency.rs | 2 +- .../src/reference_safety/mod.rs | 2 +- .../move-bytecode-verifier/src/signature.rs | 6 +- .../src/signature_v2.rs | 16 +++-- .../src/stack_usage_verifier.rs | 4 +- .../move-bytecode-verifier/src/struct_defs.rs | 4 +- .../move-bytecode-verifier/src/type_safety.rs | 52 +++++++++++++---- .../move-compiler/src/interface_generator.rs | 4 +- .../move-ir-to-bytecode/src/context.rs | 2 +- third_party/move/move-model/src/ty.rs | 2 +- .../move-vm/runtime/src/loader/type_loader.rs | 2 +- .../move-vm/types/src/values/values_impl.rs | 2 +- .../tools/move-bytecode-utils/src/layout.rs | 2 +- .../move-disassembler/src/disassembler.rs | 2 +- .../tools/move-resource-viewer/src/lib.rs | 2 +- 31 files changed, 204 insertions(+), 88 deletions(-) diff --git a/api/types/src/bytecode.rs b/api/types/src/bytecode.rs index ffdea8f6186c67..45908b2e2b7121 100644 --- a/api/types/src/bytecode.rs +++ b/api/types/src/bytecode.rs @@ -105,7 +105,7 @@ pub trait Bytecode { mutable: true, to: Box::new(self.new_move_type(t.borrow())), }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO(LAMBDA) unimplemented!("signature token function to API MoveType") }, diff --git a/aptos-move/script-composer/src/decompiler.rs b/aptos-move/script-composer/src/decompiler.rs index 147d976b90ccfe..861bbe990a6b16 100644 --- a/aptos-move/script-composer/src/decompiler.rs +++ b/aptos-move/script-composer/src/decompiler.rs @@ -208,7 +208,7 @@ impl LocalState { SignatureToken::Vector(s) => { TypeTag::Vector(Box::new(Self::type_tag_from_sig_token(script, s)?)) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO(LAMBDA) bail!("function types not yet implemented for script composer") }, diff --git a/third_party/move/move-binary-format/src/binary_views.rs b/third_party/move/move-binary-format/src/binary_views.rs index 74c9325c7e2e9b..f1093d22c45e72 100644 --- a/third_party/move/move-binary-format/src/binary_views.rs +++ b/third_party/move/move-binary-format/src/binary_views.rs @@ -343,7 +343,7 @@ impl<'a> BinaryIndexedView<'a> { Vector(ty) => AbilitySet::polymorphic_abilities(AbilitySet::VECTOR, vec![false], vec![ self.abilities(ty, constraints)?, ]), - Function(_, _, abilities) => Ok(*abilities), + Function { abilities, .. } => Ok(*abilities), Struct(idx) => { let sh = self.struct_handle_at(*idx); Ok(sh.abilities) diff --git a/third_party/move/move-binary-format/src/builders.rs b/third_party/move/move-binary-format/src/builders.rs index b78ba64c23707d..5cacb8e3e62b25 100644 --- a/third_party/move/move-binary-format/src/builders.rs +++ b/third_party/move/move-binary-format/src/builders.rs @@ -235,11 +235,15 @@ impl CompiledScriptBuilder { MutableReference(Box::new(self.import_signature_token(module, ty)?)) }, Vector(ty) => Vector(Box::new(self.import_signature_token(module, ty)?)), - Function(args, result, abilities) => Function( - import_vec(self, args)?, - import_vec(self, result)?, - *abilities, - ), + Function { + args, + results, + abilities, + } => Function { + args: import_vec(self, args)?, + results: import_vec(self, results)?, + abilities: *abilities, + }, Struct(idx) => Struct(self.import_struct(module, *idx)?), StructInstantiation(idx, inst_tys) => StructInstantiation( self.import_struct(module, *idx)?, diff --git a/third_party/move/move-binary-format/src/check_bounds.rs b/third_party/move/move-binary-format/src/check_bounds.rs index 1992a22e479689..6560f376d1d72d 100644 --- a/third_party/move/move-binary-format/src/check_bounds.rs +++ b/third_party/move/move-binary-format/src/check_bounds.rs @@ -685,8 +685,20 @@ impl<'a> BoundsChecker<'a> { for ty in ty.preorder_traversal() { match ty { - Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | TypeParameter(_) - | Reference(_) | MutableReference(_) | Vector(_) | Function(..) => (), + Bool + | U8 + | U16 + | U32 + | U64 + | U128 + | U256 + | Address + | Signer + | TypeParameter(_) + | Reference(_) + | MutableReference(_) + | Vector(_) + | Function { .. } => (), Struct(idx) => { check_bounds_impl(self.view.struct_handles(), *idx)?; if let Some(sh) = self.view.struct_handles().get(idx.into_index()) { diff --git a/third_party/move/move-binary-format/src/check_complexity.rs b/third_party/move/move-binary-format/src/check_complexity.rs index 78310559d7b961..1aa4b3d9499944 100644 --- a/third_party/move/move-binary-format/src/check_complexity.rs +++ b/third_party/move/move-binary-format/src/check_complexity.rs @@ -67,8 +67,20 @@ impl<'a> BinaryComplexityMeter<'a> { cost = cost.saturating_add(struct_name.len() as u64 * COST_PER_IDENT_BYTE); cost = cost.saturating_add(moduel_name.len() as u64 * COST_PER_IDENT_BYTE); }, - U8 | U16 | U32 | U64 | U128 | U256 | Signer | Address | Bool | Vector(_) - | Function(..) | TypeParameter(_) | Reference(_) | MutableReference(_) => (), + U8 + | U16 + | U32 + | U64 + | U128 + | U256 + | Signer + | Address + | Bool + | Vector(_) + | Function { .. } + | TypeParameter(_) + | Reference(_) + | MutableReference(_) => (), } } diff --git a/third_party/move/move-binary-format/src/constant.rs b/third_party/move/move-binary-format/src/constant.rs index 3b8d200787a4e1..1f27d9f259fe3b 100644 --- a/third_party/move/move-binary-format/src/constant.rs +++ b/third_party/move/move-binary-format/src/constant.rs @@ -17,7 +17,7 @@ fn sig_to_ty(sig: &SignatureToken) -> Option { SignatureToken::U128 => Some(MoveTypeLayout::U128), SignatureToken::U256 => Some(MoveTypeLayout::U256), SignatureToken::Vector(v) => Some(MoveTypeLayout::Vector(Box::new(sig_to_ty(v.as_ref())?))), - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO(LAMBDA): do we need representation in MoveTypeLayout? None }, diff --git a/third_party/move/move-binary-format/src/file_format.rs b/third_party/move/move-binary-format/src/file_format.rs index a2137420e3226c..7afb1891560263 100644 --- a/third_party/move/move-binary-format/src/file_format.rs +++ b/third_party/move/move-binary-format/src/file_format.rs @@ -1262,7 +1262,11 @@ pub enum SignatureToken { /// Vector Vector(Box), /// Function, with n argument types and m result types, and an associated ability set. - Function(Vec, Vec, AbilitySet), + Function { + args: Vec, + results: Vec, + abilities: AbilitySet, + }, /// User defined type Struct(StructHandleIndex), StructInstantiation(StructHandleIndex, Vec), @@ -1305,9 +1309,9 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIter<'a> { self.stack.extend(inner_toks.iter().rev()) }, - Function(args, result, _) => { + Function { args, results, .. } => { self.stack.extend(args.iter().rev()); - self.stack.extend(result.iter().rev()); + self.stack.extend(results.iter().rev()); }, Signer | Bool | Address | U8 | U16 | U32 | U64 | U128 | U256 | Struct(_) @@ -1343,11 +1347,11 @@ impl<'a> Iterator for SignatureTokenPreorderTraversalIterWithDepth<'a> { .stack .extend(inner_toks.iter().map(|tok| (tok, depth + 1)).rev()), - Function(args, result, _) => { + Function { args, results, .. } => { self.stack .extend(args.iter().map(|tok| (tok, depth + 1)).rev()); self.stack - .extend(result.iter().map(|tok| (tok, depth + 1)).rev()); + .extend(results.iter().map(|tok| (tok, depth + 1)).rev()); }, Signer | Bool | Address | U8 | U16 | U32 | U64 | U128 | U256 | Struct(_) @@ -1410,8 +1414,12 @@ impl std::fmt::Debug for SignatureToken { SignatureToken::Address => write!(f, "Address"), SignatureToken::Signer => write!(f, "Signer"), SignatureToken::Vector(boxed) => write!(f, "Vector({:?})", boxed), - SignatureToken::Function(args, result, abilities) => { - write!(f, "Function({:?}, {:?}, {})", args, result, abilities) + SignatureToken::Function { + args, + results, + abilities, + } => { + write!(f, "Function({:?}, {:?}, {})", args, results, abilities) }, SignatureToken::Reference(boxed) => write!(f, "Reference({:?})", boxed), SignatureToken::Struct(idx) => write!(f, "Struct({:?})", idx), @@ -1425,7 +1433,7 @@ impl std::fmt::Debug for SignatureToken { } impl SignatureToken { - // Returns `true` if the `SignatureToken` is an integer type. + /// Returns `true` if the `SignatureToken` is an integer type. pub fn is_integer(&self) -> bool { use SignatureToken::*; match self { @@ -1434,7 +1442,7 @@ impl SignatureToken { | Address | Signer | Vector(_) - | Function(..) + | Function { .. } | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1473,7 +1481,7 @@ impl SignatureToken { Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address => true, Vector(inner) => inner.is_valid_for_constant(), Signer - | Function(..) + | Function { .. } | Struct(_) | StructInstantiation(_, _) | Reference(_) @@ -1486,7 +1494,7 @@ impl SignatureToken { pub fn is_function(&self) -> bool { use SignatureToken::*; - matches!(self, Function(..)) + matches!(self, Function { .. }) } /// Set the index to this one. Useful for random testing. @@ -1537,8 +1545,14 @@ impl SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(ty.instantiate(subst_mapping))), - Function(args, result, abilities) => { - Function(inst_vec(args), inst_vec(result), *abilities) + Function { + args, + results, + abilities, + } => Function { + args: inst_vec(args), + results: inst_vec(results), + abilities: *abilities, }, Struct(idx) => Struct(*idx), StructInstantiation(idx, struct_type_args) => { @@ -1834,6 +1848,11 @@ pub enum Bytecode { // Here `func` is loaded from the file format, containing information like the // the function signature, the locals, and the body. + // TODO(LAMBDA) - try to loosen this restriction after MVP if we can + // prove safety using access specifiers or other mechanisms. + if conservative_invocation_mode then + if stack already contains a call to the same module as func, then abort + ty_args = if func.is_generic then func.ty_args else [] n = func.num_params @@ -3047,8 +3066,13 @@ pub enum Bytecode { Return values are pushed onto the stack from the first to the last and available to the caller after returning from the callee. - During execution of the function conservative_invocation_mode is enabled; - afterwards, it is restored to its original state. + For MVP, we dynamically disallow all module reentrancy starting with + any dynamic call. This means that normal calls need a dynamic stack check + if there is a dynamic call frame on the stack. We denote this here + using a "conservative_invocation_mode" state variable. + + During execution of the dynamic function conservative_invocation_mode + is enabled; afterwards, it is restored to its original state. "#] #[semantics = r#" stack >> function_handle @@ -3058,6 +3082,10 @@ pub enum Bytecode { n = func.num_params ty_args = if func.is_generic then func.ty_args else [] + // TODO(LAMBDA) - try to loosen this restriction after MVP if we can + // prove safety using access specifiers or other mechanisms. + if stack already contains a call to the same module as func, then abort + n = func.num_params stack >> arg_{n-1} .. diff --git a/third_party/move/move-binary-format/src/normalized.rs b/third_party/move/move-binary-format/src/normalized.rs index 4fe5b3508d0fa3..8d13b0d015478f 100644 --- a/third_party/move/move-binary-format/src/normalized.rs +++ b/third_party/move/move-binary-format/src/normalized.rs @@ -193,7 +193,7 @@ impl Type { Reference(t) => Type::Reference(Box::new(Type::new(m, t))), MutableReference(t) => Type::MutableReference(Box::new(Type::new(m, t))), - Function(..) => panic!("normalized representation does not support function types"), + Function { .. } => panic!("normalized representation does not support function types"), } } diff --git a/third_party/move/move-binary-format/src/proptest_types/functions.rs b/third_party/move/move-binary-format/src/proptest_types/functions.rs index f614ffcfbb8e69..77dbd31092cf5d 100644 --- a/third_party/move/move-binary-format/src/proptest_types/functions.rs +++ b/third_party/move/move-binary-format/src/proptest_types/functions.rs @@ -1061,8 +1061,18 @@ impl BytecodeGen { fn check_signature_token(token: &SignatureToken) -> bool { use SignatureToken::*; match token { - U8 | U16 | U32 | U64 | U128 | U256 | Bool | Address | Signer | Struct(_) - | Function(..) | TypeParameter(_) => true, + U8 + | U16 + | U32 + | U64 + | U128 + | U256 + | Bool + | Address + | Signer + | Struct(_) + | Function { .. } + | TypeParameter(_) => true, Vector(element_token) => BytecodeGen::check_signature_token(element_token), StructInstantiation(_, type_arguments) => type_arguments .iter() diff --git a/third_party/move/move-binary-format/src/proptest_types/types.rs b/third_party/move/move-binary-format/src/proptest_types/types.rs index 161586d5038040..830dd5c564298f 100644 --- a/third_party/move/move-binary-format/src/proptest_types/types.rs +++ b/third_party/move/move-binary-format/src/proptest_types/types.rs @@ -71,7 +71,7 @@ impl StDefnMaterializeState { let inner = self.potential_abilities(ty); inner.intersect(AbilitySet::VECTOR) }, - Function(_, _, a) => *a, + Function { abilities, .. } => *abilities, Struct(idx) => { let sh = &self.struct_handles[idx.0 as usize]; sh.abilities diff --git a/third_party/move/move-binary-format/src/serializer.rs b/third_party/move/move-binary-format/src/serializer.rs index 686df2927b1fa5..f9c9e519174823 100644 --- a/third_party/move/move-binary-format/src/serializer.rs +++ b/third_party/move/move-binary-format/src/serializer.rs @@ -800,7 +800,7 @@ fn serialize_signature_token_single_node_impl( binary.push(SerializedType::TYPE_PARAMETER as u8)?; serialize_type_parameter_index(binary, *idx)?; }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { unimplemented!("serialization of function types") }, } diff --git a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs index cb47be2e8e2203..0d7858b658c7da 100644 --- a/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs +++ b/third_party/move/move-bytecode-verifier/invalid-mutations/src/bounds.rs @@ -358,7 +358,17 @@ fn struct_handle(token: &SignatureToken) -> Option { Struct(sh_idx) => Some(*sh_idx), StructInstantiation(sh_idx, _) => Some(*sh_idx), Reference(token) | MutableReference(token) => struct_handle(token), - Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | Vector(_) - | TypeParameter(_) | Function(..) => None, + Bool + | U8 + | U16 + | U32 + | U64 + | U128 + | U256 + | Address + | Signer + | Vector(_) + | TypeParameter(_) + | Function { .. } => None, } } diff --git a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs index f7c3de60ff1d1c..e18baabbb313b1 100644 --- a/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/acquires_list_verifier.rs @@ -212,23 +212,20 @@ impl<'a> AcquiresVerifier<'a> { fn ld_function_acquire( &mut self, - fh_idx: FunctionHandleIndex, - offset: CodeOffset, + _fh_idx: FunctionHandleIndex, + _offset: CodeOffset, ) -> PartialVMResult<()> { - // Currenty we are disallowing acquires for any function value which - // is created, so InvokeFunction does nothing with acquires. + // Note that function values are dynamically disallowed to be called from the module hosting + // their implementation function, and to be called while their host module is on the stack, + // so we don't need to do anything about acquires at this point. // TODO(LAMBDA) In the future this may change. - let function_handle = self.module.function_handle_at(fh_idx); - let function_acquired_resources = self.function_acquired_resources(function_handle, fh_idx); - if !function_acquired_resources.is_empty() { - return Err(self.error(StatusCode::LD_FUNCTION_NONEMPTY_ACQUIRES, offset)); - } Ok(()) } fn invoke_acquire(&mut self, _offset: CodeOffset) -> PartialVMResult<()> { - // Currenty we are disallowing acquires for any function value which - // is created, so InvokeFunction does nothing with acquires. + // Note that function values are dynamically disallowed to be called from their + // hosting module, or if that module appears on the stack at all. This means + // that a dynamic call cannot affect local resources in the caller. // TODO(LAMBDA) In the future this may change. Ok(()) } diff --git a/third_party/move/move-bytecode-verifier/src/dependencies.rs b/third_party/move/move-bytecode-verifier/src/dependencies.rs index 8593100e39746c..74d86a40a5783c 100644 --- a/third_party/move/move-bytecode-verifier/src/dependencies.rs +++ b/third_party/move/move-bytecode-verifier/src/dependencies.rs @@ -456,13 +456,22 @@ fn compare_types( compare_types(context, ty1, ty2, def_module) }, ( - SignatureToken::Function(args1, result1, ab1), - SignatureToken::Function(args2, result2, ab2), + SignatureToken::Function { + args: args1, + results: result1, + abilities: abilities1, + }, + SignatureToken::Function { + args: args2, + results: result2, + abilities: abilities2, + }, ) => { compare_cross_module_signatures(context, args1, args2, def_module)?; compare_cross_module_signatures(context, result1, result2, def_module)?; - if ab1 != ab2 { - Err(PartialVMError::new(StatusCode::TYPE_MISMATCH)) + if abilities1 != abilities2 { + Err(PartialVMError::new(StatusCode::TYPE_MISMATCH) + .with_message("Function type ability constraints mismatch".to_string())) } else { Ok(()) } @@ -495,7 +504,7 @@ fn compare_types( | (SignatureToken::Address, _) | (SignatureToken::Signer, _) | (SignatureToken::Vector(_), _) - | (SignatureToken::Function(..), _) + | (SignatureToken::Function { .. }, _) | (SignatureToken::Struct(_), _) | (SignatureToken::StructInstantiation(_, _), _) | (SignatureToken::Reference(_), _) diff --git a/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs b/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs index 53896c212ee141..d1a1501d414307 100644 --- a/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs +++ b/third_party/move/move-bytecode-verifier/src/instantiation_loops.rs @@ -148,11 +148,11 @@ impl<'a> InstantiationLoopChecker<'a> { type_params.insert(*idx); }, Vector(ty) => rec(type_params, ty), - Function(args, result, _) => { + Function { args, results, .. } => { for ty in args { rec(type_params, ty); } - for ty in result { + for ty in results { rec(type_params, ty); } }, diff --git a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs index 6a00182700d90b..ba4e7d9e5c14c3 100644 --- a/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs +++ b/third_party/move/move-bytecode-verifier/src/instruction_consistency.rs @@ -266,7 +266,7 @@ impl<'a> InstructionConsistency<'a> { ) -> PartialVMResult<()> { let signature = self.resolver.signature_at(sig_index); if let Some(sig_token) = signature.0.first() { - if let SignatureToken::Function(params, _returns, _abilities) = sig_token { + if let SignatureToken::Function { args: params, .. } = sig_token { if count as usize > params.len() { return Err( PartialVMError::new(StatusCode::NUMBER_OF_TYPE_ARGUMENTS_MISMATCH) diff --git a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs index 80f3127a0b8a2d..651f54078d7849 100644 --- a/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs +++ b/third_party/move/move-bytecode-verifier/src/reference_safety/mod.rs @@ -250,7 +250,7 @@ fn fun_type( idx: SignatureIndex, ) -> PartialVMResult<(Vec, Vec)> { match verifier.resolver.signature_at(idx).0.first() { - Some(SignatureToken::Function(args, result, _)) => Ok((args.clone(), result.clone())), + Some(SignatureToken::Function { args, results, .. }) => Ok((args.clone(), results.clone())), _ => Err(PartialVMError::new( StatusCode::VERIFIER_INVARIANT_VIOLATION, )), diff --git a/third_party/move/move-bytecode-verifier/src/signature.rs b/third_party/move/move-bytecode-verifier/src/signature.rs index a7b2d4372c9ab5..a89dc52c8d7835 100644 --- a/third_party/move/move-bytecode-verifier/src/signature.rs +++ b/third_party/move/move-bytecode-verifier/src/signature.rs @@ -372,7 +372,7 @@ impl<'a> SignatureChecker<'a> { } }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { return Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())); }, @@ -429,7 +429,7 @@ impl<'a> SignatureChecker<'a> { Err(PartialVMError::new(StatusCode::INVALID_SIGNATURE_TOKEN) .with_message("reference not allowed".to_string())) }, - Function(..) => Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) + Function { .. } => Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())), Vector(ty) => self.check_signature_token(ty), StructInstantiation(_, type_arguments) => self.check_signature_tokens(type_arguments), @@ -481,7 +481,7 @@ impl<'a> SignatureChecker<'a> { type_parameters, ) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { Err(PartialVMError::new(StatusCode::UNEXPECTED_VERIFIER_ERROR) .with_message("function types not supported".to_string())) }, diff --git a/third_party/move/move-bytecode-verifier/src/signature_v2.rs b/third_party/move/move-bytecode-verifier/src/signature_v2.rs index 6a9eccf8ce9a5d..f352aee8fe922b 100644 --- a/third_party/move/move-bytecode-verifier/src/signature_v2.rs +++ b/third_party/move/move-bytecode-verifier/src/signature_v2.rs @@ -173,9 +173,13 @@ fn check_ty( param_constraints, )?; }, - Function(args, result, abilities) => { + Function { + args, + results, + abilities, + } => { assert_abilities(*abilities, required_abilities)?; - for ty in args.iter().chain(result.iter()) { + for ty in args.iter().chain(results.iter()) { check_ty( struct_handles, ty, @@ -271,8 +275,8 @@ fn check_phantom_params( match ty { Vector(ty) => check_phantom_params(struct_handles, context, false, ty)?, - Function(args, result, _) => { - for ty in args.iter().chain(result) { + Function { args, results, .. } => { + for ty in args.iter().chain(results) { check_phantom_params(struct_handles, context, false, ty)? } }, @@ -915,10 +919,10 @@ impl<'a, const N: usize> SignatureChecker<'a, N> { checked_early_bind_insts.entry((*idx, *count)) { // Note non-function case is checked in `verify_fun_sig_idx` above. - if let Some(SignatureToken::Function(params, _results, _abilities)) = + if let Some(SignatureToken::Function { args, .. }) = self.resolver.signature_at(*idx).0.first() { - if *count as usize > params.len() { + if *count as usize > args.len() { return map_err(Err(PartialVMError::new( StatusCode::NUMBER_OF_ARGUMENTS_MISMATCH, ) diff --git a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs index a76f2aab3d2af4..4fb7856670a42e 100644 --- a/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs +++ b/third_party/move/move-bytecode-verifier/src/stack_usage_verifier.rs @@ -242,7 +242,7 @@ impl<'a> StackUsageVerifier<'a> { // InvokeFunction pops a function and the number of arguments and pushes the results of the // given function type Bytecode::InvokeFunction(idx) => { - if let Some(SignatureToken::Function(args, results, _)) = + if let Some(SignatureToken::Function { args, results, .. }) = self.resolver.signature_at(*idx).0.first() { ((1 + args.len()) as u64, results.len() as u64) @@ -255,7 +255,7 @@ impl<'a> StackUsageVerifier<'a> { // EarlyBindFunction pops a function value and the captured arguments and returns 1 value Bytecode::EarlyBindFunction(idx, arg_count) => { - if let Some(SignatureToken::Function(args, _results, _)) = + if let Some(SignatureToken::Function { args, .. }) = self.resolver.signature_at(*idx).0.first() { if args.len() <= *arg_count as usize { diff --git a/third_party/move/move-bytecode-verifier/src/struct_defs.rs b/third_party/move/move-bytecode-verifier/src/struct_defs.rs index d78c9e7f58f7d5..a3ce1f606d77ef 100644 --- a/third_party/move/move-bytecode-verifier/src/struct_defs.rs +++ b/third_party/move/move-bytecode-verifier/src/struct_defs.rs @@ -123,8 +123,8 @@ impl<'a> StructDefGraphBuilder<'a> { ) }, T::Vector(inner) => self.add_signature_token(neighbors, cur_idx, inner)?, - T::Function(args, result, _) => { - for t in args.iter().chain(result) { + T::Function { args, results, .. } => { + for t in args.iter().chain(results) { self.add_signature_token(neighbors, cur_idx, t)? } }, diff --git a/third_party/move/move-bytecode-verifier/src/type_safety.rs b/third_party/move/move-bytecode-verifier/src/type_safety.rs index bc1bb05d03336a..bf9897f47ea058 100644 --- a/third_party/move/move-bytecode-verifier/src/type_safety.rs +++ b/third_party/move/move-bytecode-verifier/src/type_safety.rs @@ -307,7 +307,12 @@ fn invoke_function( offset: CodeOffset, expected_ty: &SignatureToken, ) -> PartialVMResult<()> { - let SignatureToken::Function(param_tys, ret_tys, abilities) = expected_ty else { + let SignatureToken::Function { + args: param_tys, + results: ret_tys, + abilities, + } = expected_ty + else { // The signature checker has ensured this is a function safe_assert!(false); unreachable!() @@ -322,7 +327,11 @@ fn invoke_function( .with_message("closure type mismatch".to_owned())); } // Verify that the abilities match - let SignatureToken::Function(_, _, closure_abilities) = closure_ty else { + let SignatureToken::Function { + abilities: closure_abilities, + .. + } = closure_ty + else { // Ensured above, but never panic safe_assert!(false); unreachable!() @@ -385,7 +394,11 @@ fn ld_function( verifier.push( meter, instantiate( - &SignatureToken::Function(parameters.0.to_vec(), ret_sign.0.to_vec(), abilities), + &SignatureToken::Function { + args: parameters.0.to_vec(), + results: ret_sign.0.to_vec(), + abilities, + }, type_actuals, ), ) @@ -399,7 +412,12 @@ fn early_bind_function( count: u8, ) -> PartialVMResult<()> { let count = count as usize; - let SignatureToken::Function(param_tys, ret_tys, abilities) = expected_ty else { + let SignatureToken::Function { + args: param_tys, + results: ret_tys, + abilities, + } = expected_ty + else { // The signature checker has ensured this is a function safe_assert!(false); unreachable!() @@ -414,7 +432,11 @@ fn early_bind_function( .with_message("closure type mismatch".to_owned())); } // Verify that the abilities match - let SignatureToken::Function(_, _, closure_abilities) = closure_ty else { + let SignatureToken::Function { + abilities: closure_abilities, + .. + } = closure_ty + else { // Ensured above, but never panic safe_assert!(false); unreachable!() @@ -439,11 +461,11 @@ fn early_bind_function( return Err(verifier.error(StatusCode::CALL_TYPE_MISMATCH_ERROR, offset)); } } - let result_ty = SignatureToken::Function( - (*remaining_param_tys).to_vec(), - ret_tys.to_vec(), - *abilities, - ); + let result_ty = SignatureToken::Function { + args: (*remaining_param_tys).to_vec(), + results: ret_tys.to_vec(), + abilities: *abilities, + }; verifier.push(meter, result_ty) } @@ -1321,7 +1343,15 @@ fn instantiate(token: &SignatureToken, subst: &Signature) -> SignatureToken { Address => Address, Signer => Signer, Vector(ty) => Vector(Box::new(instantiate(ty, subst))), - Function(args, result, abilities) => Function(inst_vec(args), inst_vec(result), *abilities), + Function { + args, + results, + abilities, + } => Function { + args: inst_vec(args), + results: inst_vec(results), + abilities: *abilities, + }, Struct(idx) => Struct(*idx), StructInstantiation(idx, struct_type_args) => { StructInstantiation(*idx, inst_vec(struct_type_args)) diff --git a/third_party/move/move-compiler/src/interface_generator.rs b/third_party/move/move-compiler/src/interface_generator.rs index 8a78305aecc9dc..09c4c8379eb987 100644 --- a/third_party/move/move-compiler/src/interface_generator.rs +++ b/third_party/move/move-compiler/src/interface_generator.rs @@ -365,8 +365,8 @@ fn write_signature_token(ctx: &mut Context, t: &SignatureToken) -> String { SignatureToken::Address => "address".to_string(), SignatureToken::Signer => "signer".to_string(), SignatureToken::Vector(inner) => format!("vector<{}>", write_signature_token(ctx, inner)), - SignatureToken::Function(args, result, _) => { - format!("|{}|{}", tok_list(ctx, args), tok_list(ctx, result)) + SignatureToken::Function { args, results, .. } => { + format!("|{}|{}", tok_list(ctx, args), tok_list(ctx, results)) }, SignatureToken::Struct(idx) => write_struct_handle_type(ctx, *idx), SignatureToken::StructInstantiation(idx, types) => { diff --git a/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs b/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs index c29d9235040855..3da165cdb01e1c 100644 --- a/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs +++ b/third_party/move/move-ir-compiler/move-ir-to-bytecode/src/context.rs @@ -768,7 +768,7 @@ impl<'a> Context<'a> { let correct_inner = self.reindex_signature_token(dep, *inner)?; SignatureToken::Vector(Box::new(correct_inner)) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { unimplemented!("function types not supported by MoveIR") }, SignatureToken::Reference(inner) => { diff --git a/third_party/move/move-model/src/ty.rs b/third_party/move/move-model/src/ty.rs index 6da025ea9c482e..d1eedf5ed561c1 100644 --- a/third_party/move/move-model/src/ty.rs +++ b/third_party/move/move-model/src/ty.rs @@ -1394,7 +1394,7 @@ impl Type { .collect(), ) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO: implement function conversion unimplemented!("signature token to model type") }, diff --git a/third_party/move/move-vm/runtime/src/loader/type_loader.rs b/third_party/move/move-vm/runtime/src/loader/type_loader.rs index 1aa31dcd377ea7..19099c68c5c4c2 100644 --- a/third_party/move/move-vm/runtime/src/loader/type_loader.rs +++ b/third_party/move/move-vm/runtime/src/loader/type_loader.rs @@ -31,7 +31,7 @@ pub fn intern_type( let inner_type = intern_type(module, inner_tok, struct_name_table)?; Type::Vector(TriompheArc::new(inner_type)) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { // TODO: implement closures return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE) .with_message("function types in the type loader".to_owned())); diff --git a/third_party/move/move-vm/types/src/values/values_impl.rs b/third_party/move/move-vm/types/src/values/values_impl.rs index c3171cd907588b..ffdc8634d2cf1b 100644 --- a/third_party/move/move-vm/types/src/values/values_impl.rs +++ b/third_party/move/move-vm/types/src/values/values_impl.rs @@ -4154,7 +4154,7 @@ impl Value { S::Signer => return None, S::Vector(inner) => L::Vector(Box::new(Self::constant_sig_token_to_layout(inner)?)), // Not yet supported - S::Struct(_) | S::StructInstantiation(_, _) | S::Function(..) => return None, + S::Struct(_) | S::StructInstantiation(_, _) | S::Function { .. } => return None, // Not allowed/Not meaningful S::TypeParameter(_) | S::Reference(_) | S::MutableReference(_) => return None, }) diff --git a/third_party/move/tools/move-bytecode-utils/src/layout.rs b/third_party/move/tools/move-bytecode-utils/src/layout.rs index 46863644cdc55b..cfb305dd6b408f 100644 --- a/third_party/move/tools/move-bytecode-utils/src/layout.rs +++ b/third_party/move/tools/move-bytecode-utils/src/layout.rs @@ -386,7 +386,7 @@ impl TypeLayoutBuilder { ) -> anyhow::Result { use SignatureToken::*; Ok(match s { - Function(..) => bail!("function types NYI for MoveTypeLayout"), + Function { .. } => bail!("function types NYI for MoveTypeLayout"), Vector(t) => MoveTypeLayout::Vector(Box::new(Self::build_from_signature_token( m, t, diff --git a/third_party/move/tools/move-disassembler/src/disassembler.rs b/third_party/move/tools/move-disassembler/src/disassembler.rs index f368e269e5d77e..1e4bcc1e3608f2 100644 --- a/third_party/move/tools/move-disassembler/src/disassembler.rs +++ b/third_party/move/tools/move-disassembler/src/disassembler.rs @@ -571,7 +571,7 @@ impl<'a> Disassembler<'a> { ) -> Result { Ok(match sig_tok { // TODO: function types - SignatureToken::Function(..) => unimplemented!("disassembling function sig tokens"), + SignatureToken::Function { .. } => unimplemented!("disassembling function sig tokens"), SignatureToken::Bool => "bool".to_string(), SignatureToken::U8 => "u8".to_string(), diff --git a/third_party/move/tools/move-resource-viewer/src/lib.rs b/third_party/move/tools/move-resource-viewer/src/lib.rs index ba6d7f3eca5151..3e81f50c130769 100644 --- a/third_party/move/tools/move-resource-viewer/src/lib.rs +++ b/third_party/move/tools/move-resource-viewer/src/lib.rs @@ -375,7 +375,7 @@ impl MoveValueAnnotator { SignatureToken::Vector(ty) => { FatType::Vector(Box::new(self.resolve_signature(module, ty, limit)?)) }, - SignatureToken::Function(..) => { + SignatureToken::Function { .. } => { bail!("function types NYI by fat types") }, SignatureToken::Struct(idx) => {