Repository: bytecodealliance/wasm-tools
Pull request #14 (2020-05-20 18:00:30)

Implement support for tail calls in wasmparser
This adds the two new opcodes added by the tail-call proposal,
`return_call` and `return_call_indirect`. The main complication here is
the validation.


Patch:
diff --git a/crates/wasmparser/src/binary_reader.rs b/crates/wasmparser/src/binary_reader.rs
index 96377fb3..549200ea 100644
--- a/crates/wasmparser/src/binary_reader.rs
+++ b/crates/wasmparser/src/binary_reader.rs
@@ -977,6 +977,13 @@ impl<'a> BinaryReader<'a> {
                 index: self.read_var_u32()?,
                 table_index: self.read_var_u32()?,
             },
+            0x12 => Operator::ReturnCall {
+                function_index: self.read_var_u32()?,
+            },
+            0x13 => Operator::ReturnCallIndirect {
+                index: self.read_var_u32()?,
+                table_index: self.read_var_u32()?,
+            },
             0x1a => Operator::Drop,
             0x1b => Operator::Select,
             0x1c => {
diff --git a/crates/wasmparser/src/module_resources.rs b/crates/wasmparser/src/module_resources.rs
index ba3ead27..71771721 100644
--- a/crates/wasmparser/src/module_resources.rs
+++ b/crates/wasmparser/src/module_resources.rs
@@ -304,6 +304,42 @@ pub trait WasmModuleResources {
     fn data_count(&self) -> u32;
 }
 
+impl<T> WasmModuleResources for &'_ T
+where
+    T: ?Sized + WasmModuleResources,
+{
+    type FuncType = T::FuncType;
+    type TableType = T::TableType;
+    type MemoryType = T::MemoryType;
+    type GlobalType = T::GlobalType;
+
+    fn type_at(&self, at: u32) -> Option<&Self::FuncType> {
+        T::type_at(self, at)
+    }
+    fn table_at(&self, at: u32) -> Option<&Self::TableType> {
+        T::table_at(self, at)
+    }
+    fn memory_at(&self, at: u32) -> Option<&Self::MemoryType> {
+        T::memory_at(self, at)
+    }
+    fn global_at(&self, at: u32) -> Option<&Self::GlobalType> {
+        T::global_at(self, at)
+    }
+    fn func_type_id_at(&self, at: u32) -> Option<u32> {
+        T::func_type_id_at(self, at)
+    }
+    fn element_type_at(&self, at: u32) -> Option<crate::Type> {
+        T::element_type_at(self, at)
+    }
+
+    fn element_count(&self) -> u32 {
+        T::element_count(self)
+    }
+    fn data_count(&self) -> u32 {
+        T::data_count(self)
+    }
+}
+
 impl WasmType for crate::Type {
     fn to_parser_type(&self) -> crate::Type {
         *self
diff --git a/crates/wasmparser/src/operators_validator.rs b/crates/wasmparser/src/operators_validator.rs
index 872b58f5..d81e62cb 100644
--- a/crates/wasmparser/src/operators_validator.rs
+++ b/crates/wasmparser/src/operators_validator.rs
@@ -327,6 +327,7 @@ pub struct OperatorValidatorConfig {
     pub enable_simd: bool,
     pub enable_bulk_memory: bool,
     pub enable_multi_value: bool,
+    pub enable_tail_call: bool,
 
     #[cfg(feature = "deterministic")]
     pub deterministic_only: bool,
@@ -339,6 +340,7 @@ pub(crate) const DEFAULT_OPERATOR_VALIDATOR_CONFIG: OperatorValidatorConfig =
         enable_simd: false,
         enable_bulk_memory: false,
         enable_multi_value: true,
+        enable_tail_call: false,
 
         #[cfg(feature = "deterministic")]
         deterministic_only: true,
@@ -525,6 +527,72 @@ impl OperatorValidator {
         self.check_block_return_types(self.func_state.last_block(), 0)
     }
 
+    fn check_call(
+        &mut self,
+        function_index: u32,
+        resources: impl WasmModuleResources,
+    ) -> OperatorValidatorResult<()> {
+        let type_index = match resources.func_type_id_at(function_index) {
+            Some(i) => i,
+            None => {
+                bail_op_err!(
+                    "unknown function {}: function index out of bounds",
+                    function_index
+                );
+            }
+        };
+        let ty = resources
+            .type_at(type_index)
+            .expect("function type index is out of bounds");
+        self.check_operands(wasm_func_type_inputs(ty).map(WasmType::to_parser_type))?;
+        self.func_state.change_frame_with_types(
+            ty.len_inputs(),
+            wasm_func_type_outputs(ty).map(WasmType::to_parser_type),
+        )?;
+        Ok(())
+    }
+
+    fn check_call_indirect(
+        &mut self,
+        index: u32,
+        table_index: u32,
+        resources: impl WasmModuleResources,
+    ) -> OperatorValidatorResult<()> {
+        if resources.table_at(table_index).is_none() {
+            return Err(OperatorValidatorError::new(
+                "unknown table: table index out of bounds",
+            ));
+        }
+        match resources.type_at(index) {
+            None => {
+                return Err(OperatorValidatorError::new(
+                    "unknown type: type index out of bounds",
+                ))
+            }
+            Some(ty) => {
+                let types = {
+                    let mut types = Vec::with_capacity(ty.len_inputs() + 1);
+                    types.extend(wasm_func_type_inputs(ty).map(WasmType::to_parser_type));
+                    types.push(Type::I32);
+                    types
+                };
+                self.check_operands(types.into_iter())?;
+                self.func_state.change_frame_with_types(
+                    ty.len_inputs() + 1,
+                    wasm_func_type_outputs(ty).map(WasmType::to_parser_type),
+                )?;
+            }
+        }
+        Ok(())
+    }
+
+    fn check_return(&mut self) -> OperatorValidatorResult<()> {
+        let depth = (self.func_state.blocks.len() - 1) as u32;
+        self.check_jump_from_block(depth, 0)?;
+        self.func_state.start_dead_code();
+        Ok(())
+    }
+
     fn check_jump_from_block(
         &self,
         relative_depth: u32,
@@ -927,59 +995,28 @@ impl OperatorValidator {
                 }
                 self.func_state.start_dead_code()
             }
-            Operator::Return => {
-                let depth = (self.func_state.blocks.len() - 1) as u32;
-                self.check_jump_from_block(depth, 0)?;
-                self.func_state.start_dead_code()
-            }
-            Operator::Call { function_index } => match resources.func_type_id_at(function_index) {
-                Some(type_index) => {
-                    let ty = resources
-                        .type_at(type_index)
-                        // Note: This was an out-of-bounds memory access before
-                        //       the change to return `Option` at `type_at`. So
-                        //       I assumed that invalid indices at this point are
-                        //       bugs.
-                        .expect("function type index is out of bounds");
-                    self.check_operands(wasm_func_type_inputs(ty).map(WasmType::to_parser_type))?;
-                    self.func_state.change_frame_with_types(
-                        ty.len_inputs(),
-                        wasm_func_type_outputs(ty).map(WasmType::to_parser_type),
-                    )?;
-                }
-                None => {
-                    bail_op_err!(
-                        "unknown function {}: function index out of bounds",
-                        function_index
-                    );
+            Operator::Return => self.check_return()?,
+            Operator::Call { function_index } => self.check_call(function_index, resources)?,
+            Operator::ReturnCall { function_index } => {
+                if !self.config.enable_tail_call {
+                    return Err(OperatorValidatorError::new(
+                        "tail calls support is not enabled",
+                    ));
                 }
-            },
+                self.check_call(function_index, resources)?;
+                self.check_return()?;
+            }
             Operator::CallIndirect { index, table_index } => {
-                if resources.table_at(table_index).is_none() {
+                self.check_call_indirect(index, table_index, resources)?
+            }
+            Operator::ReturnCallIndirect { index, table_index } => {
+                if !self.config.enable_tail_call {
                     return Err(OperatorValidatorError::new(
-                        "unknown table: table index out of bounds",
+                        "tail calls support is not enabled",
                     ));
                 }
-                match resources.type_at(index) {
-                    None => {
-                        return Err(OperatorValidatorError::new(
-                            "unknown type: type index out of bounds",
-                        ))
-                    }
-                    Some(ty) => {
-                        let types = {
-                            let mut types = Vec::with_capacity(ty.len_inputs() + 1);
-                            types.extend(wasm_func_type_inputs(ty).map(WasmType::to_parser_type));
-                            types.push(Type::I32);
-                            types
-                        };
-                        self.check_operands(types.into_iter())?;
-                        self.func_state.change_frame_with_types(
-                            ty.len_inputs() + 1,
-                            wasm_func_type_outputs(ty).map(WasmType::to_parser_type),
-                        )?;
-                    }
-                }
+                self.check_call_indirect(index, table_index, resources)?;
+                self.check_return()?;
             }
             Operator::Drop => {
                 self.check_frame_size(1)?;
diff --git a/crates/wasmparser/src/primitives.rs b/crates/wasmparser/src/primitives.rs
index d55dd350..c101eb69 100644
--- a/crates/wasmparser/src/primitives.rs
+++ b/crates/wasmparser/src/primitives.rs
@@ -274,6 +274,8 @@ pub enum Operator<'a> {
     Return,
     Call { function_index: u32 },
     CallIndirect { index: u32, table_index: u32 },
+    ReturnCall { function_index: u32 },
+    ReturnCallIndirect { index: u32, table_index: u32 },
     Drop,
     Select,
     TypedSelect { ty: Type },
diff --git a/crates/wasmprinter/src/lib.rs b/crates/wasmprinter/src/lib.rs
index d1ee32a3..d41ac201 100644
--- a/crates/wasmprinter/src/lib.rs
+++ b/crates/wasmprinter/src/lib.rs
@@ -517,6 +517,17 @@ impl Printer {
                 }
                 write!(self.result, " (type {})", index)?;
             }
+            ReturnCall { function_index } => {
+                self.result.push_str("return_call ");
+                self.print_func_idx(*function_index)?;
+            }
+            ReturnCallIndirect { table_index, index } => {
+                self.result.push_str("return_call_indirect");
+                if *table_index != 0 {
+                    write!(self.result, " {}", table_index)?;
+                }
+                write!(self.result, " (type {})", index)?;
+            }
 
             Drop => self.result.push_str("drop"),
             Select => self.result.push_str("select"),
diff --git a/src/bin/wasm-validate-rs.rs b/src/bin/wasm-validate-rs.rs
index 5c9409be..7a750348 100644
--- a/src/bin/wasm-validate-rs.rs
+++ b/src/bin/wasm-validate-rs.rs
@@ -18,6 +18,7 @@ fn main() -> Result<()> {
         "Enable wasm bulk memory operations feature",
     );
     opts.optflag("", "enable-multi-value", "Enable wasm multi-value feature");
+    opts.optflag("", "enable-tail-call", "Enable wasm tail-call feature");
     #[cfg(feature = "deterministic")]
     opts.optflag(
         "",
@@ -45,6 +46,7 @@ fn main() -> Result<()> {
             enable_simd: matches.opt_present("enable-simd"),
             enable_bulk_memory: matches.opt_present("enable-bulk-memory"),
             enable_multi_value: matches.opt_present("enable-multi-value"),
+            enable_tail_call: matches.opt_present("enable-tail-call"),
             #[cfg(feature = "deterministic")]
             deterministic_only: matches.opt_present("deterministic-only"),
         },


Test patch:
diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs
index 3380ede0..0c8e367d 100644
--- a/tests/roundtrip.rs
+++ b/tests/roundtrip.rs
@@ -211,11 +211,9 @@ impl TestState {
         // however, that wasmparser doesn't implement all features that wabt
         // does, so we skip some tests here too.
         //
-        // TODO: implement tails calls in wasmparser
         // TODO: implement gc types in wasmparser
         // TODO: implement exceptions in wasmparser
-        if !contents.contains("--enable-tail-call")
-            && !contents.contains("--enable-exceptions")
+        if !contents.contains("--enable-exceptions")
             && !contents.contains("--enable-gc")
             && !contents.contains("--no-check")
             && !test.ends_with("dump/event.txt")
@@ -651,11 +649,6 @@ impl TestState {
     }
 
     fn wasmparser_disabled(&self, test: &Path, line: usize) -> bool {
-        // tail calls aren't implemented yet in wasmparser
-        if test.iter().any(|t| t == "tail-call") {
-            return true;
-        }
-
         // Waiting for WebAssembly/reference-types#76 to get resolved
         if test.ends_with("reference-types/ref_func.wast") {
             return line == 108 || line == 112;
@@ -688,6 +681,7 @@ impl TestState {
                 enable_simd: true,
                 enable_bulk_memory: true,
                 enable_multi_value: true,
+                enable_tail_call: true,
             },
         };
         for part in test.iter().filter_map(|t| t.to_str()) {
@@ -697,6 +691,7 @@ impl TestState {
                     config.operator_config.enable_reference_types = false;
                     config.operator_config.enable_simd = false;
                     config.operator_config.enable_bulk_memory = false;
+                    config.operator_config.enable_tail_call = false;
                 }
                 "threads" => config.operator_config.enable_threads = true,
                 "simd" => config.operator_config.enable_simd = true,
@@ -707,6 +702,7 @@ impl TestState {
                 "bulk-memory-operations" => {
                     config.operator_config.enable_bulk_memory = true;
                 }
+                "tail-call" => config.operator_config.enable_tail_call = true,
                 _ => {}
             }
         }
@@ -841,7 +837,7 @@ fn error_matches(error: &str, message: &str) -> bool {
         return error.contains("Bad magic number");
     }
 
-    if message == "unknown function 1" {
+    if message.starts_with("unknown function ") {
         return error.contains("unknown function");
     }