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 pointer use unsigned (via >>> 0) for all malloc/realloc
calls, and for all pointer arguments while generating the shim functions.
Ideally there would be an abstraction that guarantees we do it in all
cases, but that would require a major refactor.

To test it we use gg-alloc as the global allocator while running tests.
This allocator was made specifically for testing this issue, but was
never upstreamed. It only allocates memory above 2GiB.

Fixes rustwasm#2388
Fixes rustwasm#2957
  • Loading branch information
kristoff3r committed Feb 15, 2023
1 parent d9e113b commit 2b0803e
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 20 deletions.
20 changes: 13 additions & 7 deletions crates/cli-support/src/js/binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,12 @@ impl<'a, 'b> JsBuilder<'a, 'b> {
self.stack.pop().unwrap()
}

/// Like pop but makes the argument unsigned, thus allowing pointers
/// to use all 32 bits without becoming negative
fn pop_ptr(&mut self) -> String {
format!("({} >>> 0)", self.stack.pop().unwrap())
}

fn push(&mut self, arg: String) {
self.stack.push(arg);
}
Expand Down Expand Up @@ -616,7 +622,7 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->

Instruction::Standard(wit_walrus::Instruction::MemoryToString(mem)) => {
let len = js.pop();
let ptr = js.pop();
let ptr = js.pop_ptr();
let get = js.cx.expose_get_string_from_wasm(*mem)?;
js.push(format!("{}({}, {})", get, ptr, len));
}
Expand Down Expand Up @@ -862,7 +868,7 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->
let is_err = js.pop();
let err = js.pop();
let len = js.pop();
let ptr = js.pop();
let ptr = js.pop_ptr();
let i = js.tmp();
js.prelude(&format!(
"
Expand Down Expand Up @@ -997,7 +1003,7 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->
table,
} => {
let len = js.pop();
let ptr = js.pop();
let ptr = js.pop_ptr();
let tmp = js.tmp();

let get = js.cx.expose_get_cached_string_from_wasm(*mem, *table)?;
Expand Down Expand Up @@ -1074,7 +1080,7 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->

Instruction::VectorLoad { kind, mem, free } => {
let len = js.pop();
let ptr = js.pop();
let ptr = js.pop_ptr();
let f = js.cx.expose_get_vector_from_wasm(kind.clone(), *mem)?;
let i = js.tmp();
let free = js.cx.export_name_of(*free);
Expand All @@ -1091,7 +1097,7 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->

Instruction::OptionVectorLoad { kind, mem, free } => {
let len = js.pop();
let ptr = js.pop();
let ptr = js.pop_ptr();
let f = js.cx.expose_get_vector_from_wasm(kind.clone(), *mem)?;
let i = js.tmp();
let free = js.cx.export_name_of(*free);
Expand All @@ -1111,14 +1117,14 @@ fn instruction(js: &mut JsBuilder, instr: &Instruction, log_error: &mut bool) ->

Instruction::View { kind, mem } => {
let len = js.pop();
let ptr = js.pop();
let ptr = js.pop_ptr();
let f = js.cx.expose_get_vector_from_wasm(kind.clone(), *mem)?;
js.push(format!("{f}({ptr}, {len})", ptr = ptr, len = len, f = f));
}

Instruction::OptionView { kind, mem } => {
let len = js.pop();
let ptr = js.pop();
let ptr = js.pop_ptr();
let f = js.cx.expose_get_vector_from_wasm(kind.clone(), *mem)?;
js.push(format!(
"{ptr} === 0 ? undefined : {f}({ptr}, {len})",
Expand Down
12 changes: 6 additions & 6 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1259,14 +1259,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 +1294,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 +1366,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 +1383,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 +1416,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
2 changes: 1 addition & 1 deletion crates/cli/tests/reference/anyref-import-catch.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function __wbg_foo_95fe1a04017077db() { return handleError(function () {
}, arguments) };

export function __wbindgen_throw(arg0, arg1) {
throw new Error(getStringFromWasm0(arg0, arg1));
throw new Error(getStringFromWasm0((arg0 >>> 0), arg1));
};

export function __wbindgen_init_externref_table() {
Expand Down
4 changes: 2 additions & 2 deletions crates/cli/tests/reference/result-string.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ export function exported() {
var r1 = getInt32Memory0()[retptr / 4 + 1];
var r2 = getInt32Memory0()[retptr / 4 + 2];
var r3 = getInt32Memory0()[retptr / 4 + 3];
var ptr0 = r0;
var ptr0 = (r0 >>> 0);
var len0 = r1;
if (r3) {
ptr0 = 0; len0 = 0;
throw takeObject(r2);
}
return getStringFromWasm0(ptr0, len0);
return getStringFromWasm0((ptr0 >>> 0), len0);
} finally {
wasm.__wbindgen_add_to_stack_pointer(16);
wasm.__wbindgen_free(ptr0, len0);
Expand Down
8 changes: 4 additions & 4 deletions crates/cli/tests/reference/string-arg.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,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 +69,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 All @@ -89,6 +89,6 @@ export function foo(a) {
}

export function __wbindgen_throw(arg0, arg1) {
throw new Error(getStringFromWasm0(arg0, arg1));
throw new Error(getStringFromWasm0((arg0 >>> 0), arg1));
};

1 change: 1 addition & 0 deletions crates/test/Cargo.toml
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 = "1.0"

[lib]
test = false
8 changes: 8 additions & 0 deletions crates/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@

pub use wasm_bindgen_test_macro::wasm_bindgen_test;

use gg_alloc::GgAlloc;
use std::alloc::System;

// Custom allocator that only returns pointers in the 2GB-4GB range
// To ensure we actually support more than 2GB of memory
#[global_allocator]
static A: GgAlloc<System> = GgAlloc::new(System);

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

0 comments on commit 2b0803e

Please sign in to comment.