1711817 - Assertion failure: i <= HighestValidARMImmediate, at gecko-dev/js/src/wasm/WasmTypes.cpp

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, defect, P1)

Description

[anbu]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/90.0.4430.212 Safari/537.36

Steps to reproduce:

Steps to reproduce:

spidermonkey version

checked revison: cecdac0aa5733fee515a166b6e31e38cc58abf32
build command: /bin/sh ../configure.in --enable-debug --disable-optimize

$ ./js/src/build_debug/dist/bin/js --version
JavaScript-C90.0a1

Testcase. The following test case crashes the debug version of SpiderMonkey:

var x = this;
var y = 0;
var z = new ArrayBuffer(64 * 1024 * 65500);

var foo = function (a, b, c) {
    'use asm';
    var local = new a.Int16Array(c);
    function bar() {
        return local[0] | 0;
    }
    return bar;
}(x, y, z);

foo();

Actual results:

Error

$ moz-dbg 1.js 
Assertion failure: i <= HighestValidARMImmediate, at gecko-dev/js/src/wasm/WasmTypes.cpp:1592
Segmentation fault (core dumped)

Backtrace

#0  0x00005555570457bd in js::wasm::RoundUpToNextValidARMImmediate (i=0xffdc0000) at gecko-dev/js/src/wasm/WasmTypes.cpp:1592
#1  0x0000555556e53710 in RoundUpToNextValidAsmJSHeapLength (length=0xffdc0000) at gecko-dev/js/src/wasm/AsmJS.cpp:108
#2  0x0000555556e52dbc in CheckBuffer (cx=0x7ffff6126000, metadata=..., bufferVal=..., buffer=...) at gecko-dev/js/src/wasm/AsmJS.cpp:6754
#3  0x0000555556df7ec6 in TryInstantiate (cx=0x7ffff6126000, args=..., module=..., metadata=..., instanceObj=..., exportObj=...) at gecko-dev/js/src/wasm/AsmJS.cpp:6872
#4  0x0000555556df7b43 in js::InstantiateAsmJS (cx=0x7ffff6126000, argc=0x3, vp=0x7ffff4ff00a0) at gecko-dev/js/src/wasm/AsmJS.cpp:6985
#5  0x0000555555abbd43 in CallJSNative (cx=0x7ffff6126000, native=0x555556df7990 <js::InstantiateAsmJS(JSContext*, unsigned int, JS::Value*)>, reason=js::CallReason::Call, args=...) at gecko-dev/js/src/vm/Interpreter.cpp:427
#6  0x0000555555aa97d1 in js::InternalCallOrConstruct (cx=0x7ffff6126000, args=..., construct=js::NO_CONSTRUCT, reason=js::CallReason::Call) at gecko-dev/js/src/vm/Interpreter.cpp:512
#7  0x0000555555aa9fef in InternalCall (cx=0x7ffff6126000, args=..., reason=js::CallReason::Call) at gecko-dev/js/src/vm/Interpreter.cpp:572
#8  0x0000555555aa9db2 in js::CallFromStack (cx=0x7ffff6126000, args=...) at gecko-dev/js/src/vm/Interpreter.cpp:576
#9  0x0000555555a9d4e4 in Interpret (cx=0x7ffff6126000, state=...) at gecko-dev/js/src/vm/Interpreter.cpp:3227
#10 0x0000555555a92482 in js::RunScript (cx=0x7ffff6126000, state=...) at gecko-dev/js/src/vm/Interpreter.cpp:396
#11 0x0000555555aab2fb in js::ExecuteKernel (cx=0x7ffff6126000, script=..., envChainArg=..., newTargetValue=..., evalInFrame=..., result=...) at gecko-dev/js/src/vm/Interpreter.cpp:766
#12 0x0000555555aab66f in js::Execute (cx=0x7ffff6126000, script=..., envChain=..., rval=...) at gecko-dev/js/src/vm/Interpreter.cpp:798
#13 0x0000555555cf4946 in ExecuteScript (cx=0x7ffff6126000, envChain=..., script=..., rval=...) at gecko-dev/js/src/vm/CompilationAndEvaluation.cpp:491
#14 0x0000555555cf4a77 in JS_ExecuteScript (cx=0x7ffff6126000, scriptArg=...) at gecko-dev/js/src/vm/CompilationAndEvaluation.cpp:515
#15 0x0000555555903625 in RunFile (cx=0x7ffff6126000, filename=0x7fffffffdd9d "1.js", file=0x7ffff78572e0, compileMethod=CompileUtf8::DontInflate, compileOnly=0x0) at gecko-dev/js/src/shell/js.cpp:1066
#16 0x0000555555902f61 in Process (cx=0x7ffff6126000, filename=0x7fffffffdd9d "1.js", forceTTY=0x0, kind=FileScript) at gecko-dev/js/src/shell/js.cpp:1657
#17 0x00005555558d92f9 in ProcessArgs (cx=0x7ffff6126000, op=0x7fffffffd758) at gecko-dev/js/src/shell/js.cpp:10948
#18 0x00005555558cec78 in Shell (cx=0x7ffff6126000, op=0x7fffffffd758) at gecko-dev/js/src/shell/js.cpp:11671
#19 0x00005555558c9b01 in main (argc=0x2, argv=0x7fffffffd9e8) at gecko-dev/js/src/shell/js.cpp:12635
#20 0x00007ffff7a500b3 in __libc_start_main (main=0x5555558c6cb0 <main(int, char**)>, argc=0x2, argv=0x7fffffffd9e8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd9d8) at ../csu/libc-start.c:308
#21 0x00005555558be3ce in _start ()

Expected results:

I think it would be better to exit with a error message like "Invalid heap size", rather than exiting with such an assertion error.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Javascript: WebAssembly' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Lars T Hansen [:lth]]

Must have come in with the large buffer change, we may need a guard to move.

Comment 3

[Lars T Hansen [:lth]]

When we discover that the heap length is not valid, we want to print an error message, and to be user friendly we want to include the next higher heap length in the error message so we try to round the heap length up, but if the heap length is already too large we will assert in the round-up function.

This could not happen before large buffers landed because the longest heap length was then < INT32_MAX while the max logical heap length was almost UINT32_MAX; the roundup would thus always succeed. Now we need an extra guard here so that we can distinguish the "can be rounded up" from "is too large" cases.

(Probably not s-s but I will check all cases before committing to that.)

Comment 4

[Lars T Hansen [:lth]]

Attachment: Bug 1711817 - guard an error message on a limit. r?rhunt

When reporting an invalid asm.js heap length, we must also guard the
message on the max such heap length or we risk asserting in the function
that computes the max heap length from the given invalid length.

Drive-by fix: Better error reporting for link errors.

Comment 5

[Lars T Hansen [:lth]]

There was only the one case that needed a more subtle guard.

With the attached patch, we get this, as expected:

$ dist/bin/js -w big.js
big.js:warning: Successfully compiled asm.js code (total compilation time 2ms)
big.js:12:2 warning: asm.js link error: ArrayBuffer byteLength 0xffdc0000 is not a valid heap length - it is too long. The longest valid length is 0x7f000000
big.js:warning: asm.js type error: Disabled by linker (instantiation failure)

:decoder, can you open up? This is not a security issue after all.

Comment 6

[Dorel Luca [:dluca]]

Backed out changeset a8c663dad696 (bug 1711817) for for Spidermonkey failures in js/src/jit-test/tests/asm.js/testAddressErrors.js. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=340213567&repo=autoland&lineNumber=2918

Push with failure:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=a8c663dad69688505569f73095bd2a89a900f548

Backout:
https://hg.mozilla.org/integration/autoland/rev/043aa993858012a2757c2f71f74beec08b445266

Comment 7

[Lars T Hansen [:lth]]

Forgot to update the error message pattern to account for the improved error messages.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/85e84a592715
guard an error message on a limit. r=rhunt

Comment 9

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/85e84a592715