689892 - Assertion failure: isInterpreted(), at ../../jsfun.h:199 or Crash [@ js::gc::Cell::compartment]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test asserts on mozilla-central revision 7f4867717226 (options -m -n -a):


function foo2(x, n) {
  var i = 0;
  while (--n >= 0) {
    x[i++] = 0;
  }
}
foo2([1,0Xae ,3,4,5],('0123456'));


Stepping through the assert crashes:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000412a94 in js::gc::Cell::compartment (this=0xfff8800000000000) at ../../jsgc.h:809
809         return arenaHeader()->compartment;
(gdb) bt
#0  0x0000000000412a94 in js::gc::Cell::compartment (this=0xfff8800000000000) at ../../jsgc.h:809
#1  0x00000000006e2a0e in PopActiveVMFrame (f=...) at /srv/repos/mozilla-central/js/src/methodjit/MethodJIT.cpp:143
#2  0x00000000006e2abd in JaegerTrampolineReturn () at /srv/repos/mozilla-central/js/src/methodjit/MethodJIT.cpp:153
#3  0x00007ffff7f61368 in ?? ()
#4  0x0000000000000001 in ?? ()
#5  0x0000000000000000 in ?? ()

Comment 1

[Brian Hackett [Laid off!]]

Attachment: patch

Problem with hoisting bounds checks out of loops.  We come up with the correct hoisted condition for the x[i++], except for the fact that the type of 'n' changes in the decrement --- the loop test will be on integers, but when entering the loop 'n' may still be a string (we checked the type of the decrement's result, but not its input).  When checking the hoisted condition on loop entry we use the payload of 'n' as if it is an integer and get the wrong result.  The fix makes sure that all increment/arithmetic operations used when hoisting checks have both integer results and integer inputs.

Comment 2

[Brian Hackett [Laid off!]]

Also, TI regression, so doesn't affect Firefox 8 or earlier.

Comment 3

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/28a549e90410

Comment 4

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/mozilla-central/rev/28a549e90410

Comment 5

[LegNeato]

Brian: What could be broken by this if we leave it in? How could this patch go wrong? Do we know what checking caused this other than just "TI"? Thanks!

Comment 6

[Brian Hackett [Laid off!]]

I think this logic was added by bug 650715 (added SSA analysis, a core TI patch).  I think the odds of a page stumbling across this crash by chance are low, but if found the crash could be exploited (buffer overflow).

Comment 7

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a4123ca7c79a

Comment 8

[u279076]

qa- as QA cannot easily verify this fix. Can someone who is already set up to test this bug please verify the fix?

Comment 9

[Bob Clary [:bc] (inactive)]

Automation hit something similar on Windows 7/Beta 8 debug but not nightly opt. It is not reproducible.

http://www.neimanmarcus.com/search.jhtml?No=0%26Ntt=kate%2Bspade%26_requestid=48919%26N=0%26pageSize=160

Operating system: Windows NT
                  6.1.7601 Service Pack 1
CPU: x86
     GenuineIntel family 6 model 44 stepping 2
     1 CPU

Crash reason:  EXCEPTION_ACCESS_VIOLATION_READ
Crash address: 0x0

Thread 0 (crashed)
 0  mozjs.dll!js::gc::Cell::compartment() [jsgc.h : 755 + 0x8]
    eip = 0x6794b0ef   esp = 0x00148dcc   ebp = 0x00148dd0   ebx = 0x00000000
    esi = 0x67e9aff8   edi = 0xffffff87   eax = 0x00000000   ecx = 0x00000008
    edx = 0x00000000   efl = 0x00210246
    Found by: given as instruction pointer in context
 1  mozjs.dll!JSObject::getCompartment() [jsgc.h : 1514 + 0x7]
    eip = 0x6794b0cf   esp = 0x00148dd8   ebp = 0x00148ddc
    Found by: call frame info
 2  mozjs.dll!JSAutoEnterCompartment::enter(JSContext *,JSObject *) [jsapi.cpp : 1242 + 0x7]
    eip = 0x6794b060   esp = 0x00148de4   ebp = 0x00148de8
    Found by: call frame info
 3  xul.dll!NativeInterface2JSObject [nsXPConnect.cpp : 1293 + 0x15]
    eip = 0x624f1ba5   esp = 0x00148df0   ebp = 0x00148e28
    Found by: call frame info
 4  xul.dll!nsXPConnect::WrapNativeToJSVal(JSContext *,JSObject *,nsISupports *,nsWrapperCache *,nsID const *,int,jsval_layout *,nsIXPConnectJSObjectHolder * *) [nsXPConnect.cpp : 1358 + 0x27]
    eip = 0x624f1e2b   esp = 0x00148e30   ebp = 0x00148f1c
    Found by: call frame info
 5  xul.dll!WrapNative [nsDOMClassInfo.cpp : 1754 + 0x35]
    eip = 0x620d4e2a   esp = 0x00148f24   ebp = 0x00148f50
    Found by: call frame info
 6  xul.dll!WrapNative [nsDOMClassInfo.cpp : 1765 + 0x22]
    eip = 0x620d4d76   esp = 0x00148f58   ebp = 0x00148f78
    Found by: call frame info
 7  xul.dll!nsWindowSH::GetProperty(nsIXPConnectWrappedNative *,JSContext *,JSObject *,jsid,jsval_layout *,int *) [nsDOMClassInfo.cpp : 5264 + 0x46]
    eip = 0x620e065d   esp = 0x00148f80   ebp = 0x00149010
    Found by: call frame info
 8  xul.dll!XPC_WN_Helper_GetProperty [xpcwrappednativejsops.cpp : 997 + 0x25]
    eip = 0x6254795e   esp = 0x00149018   ebp = 0x00149048
    Found by: call frame info
 9  mozjs.dll!js::CallJSPropertyOp(JSContext *,int (*)(JSContext *,JSObject *,jsid,js::Value *),JSObject *,jsid,js::Value *) [jscntxtinlines.h : 333 + 0x12]
    eip = 0x67a41f45   esp = 0x00149050   ebp = 0x00149064
    Found by: call frame info
10  mozjs.dll!js::Shape::get(JSContext *,JSObject *,JSObject *,JSObject *,js::Value *) [jsscopeinlines.h : 284 + 0x50]
    eip = 0x67a4a4c8   esp = 0x0014906c   ebp = 0x00149094
    Found by: call frame info

If that is distinct from this, let me know and I'll file a bug as soon as I can reproduce reliably.

Comment 10

[Johnny Stenback (:jst)]

Brian, any reason we wouldn't want this on beta to fix this for 8?

Comment 11

[Brian Hackett [Laid off!]]

(In reply to Johnny Stenback (:jst, [e-mail]) from comment #10)
> Brian, any reason we wouldn't want this on beta to fix this for 8?

This is a TI bug so won't affect 8.  The stack in comment 9 is different and looks like a compartment or DOM problem (crashing under WrapNative).

Comment 12

[Johnny Stenback (:jst)]

Thanks for the clarification!

Comment 13

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929