Skip to content

Commit

Permalink
[wasm64] Fix for bad Memory initial size under firefox
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 committed Sep 5, 2024
1 parent 4c71fa1 commit 0b59e0d
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 23 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
5 changes: 4 additions & 1 deletion tools/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -2098,7 +2098,10 @@ def phase_final_emitting(options, state, target, wasm_target):
# optimize by Closure, or unoptimalities that were left behind by processing
# steps that occurred after Closure.
if settings.MINIMAL_RUNTIME == 2 and settings.USE_CLOSURE_COMPILER and settings.DEBUG_LEVEL == 0:
shared.run_js_tool(utils.path_from_root('tools/unsafe_optimizations.mjs'), [final_js, '-o', final_js], cwd=utils.path_from_root('.'))
args = [final_js, '-o', final_js]
if not settings.MINIFY_WHITESPACE:
args.append('--pretty')
shared.run_js_tool(utils.path_from_root('tools/unsafe_optimizations.mjs'), args, cwd=utils.path_from_root('.'))
save_intermediate('unsafe-optimizations')
# Finally, rerun Closure compile with simple optimizations. It will be able
# to further minify the code. (n.b. it would not be safe to run in advanced
Expand Down
7 changes: 5 additions & 2 deletions tools/unsafe_optimizations.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ 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 +220,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 0b59e0d

Please sign in to comment.