1161298 - Assertion failure: value >= 0 ? (int32_t(y) >= int32_t(x)) : (int32_t(y) < int32_t(x)), at js/src/jit/x86-shared/Patching-x86-shared.h:51

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision dc5f85980a82 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-offthread-compile=off):

function m(stdlib, ffi, heap) {
    "use asm";
    var HEAP32 = new stdlib.SharedInt32Array(heap);
    var add = stdlib.Atomics.add;
    function add_sharedEv(i1) {
	i1 = i1 | 0;
	var i2 = 0;
	add(HEAP32, i2 >> 2, 1) | 0;
    }
    return {add_sharedEv:add_sharedEv};
}
var sab = new SharedArrayBuffer((2147483648));
var {add_sharedEv} = m(this, 2, sab);



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x000000000042a9f4 in js::jit::X86Encoding::AddInt32 (value=<optimized out>, where=<optimized out>) at js/src/jit/x86-shared/Patching-x86-shared.h:51
#0  0x000000000042a9f4 in js::jit::X86Encoding::AddInt32 (value=<optimized out>, where=<optimized out>) at js/src/jit/x86-shared/Patching-x86-shared.h:51
#1  0x000000000051ef05 in AddInt32 (value=<optimized out>, where=<optimized out>) at js/src/vm/ArrayBufferObject-inl.h:24
#2  js::AsmJSModule::initHeap (this=this@entry=0x7ffff69ae000, heap=heap@entry=..., cx=cx@entry=0x7ffff691b4e0) at js/src/asmjs/AsmJSModule.cpp:869
#3  0x00000000005816ef in LinkModuleToHeap (heap=..., module=..., cx=0x7ffff691b4e0) at js/src/asmjs/AsmJSLink.cpp:524
#4  DynamicallyLinkModule (module=..., cx=0x7ffff691b4e0, args=...) at js/src/asmjs/AsmJSLink.cpp:544
#5  LinkAsmJS (cx=0x7ffff691b4e0, argc=3, vp=0x7ffff51e90a0) at js/src/asmjs/AsmJSLink.cpp:1067
#6  0x0000000000678c92 in js::CallJSNative (cx=0x7ffff691b4e0, native=0x581190 <LinkAsmJS(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
#7  0x0000000000669093 in js::Invoke (cx=cx@entry=0x7ffff691b4e0, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:727
#8  0x0000000000662c47 in Interpret (cx=cx@entry=0x7ffff691b4e0, state=...) at js/src/vm/Interpreter.cpp:2956
#9  0x0000000000668b6b in js::RunScript (cx=cx@entry=0x7ffff691b4e0, state=...) at js/src/vm/Interpreter.cpp:677
#10 0x00000000006731be in js::ExecuteKernel (cx=cx@entry=0x7ffff691b4e0, script=..., script@entry=..., scopeChainArg=..., thisv=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:902
#11 0x00000000006753f9 in js::Execute (cx=cx@entry=0x7ffff691b4e0, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:942
#12 0x0000000000a5ca49 in ExecuteScript (cx=cx@entry=0x7ffff691b4e0, obj=..., scriptArg=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4159
#13 0x0000000000a5cc0b in JS_ExecuteScript (cx=cx@entry=0x7ffff691b4e0, scriptArg=..., scriptArg@entry=...) at js/src/jsapi.cpp:4181
#14 0x0000000000425a07 in RunFile (compileOnly=false, file=0x7ffff699e400, filename=0x7fffffffdf15 "min.js", cx=0x7ffff691b4e0) at js/src/shell/js.cpp:467
#15 Process (cx=cx@entry=0x7ffff691b4e0, filename=0x7fffffffdf15 "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:597
#16 0x00000000004712b1 in ProcessArgs (op=0x7fffffffd950, cx=0x7ffff691b4e0) at js/src/shell/js.cpp:5798
#17 Shell (envp=<optimized out>, op=0x7fffffffd950, cx=0x7ffff691b4e0) at js/src/shell/js.cpp:6064
#18 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6385
rax	0x0	0
rbx	0x7ffff69ae000	140737330733056
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffbfd0	140737488338896
rsp	0x7fffffffbfd0	140737488338896
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffbd90	140737488338320
r11	0x7ffff6c27960	140737333328224
r12	0x7fffffffc190	140737488339344
r13	0x1a24200	27410944
r14	0x7ffff7fe6730	140737354032944
r15	0x7fffffffc2a0	140737488339616
rip	0x42a9f4 <js::jit::X86Encoding::AddInt32(int32_t, void*)+28>
=> 0x42a9f4 <js::jit::X86Encoding::AddInt32(int32_t, void*)+28>:	movl   $0x33,0x0
   0x42a9ff <js::jit::X86Encoding::AddInt32(int32_t, void*)+39>:	callq  0x48eb50 <abort()>

Comment 1

[Lars T Hansen [:lth]]

This appears to be a simple problem with the guard on the array length in SharedArrayObject.cpp:class_constructor.  Observe that there is a guard on the length in the !ToLengthClamped case, but there needs to be a guard also if that does not fails, since ToLengthClamped clamps the length to below 2^32-2, and we need below 2^31.

Comment 2

[Lars T Hansen [:lth]]

Attachment: bug1161298-missing-guard.diff

Simple cleanup.  ToLengthClamped returns false on a bad length or overflow, so we just post-check the length if ToLengthClamped succeeded.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8602586 [details] [diff] [review]
bug1161298-missing-guard.diff

Review of attachment 8602586 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, please add the test case in the patch.

::: js/src/vm/SharedArrayObject.cpp
@@ +220,3 @@
>      uint32_t length;
>      bool overflow;
> +    if (!ToLengthClamped(cx, args.get(0), &length, &overflow) || length > INT32_MAX) {

Any chance to not have the "overflow" variable, as it's unused?

Comment 4

[Lars T Hansen [:lth]]

(In reply to Benjamin Bouvier [:bbouvier] from comment #3)
> Comment on attachment 8602586 [details] [diff] [review]
> bug1161298-missing-guard.diff
> 
> Any chance to not have the "overflow" variable, as it's unused?

Not without complicating the API or implementation of ToLengthClamped, both of which seem like worse options.  I'll rename as overflow_unused or somesuch.

Comment 5

[Lars T Hansen [:lth]]

Attachment: Corrected patch, with test, for landing.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/480e59b8e406

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/480e59b8e406