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

Move Artifact Register FrameInfo #4011

Merged
merged 20 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2d4ec98
Move Artifact Regster FrameInfo
ptitSeb Jun 20, 2023
735d47e
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 20, 2023
b1047cd
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 21, 2023
028dc59
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 22, 2023
bc2fabc
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 22, 2023
221b953
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 22, 2023
d5da2be
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 23, 2023
29fab5e
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 26, 2023
178c315
Removed Mutex for GlobalFrameInfoRegistration as it's no longer needed
ptitSeb Jun 26, 2023
660645e
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 26, 2023
cdfad46
Moved GlobalFrameInfo to codeMemory (fixes #3793)
ptitSeb Jun 26, 2023
58647db
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 27, 2023
d216a65
Update the API and added more comments
ptitSeb Jun 27, 2023
4bcbb41
Fixed spelling mystake
ptitSeb Jun 27, 2023
3bab6e5
Rename method to take_frame_info_registration
ptitSeb Jun 27, 2023
f05c3db
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 27, 2023
4d5a3a6
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 27, 2023
d5b65a6
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 27, 2023
29fd6f3
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 28, 2023
73ae525
Merge branch 'master' into move_register_frameinfo
ptitSeb Jun 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 91 additions & 44 deletions lib/compiler/src/engine/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use enumset::EnumSet;
use std::mem;
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
use std::sync::Arc;
use std::sync::Mutex;
#[cfg(feature = "static-artifact-create")]
use wasmer_object::{emit_compilation, emit_data, get_object_for_target, Object};
#[cfg(any(feature = "static-artifact-create", feature = "static-artifact-load"))]
Expand All @@ -38,12 +37,19 @@ use wasmer_vm::{FunctionBodyPtr, MemoryStyle, TableStyle, VMSharedSignatureIndex
use wasmer_vm::{InstanceAllocator, StoreObjects, TrapHandlerFn, VMConfig, VMExtern, VMInstance};

pub struct AllocatedArtifact {
// This shows if the frame info has been regestered already or not.
// Because the 'GlobalFrameInfoRegistration' ownership can be transfered to EngineInner
// this bool is needed to track the status, as 'frame_info_registration' will be None
// after the ownership is transfered.
frame_info_registered: bool,
ptitSeb marked this conversation as resolved.
Show resolved Hide resolved
// frame_info_registered is not staying there but transfered to CodeMemory from EngineInner
// using 'Artifact::transfert_frame_info_registration' method
// so the GloabelFrameInfo and MMap stays in sync and get dropped at the same time
frame_info_registration: Option<GlobalFrameInfoRegistration>,
finished_functions: BoxedSlice<LocalFunctionIndex, FunctionBodyPtr>,
finished_function_call_trampolines: BoxedSlice<SignatureIndex, VMTrampoline>,
finished_dynamic_function_trampolines: BoxedSlice<FunctionIndex, FunctionBodyPtr>,
signatures: BoxedSlice<SignatureIndex, VMSharedSignatureIndex>,
/// Some(_) only if this is not a deserialized static artifact
frame_info_registration: Option<Mutex<Option<GlobalFrameInfoRegistration>>>,
finished_function_lengths: BoxedSlice<LocalFunctionIndex, usize>,
}

Expand Down Expand Up @@ -301,18 +307,26 @@ impl Artifact {
finished_dynamic_function_trampolines.into_boxed_slice();
let signatures = signatures.into_boxed_slice();

Ok(Self {
let mut artifact = Self {
id: Default::default(),
artifact,
allocated: Some(AllocatedArtifact {
frame_info_registered: false,
frame_info_registration: None,
finished_functions,
finished_function_call_trampolines,
finished_dynamic_function_trampolines,
signatures,
frame_info_registration: Some(Mutex::new(None)),
finished_function_lengths,
}),
})
};

artifact.internal_register_frame_info();
if let Some(frame_info) = artifact.internal_transfert_frame_info_registration() {
engine_inner.register_frame_info(frame_info);
}

Ok(artifact)
}

/// Check if the provided bytes look like a serialized `ArtifactBuild`.
Expand Down Expand Up @@ -378,47 +392,81 @@ impl ArtifactCreate for Artifact {
impl Artifact {
/// Register thie `Artifact` stack frame information into the global scope.
///
/// This is required to ensure that any traps can be properly symbolicated.
pub fn register_frame_info(&self) {
if let Some(frame_info_registration) = self
/// This is not required anymore as it's done automaticaly when creating by 'Artifact::from_parts'
#[deprecated(
since = "4.0.0",
note = "done automaticaly by Artifact::from_parts, use 'transfert_frame_info_registration' if you use this method"
)]
pub fn register_frame_info(&mut self) {
self.internal_register_frame_info()
}

fn internal_register_frame_info(&mut self) {
if self
.allocated
.as_ref()
.expect("It must be allocated")
.frame_info_registration
.as_ref()
.frame_info_registered
{
let mut info = frame_info_registration.lock().unwrap();
return; // already done
}

if info.is_some() {
return;
}
let finished_function_extents = self
.allocated
.as_ref()
.expect("It must be allocated")
.finished_functions
.values()
.copied()
.zip(
self.allocated
.as_ref()
.expect("It must be allocated")
.finished_function_lengths
.values()
.copied(),
)
.map(|(ptr, length)| FunctionExtent { ptr, length })
.collect::<PrimaryMap<LocalFunctionIndex, _>>()
.into_boxed_slice();

let finished_function_extents = self
.allocated
.as_ref()
.expect("It must be allocated")
.finished_functions
.values()
.copied()
.zip(
self.allocated
.as_ref()
.expect("It must be allocated")
.finished_function_lengths
.values()
.copied(),
)
.map(|(ptr, length)| FunctionExtent { ptr, length })
.collect::<PrimaryMap<LocalFunctionIndex, _>>()
.into_boxed_slice();

let frame_infos = self.artifact.get_frame_info_ref();
*info = register_frame_info(
self.artifact.create_module_info(),
&finished_function_extents,
frame_infos.clone(),
);
}
let frame_info_registration = &mut self
.allocated
.as_mut()
.expect("It must be allocated")
.frame_info_registration;

let frame_infos = self.artifact.get_frame_info_ref();

*frame_info_registration = register_frame_info(
self.artifact.create_module_info(),
&finished_function_extents,
frame_infos.clone(),
);

self.allocated
.as_mut()
.expect("It must be allocated")
.frame_info_registered = true;
}

/// The GlobalFrameInfoRegistration needs to be transfered to EngineInner if
/// register_frame_info has been used.
#[deprecated(since = "4.0.0", note = "done automaticaly by Artifact::from_parts.")]
pub fn transfert_frame_info_registration(&mut self) -> Option<GlobalFrameInfoRegistration> {
ptitSeb marked this conversation as resolved.
Show resolved Hide resolved
self.internal_transfert_frame_info_registration()
}

fn internal_transfert_frame_info_registration(
&mut self,
) -> Option<GlobalFrameInfoRegistration> {
let frame_info_registration = &mut self
.allocated
.as_mut()
.expect("It must be allocated")
.frame_info_registration;

frame_info_registration.take()
}

/// Returns the functions allocated in memory or this `Artifact`
Expand Down Expand Up @@ -531,8 +579,6 @@ impl Artifact {
.map_err(InstantiationError::Link)?
.into_boxed_slice();

self.register_frame_info();

let handle = VMInstance::new(
allocator,
module,
Expand Down Expand Up @@ -891,14 +937,15 @@ impl Artifact {
id: Default::default(),
artifact,
allocated: Some(AllocatedArtifact {
frame_info_registered: false,
frame_info_registration: None,
finished_functions: finished_functions.into_boxed_slice(),
finished_function_call_trampolines: finished_function_call_trampolines
.into_boxed_slice(),
finished_dynamic_function_trampolines: finished_dynamic_function_trampolines
.into_boxed_slice(),
signatures: signatures.into_boxed_slice(),
finished_function_lengths,
frame_info_registration: None,
}),
})
}
Expand Down
9 changes: 9 additions & 0 deletions lib/compiler/src/engine/code_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

//! Memory management for executable code.
use super::unwind::UnwindRegistry;
use crate::GlobalFrameInfoRegistration;
use wasmer_types::{CompiledFunctionUnwindInfo, CustomSection, FunctionBody};
use wasmer_vm::{Mmap, VMFunctionBody};

Expand All @@ -19,6 +20,8 @@ const DATA_SECTION_ALIGNMENT: usize = 64;

/// Memory manager for executable code.
pub struct CodeMemory {
// frame info is placed first, to ensure it's dropped before the mmap
frame_info_registration: Option<GlobalFrameInfoRegistration>,
unwind_registry: UnwindRegistry,
mmap: Mmap,
start_of_nonexecutable_pages: usize,
Expand All @@ -31,6 +34,7 @@ impl CodeMemory {
unwind_registry: UnwindRegistry::new(),
mmap: Mmap::new(),
start_of_nonexecutable_pages: 0,
frame_info_registration: None,
}
}

Expand Down Expand Up @@ -207,6 +211,11 @@ impl CodeMemory {
let body_ptr = byte_ptr as *mut [VMFunctionBody];
unsafe { &mut *body_ptr }
}

/// Register the frame info, so it's free when the mememory gets freed
pub fn register_frame_info(&mut self, frame_info: GlobalFrameInfoRegistration) {
self.frame_info_registration = Some(frame_info);
}
}

fn round_up(size: usize, multiple: usize) -> usize {
Expand Down
11 changes: 11 additions & 0 deletions lib/compiler/src/engine/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use crate::Artifact;
use crate::BaseTunables;
#[cfg(not(target_arch = "wasm32"))]
use crate::CodeMemory;
#[cfg(not(target_arch = "wasm32"))]
use crate::GlobalFrameInfoRegistration;
#[cfg(feature = "compiler")]
use crate::{Compiler, CompilerConfig};
#[cfg(not(target_arch = "wasm32"))]
Expand Down Expand Up @@ -442,6 +444,15 @@ impl EngineInner {
pub fn signatures(&self) -> &SignatureRegistry {
&self.signatures
}

#[cfg(not(target_arch = "wasm32"))]
/// Register the frame info for the code memory
pub(crate) fn register_frame_info(&mut self, frame_info: GlobalFrameInfoRegistration) {
self.code_memory
.last_mut()
.unwrap()
.register_frame_info(frame_info);
}
}

#[cfg(feature = "compiler")]
Expand Down