Skip to content

Commit

Permalink
Fix large pointers from WASM
Browse files Browse the repository at this point in the history 
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 rustwasm#2388
Fixes rustwasm#2957
  • Loading branch information
@kristoff3r
kristoff3r committed Feb 17, 2023
1 parent 629a623 commit 149076a
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 11 deletions.
8 changes: 7 additions & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
19 changes: 12 additions & 7 deletions crates/cli-support/src/js/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
{}
Expand All @@ -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),
));
Expand Down Expand Up @@ -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}();
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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]);
Expand All @@ -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]);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1513,6 +1514,7 @@ impl<'a> Context<'a> {
self.global(&format!(
"
function {}(ptr, len) {{
ptr = ptr >>> 0;
return cachedTextDecoder.decode({}().{}(ptr, ptr + len));
}}
",
Expand Down Expand Up @@ -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 = [];
Expand All @@ -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 = [];
Expand Down Expand Up @@ -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);
}}
",
Expand Down
1 change: 1 addition & 0 deletions crates/cli/tests/reference/anyref-import-catch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ function getUint8Memory0() {
}

function getStringFromWasm0(ptr, len) {
ptr = ptr >>> 0;
return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
}

Expand Down
1 change: 1 addition & 0 deletions crates/cli/tests/reference/result-string.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ function getUint8Memory0() {
}

function getStringFromWasm0(ptr, len) {
ptr = ptr >>> 0;
return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
}
/**
Expand Down
7 changes: 4 additions & 3 deletions crates/cli/tests/reference/string-arg.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ function getUint8Memory0() {
}

function getStringFromWasm0(ptr, len) {
ptr = ptr >>> 0;
return cachedTextDecoder.decode(getUint8Memory0().subarray(ptr, ptr + len));
}

Expand All @@ -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();

Expand All @@ -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);

Expand Down
1 change: 1 addition & 0 deletions crates/test/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions crates/test/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::alloc::System> = gg_alloc::GgAlloc::new(std::alloc::System);

/// Helper macro which acts like `println!` only routes to `console.log`
/// instead.
#[macro_export]
Expand Down

0 comments on commit 149076a

Please sign in to comment.