Merge #2768

2768: Fix serialize FrameInfo on Dylib engine (for #1727) r=Amanieu a=ptitSeb # Description Add serialization of Frame Info (except when using llvm) for the Dylib engine, so Trap Information is still valid and the correct trap are triggered by the program. Co-authored-by: ptitSeb <[email protected]>
wasmerio · Jan 27, 2022 · 9d5e4ff · 9d5e4ff
2 parents cfcc891 + 8e80448
commit 9d5e4ff
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,7 @@ Looking for changes that affect our C API? See the [C API Changelog](lib/c-api/C
 - [#2717](https://github.com/wasmerio/wasmer/pull/2717) Allow `Exports` to be modified after being cloned.
 - [#2719](https://github.com/wasmerio/wasmer/pull/2719) Fixed `wasm_importtype_new`'s Rust signature to not assume boxed vectors.
 - [#2723](https://github.com/wasmerio/wasmer/pull/2723) Fixed a bug in parameter passing in the Singlepass compiler.
+- [#2768](https://github.com/wasmerio/wasmer/pull/2768) Fixed issue with Frame Info on dylib engine.
 
 ## 2.1.0 - 2021/11/30
 

diff --git a/lib/engine-dylib/src/artifact.rs b/lib/engine-dylib/src/artifact.rs
@@ -209,8 +209,11 @@ impl DylibArtifact {
             .map(|_function_body| 0u64)
             .collect::<PrimaryMap<LocalFunctionIndex, u64>>();
 
+        let function_frame_info = None;
+
         let mut metadata = ModuleMetadata {
             compile_info,
+            function_frame_info,
             prefix: engine_inner.get_prefix(&data),
             data_initializers,
             function_body_lengths,
@@ -287,6 +290,9 @@ impl DylibArtifact {
                     MetadataHeader::ALIGN as u64,
                 )
                 .map_err(to_compile_error)?;
+
+                let frame_info = compilation.get_frame_info().clone();
+
                 emit_compilation(&mut obj, compilation, &symbol_registry, &target_triple)
                     .map_err(to_compile_error)?;
                 if obj.format() == BinaryFormat::Coff {
@@ -298,6 +304,8 @@ impl DylibArtifact {
                     .tempfile()
                     .map_err(to_compile_error)?;
 
+                metadata.function_frame_info = Some(frame_info);
+
                 // Re-open it.
                 let (mut file, filepath) = file.keep().map_err(to_compile_error)?;
                 let obj_bytes = obj.write().map_err(to_compile_error)?;
@@ -552,20 +560,6 @@ impl DylibArtifact {
             }
         }
 
-        // Leaving frame infos from now, as they are not yet used
-        // however they might be useful for the future.
-        // let frame_infos = compilation
-        //     .get_frame_info()
-        //     .values()
-        //     .map(|frame_info| SerializableFunctionFrameInfo::Processed(frame_info.clone()))
-        //     .collect::<PrimaryMap<LocalFunctionIndex, _>>();
-        // Self::from_parts(&mut engine_inner, lib, metadata, )
-        // let frame_info_registration = register_frame_info(
-        //     serializable.module.clone(),
-        //     &finished_functions,
-        //     serializable.compilation.function_frame_info.clone(),
-        // );
-
         // Compute indices into the shared signature table.
         let signatures = {
             metadata
@@ -806,16 +800,20 @@ impl Artifact for DylibArtifact {
         // We sort them again, by key this time
         function_pointers.sort_by(|(k1, _v1), (k2, _v2)| k1.cmp(k2));
 
-        let frame_infos = function_pointers
-            .iter()
-            .map(|(_, extent)| CompiledFunctionFrameInfo {
-                traps: vec![],
-                address_map: FunctionAddressMap {
-                    body_len: extent.length,
-                    ..Default::default()
-                },
-            })
-            .collect::<PrimaryMap<LocalFunctionIndex, _>>();
+        let frame_infos = if self.metadata().function_frame_info.is_some() {
+            self.metadata().function_frame_info.clone().unwrap()
+        } else {
+            function_pointers
+                .iter()
+                .map(|(_, extent)| CompiledFunctionFrameInfo {
+                    traps: vec![],
+                    address_map: FunctionAddressMap {
+                        body_len: extent.length,
+                        ..Default::default()
+                    },
+                })
+                .collect::<PrimaryMap<LocalFunctionIndex, _>>()
+        };
 
         let finished_function_extents = function_pointers
             .into_iter()

diff --git a/lib/engine-dylib/src/serialize.rs b/lib/engine-dylib/src/serialize.rs
@@ -6,7 +6,10 @@ use rkyv::{
 };
 use serde::{Deserialize, Serialize};
 use std::error::Error;
-use wasmer_compiler::{CompileError, CompileModuleInfo, SectionIndex, Symbol, SymbolRegistry};
+use wasmer_compiler::{
+    CompileError, CompileModuleInfo, CompiledFunctionFrameInfo, SectionIndex, Symbol,
+    SymbolRegistry,
+};
 use wasmer_engine::DeserializeError;
 use wasmer_types::entity::{EntityRef, PrimaryMap};
 use wasmer_types::{FunctionIndex, LocalFunctionIndex, OwnedDataInitializer, SignatureIndex};
@@ -29,6 +32,7 @@ fn to_compile_error(err: impl Error) -> CompileError {
 )]
 pub struct ModuleMetadata {
     pub compile_info: CompileModuleInfo,
+    pub function_frame_info: Option<PrimaryMap<LocalFunctionIndex, CompiledFunctionFrameInfo>>,
     pub prefix: String,
     pub data_initializers: Box<[OwnedDataInitializer]>,
     // The function body lengths (used to find function by address)

diff --git a/tests/ignores.txt b/tests/ignores.txt
@@ -47,11 +47,6 @@ cranelift+macos   spec::skip_stack_guard_page # Needs investigation. process did
 llvm+macos        spec::skip_stack_guard_page # Needs investigation. process didn't exit successfully: (signal: 6, SIGABRT: process abort signal)
 dylib             spec::skip_stack_guard_page # Missing trap information in dylibs
 
-
-# TODO(https://github.com/wasmerio/wasmer/issues/1727): Traps in dylib engine
-cranelift+dylib spec::linking # Needs investigation
-cranelift+dylib spec::bulk # Needs investigation
-
 # Some SIMD opperations are not yet supported by Cranelift
 # Cranelift just added support for most of those recently, it might be easy to update
 cranelift spec::simd::simd_conversions