Validate modules while translating (#2059)

* Validate modules while translating

This commit is a change to cranelift-wasm to validate each function body
as it is translated. Additionally top-level module translation functions
will perform module validation. This commit builds on changes in
wasmparser to perform module validation interwtwined with parsing and
translation. This will be necessary for future wasm features such as
module linking where the type behind a function index, for example, can
be far away in another module. Additionally this also brings a nice
benefit where parsing the binary only happens once (instead of having an
up-front serial validation step) and validation can happen in parallel
for each function.

Most of the changes in this commit are plumbing to make sure everything
lines up right. The major functional change here is that module
compilation should be faster by validating in parallel (or skipping
function validation entirely in the case of a cache hit). Otherwise from
a user-facing perspective nothing should be that different.

This commit does mean that cranelift's translation now inherently
validates the input wasm module. This means that the Spidermonkey
integration of cranelift-wasm will also be validating the function as
it's being translated with cranelift. The associated PR for wasmparser
(bytecodealliance/wasmparser#62) provides the necessary tools to create
a `FuncValidator` for Gecko, but this is something I'll want careful
review for before landing!

* Read function operators until EOF

This way we can let the validator take care of any issues with
mismatched `end` instructions and/or trailing operators/bytes.

crates/environ/Cargo.toml

@@ -16,7 +16,7 @@ anyhow = "1.0"
 cranelift-codegen = { path = "../../cranelift/codegen", version = "0.67.0", features = ["enable-serde"] }
 cranelift-entity = { path = "../../cranelift/entity", version = "0.67.0", features = ["enable-serde"] }
 cranelift-wasm = { path = "../../cranelift/wasm", version = "0.67.0", features = ["enable-serde"] }
-wasmparser = "0.59.0"
+wasmparser = "0.62.0"
 indexmap = { version = "1.0.2", features = ["serde-1"] }
 thiserror = "1.0.4"
 serde = { version = "1.0.94", features = ["derive"] }

crates/environ/src/compilation.rs

@@ -103,7 +103,7 @@ pub trait Compiler: Send + Sync {
         &self,
         translation: &ModuleTranslation<'_>,
         index: DefinedFuncIndex,
-        data: &FunctionBodyData<'_>,
+        data: FunctionBodyData<'_>,
         isa: &dyn isa::TargetIsa,
     ) -> Result<CompiledFunction, CompileError>;
 }

crates/environ/src/module_environ.rs

@@ -15,12 +15,14 @@ use std::convert::TryFrom;
 use std::path::PathBuf;
 use std::sync::Arc;
 use wasmparser::Type as WasmType;
+use wasmparser::{FuncValidator, FunctionBody, ValidatorResources, WasmFeatures};
 
 /// Object containing the standalone environment information.
 pub struct ModuleEnvironment<'data> {
     /// The result to be filled in.
     result: ModuleTranslation<'data>,
     code_index: u32,
+    features: WasmFeatures,
 }
 
 /// The result of translating via `ModuleEnvironment`. Function bodies are not
@@ -50,13 +52,11 @@ pub struct ModuleTranslation<'data> {
 }
 
 /// Contains function data: byte code and its offset in the module.
-#[derive(Hash)]
 pub struct FunctionBodyData<'a> {
-    /// Body byte code.
-    pub data: &'a [u8],
-
-    /// Body offset in the module file.
-    pub module_offset: usize,
+    /// The body of the function, containing code and locals.
+    pub body: FunctionBody<'a>,
+    /// Validator for the function body
+    pub validator: FuncValidator<ValidatorResources>,
 }
 
 #[derive(Debug, Default)]
@@ -102,7 +102,11 @@ pub struct FunctionMetadata {
 
 impl<'data> ModuleEnvironment<'data> {
     /// Allocates the environment data structures.
-    pub fn new(target_config: TargetFrontendConfig, tunables: &Tunables) -> Self {
+    pub fn new(
+        target_config: TargetFrontendConfig,
+        tunables: &Tunables,
+        features: &WasmFeatures,
+    ) -> Self {
         Self {
             result: ModuleTranslation {
                 target_config,
@@ -118,6 +122,7 @@ impl<'data> ModuleEnvironment<'data> {
                 },
             },
             code_index: 0,
+            features: *features,
         }
     }
 
@@ -442,21 +447,15 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
 
     fn define_function_body(
         &mut self,
-        _module_translation: &ModuleTranslationState,
-        body_bytes: &'data [u8],
-        body_offset: usize,
+        validator: FuncValidator<ValidatorResources>,
+        body: FunctionBody<'data>,
     ) -> WasmResult<()> {
-        self.result.function_body_inputs.push(FunctionBodyData {
-            data: body_bytes,
-            module_offset: body_offset,
-        });
         if let Some(info) = &mut self.result.debuginfo {
             let func_index = self.code_index + self.result.module.num_imported_funcs as u32;
             let func_index = FuncIndex::from_u32(func_index);
             let sig_index = self.result.module.functions[func_index];
             let sig = &self.result.module.signatures[sig_index];
             let mut locals = Vec::new();
-            let body = wasmparser::FunctionBody::new(body_offset, body_bytes);
             for pair in body.get_locals_reader()? {
                 locals.push(pair?);
             }
@@ -465,6 +464,9 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
                 params: sig.0.params.iter().cloned().map(|i| i.into()).collect(),
             });
         }
+        self.result
+            .function_body_inputs
+            .push(FunctionBodyData { validator, body });
         self.code_index += 1;
         Ok(())
     }
@@ -564,6 +566,10 @@ and for re-adding support for interface types you can see this issue:
             _ => Ok(()),
         }
     }
+
+    fn wasm_features(&self) -> WasmFeatures {
+        self.features
+    }
 }
 
 /// Add environment-specific function parameters.