From 81d65ef41f9bb746ff4614fff01d6d8dcbb7afd2 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 7 Apr 2020 12:34:07 +0200 Subject: [PATCH 1/3] fix(interface-types) Remove allocator index from `string.lower_memory`. --- src/decoders/binary.rs | 15 +-- src/decoders/wat.rs | 10 +- src/encoders/binary.rs | 9 +- src/encoders/wat.rs | 11 +-- src/interpreter/instructions/mod.rs | 5 +- src/interpreter/instructions/strings.rs | 126 ++++-------------------- src/interpreter/mod.rs | 4 +- 7 files changed, 34 insertions(+), 146 deletions(-) diff --git a/src/decoders/binary.rs b/src/decoders/binary.rs index 5aae148..7e0684a 100644 --- a/src/decoders/binary.rs +++ b/src/decoders/binary.rs @@ -232,16 +232,7 @@ fn instruction<'input, E: ParseError<&'input [u8]>>( 0x21 => (input, Instruction::I64FromU64), 0x22 => (input, Instruction::StringLiftMemory), - 0x23 => { - consume!((input, argument_0) = uleb(input)?); - - ( - input, - Instruction::StringLowerMemory { - allocator_index: argument_0 as u32, - }, - ) - } + 0x23 => (input, Instruction::StringLowerMemory), 0x24 => (input, Instruction::StringSize), 0x25 => { @@ -764,7 +755,7 @@ mod tests { 0x20, // I64FromU32 0x21, // I64FromU64 0x22, // StringLiftMemory - 0x23, 0x01, // StringLowerMemory { allocator_index: 1 } + 0x23, // StringLowerMemory 0x24, // StringSize 0x25, 0x01, // RecordLift { type_index: 1 }, 0x26, 0x01, // RecordLower { type_index: 1 }, @@ -808,7 +799,7 @@ mod tests { Instruction::I64FromU32, Instruction::I64FromU64, Instruction::StringLiftMemory, - Instruction::StringLowerMemory { allocator_index: 1 }, + Instruction::StringLowerMemory, Instruction::StringSize, Instruction::RecordLift { type_index: 1 }, Instruction::RecordLower { type_index: 1 }, diff --git a/src/decoders/wat.rs b/src/decoders/wat.rs index 9b07be0..c0f66ad 100644 --- a/src/decoders/wat.rs +++ b/src/decoders/wat.rs @@ -309,9 +309,7 @@ impl<'a> Parse<'a> for Instruction { } else if lookahead.peek::() { parser.parse::()?; - Ok(Instruction::StringLowerMemory { - allocator_index: parser.parse()?, - }) + Ok(Instruction::StringLowerMemory) } else if lookahead.peek::() { parser.parse::()?; @@ -770,7 +768,7 @@ mod tests { "i64.from_u32", "i64.from_u64", "string.lift_memory", - "string.lower_memory 42", + "string.lower_memory", "string.size", "record.lift 42", "record.lower 42", @@ -811,9 +809,7 @@ mod tests { Instruction::I64FromU32, Instruction::I64FromU64, Instruction::StringLiftMemory, - Instruction::StringLowerMemory { - allocator_index: 42, - }, + Instruction::StringLowerMemory, Instruction::StringSize, Instruction::RecordLift { type_index: 42 }, Instruction::RecordLower { type_index: 42 }, diff --git a/src/encoders/binary.rs b/src/encoders/binary.rs index 2849dce..6788bdd 100644 --- a/src/encoders/binary.rs +++ b/src/encoders/binary.rs @@ -331,10 +331,7 @@ where Instruction::I64FromU64 => 0x21_u8.to_bytes(writer)?, Instruction::StringLiftMemory => 0x22_u8.to_bytes(writer)?, - Instruction::StringLowerMemory { allocator_index } => { - 0x23_u8.to_bytes(writer)?; - (*allocator_index as u64).to_bytes(writer)?; - } + Instruction::StringLowerMemory => 0x23_u8.to_bytes(writer)?, Instruction::StringSize => 0x24_u8.to_bytes(writer)?, Instruction::RecordLift { type_index } => { @@ -688,7 +685,7 @@ mod tests { Instruction::I64FromU32, Instruction::I64FromU64, Instruction::StringLiftMemory, - Instruction::StringLowerMemory { allocator_index: 1 }, + Instruction::StringLowerMemory, Instruction::StringSize, Instruction::RecordLift { type_index: 1 }, Instruction::RecordLower { type_index: 1 }, @@ -730,7 +727,7 @@ mod tests { 0x20, // I64FromU32 0x21, // I64FromU64 0x22, // StringLiftMemory - 0x23, 0x01, // StringLowerMemory { allocator_index: 1 } + 0x23, // StringLowerMemory 0x24, // StringSize 0x025, 0x01, // RecordLift { type_index: 1 } 0x026, 0x01, // RecordLower { type_index: 1 } diff --git a/src/encoders/wat.rs b/src/encoders/wat.rs index a141b23..a99adf4 100644 --- a/src/encoders/wat.rs +++ b/src/encoders/wat.rs @@ -135,9 +135,7 @@ impl ToString for &Instruction { Instruction::I64FromU32 => "i64.from_u32".into(), Instruction::I64FromU64 => "i64.from_u64".into(), Instruction::StringLiftMemory => "string.lift_memory".into(), - Instruction::StringLowerMemory { allocator_index } => { - format!("string.lower_memory {}", allocator_index) - } + Instruction::StringLowerMemory => "string.lower_memory".into(), Instruction::StringSize => "string.size".into(), Instruction::RecordLift { type_index } => format!("record.lift {}", type_index), Instruction::RecordLower { type_index } => format!("record.lower {}", type_index), @@ -462,10 +460,7 @@ mod tests { (&Instruction::I64FromU32).to_string(), (&Instruction::I64FromU64).to_string(), (&Instruction::StringLiftMemory).to_string(), - (&Instruction::StringLowerMemory { - allocator_index: 42, - }) - .to_string(), + (&Instruction::StringLowerMemory).to_string(), (&Instruction::StringSize).to_string(), (&Instruction::RecordLift { type_index: 42 }).to_string(), (&Instruction::RecordLower { type_index: 42 }).to_string(), @@ -506,7 +501,7 @@ mod tests { "i64.from_u32", "i64.from_u64", "string.lift_memory", - "string.lower_memory 42", + "string.lower_memory", "string.size", "record.lift 42", "record.lower 42", diff --git a/src/interpreter/instructions/mod.rs b/src/interpreter/instructions/mod.rs index 39f25ab..49ab7ca 100644 --- a/src/interpreter/instructions/mod.rs +++ b/src/interpreter/instructions/mod.rs @@ -130,10 +130,7 @@ pub enum Instruction { StringLiftMemory, /// The `string.lower_memory` instruction. - StringLowerMemory { - /// The allocator function index. - allocator_index: u32, - }, + StringLowerMemory, /// The `string.size` instruction. StringSize, diff --git a/src/interpreter/instructions/strings.rs b/src/interpreter/instructions/strings.rs index 0a332cb..7e52110 100644 --- a/src/interpreter/instructions/strings.rs +++ b/src/interpreter/instructions/strings.rs @@ -2,13 +2,7 @@ use super::to_native; use crate::{ ast::InterfaceType, errors::{InstructionError, InstructionErrorKind}, - interpreter::{ - wasm::{ - structures::{FunctionIndex, TypedIndex}, - values::InterfaceValue, - }, - Instruction, - }, + interpreter::{wasm::values::InterfaceValue, Instruction}, }; use std::{cell::Cell, convert::TryInto}; @@ -75,37 +69,16 @@ executable_instruction!( ); executable_instruction!( - string_lower_memory(allocator_index: u32, instruction: Instruction) -> _ { + string_lower_memory(instruction: Instruction) -> _ { move |runtime| -> _ { - let instance = &mut runtime.wasm_instance; - let index = FunctionIndex::new(allocator_index as usize); - - let allocator = instance.local_or_import(index).ok_or_else(|| { + let inputs = runtime.stack.pop(2).ok_or_else(|| { InstructionError::new( instruction, - InstructionErrorKind::LocalOrImportIsMissing { function_index: allocator_index }, + InstructionErrorKind::StackIsTooSmall { needed: 2 }, ) })?; - if allocator.inputs() != [InterfaceType::I32] || allocator.outputs() != [InterfaceType::I32] { - return Err(InstructionError::new( - instruction, - InstructionErrorKind::LocalOrImportSignatureMismatch { - function_index: allocator_index, - expected: (vec![InterfaceType::I32], vec![InterfaceType::I32]), - received: (allocator.inputs().to_vec(), allocator.outputs().to_vec()), - }, - )) - } - - let string = runtime.stack.pop1().ok_or_else(|| { - InstructionError::new( - instruction, - InstructionErrorKind::StackIsTooSmall { needed: 1 }, - ) - })?; - - let string: String = to_native(&string, instruction)?; + let string: String = to_native(&inputs[0], instruction)?; let string_bytes = string.as_bytes(); let string_length: i32 = string_bytes.len().try_into().map_err(|_| { InstructionError::new( @@ -113,20 +86,12 @@ executable_instruction!( InstructionErrorKind::NegativeValue { subject: "string_length" }, ) })?; + let string_pointer: usize = to_native::(&inputs[1], instruction)? + .try_into() + .map_err(|e| (e, "pointer").into()) + .map_err(|k| InstructionError::new(instruction, k))?; - let outputs = allocator.call(&[InterfaceValue::I32(string_length)]).map_err(|_| { - InstructionError::new( - instruction, - InstructionErrorKind::LocalOrImportCall { function_index: allocator_index }, - ) - })?; - let string_pointer: u32 = to_native::(&outputs[0], instruction)?.try_into().map_err(|_| { - InstructionError::new( - instruction, - InstructionErrorKind::NegativeValue { subject: "string_pointer" }, - ) - })?; - + let instance = &mut runtime.wasm_instance; let memory_index: u32 = 0; let memory_view = instance .memory(memory_index as usize) @@ -313,7 +278,10 @@ mod tests { test_string_lower_memory = instructions: [ Instruction::ArgumentGet { index: 0 }, - Instruction::StringLowerMemory { allocator_index: 43 }, + Instruction::StringSize, + Instruction::CallCore { function_index: 43 }, + Instruction::StringLowerMemory, + ], invocation_inputs: [InterfaceValue::String("Hello, World!".into())], instance: Instance::new(), @@ -329,7 +297,9 @@ mod tests { test_string__roundtrip = instructions: [ Instruction::ArgumentGet { index: 0 }, - Instruction::StringLowerMemory { allocator_index: 43 }, + Instruction::StringSize, + Instruction::CallCore { function_index: 43 }, + Instruction::StringLowerMemory, Instruction::StringLiftMemory, ], invocation_inputs: [InterfaceValue::String("Hello, World!".into())], @@ -337,70 +307,14 @@ mod tests { stack: [InterfaceValue::String("Hello, World!".into())], ); - test_executable_instruction!( - test_string_lower_memory__allocator_does_not_exist = - instructions: [Instruction::StringLowerMemory { allocator_index: 43 }], - invocation_inputs: [], - instance: Instance { ..Default::default() }, - error: r#"`string.lower_memory 43` the local or import function `43` doesn't exist"#, - ); - test_executable_instruction!( test_string_lower_memory__stack_is_too_small = instructions: [ - Instruction::StringLowerMemory { allocator_index: 43 } - // ^^ `43` expects 1 value on the stack, none is present + Instruction::StringLowerMemory, ], - invocation_inputs: [InterfaceValue::String("Hello, World!".into())], + invocation_inputs: [], instance: Instance::new(), - error: r#"`string.lower_memory 43` needed to read `1` value(s) from the stack, but it doesn't contain enough data"#, - ); - - test_executable_instruction!( - test_string_lower_memory__failure_when_calling_the_allocator = - instructions: [ - Instruction::ArgumentGet { index: 0 }, - Instruction::StringLowerMemory { allocator_index: 153 } - ], - invocation_inputs: [InterfaceValue::String("Hello, World!".into())], - instance: { - let mut instance = Instance::new(); - instance.locals_or_imports.insert( - 153, - LocalImport { - inputs: vec![InterfaceType::I32], - outputs: vec![InterfaceType::I32], - function: |_| Err(()), - // ^^^^^^^ function fails - }, - ); - - instance - }, - error: r#"`string.lower_memory 153` failed while calling the local or import function `153`"#, - ); - - test_executable_instruction!( - test_string_lower_memory__invalid_allocator_signature = - instructions: [ - Instruction::ArgumentGet { index: 0 }, - Instruction::StringLowerMemory { allocator_index: 153 } - ], - invocation_inputs: [InterfaceValue::String("Hello, World!".into())], - instance: { - let mut instance = Instance::new(); - instance.locals_or_imports.insert( - 153, - LocalImport { - inputs: vec![InterfaceType::I32, InterfaceType::I32], - outputs: vec![], - function: |_| Err(()), - }, - ); - - instance - }, - error: r#"`string.lower_memory 153` the local or import function `153` has the signature `[I32] -> [I32]` but it received values of kind `[I32, I32] -> []`"#, + error: r#"`string.lower_memory` needed to read `2` value(s) from the stack, but it doesn't contain enough data"#, ); test_executable_instruction!( diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 7486c81..0d6b331 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -230,9 +230,7 @@ where Instruction::I64FromU64 => instructions::i64_from_u64(*instruction), Instruction::StringLiftMemory => instructions::string_lift_memory(*instruction), - Instruction::StringLowerMemory { allocator_index } => { - instructions::string_lower_memory(*allocator_index, *instruction) - } + Instruction::StringLowerMemory => instructions::string_lower_memory(*instruction), Instruction::StringSize => instructions::string_size(*instruction), Instruction::RecordLift { type_index } => { From 2f17a52373ddf27697c2d5ab6a32f8a7349bf7e9 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 7 Apr 2020 12:34:30 +0200 Subject: [PATCH 2/3] test(interface-types) Update the `test_executable_instruction` macro. It provides a better failure message. --- src/macros.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/macros.rs b/src/macros.rs index 8ff4109..be60ac0 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -111,7 +111,12 @@ macro_rules! test_executable_instruction { let mut instance = $instance; let run = interpreter.run(&invocation_inputs, &mut instance); - assert!(run.is_ok()); + let err = match &run { + Ok(_) => "".to_string(), + Err(e) => e.to_string(), + }; + + assert!(run.is_ok(), err); let stack = run.unwrap(); From 5cb07c9d692049587b8cbb65b65b46b0ea0387c2 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 7 Apr 2020 12:40:14 +0200 Subject: [PATCH 3/3] feat(interface-types) `string.size` pops the string. Previously, `string.size` was just peeking the string. --- src/interpreter/instructions/strings.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/interpreter/instructions/strings.rs b/src/interpreter/instructions/strings.rs index 7e52110..d84e631 100644 --- a/src/interpreter/instructions/strings.rs +++ b/src/interpreter/instructions/strings.rs @@ -78,7 +78,11 @@ executable_instruction!( ) })?; - let string: String = to_native(&inputs[0], instruction)?; + let string_pointer: usize = to_native::(&inputs[0], instruction)? + .try_into() + .map_err(|e| (e, "pointer").into()) + .map_err(|k| InstructionError::new(instruction, k))?; + let string: String = to_native(&inputs[1], instruction)?; let string_bytes = string.as_bytes(); let string_length: i32 = string_bytes.len().try_into().map_err(|_| { InstructionError::new( @@ -86,10 +90,6 @@ executable_instruction!( InstructionErrorKind::NegativeValue { subject: "string_length" }, ) })?; - let string_pointer: usize = to_native::(&inputs[1], instruction)? - .try_into() - .map_err(|e| (e, "pointer").into()) - .map_err(|k| InstructionError::new(instruction, k))?; let instance = &mut runtime.wasm_instance; let memory_index: u32 = 0; @@ -118,7 +118,7 @@ executable_instruction!( executable_instruction!( string_size(instruction: Instruction) -> _ { move |runtime| -> _ { - match runtime.stack.peek1() { + match runtime.stack.pop1() { Some(InterfaceValue::String(string)) => { let length = string.len() as i32; runtime.stack.push(InterfaceValue::I32(length)); @@ -130,7 +130,7 @@ executable_instruction!( instruction, InstructionErrorKind::InvalidValueOnTheStack { expected_type: InterfaceType::String, - received_type: value.into(), + received_type: (&value).into(), }, )), @@ -280,6 +280,7 @@ mod tests { Instruction::ArgumentGet { index: 0 }, Instruction::StringSize, Instruction::CallCore { function_index: 43 }, + Instruction::ArgumentGet { index: 0 }, Instruction::StringLowerMemory, ], @@ -299,6 +300,7 @@ mod tests { Instruction::ArgumentGet { index: 0 }, Instruction::StringSize, Instruction::CallCore { function_index: 43 }, + Instruction::ArgumentGet { index: 0 }, Instruction::StringLowerMemory, Instruction::StringLiftMemory, ], @@ -325,7 +327,7 @@ mod tests { ], invocation_inputs: [InterfaceValue::String("Hello, World!".into())], instance: Instance::new(), - stack: [InterfaceValue::String("Hello, World!".into()), InterfaceValue::I32(13)], + stack: [InterfaceValue::I32(13)], ); test_executable_instruction!(