Skip to content

Commit

Permalink
[wasm64] Fix for bad Memory initial size under firefox (#22497)
Browse files Browse the repository at this point in the history
Recent versions of firefox started requiring bigint values for initial
and max memory.

See WebAssembly/memory64#68

Fixes: #22486
  • Loading branch information
sbc100 authored Sep 5, 2024
1 parent 36ec31f commit 12770b1
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 22 deletions.
5 changes: 4 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,10 @@ jobs:
- install-emsdk
- run-tests-firefox:
title: "browser64"
test_targets: "browser64.test_sdl_image"
test_targets: "
browser64.test_sdl_image
browser64.test_dylink_many
"
# TODO(sbc): Re-enable once we figure out why the emrun tests are
# locking up.
#test-browser-chrome-emrun:
Expand Down
17 changes: 9 additions & 8 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -1966,41 +1966,42 @@ addToLibrary({
$setWasmTableEntry__internal: true,
$setWasmTableEntry__deps: ['$wasmTableMirror', '$wasmTable'],
$setWasmTableEntry: (idx, func) => {
wasmTable.set(idx, func);
wasmTable.set({{{ toIndexType('idx') }}}, func);
// With ABORT_ON_WASM_EXCEPTIONS wasmTable.get is overridden to return wrapped
// functions so we need to call it here to retrieve the potential wrapper correctly
// instead of just storing 'func' directly into wasmTableMirror
wasmTableMirror[idx] = wasmTable.get(idx);
wasmTableMirror[idx] = wasmTable.get({{{ toIndexType('idx') }}});
},

$getWasmTableEntry__internal: true,
$getWasmTableEntry__deps: ['$wasmTableMirror', '$wasmTable'],
$getWasmTableEntry: (funcPtr) => {
#if MEMORY64
// Function pointers are 64-bit, but wasmTable.get() requires a Number.
// Function pointers should show up as numbers, even under wasm64, but
// we still have some places where bigint values can flow here.
// https://github.com/emscripten-core/emscripten/issues/18200
funcPtr = Number(funcPtr);
#endif
var func = wasmTableMirror[funcPtr];
if (!func) {
if (funcPtr >= wasmTableMirror.length) wasmTableMirror.length = funcPtr + 1;
wasmTableMirror[funcPtr] = func = wasmTable.get(funcPtr);
wasmTableMirror[funcPtr] = func = wasmTable.get({{{ toIndexType('funcPtr') }}});
#if ASYNCIFY == 2
if (Asyncify.isAsyncExport(func)) {
wasmTableMirror[funcPtr] = func = Asyncify.makeAsyncFunction(func);
}
#endif
}
#if ASSERTIONS && ASYNCIFY != 2 // With JSPI the function stored in the table will be a wrapper.
assert(wasmTable.get(funcPtr) == func, 'JavaScript-side Wasm function table mirror is out of date!');
assert(wasmTable.get({{{ toIndexType('funcPtr') }}}) == func, 'JavaScript-side Wasm function table mirror is out of date!');
#endif
return func;
},

#else

$setWasmTableEntry__deps: ['$wasmTable'],
$setWasmTableEntry: (idx, func) => wasmTable.set(idx, func),
$setWasmTableEntry: (idx, func) => wasmTable.set({{{ toIndexType('idx') }}}, func),

$getWasmTableEntry__deps: ['$wasmTable'],
$getWasmTableEntry: (funcPtr) => {
Expand Down Expand Up @@ -2391,9 +2392,9 @@ addToLibrary({
#if RELOCATABLE
// In RELOCATABLE mode we create the table in JS.
$wasmTable: `=new WebAssembly.Table({
'initial': {{{ INITIAL_TABLE }}},
'initial': {{{ toIndexType(INITIAL_TABLE) }}},
#if !ALLOW_TABLE_GROWTH
'maximum': {{{ INITIAL_TABLE }}},
'maximum': {{{ toIndexType(INITIAL_TABLE) }}},
#endif
#if MEMORY64 == 1
'index': 'i64',
Expand Down
6 changes: 3 additions & 3 deletions src/library_addfunction.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,14 @@ addToLibrary({
}
// Grow the table
try {
wasmTable.grow(1);
wasmTable.grow({{{ toIndexType('1') }}});
} catch (err) {
if (!(err instanceof RangeError)) {
throw err;
}
throw 'Unable to grow wasm table. Set ALLOW_TABLE_GROWTH.';
}
return wasmTable.length - 1;
return {{{ from64Expr('wasmTable.length') }}} - 1;
},

$updateTableMap__deps: ['$getWasmTableEntry'],
Expand All @@ -179,7 +179,7 @@ addToLibrary({
// First, create the map if this is the first use.
if (!functionsInTableMap) {
functionsInTableMap = new WeakMap();
updateTableMap(0, wasmTable.length);
updateTableMap(0, {{{ from64Expr('wasmTable.length') }}});
}
return functionsInTableMap.get(func) || 0;
},
Expand Down
6 changes: 4 additions & 2 deletions src/library_dylink.js
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,10 @@ var LibraryDylink = {
#if DYLINK_DEBUG
$dumpTable__deps: ['$wasmTable'],
$dumpTable: () => {
for (var i = 0; i < wasmTable.length; i++)
var len = wasmTable.length;
for (var i = {{{ toIndexType(0) }}} ; i < len; i++) {
dbg(`table: ${i} : ${wasmTable.get(i)}`);
}
},
#endif

Expand Down Expand Up @@ -641,7 +643,7 @@ var LibraryDylink = {
tableBase = {{{ makeGetValue('handle', C_STRUCTS.dso.table_addr, '*') }}};
}

var tableGrowthNeeded = tableBase + metadata.tableSize - wasmTable.length;
var tableGrowthNeeded = tableBase + metadata.tableSize - {{{ from64Expr('wasmTable.length') }}};
if (tableGrowthNeeded > 0) {
#if DYLINK_DEBUG
dbg("loadModule: growing table: " + tableGrowthNeeded);
Expand Down
6 changes: 6 additions & 0 deletions src/parseTools.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,11 @@ function from64Expr(x, assign = true) {
return `Number(${x})`;
}

function toIndexType(x) {
if (MEMORY64 != 1) return x;
return `toIndexType(${x})`;
}

function to64(x) {
if (!MEMORY64) return x;
return `BigInt(${x})`;
Expand Down Expand Up @@ -1153,4 +1158,5 @@ addToCompileTimeContext({
splitI64,
storeException,
to64,
toIndexType,
});
6 changes: 3 additions & 3 deletions src/runtime_init_memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ if (!ENVIRONMENT_IS_PTHREAD) {
assert(INITIAL_MEMORY >= {{{STACK_SIZE}}}, 'INITIAL_MEMORY should be larger than STACK_SIZE, was ' + INITIAL_MEMORY + '! (STACK_SIZE=' + {{{STACK_SIZE}}} + ')');
#endif
wasmMemory = new WebAssembly.Memory({
'initial': INITIAL_MEMORY / {{{ WASM_PAGE_SIZE }}},
'initial': {{{ toIndexType(`INITIAL_MEMORY / ${WASM_PAGE_SIZE}`) }}},
#if ALLOW_MEMORY_GROWTH
// In theory we should not need to emit the maximum if we want "unlimited"
// or 4GB of memory, but VMs error on that atm, see
// https://github.com/emscripten-core/emscripten/issues/14130
// And in the pthreads case we definitely need to emit a maximum. So
// always emit one.
'maximum': {{{ MAXIMUM_MEMORY }}} / {{{ WASM_PAGE_SIZE }}},
'maximum': {{{ toIndexType(MAXIMUM_MEMORY / WASM_PAGE_SIZE) }}},
#else
'maximum': INITIAL_MEMORY / {{{ WASM_PAGE_SIZE }}},
'maximum': {{{ toIndexType(`INITIAL_MEMORY / ${WASM_PAGE_SIZE}`) }}},
#endif // ALLOW_MEMORY_GROWTH
#if SHARED_MEMORY
'shared': true,
Expand Down
16 changes: 16 additions & 0 deletions src/runtime_shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,19 @@ function updateMemoryViews() {
{{{ maybeExportHeap('HEAPU64') }}}HEAPU64 = new BigUint64Array(b);
#endif
}

#if MEMORY64 == 1
var toIndexType = (function() {
// Probe for support of bigint bounds with memory64.
// TODO(sbc): Remove this once all browsers start requiring bigint here.
// See https://github.com/WebAssembly/memory64/issues/68
var bigintMemoryBounds = 1;
try {
/** @suppress {checkTypes} */
new WebAssembly.Memory({'initial': 1n, 'index': 'i64'});
} catch (e) {
bigintMemoryBounds = 0;
}
return (i) => bigintMemoryBounds ? BigInt(i) : i;
})();
#endif
6 changes: 3 additions & 3 deletions test/no_this_in_dyncall.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
addToLibrary({
$classLike: {
fnptr: 0,
$classLike: {
fnptr: 0,
call: function(val) {
{{{ makeDynCall('vp', 'this.fnptr') }}}(val);
}
},
},

test__deps: ['$classLike'],
test: function(fnptr, val) {
Expand Down
10 changes: 8 additions & 2 deletions tools/unsafe_optimizations.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ function optPassRemoveRedundantOperatorNews(ast) {
// in emscripten with real side effects. For example, see
// loadWasmModuleToWorker which returns a `new Promise` that is never
// referenced (a least in some builds).
if (n.expression.callee.name !== 'Promise') {
//
// Another exception is made for `new WebAssembly.*` since we create and
// unused `WebAssembly.Memory` when probing for wasm64 fatures.
if (
n.expression.callee.name !== 'Promise' &&
n.expression.callee.object?.name !== 'WebAssembly'
) {
nodeArray.splice(i--, 1);
}
}
Expand Down Expand Up @@ -217,7 +223,7 @@ function runOnJsText(js, pretty = false) {
const output = terserAst.print_to_string({
wrap_func_args: false,
beautify: pretty,
indent_level: pretty ? 1 : 0,
indent_level: pretty ? 2 : 0,
});

return output;
Expand Down

0 comments on commit 12770b1

Please sign in to comment.