From 1aee7e8e275c8399f5981eedd8e33a1f61a44b40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20S=C3=B8holm?= Date: Fri, 17 Feb 2023 16:10:06 +0100 Subject: [PATCH] Fix large pointers from WASM (#3310) Pointers being passed from WASM to JS are interpreted as signed, so large pointers ended up negative. This prevented WASM programs from allocating more than 2GB. To fix it we make all pointers unsigned (via >>> 0) for all malloc/realloc calls and inside shim functions. Ideally we would have an abstraction that guarantees we catch all cases, but that would require a major refactor. To test it we add gg-alloc as an optional system allocator for wasm-bindgen-test. It only allocates in the upper 2GB range and was made to test this exact issue but was never upstreamed. Fixes #2388 Fixes #2957 --- Cargo.toml | 8 +++++++- crates/cli-support/src/js/mod.rs | 19 ++++++++++++------- .../tests/reference/anyref-import-catch.js | 1 + crates/cli/tests/reference/result-string.js | 1 + crates/cli/tests/reference/string-arg.js | 7 ++++--- crates/test/Cargo.toml | 1 + crates/test/src/lib.rs | 6 ++++++ 7 files changed, 32 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ebbeda2cf8f..d7179ac09a7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,9 +30,15 @@ enable-interning = ["std"] # all unused attributes strict-macro = ["wasm-bindgen-macro/strict-macro"] +# Enables gg-alloc as system allocator when using wasm-bindgen-test to check that large pointers +# are handled correctly +gg-alloc = ["wasm-bindgen-test/gg-alloc"] + # This is only for debugging wasm-bindgen! No stability guarantees, so enable # this at your own peril! -xxx_debug_only_print_generated_code = ["wasm-bindgen-macro/xxx_debug_only_print_generated_code"] +xxx_debug_only_print_generated_code = [ + "wasm-bindgen-macro/xxx_debug_only_print_generated_code", +] [dependencies] wasm-bindgen-macro = { path = "crates/macro", version = "=0.2.84" } diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index aa88537370f..1a1de40e909 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -920,6 +920,7 @@ impl<'a> Context<'a> { dst.push_str(&format!( " static __wrap(ptr) {{ + ptr = ptr >>> 0; const obj = Object.create({}.prototype); obj.ptr = ptr; {} @@ -937,7 +938,7 @@ impl<'a> Context<'a> { if self.config.weak_refs { self.global(&format!( - "const {}Finalization = new FinalizationRegistry(ptr => wasm.{}(ptr));", + "const {}Finalization = new FinalizationRegistry(ptr => wasm.{}(ptr >>> 0));", name, wasm_bindgen_shared::free_function(&name), )); @@ -1259,14 +1260,14 @@ impl<'a> Context<'a> { "\ if (realloc === undefined) {{ const buf = cachedTextEncoder.encode(arg); - const ptr = malloc(buf.length); + const ptr = malloc(buf.length) >>> 0; {mem}().subarray(ptr, ptr + buf.length).set(buf); WASM_VECTOR_LEN = buf.length; return ptr; }} let len = arg.length; - let ptr = malloc(len); + let ptr = malloc(len) >>> 0; const mem = {mem}(); @@ -1294,7 +1295,7 @@ impl<'a> Context<'a> { if (offset !== 0) {{ arg = arg.slice(offset); }} - ptr = realloc(ptr, len, len = offset + arg.length * 3); + ptr = realloc(ptr, len, len = offset + arg.length * 3) >>> 0; const view = {mem}().subarray(ptr + offset, ptr + len); const ret = encodeString(arg, view); {debug_end} @@ -1366,7 +1367,7 @@ impl<'a> Context<'a> { self.global(&format!( " function {}(array, malloc) {{ - const ptr = malloc(array.length * 4); + const ptr = malloc(array.length * 4) >>> 0; const mem = {}(); for (let i = 0; i < array.length; i++) {{ mem[ptr / 4 + i] = {}(array[i]); @@ -1383,7 +1384,7 @@ impl<'a> Context<'a> { self.global(&format!( " function {}(array, malloc) {{ - const ptr = malloc(array.length * 4); + const ptr = malloc(array.length * 4) >>> 0; const mem = {}(); for (let i = 0; i < array.length; i++) {{ mem[ptr / 4 + i] = addHeapObject(array[i]); @@ -1416,7 +1417,7 @@ impl<'a> Context<'a> { self.global(&format!( " function {}(arg, malloc) {{ - const ptr = malloc(arg.length * {size}); + const ptr = malloc(arg.length * {size}) >>> 0; {}().set(arg, ptr / {size}); WASM_VECTOR_LEN = arg.length; return ptr; @@ -1513,6 +1514,7 @@ impl<'a> Context<'a> { self.global(&format!( " function {}(ptr, len) {{ + ptr = ptr >>> 0; return cachedTextDecoder.decode({}().{}(ptr, ptr + len)); }} ", @@ -1583,6 +1585,7 @@ impl<'a> Context<'a> { self.global(&format!( " function {}(ptr, len) {{ + ptr = ptr >>> 0; const mem = {}(); const slice = mem.subarray(ptr / 4, ptr / 4 + len); const result = []; @@ -1601,6 +1604,7 @@ impl<'a> Context<'a> { self.global(&format!( " function {}(ptr, len) {{ + ptr = ptr >>> 0; const mem = {}(); const slice = mem.subarray(ptr / 4, ptr / 4 + len); const result = []; @@ -1683,6 +1687,7 @@ impl<'a> Context<'a> { self.global(&format!( " function {name}(ptr, len) {{ + ptr = ptr >>> 0; return {mem}().subarray(ptr / {size}, ptr / {size} + len); }} ", diff --git a/crates/cli/tests/reference/anyref-import-catch.js b/crates/cli/tests/reference/anyref-import-catch.js index d73f9ff7f3d..f7727f5e627 100644 --- a/crates/cli/tests/reference/anyref-import-catch.js +++ b/crates/cli/tests/reference/anyref-import-catch.js @@ -20,6 +20,7 @@ function getUint8Memory0() { } function getStringFromWasm0(ptr, len) { + ptr = ptr >>> 0; return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len)); } diff --git a/crates/cli/tests/reference/result-string.js b/crates/cli/tests/reference/result-string.js index 1ade844742c..6d28e06c7e4 100644 --- a/crates/cli/tests/reference/result-string.js +++ b/crates/cli/tests/reference/result-string.js @@ -58,6 +58,7 @@ function getUint8Memory0() { } function getStringFromWasm0(ptr, len) { + ptr = ptr >>> 0; return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len)); } /** diff --git a/crates/cli/tests/reference/string-arg.js b/crates/cli/tests/reference/string-arg.js index bf5ba77b739..7921d3ca422 100644 --- a/crates/cli/tests/reference/string-arg.js +++ b/crates/cli/tests/reference/string-arg.js @@ -20,6 +20,7 @@ function getUint8Memory0() { } function getStringFromWasm0(ptr, len) { + ptr = ptr >>> 0; return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len)); } @@ -46,14 +47,14 @@ function passStringToWasm0(arg, malloc, realloc) { if (realloc === undefined) { const buf = cachedTextEncoder.encode(arg); - const ptr = malloc(buf.length); + const ptr = malloc(buf.length) >>> 0; getUint8Memory0().subarray(ptr, ptr + buf.length).set(buf); WASM_VECTOR_LEN = buf.length; return ptr; } let len = arg.length; - let ptr = malloc(len); + let ptr = malloc(len) >>> 0; const mem = getUint8Memory0(); @@ -69,7 +70,7 @@ function passStringToWasm0(arg, malloc, realloc) { if (offset !== 0) { arg = arg.slice(offset); } - ptr = realloc(ptr, len, len = offset + arg.length * 3); + ptr = realloc(ptr, len, len = offset + arg.length * 3) >>> 0; const view = getUint8Memory0().subarray(ptr + offset, ptr + len); const ret = encodeString(arg, view); diff --git a/crates/test/Cargo.toml b/crates/test/Cargo.toml index 7b7ef2192c1..ff6b046f289 100644 --- a/crates/test/Cargo.toml +++ b/crates/test/Cargo.toml @@ -14,6 +14,7 @@ scoped-tls = "1.0" wasm-bindgen = { path = '../..', version = '0.2.84' } wasm-bindgen-futures = { path = '../futures', version = '0.4.34' } wasm-bindgen-test-macro = { path = '../test-macro', version = '=0.3.34' } +gg-alloc = { version = "1.0", optional = true } [lib] test = false diff --git a/crates/test/src/lib.rs b/crates/test/src/lib.rs index 7fc5bc0a239..4fcb28aeb49 100644 --- a/crates/test/src/lib.rs +++ b/crates/test/src/lib.rs @@ -6,6 +6,12 @@ pub use wasm_bindgen_test_macro::wasm_bindgen_test; +// Custom allocator that only returns pointers in the 2GB-4GB range +// To ensure we actually support more than 2GB of memory +#[cfg(all(test, feature = "gg-alloc"))] +#[global_allocator] +static A: gg_alloc::GgAlloc = gg_alloc::GgAlloc::new(std::alloc::System); + /// Helper macro which acts like `println!` only routes to `console.log` /// instead. #[macro_export]