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 #3197 #3221

Merged
merged 17 commits into from
Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from 10 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
7 changes: 7 additions & 0 deletions lib/api/src/js/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ impl VMMemory {
Self { memory, ty }
}

pub fn get_runtime_size(&self) -> u32 {
self.memory.get_runtime_size()
}

/// 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 +60,9 @@ impl VMTable {
pub(crate) fn new(table: Table, ty: TableType) -> Self {
Self { table, ty }
}
pub fn get_runtime_size(&self) -> u32 {
self.table.get_runtime_size()
}
}

#[derive(Clone)]
Expand Down
15 changes: 15 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,18 @@ impl<'a> Exportable<'a> for Table {
}
}
}

#[test]
fn test_table_grow_3197() {
fschutt marked this conversation as resolved.
Show resolved Hide resolved
use crate::{imports, Instance, Module, Store, Table, TableType, Type, Value};

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

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();
let imports = imports! {"env" => {"table" => table}};
let _instance = Instance::new(&mut store, &module, &imports).unwrap();
fschutt marked this conversation as resolved.
Show resolved Hide resolved
}
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
16 changes: 10 additions & 6 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,13 @@ 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, _: Option<u32>) -> bool {
let MemoryType {
minimum: exported_minimum,
maximum: exported_maximum,
Expand Down Expand Up @@ -211,12 +215,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 size of the allocated memory block
pub fn get_runtime_size(&self) -> u32 {
unsafe { self.0.vmmemory().as_ref() }.current_length as u32
}

/// 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 {
fschutt marked this conversation as resolved.
Show resolved Hide resolved
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