Merge #1337

1337: feat(interface-types) Better handling of i32 to usize casts r=MarkMcCaskey a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>

lib/interface-types/src/decoders/binary.rs

@@ -169,7 +169,7 @@ fn instruction<'input, E: ParseError<&'input [u8]>>(
             (
                 input,
                 Instruction::CallCore {
-                    function_index: argument_0 as usize,
+                    function_index: argument_0 as u32,
                 },
             )
         }

lib/interface-types/src/decoders/wat.rs

@@ -146,7 +146,7 @@ impl<'a> Parse<'a> for Instruction {
             parser.parse::<keyword::call_core>()?;
 
             Ok(Instruction::CallCore {
-                function_index: parser.parse::<u64>()? as usize,
+                function_index: parser.parse::<u32>()?,
             })
         } else if lookahead.peek::<keyword::s8_from_i32>() {
             parser.parse::<keyword::s8_from_i32>()?;

lib/interface-types/src/errors.rs

@@ -5,6 +5,7 @@ use crate::{ast::InterfaceType, interpreter::Instruction};
 use std::{
     error::Error,
     fmt::{self, Display, Formatter},
+    num::TryFromIntError,
     result::Result,
     string::{self, ToString},
 };
@@ -149,6 +150,12 @@ pub enum InstructionErrorKind {
 
     /// The string contains invalid UTF-8 encoding.
     String(string::FromUtf8Error),
+
+    /// Out of range integral type conversion attempted.
+    NegativeValue {
+        /// The variable name that triggered the error.
+        subject: &'static str,
+    },
 }
 
 impl Error for InstructionErrorKind {}
@@ -227,6 +234,18 @@ impl Display for InstructionErrorKind {
                 "{}",
                 error
             ),
+
+            Self::NegativeValue { subject } => write!(
+                formatter,
+                "attempted to convert `{}` but it appears to be a negative value",
+                subject
+            ),
         }
     }
 }
+
+impl From<(TryFromIntError, &'static str)> for InstructionErrorKind {
+    fn from((_, subject): (TryFromIntError, &'static str)) -> Self {
+        InstructionErrorKind::NegativeValue { subject }
+    }
+}

lib/interface-types/src/interpreter/instruction.rs

@@ -12,7 +12,7 @@ pub enum Instruction {
     /// The `call-core` instruction.
     CallCore {
         /// The function index.
-        function_index: usize,
+        function_index: u32,
     },
 
     /// The `s8.from_i32` instruction.

lib/interface-types/src/interpreter/instructions/argument_get.rs

@@ -8,7 +8,7 @@ executable_instruction!(
         move |runtime| -> _ {
             let invocation_inputs = runtime.invocation_inputs;
 
-            if index >= (invocation_inputs.len() as u32) {
+            if (index as usize) >= invocation_inputs.len() {
                 return Err(InstructionError::new(
                     instruction,
                     InstructionErrorKind::InvocationInputIsMissing { index },

lib/interface-types/src/interpreter/instructions/call_core.rs

@@ -8,16 +8,16 @@ use crate::{
 };
 
 executable_instruction!(
-    call_core(function_index: usize, instruction: Instruction) -> _ {
+    call_core(function_index: u32, instruction: Instruction) -> _ {
         move |runtime| -> _ {
             let instance = &mut runtime.wasm_instance;
-            let index = FunctionIndex::new(function_index);
+            let index = FunctionIndex::new(function_index as usize);
 
             let local_or_import = instance.local_or_import(index).ok_or_else(|| {
                 InstructionError::new(
                     instruction,
                     InstructionErrorKind::LocalOrImportIsMissing {
-                        function_index: function_index as u32,
+                        function_index: function_index,
                     },
                 )
             })?;
@@ -40,7 +40,7 @@ executable_instruction!(
                 return Err(InstructionError::new(
                     instruction,
                     InstructionErrorKind::LocalOrImportSignatureMismatch {
-                        function_index: function_index as u32,
+                        function_index: function_index,
                         expected: (local_or_import.inputs().to_vec(), vec![]),
                         received: (input_types, vec![]),
                     },
@@ -51,7 +51,7 @@ executable_instruction!(
                 InstructionError::new(
                     instruction,
                     InstructionErrorKind::LocalOrImportCall {
-                        function_index: function_index as u32,
+                        function_index: function_index,
                     },
                 )
             })?;

lib/interface-types/src/interpreter/instructions/strings.rs

@@ -10,7 +10,7 @@ use crate::{
         Instruction,
     },
 };
-use std::cell::Cell;
+use std::{cell::Cell, convert::TryInto};
 
 executable_instruction!(
     string_lift_memory(instruction: Instruction) -> _ {
@@ -23,7 +23,6 @@ executable_instruction!(
             })?;
 
             let memory_index: u32 = 0;
-
             let memory = runtime
                 .wasm_instance
                 .memory(memory_index as usize)
@@ -34,8 +33,14 @@ executable_instruction!(
                     )
                 })?;
 
-            let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
-            let length = to_native::<i32>(&inputs[1], instruction)? as usize;
+            let pointer: usize = to_native::<i32>(&inputs[0], instruction)?
+                .try_into()
+                .map_err(|e| (e, "pointer").into())
+                .map_err(|k| InstructionError::new(instruction, k))?;
+            let length: usize = to_native::<i32>(&inputs[1], instruction)?
+                .try_into()
+                .map_err(|e| (e, "length").into())
+                .map_err(|k| InstructionError::new(instruction, k))?;
             let memory_view = memory.view();
 
             if length == 0 {
@@ -102,15 +107,25 @@ executable_instruction!(
 
             let string: String = to_native(&string, instruction)?;
             let string_bytes = string.as_bytes();
-            let string_length = string_bytes.len() as i32;
+            let string_length: i32 = string_bytes.len().try_into().map_err(|_| {
+                InstructionError::new(
+                    instruction,
+                    InstructionErrorKind::NegativeValue { subject: "string_length" },
+                )
+            })?;
 
             let outputs = allocator.call(&[InterfaceValue::I32(string_length)]).map_err(|_| {
                 InstructionError::new(
                     instruction,
                     InstructionErrorKind::LocalOrImportCall { function_index: allocator_index },
                 )
             })?;
-            let string_pointer: i32 = to_native(&outputs[0], instruction)?;
+            let string_pointer: u32 = to_native::<i32>(&outputs[0], instruction)?.try_into().map_err(|_| {
+                InstructionError::new(
+                    instruction,
+                    InstructionErrorKind::NegativeValue { subject: "string_pointer" },
+                )
+            })?;
 
             let memory_index: u32 = 0;
             let memory_view = instance
@@ -127,7 +142,7 @@ executable_instruction!(
                 memory_view[string_pointer as usize + nth].set(*byte);
             }
 
-            runtime.stack.push(InterfaceValue::I32(string_pointer));
+            runtime.stack.push(InterfaceValue::I32(string_pointer as i32));
             runtime.stack.push(InterfaceValue::I32(string_length));
 
             Ok(())
@@ -203,6 +218,42 @@ mod tests {
             stack: [InterfaceValue::String("".into())],
     );
 
+    test_executable_instruction!(
+        test_string_lift_memory__negative_pointer =
+            instructions: [
+                Instruction::ArgumentGet { index: 0 },
+                Instruction::ArgumentGet { index: 1 },
+                Instruction::StringLiftMemory,
+            ],
+            invocation_inputs: [
+                InterfaceValue::I32(-42),
+                InterfaceValue::I32(13),
+            ],
+            instance: Instance {
+                memory: Memory::new("Hello!".as_bytes().iter().map(|u| Cell::new(*u)).collect()),
+                ..Default::default()
+            },
+            error: r#"`string.lift_memory` attempted to convert `pointer` but it appears to be a negative value"#,
+    );
+
+    test_executable_instruction!(
+        test_string_lift_memory__negative_length =
+            instructions: [
+                Instruction::ArgumentGet { index: 0 },
+                Instruction::ArgumentGet { index: 1 },
+                Instruction::StringLiftMemory,
+            ],
+            invocation_inputs: [
+                InterfaceValue::I32(0),
+                InterfaceValue::I32(-1),
+            ],
+            instance: Instance {
+                memory: Memory::new("Hello!".as_bytes().iter().map(|u| Cell::new(*u)).collect()),
+                ..Default::default()
+            },
+            error: r#"`string.lift_memory` attempted to convert `length` but it appears to be a negative value"#,
+    );
+
     test_executable_instruction!(
         test_string_lift_memory__read_out_of_memory =
             instructions: [