Closed Bug 1183060 Opened 4 years ago Closed 4 years ago

Crash [@ ??] with atomics in asm.js

Categories

(Core :: JavaScript Engine, defect, critical)

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla42
Tracking Status
firefox40 --- unaffected
firefox41 --- unaffected
firefox42 --- verified
firefox43 --- verified
firefox-esr31 --- unaffected
firefox-esr38 --- unaffected
b2g-v2.1S --- unaffected
b2g-v2.2 --- unaffected
b2g-v2.2r --- unaffected
b2g-master --- fixed

People

(Reporter: decoder, Assigned: lth)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(1 file)

The following testcase crashes on mozilla-central revision eab21ec484bb (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --baseline-eager --ion-offthread-compile=off --ion-extra-checks):

function loadModule_uint16(stdlib, foreign, heap) {
    "use asm";
    var atomic_or = stdlib.Atomics.or;
    var i16a = new stdlib.SharedUint16Array(heap);
    function do_or_i(i) {
	i = i|0;
	var v = 0;
	v = atomic_or(i16a, i>>1, 0x3333)|0;
    }
    return { or_i: do_or_i };
}
function test_uint16(heap) {
    var i16m = loadModule_uint16(this, {}, heap);
    var size = SharedUint16Array.BYTES_PER_ELEMENT;
    i16m.or_i(size*40)
}
var heap = new SharedArrayBuffer(65536);
test_uint16(heap);



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0xf7c88040 in ?? ()
#0  0xf7c88040 in ?? ()
#1  0x08310992 in js::CallJSNative (cx=0xf7a7f040, native=0x8205d00 <CallAsmJS(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
#2  0x082fd3f6 in js::Invoke (cx=cx@entry=0xf7a7f040, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:720
#3  0x082ff203 in js::Invoke (cx=cx@entry=0xf7a7f040, thisv=..., fval=..., argc=argc@entry=1, argv=argv@entry=0xffffb8b0, rval=rval@entry=...) at js/src/vm/Interpreter.cpp:775
#4  0x08535e0e in js::jit::DoCallFallback (cx=0xf7a7f040, frame=0xffffb8f0, stub_=0xf7a21728, argc=1, vp=0xffffb8a0, res=...) at js/src/jit/BaselineIC.cpp:9859
#5  0xf7fccd2e in ?? ()
#6  0xf7a21728 in ?? ()
#7  0xf7fc8c49 in ?? ()
#8  0x084b3f65 in EnterBaseline (cx=0xf7a21728, cx@entry=0xf7a7f040, data=...) at js/src/jit/BaselineJIT.cpp:125
#9  0x084c63bd in js::jit::EnterBaselineMethod (cx=cx@entry=0xf7a7f040, state=...) at js/src/jit/BaselineJIT.cpp:156
#10 0x082fce78 in js::RunScript (cx=cx@entry=0xf7a7f040, state=...) at js/src/vm/Interpreter.cpp:651
#11 0x082fd510 in js::Invoke (cx=cx@entry=0xf7a7f040, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:738
#12 0x082ff203 in js::Invoke (cx=cx@entry=0xf7a7f040, thisv=..., fval=..., argc=argc@entry=1, argv=argv@entry=0xffffc1f0, rval=rval@entry=...) at js/src/vm/Interpreter.cpp:775
#13 0x08535e0e in js::jit::DoCallFallback (cx=0xf7a7f040, frame=0xffffc220, stub_=0xf7a21350, argc=1, vp=0xffffc1e0, res=...) at js/src/jit/BaselineIC.cpp:9859
#14 0xf7fccd2e in ?? ()
#15 0xf7a21350 in ?? ()
#16 0xf7fc8c49 in ?? ()
#17 0x084b3f65 in EnterBaseline (cx=0xf7a21350, cx@entry=0xf7a7f040, data=...) at js/src/jit/BaselineJIT.cpp:125
#18 0x084c63bd in js::jit::EnterBaselineMethod (cx=cx@entry=0xf7a7f040, state=...) at js/src/jit/BaselineJIT.cpp:156
#19 0x082fce78 in js::RunScript (cx=cx@entry=0xf7a7f040, state=...) at js/src/vm/Interpreter.cpp:651
#20 0x08308246 in js::ExecuteKernel (cx=cx@entry=0xf7a7f040, script=..., script@entry=..., scopeChainArg=..., thisv=..., newTargetValue=..., type=type@entry=js::EXECUTE_GLOBAL, evalInFrame=evalInFrame@entry=..., result=result@entry=0x0) at js/src/vm/Interpreter.cpp:902
#21 0x0830a6a0 in js::Execute (cx=cx@entry=0xf7a7f040, script=script@entry=..., scopeChainArg=..., rval=rval@entry=0x0) at js/src/vm/Interpreter.cpp:936
#22 0x08737a4b in ExecuteScript (cx=cx@entry=0xf7a7f040, scope=..., script=script@entry=..., rval=rval@entry=0x0) at js/src/jsapi.cpp:4331
#23 0x08737b86 in JS_ExecuteScript (cx=cx@entry=0xf7a7f040, scriptArg=scriptArg@entry=...) at js/src/jsapi.cpp:4362
#24 0x0806b167 in RunFile (compileOnly=false, file=0xf7ae49e0, filename=0xffffcf94 "min.js", cx=0xf7a7f040) at js/src/shell/js.cpp:458
#25 Process (cx=cx@entry=0xf7a7f040, filename=0xffffcf94 "min.js", forceTTY=forceTTY@entry=false) at js/src/shell/js.cpp:576
#26 0x080ce597 in ProcessArgs (op=0xffffcbf0, cx=<optimized out>) at js/src/shell/js.cpp:5920
#27 Shell (envp=<optimized out>, op=0xffffcbf0, cx=<optimized out>) at js/src/shell/js.cpp:6189
#28 main (argc=7, argv=0xffffcd44, envp=0xffffcd64) at js/src/shell/js.cpp:6533
eax	0x50	80
ebx	0x9757c6c	158694508
ecx	0xf7c78050	-137920432
edx	0x50	80
esi	0xf7c89000	-137850880
edi	0xf58c7a00	-175343104
ebp	0xffffb3a8	4294947752
esp	0xffffb0a8	4294946984
eip	0xf7c88040	4157112384
=> 0xf7c88040:	add    %al,(%eax)
   0xf7c88042:	xchg   %ax,%ax


Marking s-s because this crash is on the heap. This could be an invalid jump or call from CallJSNative which might be severe in terms of security.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/7cf3406ac1cd
user:        Lars T Hansen
date:        Wed Mar 25 10:51:12 2015 +0100
summary:     Bug 1141121 - Immediate operands to atomics, x86 and x64.  r=h4writer

This iteration took 250.646 seconds to run.
Needinfo from Lars based on comment 1.
Flags: needinfo?(lhansen)
Summary: Crash [@ ??] with asm.js → Crash [@ ??] with atomics in asm.js
Able to repro, there's what appears to be a garbage instruction following the synchronizing instruction.  However since that instruction has encoding 00 00 and is followed by a nop i'm guessing it's filler space that has not been properly nop'ed out.  Investigating.
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Flags: needinfo?(lhansen)
The underlying bug is that the encodings of the 16-bit lock-prefixed instructions ADDW, SUBW, ANDW, ORW, and XOR Ware incorrect: they emit a 32-bit immediate operand instead of a 16-bit immediate operand, leading to two trailing zero bytes, which are interpreted as "add al, (eax)", which will crash if eax is not a valid memory address.
BTW this should be a problem on x86_64 too (the assembler code is shared) but it depends on actual register values, so whether it fails depends on the dynamic history of the program and the register usage of the code preceding the offending operation.
Fixes the problem by introducing proper BaseAssembler implementations for the word operations.  (So far as I can tell from the Intel manuals the optimizations for byte operands and dest=EAX carry over even to 16-bit operations with the prefix.)

These are hard to test; I've generalized the test case from the bug report as a stopgap.  The JS shell does crash with this test case, without the patch.
Attachment #8643007 - Flags: review?(sunfish)
Comment on attachment 8643007 [details] [diff] [review]
bug1183060-x86-word-ops.diff

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

Looks good.
Attachment #8643007 - Flags: review?(sunfish) → review+
https://hg.mozilla.org/mozilla-central/rev/3ed3258d336a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx42
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.