Improve handling of strings for backtraces (#843)

* Improve handling of strings for backtraces

Largely avoid storing strings at all in the `wasmtime-*` internal
crates, and instead only store strings in a separate global cache
specific to the `wasmtime` crate itself. This global cache is inserted
and removed from dynamically as modules are created and deallocated, and
the global cache is consulted whenever a `Trap` is created to
symbolicate any wasm frames.

This also avoids the need to thread `module_name` through the jit crates
and back, and additionally removes the need for `ModuleSyncString`.

* Run rustfmt
This commit is contained in:
Alex Crichton
2020-01-24 11:53:55 -06:00
committed by GitHub
parent 21e0a99884
commit 3db1074c15
17 changed files with 139 additions and 156 deletions

View File

@@ -20,6 +20,7 @@ libc = "0.2"
cfg-if = "0.1.9"
backtrace = "0.3.42"
rustc-demangle = "0.1.16"
lazy_static = "1.4"
[target.'cfg(target_os = "windows")'.dependencies]
winapi = "0.3.7"

View File

@@ -108,8 +108,7 @@ impl Instance {
/// [issue]: https://github.com/bytecodealliance/wasmtime/issues/727
pub fn new(module: &Module, imports: &[Extern]) -> Result<Instance, Error> {
let store = module.store();
let mut instance_handle =
instantiate(module.compiled_module().expect("compiled_module"), imports)?;
let mut instance_handle = instantiate(module.compiled_module(), imports)?;
let exports = {
let mut exports = Vec::with_capacity(module.exports().len());
@@ -124,6 +123,7 @@ impl Instance {
}
exports.into_boxed_slice()
};
module.register_names();
Ok(Instance {
instance_handle,
module: module.clone(),

View File

@@ -4,7 +4,11 @@ use crate::types::{
TableType, ValType,
};
use anyhow::{Error, Result};
use lazy_static::lazy_static;
use std::cell::Cell;
use std::collections::HashMap;
use std::rc::Rc;
use std::sync::{Arc, RwLock};
use wasmparser::{
validate, CustomSectionKind, ExternalKind, ImportSectionEntryType, ModuleReader, Name,
OperatorValidatorConfig, SectionCode, ValidatingParserConfig,
@@ -81,8 +85,25 @@ struct ModuleInner {
store: Store,
imports: Box<[ImportType]>,
exports: Box<[ExportType]>,
name: Option<String>,
compiled: Option<CompiledModule>,
compiled: CompiledModule,
registered_names: Cell<bool>,
names: Arc<Names>,
}
pub struct Names {
pub module: Arc<wasmtime_environ::Module>,
pub module_name: Option<String>,
}
lazy_static! {
/// This is a global cache of names known for all compiled modules in this
/// process.
///
/// This global cache is used during `Trap` creation to symbolicate frames.
/// This is populated on module compilation, and it is cleared out whenever
/// all references to a module are dropped, aka the `Drop for ModuleInner`
/// below.
pub static ref NAMES: RwLock<HashMap<usize, Arc<Names>>> = RwLock::default();
}
impl Module {
@@ -123,10 +144,7 @@ impl Module {
/// [binary]: https://webassembly.github.io/spec/core/binary/index.html
pub fn new(store: &Store, binary: &[u8]) -> Result<Module> {
Module::validate(store, binary)?;
// Note that the call to `unsafe` here should be ok because we
// previously validated the binary, meaning we're guaranteed to pass a
// valid binary for `store`.
unsafe { Module::new_internal(store, binary, None) }
unsafe { Module::new_unchecked(store, binary) }
}
/// Creates a new WebAssembly `Module` from the given in-memory `binary`
@@ -134,11 +152,10 @@ impl Module {
///
/// See [`Module::new`] for other details.
pub fn new_with_name(store: &Store, binary: &[u8], name: &str) -> Result<Module> {
Module::validate(store, binary)?;
// Note that the call to `unsafe` here should be ok because we
// previously validated the binary, meaning we're guaranteed to pass a
// valid binary for `store`.
unsafe { Module::new_internal(store, binary, Some(name)) }
let mut module = Module::new(store, binary)?;
let inner = Rc::get_mut(&mut module.inner).unwrap();
Arc::get_mut(&mut inner.names).unwrap().module_name = Some(name.to_string());
Ok(module)
}
/// Creates a new WebAssembly `Module` from the given in-memory `binary`
@@ -168,20 +185,8 @@ impl Module {
/// be somewhat valid for decoding purposes, and the basics of decoding can
/// still fail.
pub unsafe fn new_unchecked(store: &Store, binary: &[u8]) -> Result<Module> {
Module::new_internal(store, binary, None)
}
/// Creates a new `Module` and compiles it without doing any validation.
unsafe fn new_internal(store: &Store, binary: &[u8], name: Option<&str>) -> Result<Module> {
let mut ret = Module::empty(store);
let mut ret = Module::compile(store, binary)?;
ret.read_imports_and_exports(binary)?;
let inner = Rc::get_mut(&mut ret.inner).unwrap();
if let Some(name) = name {
// Assign or override the module's name if supplied.
inner.name = Some(name.to_string());
}
inner.compiled = Some(compile(store, binary, inner.name.as_deref())?);
Ok(ret)
}
@@ -220,31 +225,42 @@ impl Module {
#[doc(hidden)]
pub fn from_exports(store: &Store, exports: Box<[ExportType]>) -> Self {
let mut ret = Module::empty(store);
let mut ret = unsafe { Module::compile(store, b"\0asm\x01\0\0\0").unwrap() };
Rc::get_mut(&mut ret.inner).unwrap().exports = exports;
return ret;
}
fn empty(store: &Store) -> Self {
Module {
unsafe fn compile(store: &Store, binary: &[u8]) -> Result<Self> {
let compiled = CompiledModule::new(
&mut store.compiler_mut(),
binary,
store.engine().config().debug_info,
)?;
let names = Arc::new(Names {
module_name: None,
module: compiled.module().clone(),
});
Ok(Module {
inner: Rc::new(ModuleInner {
store: store.clone(),
imports: Box::new([]),
exports: Box::new([]),
name: None,
compiled: None,
names,
compiled,
registered_names: Cell::new(false),
}),
}
})
}
pub(crate) fn compiled_module(&self) -> Option<&CompiledModule> {
self.inner.compiled.as_ref()
pub(crate) fn compiled_module(&self) -> &CompiledModule {
&self.inner.compiled
}
/// Returns identifier/name that this [`Module`] has. This name
/// is used in traps/backtrace details.
pub fn name(&self) -> Option<&str> {
self.inner.name.as_deref()
self.inner.names.module_name.as_deref()
}
/// Returns the list of imports that this [`Module`] has and must be
@@ -375,7 +391,8 @@ impl Module {
while let Ok(entry) = reader.read() {
if let Name::Module(name) = entry {
if let Ok(name) = name.get_name() {
inner.name = Some(name.to_string());
Arc::get_mut(&mut inner.names).unwrap().module_name =
Some(name.to_string());
}
break;
}
@@ -392,14 +409,24 @@ impl Module {
inner.exports = exports.into();
Ok(())
}
/// Register this module's names in the global map of module names.
///
/// This is required to ensure that any traps can be properly symbolicated.
pub(crate) fn register_names(&self) {
if self.inner.registered_names.get() {
return;
}
let names = self.inner.names.clone();
NAMES.write().unwrap().insert(names.module.id, names);
self.inner.registered_names.set(true);
}
}
fn compile(store: &Store, binary: &[u8], module_name: Option<&str>) -> Result<CompiledModule> {
let compiled_module = CompiledModule::new(
&mut store.compiler_mut(),
binary,
module_name,
store.engine().config().debug_info,
)?;
Ok(compiled_module)
impl Drop for ModuleInner {
fn drop(&mut self) {
if self.registered_names.get() {
NAMES.write().unwrap().remove(&self.names.module.id);
}
}
}

View File

@@ -4,7 +4,7 @@ use crate::runtime::Store;
use anyhow::Result;
use std::any::Any;
use std::collections::HashSet;
use std::rc::Rc;
use std::sync::Arc;
use wasmtime_environ::entity::PrimaryMap;
use wasmtime_environ::wasm::DefinedFuncIndex;
use wasmtime_environ::Module;
@@ -37,7 +37,7 @@ pub(crate) fn create_handle(
.unwrap_or_else(PrimaryMap::new);
Ok(InstanceHandle::new(
Rc::new(module),
Arc::new(module),
finished_functions.into_boxed_slice(),
imports,
&data_initializers,

View File

@@ -1,6 +1,8 @@
use crate::module::NAMES;
use backtrace::Backtrace;
use std::fmt;
use std::sync::Arc;
use wasmtime_environ::entity::EntityRef;
/// A struct representing an aborted instruction execution, with a message
/// indicating the cause.
@@ -36,15 +38,22 @@ impl Trap {
fn new_with_trace(message: String, native_trace: Backtrace) -> Self {
let mut wasm_trace = Vec::new();
let names = NAMES.read().unwrap();
for frame in native_trace.frames() {
let pc = frame.ip() as usize;
if let Some(info) = wasmtime_runtime::jit_function_registry::find(pc) {
wasm_trace.push(FrameInfo {
func_index: info.func_index as u32,
module_name: info.module_id.get().map(|s| s.to_string()),
func_name: info.func_name.get().map(|s| s.to_string()),
})
}
let info = match wasmtime_runtime::jit_function_registry::find(pc) {
Some(info) => info,
None => continue,
};
let names = match names.get(&info.module_id) {
Some(names) => names,
None => continue,
};
wasm_trace.push(FrameInfo {
func_index: info.func_index.index() as u32,
module_name: names.module_name.clone(),
func_name: names.module.func_names.get(&info.func_index).cloned(),
})
}
Trap {
inner: Arc::new(TrapInner {

View File

@@ -54,7 +54,7 @@ pub use crate::func_environ::BuiltinFunctionIndex;
#[cfg(feature = "lightbeam")]
pub use crate::lightbeam::Lightbeam;
pub use crate::module::{
Export, MemoryPlan, MemoryStyle, Module, ModuleSyncString, TableElements, TablePlan, TableStyle,
Export, MemoryPlan, MemoryStyle, Module, TableElements, TablePlan, TableStyle,
};
pub use crate::module_environ::{
translate_signature, DataInitializer, DataInitializerLocation, FunctionBodyData,

View File

@@ -3,15 +3,16 @@
use crate::module_environ::FunctionBodyData;
use crate::tunables::Tunables;
use cranelift_codegen::ir;
use cranelift_entity::{EntityRef, PrimaryMap, SecondaryMap};
use cranelift_entity::{EntityRef, PrimaryMap};
use cranelift_wasm::{
DefinedFuncIndex, DefinedGlobalIndex, DefinedMemoryIndex, DefinedTableIndex, FuncIndex, Global,
GlobalIndex, Memory, MemoryIndex, SignatureIndex, Table, TableIndex,
};
use indexmap::IndexMap;
use more_asserts::assert_ge;
use std::collections::HashMap;
use std::hash::{Hash, Hasher};
use std::sync::Arc;
use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
/// A WebAssembly table initializer.
#[derive(Clone, Debug, Hash)]
@@ -129,33 +130,14 @@ impl TablePlan {
}
}
/// Allows module strings to be cached as reused across
/// multiple threads. Useful for debug/trace information.
#[derive(Debug, Clone)]
pub struct ModuleSyncString(Option<Arc<String>>);
impl ModuleSyncString {
/// Gets optional string reference.
pub fn get(&self) -> Option<&str> {
self.0.as_deref().map(|s| s.as_str())
}
/// Constructs the string.
pub fn new(s: Option<&str>) -> Self {
ModuleSyncString(s.map(|s| Arc::new(s.to_string())))
}
}
impl Default for ModuleSyncString {
fn default() -> Self {
ModuleSyncString(None)
}
}
/// A translated WebAssembly module, excluding the function bodies and
/// memory initializers.
// WARNING: when modifying, make sure that `hash_for_cache` is still valid!
#[derive(Debug)]
pub struct Module {
/// A unique identifier (within this process) for this module.
pub id: usize,
/// Unprocessed signatures exactly as provided by `declare_signature()`.
pub signatures: PrimaryMap<SignatureIndex, ir::Signature>,
@@ -193,17 +175,17 @@ pub struct Module {
/// WebAssembly table initializers.
pub table_elements: Vec<TableElements>,
/// Module name.
pub name: ModuleSyncString,
/// Function names.
pub func_names: SecondaryMap<FuncIndex, ModuleSyncString>,
/// WebAssembly table initializers.
pub func_names: HashMap<FuncIndex, String>,
}
impl Module {
/// Allocates the module data structures.
pub fn new() -> Self {
static NEXT_ID: AtomicUsize = AtomicUsize::new(0);
Self {
id: NEXT_ID.fetch_add(1, SeqCst),
signatures: PrimaryMap::new(),
imported_funcs: PrimaryMap::new(),
imported_tables: PrimaryMap::new(),
@@ -216,8 +198,7 @@ impl Module {
exports: IndexMap::new(),
start_func: None,
table_elements: Vec::new(),
name: Default::default(),
func_names: SecondaryMap::new(),
func_names: HashMap::new(),
}
}

View File

@@ -1,5 +1,5 @@
use crate::func_environ::FuncEnvironment;
use crate::module::{Export, MemoryPlan, Module, ModuleSyncString, TableElements, TablePlan};
use crate::module::{Export, MemoryPlan, Module, TableElements, TablePlan};
use crate::tunables::Tunables;
use cranelift_codegen::ir;
use cranelift_codegen::ir::{AbiParam, ArgumentPurpose};
@@ -371,7 +371,10 @@ impl<'data> cranelift_wasm::ModuleEnvironment<'data> for ModuleEnvironment<'data
}
fn declare_func_name(&mut self, func_index: FuncIndex, name: &'data str) -> WasmResult<()> {
self.result.module.func_names[func_index] = ModuleSyncString::new(Some(name));
self.result
.module
.func_names
.insert(func_index, name.to_string());
Ok(())
}
}

View File

@@ -167,15 +167,9 @@ impl Compiler {
self.jit_function_ranges
.push((ptr as usize, ptr as usize + body_len));
let func_index = module.func_index(i);
let func_name = module
.func_names
.get(func_index)
.cloned()
.unwrap_or_else(Default::default);
let tag = jit_function_registry::JITFunctionTag {
module_id: module.name.clone(),
func_index: func_index.index(),
func_name,
module_id: module.id,
func_index,
};
jit_function_registry::register(ptr as usize, ptr as usize + body_len, tag);
}

View File

@@ -109,13 +109,7 @@ impl Context {
self.validate(&data).map_err(SetupError::Validate)?;
let debug_info = self.debug_info();
instantiate(
&mut *self.compiler,
&data,
None,
&mut self.namespace,
debug_info,
)
instantiate(&mut *self.compiler, &data, &mut self.namespace, debug_info)
}
/// Return the instance associated with the given name.
@@ -146,7 +140,7 @@ impl Context {
self.validate(&data).map_err(SetupError::Validate)?;
let debug_info = self.debug_info();
CompiledModule::new(&mut *self.compiler, data, None, debug_info)
CompiledModule::new(&mut *self.compiler, data, debug_info)
}
/// If `name` isn't None, register it for the given instance.

View File

@@ -9,13 +9,13 @@ use crate::link::link_module;
use crate::resolver::Resolver;
use std::io::Write;
use std::rc::Rc;
use std::sync::Arc;
use thiserror::Error;
use wasmtime_debug::read_debuginfo;
use wasmtime_environ::entity::{BoxedSlice, PrimaryMap};
use wasmtime_environ::wasm::{DefinedFuncIndex, SignatureIndex};
use wasmtime_environ::{
CompileError, DataInitializer, DataInitializerLocation, Module, ModuleEnvironment,
ModuleSyncString,
};
use wasmtime_runtime::{
GdbJitImageRegistration, InstanceHandle, InstantiationError, VMFunctionBody,
@@ -59,12 +59,11 @@ impl<'data> RawCompiledModule<'data> {
fn new(
compiler: &mut Compiler,
data: &'data [u8],
module_name: Option<&str>,
debug_info: bool,
) -> Result<Self, SetupError> {
let environ = ModuleEnvironment::new(compiler.frontend_config(), compiler.tunables());
let mut translation = environ
let translation = environ
.translate(data)
.map_err(|error| SetupError::Compile(CompileError::Wasm(error)))?;
@@ -74,8 +73,6 @@ impl<'data> RawCompiledModule<'data> {
None
};
translation.module.name = ModuleSyncString::new(module_name);
let (allocated_functions, jt_offsets, relocations, dbg_image) = compiler.compile(
&translation.module,
translation.module_translation.as_ref().unwrap(),
@@ -136,7 +133,7 @@ impl<'data> RawCompiledModule<'data> {
/// A compiled wasm module, ready to be instantiated.
pub struct CompiledModule {
module: Rc<Module>,
module: Arc<Module>,
finished_functions: BoxedSlice<DefinedFuncIndex, *const VMFunctionBody>,
data_initializers: Box<[OwnedDataInitializer]>,
signatures: BoxedSlice<SignatureIndex, VMSharedSignatureIndex>,
@@ -148,10 +145,9 @@ impl CompiledModule {
pub fn new<'data>(
compiler: &mut Compiler,
data: &'data [u8],
module_name: Option<&str>,
debug_info: bool,
) -> Result<Self, SetupError> {
let raw = RawCompiledModule::<'data>::new(compiler, data, module_name, debug_info)?;
let raw = RawCompiledModule::<'data>::new(compiler, data, debug_info)?;
Ok(Self::from_parts(
raw.module,
@@ -175,7 +171,7 @@ impl CompiledModule {
dbg_jit_registration: Option<GdbJitImageRegistration>,
) -> Self {
Self {
module: Rc::new(module),
module: Arc::new(module),
finished_functions,
data_initializers,
signatures,
@@ -202,7 +198,7 @@ impl CompiledModule {
.collect::<Vec<_>>();
let imports = resolve_imports(&self.module, resolver)?;
InstanceHandle::new(
Rc::clone(&self.module),
Arc::clone(&self.module),
self.finished_functions.clone(),
imports,
&data_initializers,
@@ -213,8 +209,8 @@ impl CompiledModule {
}
/// Return a reference-counting pointer to a module.
pub fn module(&self) -> Rc<Module> {
self.module.clone()
pub fn module(&self) -> &Arc<Module> {
&self.module
}
/// Return a reference to a module.
@@ -250,21 +246,9 @@ impl OwnedDataInitializer {
pub fn instantiate(
compiler: &mut Compiler,
data: &[u8],
module_name: Option<&str>,
resolver: &mut dyn Resolver,
debug_info: bool,
) -> Result<InstanceHandle, SetupError> {
let raw = RawCompiledModule::new(compiler, data, module_name, debug_info)?;
let imports = resolve_imports(&raw.module, resolver)
.map_err(|err| SetupError::Instantiate(InstantiationError::Link(err)))?;
InstanceHandle::new(
Rc::new(raw.module),
raw.finished_functions,
imports,
&*raw.data_initializers,
raw.signatures,
raw.dbg_jit_registration.map(Rc::new),
Box::new(()),
)
.map_err(SetupError::Instantiate)
let instance = CompiledModule::new(compiler, data, debug_info)?.instantiate(resolver)?;
Ok(instance)
}

View File

@@ -23,6 +23,7 @@ use std::collections::HashSet;
use std::convert::TryFrom;
use std::ptr::NonNull;
use std::rc::Rc;
use std::sync::Arc;
use std::{mem, ptr, slice};
use thiserror::Error;
use wasmtime_environ::entity::{BoxedSlice, EntityRef, PrimaryMap};
@@ -288,7 +289,7 @@ pub(crate) struct Instance {
mmap: Mmap,
/// The `Module` this `Instance` was instantiated from.
module: Rc<Module>,
module: Arc<Module>,
/// Offsets in the `vmctx` region.
offsets: VMOffsets,
@@ -748,7 +749,7 @@ impl InstanceHandle {
/// Create a new `InstanceHandle` pointing at a new `Instance`.
pub fn new(
module: Rc<Module>,
module: Arc<Module>,
finished_functions: BoxedSlice<DefinedFuncIndex, *const VMFunctionBody>,
imports: Imports,
data_initializers: &[DataInitializer<'_>],
@@ -895,8 +896,8 @@ impl InstanceHandle {
}
/// Return a reference-counting pointer to a module.
pub fn module(&self) -> Rc<Module> {
self.instance().module.clone()
pub fn module(&self) -> &Arc<Module> {
&self.instance().module
}
/// Return a reference to a module.
@@ -1110,7 +1111,7 @@ fn lookup_by_declaration(
}
fn check_table_init_bounds(instance: &mut Instance) -> Result<(), InstantiationError> {
let module = Rc::clone(&instance.module);
let module = Arc::clone(&instance.module);
for init in &module.table_elements {
let start = get_table_init_start(init, instance);
let slice = get_table_slice(
@@ -1234,7 +1235,7 @@ fn get_table_slice<'instance>(
/// Initialize the table memory from the provided initializers.
fn initialize_tables(instance: &mut Instance) -> Result<(), InstantiationError> {
let vmctx: *mut VMContext = instance.vmctx_mut();
let module = Rc::clone(&instance.module);
let module = Arc::clone(&instance.module);
for init in &module.table_elements {
let start = get_table_init_start(init, instance);
let slice = get_table_slice(
@@ -1311,7 +1312,7 @@ fn create_globals(module: &Module) -> BoxedSlice<DefinedGlobalIndex, VMGlobalDef
}
fn initialize_globals(instance: &mut Instance) {
let module = Rc::clone(&instance.module);
let module = Arc::clone(&instance.module);
let num_imports = module.imported_globals.len();
for (index, global) in module.globals.iter().skip(num_imports) {
let def_index = module.defined_global_index(index).unwrap();

View File

@@ -3,7 +3,7 @@
use lazy_static::lazy_static;
use std::collections::BTreeMap;
use std::sync::{Arc, RwLock};
use wasmtime_environ::ModuleSyncString;
use wasmtime_environ::wasm::FuncIndex;
lazy_static! {
static ref REGISTRY: RwLock<JITFunctionRegistry> = RwLock::new(JITFunctionRegistry::default());
@@ -11,20 +11,8 @@ lazy_static! {
#[derive(Clone)]
pub struct JITFunctionTag {
pub module_id: ModuleSyncString,
pub func_index: usize,
pub func_name: ModuleSyncString,
}
impl std::fmt::Debug for JITFunctionTag {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(ref module_id) = self.module_id.get() {
write!(f, "{}", module_id)?;
} else {
write!(f, "(module)")?;
}
write!(f, ":{}", self.func_index)
}
pub module_id: usize,
pub func_index: FuncIndex,
}
struct JITFunctionRegistry {

View File

@@ -3,7 +3,7 @@ use cranelift_codegen::{ir, isa};
use cranelift_entity::PrimaryMap;
use cranelift_wasm::DefinedFuncIndex;
use std::fs::File;
use std::rc::Rc;
use std::sync::Arc;
use target_lexicon::HOST;
use wasi_common::hostcalls;
use wasi_common::wasi;
@@ -79,7 +79,7 @@ pub fn instantiate_wasi_with_context(
let signatures = PrimaryMap::new();
InstanceHandle::new(
Rc::new(module),
Arc::new(module),
finished_functions.into_boxed_slice(),
imports,
&data_initializers,

View File

@@ -3,7 +3,7 @@ use cranelift_codegen::{ir, isa};
use cranelift_entity::PrimaryMap;
use cranelift_wasm::DefinedFuncIndex;
use std::fs::File;
use std::rc::Rc;
use std::sync::Arc;
use target_lexicon::HOST;
use wasi_common::old::snapshot_0::hostcalls;
use wasi_common::old::snapshot_0::{WasiCtx, WasiCtxBuilder};
@@ -79,7 +79,7 @@ pub fn instantiate_wasi_with_context(
let signatures = PrimaryMap::new();
InstanceHandle::new(
Rc::new(module),
Arc::new(module),
finished_functions.into_boxed_slice(),
imports,
&data_initializers,