Closed Bug 625251 Opened 13 years ago Closed 13 years ago

Assertion failure: strComp == right->asCell()->compartment() || right->asCell()->compartment() == rt->defaultCompartment

Categories

(Core :: JavaScript Engine, defect)

x86
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: bc, Assigned: luke)

References

()

Details

(Keywords: assertion, reproducible, testcase, Whiteboard: [compartments][hardblocker][fixed-in-tracemonkey][fx4-fixed-bugday])

Attachments

(2 files)

1. http://robstendreams.blogspot.com/2011/01/award-show-schedule-as-we-know-it.html
2. Assertion failure: strComp == right->asCell()->compartment() || right->asCell()->compartment() == rt->defaultCompartment, at c:\work\mozilla\builds\2.0.0\mozilla\js\src\jsgcinlines.h:491

So far I can reproduce this from the url but not from a saved copy so I might not be able to get a test case. You should look soon in case something changes.

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

Crash reason:  EXCEPTION_ACCESS_VIOLATION_WRITE
Crash address: 0x0

Thread 0 (crashed)
 0  mozjs.dll!JS_Assert [jsutil.cpp : 73 + 0x0]
    eip = 0x0080115a   esp = 0x0012d08c   ebp = 0x0012d08c   ebx = 0x00000001
    esi = 0x01716e30   edi = 0x00000000   eax = 0x00000000   ecx = 0x78e6bb55
    edx = 0x00613d38   efl = 0x00210202
    Found by: given as instruction pointer in context
 1  mozjs.dll!js::gc::TypedMarker [jsgcinlines.h : 491 + 0x50]
    eip = 0x0068d7f2   esp = 0x0012d094   ebp = 0x0012d0bc
    Found by: previous frame's frame pointer
 2  mozjs.dll!js::gc::Mark<JSString> [jsgcinlines.h : 222 + 0xc]
    eip = 0x0068d4be   esp = 0x0012d0c4   ebp = 0x0012d0d4
    Found by: call frame info
 3  mozjs.dll!js::gc::MarkString [jsgcinlines.h : 239 + 0xc]
    eip = 0x0068d2db   esp = 0x0012d0dc   ebp = 0x0012d0e4
    Found by: call frame info
 4  mozjs.dll!js::gc::MarkKind [jsgcinlines.h : 582 + 0xc]
    eip = 0x0068d136   esp = 0x0012d0ec   ebp = 0x0012d0f8
    Found by: call frame info
 5  mozjs.dll!js::gc::MarkValueRaw [jsgcinlines.h : 600 + 0x1a]
    eip = 0x0068d002   esp = 0x0012d100   ebp = 0x0012d10c
    Found by: call frame info
 6  mozjs.dll!js::gc::MarkValue [jsgcinlines.h : 608 + 0xc]
    eip = 0x0068cf91   esp = 0x0012d114   ebp = 0x0012d11c
    Found by: call frame info
 7  mozjs.dll!array_trace [jsarray.cpp : 933 + 0x1a]
    eip = 0x0068cf2e   esp = 0x0012d124   ebp = 0x0012d138
    Found by: call frame info
 8  mozjs.dll!js::gc::MarkChildren [jsgcinlines.h : 289 + 0x1f]
    eip = 0x0075f7fd   esp = 0x0012d140   ebp = 0x0012d160
    Found by: call frame info
 9  mozjs.dll!js::gc::TypedMarker [jsgcinlines.h : 347 + 0xc]
    eip = 0x0075f6e0   esp = 0x0012d168   ebp = 0x0012d174
    Found by: call frame info
10  mozjs.dll!js::gc::Mark<JSObject> [jsgcinlines.h : 222 + 0xc]
    eip = 0x0075f5be   esp = 0x0012d17c   ebp = 0x0012d18c
    Found by: call frame info
In case it wasn't clear, this is gc compartments.
blocking2.0: --- → ?
I can't reproduce here. I don't have windows but I tried with Mac and Linux.
I can reproduce on mac os x 10.5 intel with a debug 2.0 build.

clean up the summary and leave a bread crumb for matching related issues:
JS_Assert | js::gc::TypedMarker js::gc::Mark js::gc::MarkString js::gc::MarkKind js::gc::MarkValueRaw
OS: Windows XP → All
Summary: Assertion failure: strComp == right->asCell()->compartment() || right->asCell()->compartment() == rt->defaultCompartment JS_Assert | js::gc::TypedMarker js::gc::Mark js::gc::MarkString js::gc::MarkKind js::gc::MarkValueRaw → Assertion failure: strComp == right->asCell()->compartment() || right->asCell()->compartment() == rt->defaultCompartment
Assignee: general → anygregor
blocking2.0: ? → betaN+
We have to find out whats up here. bc, can you post STR?
Attached file test
Wait a few seconds for the assert to fire.

<iframe src="http://chatroll.com/embed/chat/robsten-dreams-im?id=QU1Q-Lpo86t&amp;platform=blogger&amp;w=$0">
</iframe>

It seems to require loading via an iframe. I haven't been able to get it to reproduce running the included page by itself either online or locally.

Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x066bfcd1 in JS_Assert (s=0x6baad9c "strComp == right->asCell()->compartment() || right->asCell()->compartment() == rt->defaultCompartment", file=0x6baa4c8 "/work/mozilla/builds/2.0.0/mozilla/js/src/jsgcinlines.h", ln=491) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsutil.cpp:80
80	    *((int *) NULL) = 0;  /* To continue from here in GDB: "return" then "continue". */
(gdb) bt
#0  0x066bfcd1 in JS_Assert (s=0x6baad9c "strComp == right->asCell()->compartment() || right->asCell()->compartment() == rt->defaultCompartment", file=0x6baa4c8 "/work/mozilla/builds/2.0.0/mozilla/js/src/jsgcinlines.h", ln=491) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsutil.cpp:80
#1  0x0654a260 in js::gc::TypedMarker (trc=0xbfffcd9c, str=0x282162c0) at jsgcinlines.h:489
#2  0x0654a49c in js::gc::Mark<JSString> (trc=0xbfffcd9c, thing=0x28216310) at jsgcinlines.h:222
#3  0x0654a549 in js::gc::MarkString (trc=0xbfffcd9c, str=0x28216310) at jsgcinlines.h:239
#4  0x0654d32d in js::gc::MarkKind (trc=0xbfffcd9c, thing=0x28216310, kind=1) at jsgcinlines.h:582
#5  0x0654d3db in js::gc::MarkValueRaw (trc=0xbfffcd9c, v=@0x23fefdf8) at jsgcinlines.h:600
#6  0x0654d41b in js::gc::MarkValue (trc=0xbfffcd9c, v=@0x23fefdf8, name=0x6bb14d3 "dense_array_elems") at jsgcinlines.h:608
#7  0x0654d496 in array_trace (trc=0xbfffcd9c, obj=0x23fefdd0) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsarray.cpp:933
#8  0x0660e0e0 in js::gc::MarkChildren (trc=0xbfffcd9c, obj=0x23fefdd0) at jsgcinlines.h:289
#9  0x0660dcc1 in js::gc::TypedMarker (trc=0xbfffcd9c, thing=0x23fefdd0) at jsgcinlines.h:347
#10 0x0660de28 in js::gc::Mark<JSObject> (trc=0xbfffcd9c, thing=0x23fefdd0) at jsgcinlines.h:222
#11 0x0660e39c in js::gc::MarkKind (trc=0xbfffcd9c, thing=0x23fefdd0, kind=0) at jsgcinlines.h:579
#12 0x0660e457 in js::gc::MarkValueRaw (trc=0xbfffcd9c, v=@0x23fed7c0) at jsgcinlines.h:600
#13 0x06610700 in js_TraceObject (trc=0xbfffcd9c, obj=0x23fed798) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsobj.cpp:6485
#14 0x0660e0e0 in js::gc::MarkChildren (trc=0xbfffcd9c, obj=0x23fed798) at jsgcinlines.h:289
#15 0x0660dcc1 in js::gc::TypedMarker (trc=0xbfffcd9c, thing=0x23fed798) at jsgcinlines.h:347
#16 0x0660de28 in js::gc::Mark<JSObject> (trc=0xbfffcd9c, thing=0x23fed798) at jsgcinlines.h:222
#17 0x0660e39c in js::gc::MarkKind (trc=0xbfffcd9c, thing=0x23fed798, kind=0) at jsgcinlines.h:579
#18 0x0660e457 in js::gc::MarkValueRaw (trc=0xbfffcd9c, v=@0x2644a178) at jsgcinlines.h:600
#19 0x06610700 in js_TraceObject (trc=0xbfffcd9c, obj=0x23f430a0) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsobj.cpp:6485
#20 0x0652bc68 in js::gc::MarkChildren (trc=0xbfffcd9c, obj=0x23f430a0) at jsgcinlines.h:289
#21 0x0652b6c5 in js::gc::TypedMarker (trc=0xbfffcd9c, thing=0x23f430a0) at jsgcinlines.h:347
#22 0x0652b82c in js::gc::Mark<JSObject> (trc=0xbfffcd9c, thing=0x23f430a0) at jsgcinlines.h:222
#23 0x0652bb62 in js::gc::MarkObject (trc=0xbfffcd9c, obj=@0x23f430a0, name=0x6ac6ef0 "parent") at jsgcinlines.h:263
#24 0x0652bbd2 in js::gc::MarkChildren (trc=0xbfffcd9c, obj=0x23f430f0) at jsgcinlines.h:277
#25 0x0652b6c5 in js::gc::TypedMarker (trc=0xbfffcd9c, thing=0x23f430f0) at jsgcinlines.h:347
#26 0x0652b82c in js::gc::Mark<JSObject> (trc=0xbfffcd9c, thing=0x23f430f0) at jsgcinlines.h:222
#27 0x0652bb62 in js::gc::MarkObject (trc=0xbfffcd9c, obj=@0x23f430f0, name=0x6bb6b9c "proto") at jsgcinlines.h:263
#28 0x0652bbad in js::gc::MarkChildren (trc=0xbfffcd9c, obj=0x23f43118) at jsgcinlines.h:275
#29 0x0652b6c5 in js::gc::TypedMarker (trc=0xbfffcd9c, thing=0x23f43118) at jsgcinlines.h:347
#30 0x0652b82c in js::gc::Mark<JSObject> (trc=0xbfffcd9c, thing=0x23f43118) at jsgcinlines.h:222
#31 0x0652bb62 in js::gc::MarkObject (trc=0xbfffcd9c, obj=@0x23f43118, name=0x6bb6b9c "proto") at jsgcinlines.h:263
#32 0x0652bbad in js::gc::MarkChildren (trc=0xbfffcd9c, obj=0x23f45150) at jsgcinlines.h:275
#33 0x0652b6c5 in js::gc::TypedMarker (trc=0xbfffcd9c, thing=0x23f45150) at jsgcinlines.h:347
#34 0x0652b82c in js::gc::Mark<JSObject> (trc=0xbfffcd9c, thing=0x23f45150) at jsgcinlines.h:222
#35 0x0652bb62 in js::gc::MarkObject (trc=0xbfffcd9c, obj=@0x23f45150, name=0x6bb6b9c "proto") at jsgcinlines.h:263
#36 0x0652bbad in js::gc::MarkChildren (trc=0xbfffcd9c, obj=0x23f47048) at jsgcinlines.h:275
#37 0x0652b6c5 in js::gc::TypedMarker (trc=0xbfffcd9c, thing=0x23f47048) at jsgcinlines.h:347
#38 0x0652b82c in js::gc::Mark<JSObject> (trc=0xbfffcd9c, thing=0x23f47048) at jsgcinlines.h:222
#39 0x0652ffc4 in js::gc::MarkKind (trc=0xbfffcd9c, thing=0x23f47048, kind=0) at jsgcinlines.h:579
#40 0x065301db in JS_CallTracer (trc=0xbfffcd9c, thing=0x23f47048, kind=0) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsapi.cpp:2141
#41 0x05aa9fc1 in XPCJSRuntime::TraceXPConnectRoots (this=0x5445c0, trc=0xbfffcd9c) at /work/mozilla/builds/2.0.0/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp:411
#42 0x05aaa2b0 in XPCJSRuntime::TraceJS (trc=0xbfffcd9c, data=0x5445c0) at /work/mozilla/builds/2.0.0/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp:372
#43 0x065b64d0 in js::MarkRuntime (trc=0xbfffcd9c) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsgc.cpp:1751
#44 0x065b679c in MarkAndSweep (cx=0x1a1d0fd0, gckind=GC_NORMAL) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsgc.cpp:2407
#45 0x065b6e35 in GCUntilDone (cx=0x1a1d0fd0, comp=0x0, gckind=GC_NORMAL) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsgc.cpp:2733
#46 0x065b702f in js_GC (cx=0x1a1d0fd0, comp=0x0, gckind=GC_NORMAL) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsgc.cpp:2804
#47 0x0652fd3a in JS_GC (cx=0x1a1d0fd0) at /work/mozilla/builds/2.0.0/mozilla/js/src/jsapi.cpp:2547
#48 0x05a78ac8 in nsXPConnect::Collect (this=0x544530) at /work/mozilla/builds/2.0.0/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:407
#49 0x05a72034 in nsXPConnect::GarbageCollect (this=0x544530) at /work/mozilla/builds/2.0.0/mozilla/js/src/xpconnect/src/nsXPConnect.cpp:415
#50 0x054f9145 in nsJSContext::CC (aListener=0x0) at /work/mozilla/builds/2.0.0/mozilla/dom/base/nsJSEnvironment.cpp:3630
#51 0x054f9202 in nsJSContext::IntervalCC () at /work/mozilla/builds/2.0.0/mozilla/dom/base/nsJSEnvironment.cpp:3735
#52 0x054f93dc in nsJSContext::CCIfUserInactive () at /work/mozilla/builds/2.0.0/mozilla/dom/base/nsJSEnvironment.cpp:3716
#53 0x054f9616 in GCTimerFired (aTimer=0x277f63b0, aClosure=0x0) at /work/mozilla/builds/2.0.0/mozilla/dom/base/nsJSEnvironment.cpp:3761
We are performing a global GC (MarkAndSweep) and we try to mark a rope string.
One of the right rope-nodes is in a different compartment that is not the defaultCompartment.

I guess we need some compartment check in the rope construction code. Currently it looks like:

    JSString *newRoot = js_NewGCString(cx);
    if (!newRoot)
        return NULL;

    newRoot->initRopeNode(left, right, wholeLength);
I added another assert in js_ConcatStrings that all strings are in the same compartment. 
I hit this assertion now with this site:

 	mozjs.dll!JS_Assert(const char * s=0x0096d050, const char * file=0x0096d038, int ln=290)  Line 73	C++
>	mozjs.dll!js_ConcatStrings(JSContext * cx=0x026b29f8, JSString * left=0x055c33d0, JSString * right=0x0fd277f0)  Line 290 + 0x54 bytes	C++
 	mozjs.dll!js::mjit::stubs::Add(js::VMFrame & f={...})  Line 1090 + 0xf bytes	C++
 	19cc57a0()	
 	xul.dll!nsPrincipal::Equals(nsIPrincipal * aOther=0x020e0050, int * aResult=0x0012aca8)  Line 374 + 0xa bytes	C++
 	mozjs.dll!JS_FrameIterator(JSContext * cx=0x026b29f8, JSStackFrame * * iteratorp=0x0012ac80)  Line 1189 + 0x11 bytes	C++
 	xul.dll!nsScriptSecurityManager::GetPrincipalAndFrame(JSContext * cx=0x026b29f8, JSStackFrame * * frameResult=0x0012acc0, unsigned int * rv=0x0012acd4)  Line 2343 + 0xe bytes	C++
 	xul.dll!nsScriptSecurityManager::GetSubjectPrincipal(JSContext * cx=0x026b29f8, unsigned int * rv=0x0012acd4)  Line 2359	C++
Whiteboard: [compartments] → [compartments][hardblocker]
The string in question is "u = {chars=0x15fa8aa0 "Permission denied for <http://chatroll.com> to call method Location.toString" left=0x15fa8aa0 }"
I guess this needs a fix in xpconnect. "Permission denied for ...." seems to be a system string. This string is passed to JS_ConcatStrings with a string that was allocated in another compartment. Others have more experience fixing these kind of bugs :) Andreas, Blake?

I could add something in js_ConcatStrings that checks if the strings are in a different compartment than the current one and make a copy but I don't think that's what we want.
(In reply to comment #9)
> I could add something in js_ConcatStrings that checks if the strings are in a
> different compartment than the current one and make a copy but I don't think
> that's what we want.

Agreed, js_ConcatStrings is a very hot path, best to keep as much stuff of it as possible.
I dug in a little.  Fist of all, our great wall of assertSameCompartment does not check same-compartment-ness for strings.  From that, it looks like the wrong-compartment string is creeping in through a JS_CallFunctionValue argument, passed by nsJSContext::CallEventHandler.
So the mismatch occurs from within the CallEventHandler call to ConvertSupportsTojsvals (for the argument array).  CallEventHandler passes the correct destination scope which makes its way down to XPCVariant::VariantData2JS which calls XPCConvert::NativeStringWithSize2JS (leaving the scope behind) which uses the cx's current compartment, which is wrong.

Other code in NativeStringWithSize2JS JSAutoEnterCompartment on scope, so wrapping that around NativeStringWithSizeJS seems like the simple fix.  Without knowing too much about this code, I wonder why we can't just have one JSAutoEnterCompartment either at the top of this function (to cover everything) or even higher up.
Attached patch fix (?)Splinter Review
The underlying issue is that the nsJSEnvironment's context has code running unrelated to the error event being delivered.  This is already dealt with by the PushContextPrincipal/JSAutoEnterCompartment immediately after ConvertSupportsTojsvals.  Swapping the order of these (as attached) thus makes the context's scope chain match the target and makes the assert go away.

So, looking more at VariantDataToJS, I see a bunch of places where, if the context is not in the right compartment, we create gc things in the wrong compartment.  Maybe I'm missing why most of these cases are ok, but it makes me think that maybe, instead of these scattered calls to AutoEnterCompartment, we should just require that the context is in the desired compartment (and take out the scope parameter and all the few existing AutoEnterCompartment calls).  Passing around a wrong-compartment context seems like asking for trouble.

Blake, Andreas?
Assignee: anygregor → lw
Status: NEW → ASSIGNED
Attachment #505990 - Flags: feedback?(mrbkap)
Attachment #505990 - Flags: feedback?(mrbkap) → review?(mrbkap)
The new assert and a single enter around it sounds very reasonable.
Attachment #505990 - Flags: review?(mrbkap) → review+
http://hg.mozilla.org/tracemonkey/rev/c22d4fa6edee
Whiteboard: [compartments][hardblocker] → [compartments][hardblocker][fixed-in-tracemonkey]
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre
Status: RESOLVED → VERIFIED
Whiteboard: [compartments][hardblocker][fixed-in-tracemonkey] → [compartments][hardblocker][fixed-in-tracemonkey][fx4-fixed-bugday]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: