Closed Bug 1450796 Opened 2 years ago Closed 2 years ago

Assertion failure: index != length, at js/src/jit/x64/MacroAssembler-x64-inl.h:859 with TypedArray

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: decoder, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 8063b0c54b88 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --cpu-count=2 --ion-eager --baseline-eager):

var lfLogBuffer = `
function S() {
  var t = new Float32Array(1);
  var k = t.length;
  var xx = 19;
  var gridRes = 64;
  t[k] = -1 + 2 * xx / gridRes;
}
assertEq(S(), -0.40625);
`;
loadFile(lfLogBuffer);
loadFile(lfLogBuffer);
function loadFile(lfVarx) {
    try {
      evaluate(lfVarx);
    } catch (lfVare) {}
}


Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x00000000006cefe8 in js::jit::MacroAssembler::spectreBoundsCheck32 (this=<optimized out>, index=..., index@entry=..., length=..., maybeScratch=..., maybeScratch@entry=..., failure=failure@entry=0x7ffff5cfcb60) at js/src/jit/x64/MacroAssembler-x64-inl.h:859
#1  0x000000000069eb9f in js::jit::CodeGenerator::visitStoreTypedArrayElementHole (this=this@entry=0x7ffff54d3000, lir=lir@entry=0x7ffff5fd84e0) at js/src/jit/CodeGenerator.cpp:11837
#2  0x00000000006b9c76 in js::jit::CodeGenerator::generateBody (this=this@entry=0x7ffff54d3000) at js/src/jit/CodeGenerator.cpp:5822
#3  0x00000000006bb000 in js::jit::CodeGenerator::generate (this=this@entry=0x7ffff54d3000) at js/src/jit/CodeGenerator.cpp:10166
#4  0x00000000006e74a6 in js::jit::GenerateCode (mir=mir@entry=0x7ffff5fd11c0, lir=0x7ffff5fd6918) at js/src/jit/Ion.cpp:1976
#5  0x0000000000740b29 in js::jit::CompileBackEnd (mir=mir@entry=0x7ffff5fd11c0) at js/src/jit/Ion.cpp:1998
#6  0x0000000000b15c5a in js::HelperThread::handleIonWorkload (this=0x7ffff5f27098, locked=...) at js/src/vm/HelperThreads.cpp:1837
#7  0x0000000000aff6ac in js::HelperThread::threadLoop (this=this@entry=0x7ffff5f27098) at js/src/vm/HelperThreads.cpp:2249
#8  0x0000000000aff7e0 in js::HelperThread::ThreadMain (arg=0x7ffff5f27098) at js/src/vm/HelperThreads.cpp:1734
#9  0x0000000000b16fe2 in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::callMain<0ul> (this=0x7ffff5f18060) at js/src/threading/Thread.h:242
#10 js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start (aPack=0x7ffff5f18060) at js/src/threading/Thread.h:235
#11 0x00007ffff7bc16ba in start_thread (arg=0x7ffff5cfe700) at pthread_create.c:333
#12 0x00007ffff6c383dd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
rax	0x0	0
rbx	0x7ffff54d3000	140737308864512
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7ffff5cfcb40	140737317423936
rsp	0x7ffff5cfcac0	140737317423808
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff5cfe700	140737317431040
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7ffff5cfcb60	140737317423968
r13	0x7ffff5fd84e0	140737320420576
r14	0x0	0
r15	0x6	6
rip	0x6cefe8 <js::jit::MacroAssembler::spectreBoundsCheck32(js::jit::Register, js::jit::Register, js::jit::Register, js::jit::Label*)+488>
=> 0x6cefe8 <js::jit::MacroAssembler::spectreBoundsCheck32(js::jit::Register, js::jit::Register, js::jit::Register, js::jit::Label*)+488>:	movl   $0x0,0x0
   0x6ceff3 <js::jit::MacroAssembler::spectreBoundsCheck32(js::jit::Register, js::jit::Register, js::jit::Register, js::jit::Label*)+499>:	ud2


Test uses TypedArray, assertion about array index and length, plus this seems to be about Spectre and bounds checking, marking s-s.
Probably a bogus assert.
Flags: needinfo?(jdemooij)
Group: javascript-core-security
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
Just an overzealous assert - the |index == length| case is silly but the code still works fine.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8965729 - Flags: review?(luke)
Attachment #8965729 - Flags: review?(luke) → review+
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/7aa5b7d5198c
user:        Jan de Mooij
date:        Wed Mar 28 16:07:10 2018 +0200
summary:     Bug 1445235 part 3 - Use Spectre-safe bounds check for LStoreTypedArrayElementHole. r=nbp

This iteration took 282.483 seconds to run.
Comment on attachment 8965729 [details] [diff] [review]
Patch

Removes a spurious assert added by the soon to be uplifted bug 1445235. Approved for 60.0b11.
Attachment #8965729 - Flags: approval-mozilla-beta+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9cecc20bead
Remove overzealous assert in spectreBoundsCheck32. r=luke
https://hg.mozilla.org/mozilla-central/rev/b9cecc20bead
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.