From 196bcec6cfd495a5c75ea169f75a878b5d78ffaf Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 20 Apr 2021 16:52:51 -0500 Subject: [PATCH] Process declared element segments for "possibly exported funcs" (#2851) Now that we're using "possibly exported" as an impactful decision for codegen (which trampolines to generate and which ABI a function has) it's important that we calculate this property of a wasm function correctly! Previously Wasmtime forgot to processed "declared" elements in apart from active/passive element segments, but this updates Wasmtime to ensure that these entries are processed and all the functions contained within are flagged as "possibly exported". Closes #2850 --- cranelift/wasm/src/environ/spec.rs | 7 ++++++ cranelift/wasm/src/sections_translator.rs | 2 +- crates/environ/src/module_environ.rs | 7 ++++++ tests/all/func.rs | 27 +++++++++++++++++++++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/cranelift/wasm/src/environ/spec.rs b/cranelift/wasm/src/environ/spec.rs index ab7974f576..31c8d86a4e 100644 --- a/cranelift/wasm/src/environ/spec.rs +++ b/cranelift/wasm/src/environ/spec.rs @@ -948,6 +948,13 @@ pub trait ModuleEnvironment<'data>: TargetEnvironment { elements: Box<[FuncIndex]>, ) -> WasmResult<()>; + /// Indicates that a declarative element segment was seen in the wasm + /// module. + fn declare_elements(&mut self, elements: Box<[FuncIndex]>) -> WasmResult<()> { + drop(elements); + Ok(()) + } + /// Provides the number of passive data segments up front. /// /// By default this does nothing, but implementations may use this to diff --git a/cranelift/wasm/src/sections_translator.rs b/cranelift/wasm/src/sections_translator.rs index 3ce00e61de..bd4dcd1136 100644 --- a/cranelift/wasm/src/sections_translator.rs +++ b/cranelift/wasm/src/sections_translator.rs @@ -400,7 +400,7 @@ pub fn parse_element_section<'data>( environ.declare_passive_element(index, segments)?; } ElementKind::Declared => { - // Nothing to do here. + environ.declare_elements(segments)?; } } } diff --git a/crates/environ/src/module_environ.rs b/crates/environ/src/module_environ.rs index 393a83975e..324c4b9296 100644 --- a/crates/environ/src/module_environ.rs +++ b/crates/environ/src/module_environ.rs @@ -746,6 +746,13 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data Ok(()) } + fn declare_elements(&mut self, segments: Box<[FuncIndex]>) -> WasmResult<()> { + for element in segments.iter() { + self.flag_func_possibly_exported(*element); + } + Ok(()) + } + fn reserve_function_bodies(&mut self, _count: u32, offset: u64) { self.result.debuginfo.wasm_file.code_section_offset = offset; } diff --git a/tests/all/func.rs b/tests/all/func.rs index e3bee038c2..67e4f9ddc2 100644 --- a/tests/all/func.rs +++ b/tests/all/func.rs @@ -774,3 +774,30 @@ fn wrap_multiple_results() -> anyhow::Result<()> { f64 "f64" F64 f64::from_bits(a), } } + +#[test] +fn trampoline_for_declared_elem() -> anyhow::Result<()> { + let engine = Engine::default(); + + let module = Module::new( + &engine, + r#" + (module + (elem declare func $f) + (func $f) + (func (export "g") (result funcref) + (ref.func $f) + ) + ) + "#, + )?; + + let store = Store::new(&engine); + let instance = Instance::new(&store, &module, &[])?; + + let g = instance.get_typed_func::<(), Option>("g")?; + + let func = g.call(())?; + func.unwrap().call(&[])?; + Ok(()) +}