668643 - TI+JM: No comply demo crashes in mjit-generated code

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

https://mozillademos.org/demos/nocomply/demo.html

crashes on OS X, both 32-bit and 64-bit, using a nightly build from revision ffddf68888de.

It looks like a crash in LICM code:
======
0x3650d11:	call   0x13b1d00 <_ZN2js4mjit5stubs16InvariantFailureERNS_7VMFrameEPv>
0x3650d16:	mov    0x1c(%esp),%ebp

// Hoisted dense array slots?
0x3650d1a:	cmpl   $0xffff0007,0x3c(%ebp)
0x3650d21:	jne    0x3650dc4
0x3650d27:	mov    0x38(%ebp),%ebx
0x3650d2a:	mov    0x24(%ebx),%ebx
0x3650d2d:	mov    %ebx,0xf0(%ebp)

// Hoisted bounds check?
0x3650d33:	mov    0xf8(%ebp),%ebx
0x3650d39:	mov    0x10(%ebx),%ebx <= CRASH
0x3650d3c:	mov    0x58(%ebp),%edx
0x3650d3f:	cmp    %edx,%ebx
0x3650d41:	jle    0x3650dc4

0x3650d47:	cmpl   $0xffff0007,-0x4(%ebp)
0x3650d4e:	jne    0x3650dc4
======
ebx has value 139 so it looks like 0xf8(%ebp) is bogus

Comment 1

[Brian Hackett [Laid off!]]

Kind of tricky (yeah, LICM), when removing redundant hoisted checks in a loop we could violate constraints on how the invariant entries must be ordered.  In this case we are hoisting a check on an access on an array which is itself a loop invaraint property access.  We reordered the check before the invariant entry which computed the array itself, so when we tried to read the temporary at 0xf8 it hadn't been filled in yet (if you disassemble more the writes to 0xf8 appear later).  The fix preserves ordering constraints when removing redundant loop invariant entries.

http://hg.mozilla.org/projects/jaegermonkey/rev/90768623f7ec