Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix serialize FrameInfo on Dylib engine (for #1727) #2768

Merged
merged 4 commits into from Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -21,6 +21,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

Expand Down
46 changes: 22 additions & 24 deletions lib/engine-dylib/src/artifact.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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)?;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 5 additions & 1 deletion lib/engine-dylib/src/serialize.rs
Expand Up @@ -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};
Expand All @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions tests/ignores.txt
Expand Up @@ -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
Expand Down