Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Move to upstream wasmtime, refactor globals snapshot #6759

Merged
10 commits merged into from
Aug 11, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 27 additions & 30 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 2 additions & 6 deletions client/executor/wasmtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,8 @@ sp-wasm-interface = { version = "2.0.0-rc5", path = "../../../primitives/wasm-in
sp-runtime-interface = { version = "2.0.0-rc5", path = "../../../primitives/runtime-interface" }
sp-core = { version = "2.0.0-rc5", path = "../../../primitives/core" }
sp-allocator = { version = "2.0.0-rc5", path = "../../../primitives/allocator" }
wasmtime = { package = "substrate-wasmtime", version = "0.19.0" }
wasmtime-runtime = { version = "0.19.0" }
wasmtime-environ = { version = "0.19.0" }
cranelift-wasm = { version = "0.66.0" }
cranelift-codegen = { version = "0.66.0" }

wasmtime = "0.19"
pwasm-utils = "0.14.0"

[dev-dependencies]
assert_matches = "1.3.0"
4 changes: 2 additions & 2 deletions client/executor/wasmtime/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ fn into_wasmtime_val_type(val_ty: ValueType) -> wasmtime::ValType {
/// Converts a `Val` into a substrate runtime interface `Value`.
///
/// Panics if the given value doesn't have a corresponding variant in `Value`.
fn into_value(val: Val) -> Value {
pub fn into_value(val: Val) -> Value {
match val {
Val::I32(v) => Value::I32(v),
Val::I64(v) => Value::I64(v),
Expand All @@ -304,7 +304,7 @@ fn into_value(val: Val) -> Value {
}
}

fn into_wasmtime_val(value: Value) -> wasmtime::Val {
pub fn into_wasmtime_val(value: Value) -> wasmtime::Val {
match value {
Value::I32(v) => Val::I32(v),
Value::I64(v) => Val::I64(v),
Expand Down
20 changes: 8 additions & 12 deletions client/executor/wasmtime/src/instance_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,36 +29,36 @@ use sc_executor_common::{
};
use sp_wasm_interface::{Pointer, WordSize, Value};
use wasmtime::{Engine, Instance, Module, Memory, Table, Val, Func, Extern, Global, Store};
use parity_wasm::elements;

mod globals_snapshot;

pub use globals_snapshot::GlobalsSnapshot;

pub struct ModuleWrapper {
imported_globals_count: u32,
globals_count: u32,
module: Module,
data_segments_snapshot: DataSegmentsSnapshot,
}

impl ModuleWrapper {
pub fn new(engine: &Engine, code: &[u8]) -> Result<Self> {
let module = Module::new(engine, code)
let mut raw_module: elements::Module = elements::deserialize_buffer(code)
.map_err(|e| Error::from(format!("cannot decode module: {}", e)))?;
pwasm_utils::export_mutable_globals(&mut raw_module, "exported_internal_global");
let instrumented_code = elements::serialize(raw_module)
.map_err(|e| Error::from(format!("cannot encode module: {}", e)))?;

let module = Module::new(engine, &instrumented_code)
.map_err(|e| Error::from(format!("cannot create module: {}", e)))?;

let module_info = WasmModuleInfo::new(code)
.ok_or_else(|| Error::from("cannot deserialize module".to_string()))?;
let declared_globals_count = module_info.declared_globals_count();
let imported_globals_count = module_info.imported_globals_count();
let globals_count = imported_globals_count + declared_globals_count;

let data_segments_snapshot = DataSegmentsSnapshot::take(&module_info)
.map_err(|e| Error::from(format!("cannot take data segments snapshot: {}", e)))?;

Ok(Self {
module,
imported_globals_count,
globals_count,
data_segments_snapshot,
})
}
Expand All @@ -78,8 +78,6 @@ impl ModuleWrapper {
/// routines.
pub struct InstanceWrapper {
instance: Instance,
globals_count: u32,
imported_globals_count: u32,
// The memory instance of the `instance`.
//
// It is important to make sure that we don't make any copies of this to make it easier to proof
Expand Down Expand Up @@ -143,8 +141,6 @@ impl InstanceWrapper {
Ok(Self {
table: get_table(&instance),
instance,
globals_count: module_wrapper.globals_count,
imported_globals_count: module_wrapper.imported_globals_count,
memory,
_not_send_nor_sync: marker::PhantomData,
})
Expand Down
133 changes: 43 additions & 90 deletions client/executor/wasmtime/src/instance_wrapper/globals_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,115 +17,68 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use super::InstanceWrapper;
use sc_executor_common::{
error::{Error, Result},
};
use sc_executor_common::error::{Result, Error};
use sp_wasm_interface::Value;
use cranelift_codegen::ir;
use cranelift_wasm::GlobalIndex;
use wasmtime_runtime::{ExportGlobal, Export};
use crate::imports::{into_value, into_wasmtime_val};

/// Saved value of particular exported global.
struct SavedValue {
/// Index of the export.
index: usize,
NikVolf marked this conversation as resolved.
Show resolved Hide resolved
/// Global value.
value: Value,
}

/// A snapshot of a global variables values. This snapshot can be used later for restoring the
/// values to the preserved state.
///
/// Technically, a snapshot stores only values of mutable global variables. This is because
/// immutable global variables always have the same values.
pub struct GlobalsSnapshot {
handle: wasmtime_runtime::InstanceHandle,
preserved_mut_globals: Vec<(*mut wasmtime_runtime::VMGlobalDefinition, Value)>,
}
pub struct GlobalsSnapshot(Vec<SavedValue>);

impl GlobalsSnapshot {
/// Take a snapshot of global variables for a given instance.
pub fn take(instance_wrapper: &InstanceWrapper) -> Result<Self> {
// EVIL:
// Usage of an undocumented function.
let handle = instance_wrapper.instance.handle().clone().handle;

let mut preserved_mut_globals = vec![];

for global_idx in instance_wrapper.imported_globals_count..instance_wrapper.globals_count {
let (def, global) = match handle.lookup_by_declaration(
&wasmtime_environ::EntityIndex::Global(GlobalIndex::from_u32(global_idx)),
) {
Export::Global(ExportGlobal { definition, global, .. }) => (definition, global),
_ => unreachable!("only globals can be returned for a global request"),
};

// skip immutable globals.
if !global.mutability {
continue;
}

let value = unsafe {
// Safety of this function solely depends on the correctness of the reference and
// the type information of the global.
read_global(def, global.ty)?
};
preserved_mut_globals.push((def, value));
}

Ok(Self {
preserved_mut_globals,
handle,
})
let data = instance_wrapper.instance
.exports()
.enumerate()
.filter_map(|(index, export)| {
if export.name().starts_with("exported_internal_global") {
export.into_global().map(
|g| SavedValue { index, value: into_value(g.get()) }
)
} else { None }
})
.collect::<Vec<_>>();

Ok(Self(data))
}

/// Apply the snapshot to the given instance.
///
/// This instance must be the same that was used for creation of this snapshot.
pub fn apply(&self, instance_wrapper: &InstanceWrapper) -> Result<()> {
if instance_wrapper.instance.handle().handle != self.handle {
return Err(Error::from("unexpected instance handle".to_string()));
}

for (def, value) in &self.preserved_mut_globals {
unsafe {
// The following writes are safe if the precondition that this is the same instance
// this snapshot was created with:
//
// 1. These pointers must be still not-NULL and allocated.
// 2. The set of global variables is fixed for the lifetime of the same instance.
// 3. We obviously assume that the wasmtime references are correct in the first place.
// 4. We write the data with the same type it was read in the first place.
write_global(*def, *value)?;
// This is a pointer over saved items, it moves forward when the loop value below takes over it's current value.
// Since both pointers (`current` and `index` below) are over ordered lists, they eventually hit all
// equal referenced values.
let mut current = 0;
for (index, export) in instance_wrapper.instance.exports().enumerate() {
if current >= self.0.len() { break; }
let current_saved = &self.0[current];
if index < current_saved.index { continue; }
else if index > current_saved.index { current += 1; continue; }
else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I am not sure what we are doing here.

Can't we just iterate over self.0 and then set the item under the corresponding index from exports? It is an iterator, but nth should work and since it is ExactSizeIterator it should be efficient enough.

Alternatively, since a Global is essentially ref-counted smart-pointer, we could store it in SavedValue and avoid accessing exports at all.

Copy link
Contributor Author

@NikVolf NikVolf Aug 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we do here is essentiallly move two pointers over ordered lists together to see when referenced members have equal values. I am not sure that nth won't iterate over all iterator every time even for ExactSizeIterator (methods of this trait have nothing about this). Can you provide proof of that? :)

Global is not Sync/Send, so I'd avoid saving it anywhere if possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah ok don't think this is worth arguing about. The comments improve the situation as well.

export.into_global()
.ok_or_else(|| Error::Other(
"Wrong instance in GlobalsSnapshot::apply: what should be global is not global.".to_string()
))?
.set(into_wasmtime_val(current_saved.value))
.map_err(|_e| Error::Other(
"Wrong instance in GlobalsSnapshot::apply: global saved type does not matched applied.".to_string()
))?;
}
}
Ok(())
}
}

unsafe fn read_global(
def: *const wasmtime_runtime::VMGlobalDefinition,
ty: ir::Type,
) -> Result<Value> {
let def = def
.as_ref()
.ok_or_else(|| Error::from("wasmtime global reference is null during read".to_string()))?;
let val = match ty {
ir::types::I32 => Value::I32(*def.as_i32()),
ir::types::I64 => Value::I64(*def.as_i64()),
ir::types::F32 => Value::F32(*def.as_u32()),
ir::types::F64 => Value::F64(*def.as_u64()),
_ => {
return Err(Error::from(format!(
"unsupported global variable type: {}",
ty
)))
}
};
Ok(val)
}

unsafe fn write_global(def: *mut wasmtime_runtime::VMGlobalDefinition, value: Value) -> Result<()> {
let def = def
.as_mut()
.ok_or_else(|| Error::from("wasmtime global reference is null during write".to_string()))?;
match value {
Value::I32(v) => *def.as_i32_mut() = v,
Value::I64(v) => *def.as_i64_mut() = v,
Value::F32(v) => *def.as_u32_mut() = v,
Value::F64(v) => *def.as_u64_mut() = v,
Ok(())
}
Ok(())
}
1 change: 1 addition & 0 deletions client/network/src/service/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ fn build_nodes_one_proto()
(node1, events_stream1, node2, events_stream2)
}

#[ignore]
Copy link
Contributor

@tomaka tomaka Apr 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please at least notify us or open an issue in the future if you ignore a test? 🙏

#[test]
fn notifications_state_consistent() {
// Runs two nodes and ensures that events are propagated out of the API in a consistent
Expand Down