Skip to content

Commit

Permalink
Check version when deserializing closure opcodes.
Browse files Browse the repository at this point in the history
  • Loading branch information
wrwg committed Jan 18, 2025
1 parent c879216 commit 58fbf2b
Show file tree
Hide file tree
Showing 12 changed files with 28 additions and 13 deletions.
10 changes: 10 additions & 0 deletions third_party/move/move-binary-format/src/deserializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
)),
);
},
_ => {},
};

Expand Down
7 changes: 4 additions & 3 deletions third_party/move/move-binary-format/src/file_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3106,8 +3106,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
Expand All @@ -3122,7 +3122,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 = "[]"]
Expand Down
5 changes: 3 additions & 2 deletions third_party/move/move-binary-format/src/file_format_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -513,12 +514,12 @@ 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
/// are added.
pub const VERSION_MAX: u32 = VERSION_7;
pub const VERSION_MAX: u32 = VERSION_8;

/// Mark which version is the default version. This is the version used by default by tools like
/// the compiler. Notice that this version might be different from the one supported on nodes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())));
}
Expand Down
2 changes: 1 addition & 1 deletion third_party/move/move-core/types/src/vm_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion third_party/move/move-model/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1395,7 +1395,7 @@ impl Type {
)
},
SignatureToken::Function(..) => {
// TODO: implement function conversion
// TODO(#15664): implement function conversion
unimplemented!("signature token to model type")
},
}
Expand Down
2 changes: 1 addition & 1 deletion third_party/move/move-vm/runtime/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1728,7 +1728,7 @@ impl Frame {
)?;

match instruction {
// TODO: implement closures
// TODO(#15664): implement closures
Bytecode::PackClosure(..)
| Bytecode::PackClosureGeneric(..)
| Bytecode::CallClosure(..) => {
Expand Down
2 changes: 1 addition & 1 deletion third_party/move/move-vm/runtime/src/loader/type_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
},
Expand Down
4 changes: 2 additions & 2 deletions third_party/move/move-vm/runtime/src/runtime_type_checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(..) => {
Expand Down Expand Up @@ -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(..) => {
Expand Down
3 changes: 2 additions & 1 deletion third_party/move/tools/move-disassembler/src/disassembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions third_party/move/tools/move-resource-viewer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit 58fbf2b

Please sign in to comment.