Skip to content

Commit

Permalink
Merge pull request #3221 from wasmerio/fix-3197
Browse files Browse the repository at this point in the history
Fix #3197
  • Loading branch information
syrusakbary authored Oct 18, 2022
2 parents e1e08f4 + 641246b commit 2fa4b90
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 9 deletions.
13 changes: 13 additions & 0 deletions Cargo.lock

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

3 changes: 3 additions & 0 deletions lib/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ wasmer-derive = { path = "../derive", version = "=3.0.0-beta.2" }
# - Optional dependencies for `js`.
wasmparser = { version = "0.83", default-features = false, optional = true }
hashbrown = { version = "0.11", optional = true }
serde-wasm-bindgen = { version = "0.4.5" }
serde = { version = "1.0", features = ["derive"] }

# - Development Dependencies for `js`.
[target.'cfg(target_arch = "wasm32")'.dev-dependencies]
wat = "1.0"
Expand Down
24 changes: 23 additions & 1 deletion lib/api/src/js/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ use crate::js::store::{AsStoreMut, AsStoreRef, InternalStoreHandle};
use crate::js::wasm_bindgen_polyfill::Global;
use js_sys::Function;
use js_sys::WebAssembly::{Memory, Table};
use serde::{Deserialize, Serialize};
use std::fmt;
use wasm_bindgen::{JsCast, JsValue};
use wasmer_types::{ExternType, FunctionType, GlobalType, MemoryType, TableType};
use wasmer_types::{ExternType, FunctionType, GlobalType, MemoryType, TableType, WASM_PAGE_SIZE};

/// Represents linear memory that is managed by the javascript runtime
#[derive(Clone, Debug, PartialEq)]
Expand All @@ -17,11 +18,29 @@ pub struct VMMemory {
unsafe impl Send for VMMemory {}
unsafe impl Sync for VMMemory {}

#[derive(Serialize, Deserialize)]
struct DummyBuffer {
#[serde(rename = "byteLength")]
byte_length: u32,
}

impl VMMemory {
pub(crate) fn new(memory: Memory, ty: MemoryType) -> Self {
Self { memory, ty }
}

/// Returns the size of the memory buffer in pages
pub fn get_runtime_size(&self) -> u32 {
let dummy: DummyBuffer = match serde_wasm_bindgen::from_value(self.memory.buffer()) {
Ok(o) => o,
Err(_) => return 0,
};
if dummy.byte_length == 0 {
return 0;
}
dummy.byte_length / WASM_PAGE_SIZE as u32
}

/// Attempts to clone this memory (if its clonable)
pub(crate) fn try_clone(&self) -> Option<VMMemory> {
Some(self.clone())
Expand Down Expand Up @@ -56,6 +75,9 @@ impl VMTable {
pub(crate) fn new(table: Table, ty: TableType) -> Self {
Self { table, ty }
}
pub fn get_runtime_size(&self) -> u32 {
self.table.length()
}
}

#[derive(Clone)]
Expand Down
19 changes: 19 additions & 0 deletions lib/api/src/sys/externals/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,22 @@ impl<'a> Exportable<'a> for Table {
}
}
}

/// Check the example from <https://github.com/wasmerio/wasmer/issues/3197>.
#[test]
fn test_table_grow_issue_3197() {
use crate::{imports, Instance, Module, Store, Table, TableType, Type, Value};

const WAT: &str = r#"(module (table (import "env" "table") 100 funcref))"#;

// Tests that the table type of `table` is compatible with the export in the WAT
// This tests that `wasmer_types::types::is_table_compatible` works as expected.
let mut store = Store::default();
let module = Module::new(&store, WAT).unwrap();
let ty = TableType::new(Type::FuncRef, 0, None);
let table = Table::new(&mut store, ty, Value::FuncRef(None)).unwrap();
table.grow(&mut store, 100, Value::FuncRef(None)).unwrap();
assert_eq!(table.ty(&store).minimum, 0);
let imports = imports! {"env" => {"table" => table}};
let _instance = Instance::new(&mut store, &module, &imports).unwrap();
}
11 changes: 10 additions & 1 deletion lib/compiler/src/engine/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ fn get_extern_type(context: &StoreObjects, extern_: &VMExtern) -> ExternType {
}
}

fn get_runtime_size(context: &StoreObjects, extern_: &VMExtern) -> Option<u32> {
match extern_ {
VMExtern::Table(t) => Some(t.get(context).get_runtime_size()),
VMExtern::Memory(m) => Some(m.get(context).get_runtime_size()),
_ => None,
}
}

/// This function allows to match all imports of a `ModuleInfo` with concrete definitions provided by
/// a `Resolver`.
///
Expand Down Expand Up @@ -85,7 +93,8 @@ pub fn resolve_imports(
));
};
let extern_type = get_extern_type(context, resolved);
if !extern_type.is_compatible_with(&import_extern) {
let runtime_size = get_runtime_size(context, resolved);
if !extern_type.is_compatible_with(&import_extern, runtime_size) {
return Err(LinkError::Import(
module_name.to_string(),
field.to_string(),
Expand Down
22 changes: 15 additions & 7 deletions lib/types/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,11 @@ fn is_table_element_type_compatible(exported_type: Type, imported_type: Type) ->
}
}

fn is_table_compatible(exported: &TableType, imported: &TableType) -> bool {
fn is_table_compatible(
exported: &TableType,
imported: &TableType,
imported_runtime_size: Option<u32>,
) -> bool {
let TableType {
ty: exported_ty,
minimum: exported_minimum,
Expand All @@ -154,13 +158,17 @@ fn is_table_compatible(exported: &TableType, imported: &TableType) -> bool {
} = imported;

is_table_element_type_compatible(*exported_ty, *imported_ty)
&& imported_minimum <= exported_minimum
&& *imported_minimum <= imported_runtime_size.unwrap_or(*exported_minimum)
&& (imported_maximum.is_none()
|| (!exported_maximum.is_none()
&& imported_maximum.unwrap() >= exported_maximum.unwrap()))
}

fn is_memory_compatible(exported: &MemoryType, imported: &MemoryType) -> bool {
fn is_memory_compatible(
exported: &MemoryType,
imported: &MemoryType,
imported_runtime_size: Option<u32>,
) -> bool {
let MemoryType {
minimum: exported_minimum,
maximum: exported_maximum,
Expand All @@ -172,7 +180,7 @@ fn is_memory_compatible(exported: &MemoryType, imported: &MemoryType) -> bool {
shared: imported_shared,
} = imported;

imported_minimum <= exported_minimum
imported_minimum.0 <= imported_runtime_size.unwrap_or(exported_minimum.0)
&& (imported_maximum.is_none()
|| (!exported_maximum.is_none()
&& imported_maximum.unwrap() >= exported_maximum.unwrap()))
Expand Down Expand Up @@ -211,12 +219,12 @@ impl ExternType {
(Memory(MemoryType) memory unwrap_memory)
}
/// Check if two externs are compatible
pub fn is_compatible_with(&self, other: &Self) -> bool {
pub fn is_compatible_with(&self, other: &Self, runtime_size: Option<u32>) -> bool {
match (self, other) {
(Self::Function(a), Self::Function(b)) => a == b,
(Self::Global(a), Self::Global(b)) => is_global_compatible(*a, *b),
(Self::Table(a), Self::Table(b)) => is_table_compatible(a, b),
(Self::Memory(a), Self::Memory(b)) => is_memory_compatible(a, b),
(Self::Table(a), Self::Table(b)) => is_table_compatible(a, b, runtime_size),
(Self::Memory(a), Self::Memory(b)) => is_memory_compatible(a, b, runtime_size),
// The rest of possibilities, are not compatible
_ => false,
}
Expand Down
5 changes: 5 additions & 0 deletions lib/vm/src/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,11 @@ impl VMMemory {
Ok(Self(Box::new(VMOwnedMemory::new(memory, style)?)))
}

/// Returns the number of pages in the allocated memory block
pub fn get_runtime_size(&self) -> u32 {
self.0.size().0
}

/// Create a new linear memory instance with specified minimum and maximum number of wasm pages.
///
/// This creates a `Memory` with metadata owned by a VM, pointed to by
Expand Down
5 changes: 5 additions & 0 deletions lib/vm/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ impl VMTable {
unsafe { Self::new_inner(table, style, None) }
}

/// Returns the size of the table
pub fn get_runtime_size(&self) -> u32 {
self.vec.len() as u32
}

/// Create a new linear table instance with specified minimum and maximum number of elements.
///
/// This creates a `Table` with metadata owned by a VM, pointed to by
Expand Down

0 comments on commit 2fa4b90

Please sign in to comment.