Check version when deserializing closure opcodes.

third_party/move/move-binary-format/src/deserializer.rs

@@ -1650,6 +1650,16 @@ fn load_code(cursor: &mut VersionedCursor, code: &mut Vec<Bytecode>) -> BinaryLo
                     )),
                 );
             },
+            Opcodes::PACK_CLOSURE | Opcodes::PACK_CLOSURE_GENERIC | Opcodes::CALL_CLOSURE
+                if cursor.version() < VERSION_8 =>
+            {
+                return Err(
+                    PartialVMError::new(StatusCode::MALFORMED).with_message(format!(
+                        "Closure operations not available before bytecode version {}",
+                        VERSION_8
+                    )),
+                );
+            },
             _ => {},
         };
 

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,
 };
@@ -1568,16 +1567,14 @@ impl ClosureMask {
     /// 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<T: Clone>(&self, tys: &[T], collect_captured: bool) -> Vec<T> {
-        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
-                }
+    pub fn extract<'a, T: Clone>(&self, values: &'a [T], collect_captured: bool) -> Vec<&'a T> {
+        let mut mask = self.mask;
+        values
+            .iter()
+            .filter(|_| {
+                let set = mask & 0x1 != 0;
+                mask >>= 1;
+                set && collect_captured || !set && !collect_captured
             })
             .collect()
     }
@@ -1590,39 +1587,29 @@ impl ClosureMask {
     /// 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<T: Clone>(&self, captured: &[T], provided: &[T]) -> Option<Vec<T>> {
-        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
+    pub fn compose<T: Clone>(
+        &self,
+        captured: impl IntoIterator<Item = T>,
+        provided: impl IntoIterator<Item = T>,
+    ) -> Option<Vec<T>> {
+        let mut captured = captured.into_iter();
+        let mut provided = provided.into_iter();
+        let mut result = vec![];
+        let mut mask = self.mask;
+        while mask != 0 {
+            if mask & 0x1 != 0 {
+                result.push(captured.next()?)
             } else {
-                if pro_idx >= provided.len() {
-                    // Inconsistency
-                    return None;
-                }
-                result.insert(i, provided[pro_idx].clone());
-                pro_idx += 1
+                result.push(provided.next()?)
             }
+            mask >>= 1;
         }
-        let map_len = result.len();
-        let vec = result.into_values().collect::<Vec<_>>();
-        if vec.len() != map_len {
-            // Inconsistency: all indices must be contiguously covered
-            None
-        } else {
-            Some(vec)
+        if captured.next().is_some() {
+            // Not all captured arguments consumed
+            return None;
         }
+        result.extend(provided);
+        Some(result)
     }
 
     /// Return the max index of captured arguments
@@ -3106,8 +3093,8 @@ pub enum Bytecode {
 
     #[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.
+        `CallClosure(|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
@@ -3122,7 +3109,8 @@ pub enum Bytecode {
         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))
+          CallClosure(PackClosure(f, mask, c1..cn), a1..am) ==
+             f(mask.compose(c1..cn, a1..am))
         ```
     "#]
     #[static_operands = "[]"]

third_party/move/move-binary-format/src/file_format_common.rs

@@ -299,6 +299,7 @@ pub enum Opcodes {
     UNPACK_VARIANT_GENERIC      = 0x55,
     TEST_VARIANT                = 0x56,
     TEST_VARIANT_GENERIC        = 0x57,
+    // Since bytecode version 8
     PACK_CLOSURE                = 0x58,
     PACK_CLOSURE_GENERIC        = 0x59,
     CALL_CLOSURE                = 0x5A,
@@ -513,7 +514,7 @@ pub const VERSION_6: u32 = 6;
 pub const VERSION_7: u32 = 7;
 
 /// Version 8: changes compared to version 7
-/// + TBD
+/// + closure instructions
 pub const VERSION_8: u32 = 8;
 
 /// Mark which version is the latest version. Should be set to v8 once features

third_party/move/move-bytecode-verifier/src/signature_v2.rs

@@ -902,7 +902,7 @@ impl<'a, const N: usize> SignatureChecker<'a, N> {
                     let sgn = self.resolver.signature_at(*idx);
                     if sgn.len() != 1 || !matches!(&sgn.0[0], SignatureToken::Function(..)) {
                         return map_err(Err(PartialVMError::new(
-                            StatusCode::CLOSURE_EVAL_REQUIRES_FUNCTION,
+                            StatusCode::CLOSURE_CALL_REQUIRES_FUNCTION,
                         )
                         .with_message("expected a function type for closure call".to_string())));
                     }

third_party/move/move-bytecode-verifier/src/type_safety.rs

@@ -360,7 +360,7 @@ fn clos_pack(
     let param_sgn = verifier.resolver.signature_at(func_handle.parameters);
     let captured_param_tys = mask.extract(&param_sgn.0, true);
     let mut abilities = AbilitySet::ALL;
-    for ty in captured_param_tys.iter().rev() {
+    for ty in captured_param_tys.into_iter().rev() {
         abilities = abilities.intersect(verifier.abilities(ty)?);
         let arg = safe_unwrap!(verifier.stack.pop());
         if (type_actuals.is_empty() && &arg != ty)
@@ -402,7 +402,11 @@ fn clos_pack(
     abilities.remove(Ability::Key);
 
     // Construct the resulting function type
-    let not_captured_param_tys = mask.extract(&param_sgn.0, false);
+    let not_captured_param_tys = mask
+        .extract(&param_sgn.0, false)
+        .into_iter()
+        .cloned()
+        .collect();
     let ret_sign = verifier.resolver.signature_at(func_handle.return_);
     verifier.push(
         meter,

third_party/move/move-core/types/src/vm_status.rs

@@ -747,7 +747,7 @@ pub enum StatusCode {
     // Closure mask invalid
     INVALID_CLOSURE_MASK = 1132,
     // Closure eval type is not a function
-    CLOSURE_EVAL_REQUIRES_FUNCTION = 1133,
+    CLOSURE_CALL_REQUIRES_FUNCTION = 1133,
 
     // Reserved error code for future use
     RESERVED_VERIFICATION_ERROR_2 = 1134,

third_party/move/move-model/bytecode/src/stackless_bytecode_generator.rs

@@ -296,6 +296,7 @@ impl<'a> StacklessBytecodeGenerator<'a> {
             MoveBytecode::PackClosure(..)
             | MoveBytecode::PackClosureGeneric(..)
             | MoveBytecode::CallClosure(..) => {
+                // TODO(#15664): implement for closures
                 unimplemented!("stackless bytecode generation for closure opcodes")
             },
             MoveBytecode::Pop => {

third_party/move/move-model/src/ty.rs

@@ -1395,7 +1395,7 @@ impl Type {
                 )
             },
             SignatureToken::Function(..) => {
-                // TODO: implement function conversion
+                // TODO(#15664): implement function conversion
                 unimplemented!("signature token to model type")
             },
         }

third_party/move/move-vm/runtime/src/interpreter.rs

@@ -1728,7 +1728,7 @@ impl Frame {
                 )?;
 
                 match instruction {
-                    // TODO: implement closures
+                    // TODO(#15664): implement closures
                     Bytecode::PackClosure(..)
                     | Bytecode::PackClosureGeneric(..)
                     | Bytecode::CallClosure(..) => {

third_party/move/move-vm/runtime/src/loader/type_loader.rs

@@ -32,7 +32,7 @@ pub fn intern_type(
             Type::Vector(TriompheArc::new(inner_type))
         },
         SignatureToken::Function(..) => {
-            // TODO: implement closures
+            // TODO(#15664): implement closures
             return Err(PartialVMError::new(StatusCode::UNIMPLEMENTED_FEATURE)
                 .with_message("function types in the type loader".to_owned()));
         },

third_party/move/move-vm/runtime/src/runtime_type_checks.rs

@@ -120,7 +120,7 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck {
         instruction: &Bytecode,
     ) -> PartialVMResult<()> {
         match instruction {
-            // TODO: implement closures
+            // TODO(#15664): implement closures
             Bytecode::PackClosure(..)
             | Bytecode::PackClosureGeneric(..)
             | Bytecode::CallClosure(..) => {
@@ -254,7 +254,7 @@ impl RuntimeTypeCheck for FullRuntimeTypeCheck {
         let ty_builder = resolver.loader().ty_builder();
 
         match instruction {
-            // TODO: implement closures
+            // TODO(#15664): implement closures
             Bytecode::PackClosure(..)
             | Bytecode::PackClosureGeneric(..)
             | Bytecode::CallClosure(..) => {

third_party/move/tools/move-disassembler/src/disassembler.rs

@@ -570,7 +570,7 @@ impl<'a> Disassembler<'a> {
         type_param_context: &[SourceName],
     ) -> Result<String> {
         Ok(match sig_tok {
-            // TODO: function types
+            // TODO(#15664): function types
             SignatureToken::Function(..) => unimplemented!("disassembling function sig tokens"),
 
             SignatureToken::Bool => "bool".to_string(),
@@ -647,6 +647,7 @@ impl<'a> Disassembler<'a> {
             Bytecode::PackClosure(..)
             | Bytecode::PackClosureGeneric(..)
             | Bytecode::CallClosure(..) => {
+                // TODO(#15664): implement
                 bail!("closure opcodes not implemented")
             },
             Bytecode::LdConst(idx) => {

third_party/move/tools/move-resource-viewer/src/lib.rs

@@ -376,6 +376,7 @@ impl<V: CompiledModuleView> MoveValueAnnotator<V> {
                 FatType::Vector(Box::new(self.resolve_signature(module, ty, limit)?))
             },
             SignatureToken::Function(..) => {
+                // TODO(#15664): implement
                 bail!("function types NYI by fat types")
             },
             SignatureToken::Struct(idx) => {