cli: allow running wasm in limited vmemory with --disable-wasm-trap-handler

joyeecheung commented 2 weeks ago

tools: support != in test status files

tools: support max_virtual_memory test configuration

tools: fix get_asan_state() in tools/test.py

The output of node -p process.config.variables.asan includes a newline character so it's never exactly "1", which means asan is always "off" for the status files. This fixes the detection by stripping whitespaces from the output.

cli: allow running wasm in limited vmemory with --disable-wasm-trap-handler

By default, Node.js enables trap-handler-based WebAssembly bound checks. As a result, V8 does not need to insert inline bound checks int the code compiled from WebAssembly which may speedup WebAssembly execution significantly, but this optimization requires allocating a big virtual memory cage (currently 10GB). If the Node.js process does not have access to a large enough virtual memory address space due to system configurations or hardware limitations, users won't be able to run any WebAssembly that involves allocation in this virtual memory cage and will see an out-of-memory error.

$ ulimit -v 5000000
$ node -p "new WebAssembly.Memory({ initial: 10, maximum: 100 });"
[eval]:1
new WebAssembly.Memory({ initial: 10, maximum: 100 });
^

RangeError: WebAssembly.Memory(): could not allocate memory
    at [eval]:1:1
    at runScriptInThisContext (node:internal/vm:209:10)
    at node:internal/process/execution:118:14
    at [eval]-wrapper:6:24
    at runScript (node:internal/process/execution:101:62)
    at evalScript (node:internal/process/execution:136:3)
    at node:internal/main/eval_string:49:3

--disable-wasm-trap-handler disables this optimization so that users can at least run WebAssembly (with a less optimial performance) when the virtual memory address space available to their Node.js process is lower than what the V8 WebAssembly memory cage needs.

Background: https://docs.google.com/document/u/3/d/1PM4Zqmlt8ac5O8UNQfY7fOsem-6MhbsB-vjFI-9XK6w/edit#heading=h.diogznvalour

nodejs-github-bot commented 2 weeks ago

Review requested:

[ ] @nodejs/startup

joyeecheung commented 2 weeks ago

A more real world example of what this PR fixes:

$ ulimit -v 5000000
$ mkdir test-webpack
$ npm install --save webpack
$ node ./node_modules/.bin/webpack

[webpack-cli] RangeError: WebAssembly.Instance(): Out of memory: Cannot allocate Wasm memory for new instance
    at create (/home/ubuntu/test-webpack/node_modules/webpack/lib/util/hash/wasm-hash.js:155:4)
    at module.exports (/home/ubuntu/test-webpack/node_modules/webpack/lib/util/createHash.js:176:6)
    at /home/ubuntu/test-webpack/node_modules/webpack/lib/DefinePlugin.js:351:22
    at _next31 (eval at create (/home/ubuntu/test-webpack/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:42:1)
    at _next9 (eval at create (/home/ubuntu/test-webpack/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:97:1)
    at Hook.eval [as call] (eval at create (/home/ubuntu/test-webpack/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:119:1)
    at Hook.CALL_DELEGATE [as _call] (/home/ubuntu/test-webpack/node_modules/tapable/lib/Hook.js:14:14)
    at Compiler.newCompilation (/home/ubuntu/test-webpack/node_modules/webpack/lib/Compiler.js:1255:26)
    at /home/ubuntu/test-webpack/node_modules/webpack/lib/Compiler.js:1299:29
    at Hook.eval [as callAsync] (eval at create (/home/ubuntu/test-webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)

(This happens because Webpack uses a wasm implementation of md4)

nodejs-github-bot commented 2 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/58823/

nodejs-github-bot commented 2 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/58840/

nodejs-github-bot commented 2 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/58871/

nodejs-github-bot commented 2 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/58875/

nodejs-github-bot commented 2 weeks ago

CI: https://ci.nodejs.org/job/node-test-pull-request/58876/

nodejs-github-bot commented 1 week ago

CI: https://ci.nodejs.org/job/node-test-pull-request/58908/

nodejs-github-bot commented 1 week ago

CI: https://ci.nodejs.org/job/node-test-pull-request/58997/

nodejs-github-bot commented 1 week ago

CI: https://ci.nodejs.org/job/node-test-pull-request/59000/

joyeecheung commented 1 week ago

cc @nodejs/cpp-reviewers @nodejs/build

joyeecheung commented 1 week ago

Added YAML and manpage entries, thanks for the reminder.

As for stability index, it seems unnecessary for a flag that serves as an escape hatch in a special environment - we don't usually add stability index to this kind of flags, either, and there is no clear criteria when this should be considered stable or how it could change over time - I expect the flag to just keep working as it is. If we must assign an index, I think it's fine to just mark it stable. But I'd prefer to just leave it out like most other optional CLI flags.

nodejs-github-bot commented 1 week ago

CI: https://ci.nodejs.org/job/node-test-pull-request/59069/

nodejs-github-bot commented 1 week ago

CI: https://ci.nodejs.org/job/node-test-pull-request/59073/

joyeecheung commented 6 days ago

Landed in e03529ec27746a418d66598890b920fd0b733619...77fabfb2ab5b5806b68f06d68b2bbe4981295747

targos commented 3 days ago

This PR adds a feature so it should be labeled https://github.com/nodejs/node/labels/semver-minor (this is not supposed to be done by releasers).

nodejs / node