765416 - weird stuff with cross fuzzing

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, defect)

Description

[Mats Palmgren (inactive)]

Attachment: js::CompartmentChecker::fail from nsXPConnect::MoveWrappers

+++ This bug was initially created as a clone of Bug #760745 +++

Using a local mozilla-inbound debug build I get a couple
of compartment assertion when running cross_fuzz.

1. Start Browser with new profile
2. Uncheck "block popup windows" pref (not sure if this is needed)
3. Open URL
4. every now and then, manually focus the tab opened in 3,
   close popup windows, close other tabs.  (it seems the test
   runs slowly when the original tab is in the background)

ACTUAL RESULTS
###!!! ASSERTION: Weird scope returned: 'betterScope == newScope', file d:/moz/inbound/js/xpconnect/src/nsXPConnect.cpp, line 1637
*** Compartment mismatch 04D630A8 vs. 0BF53A98
Assertion failure: compartment mismatched, at d:\moz\inbound\js\src\jscntxtinlines.h:237


It might be a regression in the last week or so - I didn't
get these compartment assertions when testing bug 760745
a week ago.

Comment 1

[Mats Palmgren (inactive)]

Attachment: js::CompartmentChecker::fail from js::CrossCompartmentWrapper::get

Comment 2

[Mats Palmgren (inactive)]

BTW, I still have the first crash in the debugger, let me know if you want
any specific values.

Comment 3

[Mats Palmgren (inactive)]

Attachment: JS_ASSERT in EndMarkPhase

Another JS assertion for the same test, not sure if it's related
to the compartment issue.

Comment 4

[Bobby Holley (:bholley)]

Let's make this s-s for now.

I wonder if this is the same as bug 732870. Can you reproduce it with that patch?

Comment 5

[Bobby Holley (:bholley)]

Also, if you do have the first crash - can you go up to the MoveWrapper frame, and do wrapper->mFlatJSObject->dump()?

Comment 6

[Mats Palmgren (inactive)]

I had do something else on this box so the debugger session was lost, sorry.

Comment 7

[Mats Palmgren (inactive)]

Attachment: stack 4, CompartmentChecker::fail

This is the latest mozilla-inbound with the patch in bug 732870 applied.
I'll leave the debugger open, let me know if you want any values.

Comment 8

[Bobby Holley (:bholley)]

can you do obj->dump() on frame 3?

Comment 9

[Mats Palmgren (inactive)]

object 0BB2C650
class 7077FE08 Proxy
flags:
proto <Proxy object at 0BB2C600>
parent <Window object at 0BB14040>
not native
slots:
   0 (reserved) = 9,32335e-315
   1 (reserved) = <function eval at 10543480>
   2 (reserved) = undefined
   3 (reserved) = undefined
   4 (reserved) = <function eval at 10543480>
   5 (reserved) = undefined

Comment 10

[Bobby Holley (:bholley)]

js::UnwrapObject(obj)->dump()?

Comment 11

[Mats Palmgren (inactive)]

object 10543480
class 7077DF38 Function
flags:
proto <unnamed function (:0) at 10543020>
parent <Window object at 105E6040>
properties:
    ((Shape *) 117CD8B0) readonly permanent JSString* (04612800) = jschar * (04612808) = "name"
: slot 0 = JSString* (04612660) = jschar * (04612668) = "eval"

Comment 12

[Bobby Holley (:bholley)]

Ok, I'll try to take a look at it sometime soon. I'm generally loath to debug crossfuzz testcases because they're such a pain to reproduce. But I'll give it a once I dig myself out from under my existing pile of security bugs.

mccr8 might also be interested in looking at this if he has the cycles.

Comment 13

[Bobby Holley (:bholley)]

I tried reproducing this for a while, but couldn't (though at one point I hit an assertion in the gc). :-(

Comment 14

[Daniel Veditz [:dveditz]]

Jesse: can you take a look at this and see if your fuzzer's incorporation of cross_fuzz techniques might hit this error? Or if not see if there are some tweaks we need?

Comment 15

[Jesse Ruderman]

This might be a dup of bug 732870.  Even if it's not, I wouldn't start worrying unless this bug persists after bug 766430, bug 767273, and bug 772215 are fixed.

Comment 16

[Alex Keybl [:akeybl]]

(In reply to Jesse Ruderman from comment #15)
> This might be a dup of bug 732870.  Even if it's not, I wouldn't start
> worrying unless this bug persists after bug 766430, bug 767273, and bug
> 772215 are fixed.

Are we seeing any more assertions in FF15 after bug 732870 landed?

Comment 17

[Mats Palmgren (inactive)]

Yes, the fatal "compartment mismatched" assertion is still easy to
reproduce.  The non-fatal "Weird scope" assertion that preceded it
seems to be gone now. (mozilla-inbound debug build on Win7)

Comment 18

[David Bolter [:davidb] (NeedInfo me for attention)]

Bobby are you back from holiday?

Comment 19

[Bobby Holley (:bholley)]

I just spent another half hour messing around with the testcase (on osx 10.7). At least for me, it's really erratic. I once managed to reproduce the compartment mismatch, but couldn't reproduce it in a debugger (though I managed to reproduce a number of other assertions/aborts from all over the place - svg etc).

Given that I only managed to reproduce the issue once in that span of time, and given that fixing/verifying these kinds of bugs generally requires reproducing them dozens of times, the path ahead doesn't seem tractable enough to make this a priority for me. I'm unassigning myself from this bug to draw attention to the fact that I consider myself blocked here. Happy to talk about this more on IRC etc.

Comment 20

[Mats Palmgren (inactive)]

Attachment: disable-svg-abort

Sorry if I didn't mention it, but XPCOM_DEBUG_BREAK=warn is assumed
since you wont get very far without that.

Also, disabling this fatal SVG assertion helps.

Comment 21

[Mats Palmgren (inactive)]

Disabling all plugins also helps avoiding unrelated crashes and
assertions.  Unchecking the pref "Open new windows in a new tab
instead" might help too.

Comment 22

[Mats Palmgren (inactive)]

Attachment: crash @XPCWrappedNative::IsValid

I ran the test again and now I either get this crash in
XPCWrappedNative::IsValid with a null 'this', or a hang
somewhere in JS GC.

Comment 23

[Andrew McCreight [:mccr8]]

The IsValid thing sounds like bug 781645.

Comment 24

[Mats Palmgren (inactive)]

Yeah, it's probably not worth looking at this before that bug is fixed.

Comment 25

[David Bolter [:davidb] (NeedInfo me for attention)]

Andrew, mind sitting on owning this one at least while you are active bug 781645 (see what happens when you miss critsmash ;) )

Comment 26

[Daniel Veditz [:dveditz]]

lowering severity because the original symptoms are gone and it's been hard to reproduce even in affected versions. There may be a sec-critical bug hiding in here but we can better spend our time fixing other sec-criticals that are easier to debug and reproduce (and potentially, easier to be found by bad guys).

Comment 27

[Andrew McCreight [:mccr8]]

Mats, do you still see any problems with this?  I tried running it and didn't hit anything fatal. Well, one time it hung somewhere in JS land, but I'm not sure what that was about.

Comment 28

[Mats Palmgren (inactive)]

I tested this a couple of day ago and got bug 790649.
I can't tell for sure if the original problem is gone before
the blocking bugs are resolved.  I agree with dveditz that
this bug is low priority until then.

Comment 29

[Andrew McCreight [:mccr8]]

I'm going to unassign myself until this is more actionable, and I'm updating the summary to reflect the current state of affairs for this bug.

Comment 30

[Mats Palmgren (inactive)]

Attachment: JS stack at the "compartment mismatch" assertion

The "compartment mismatch" assertion still occurs for this URL.
This is the DumpJSStack() output when that occurred.

(with the patch in bug 790649 to avoid hitting that crash)

Comment 31

[Mats Palmgren (inactive)]

I can still reproduce the "compartment mismatch" assertion after bug 790865
was fixed, using the URL in this bug.

Comment 32

[Jesse Ruderman]

On mozilla-central?

I'm a little suspicious of bug 791896, but it's possible we'll have to debug cross_fuzz :(

Comment 33

[Mats Palmgren (inactive)]

mozilla-inbound, rev 13ae166abd45.  I'll wait until that bug is fixed
before testing again.  Thanks.

Comment 34

[Andrew McCreight [:mccr8]]

Mats, does this reproduce for you now?  We've added a bunch more compartment checking, and fixed some things, so this may be fixed, or we may at least get a better stack.

Comment 35

[Mats Palmgren (inactive)]

Attachment: stack for "Assertion failure: (obj2)->zone()->isGCMarking()"

Assertion failure: (obj2)->zone()->isGCMarking(), at js/src/gc/Marking.cpp:1339

Comment 36

[Mats Palmgren (inactive)]

I got the "Assertion failure: (obj2)->zone()->isGCMarking()" again just now.
(I'll leave it in the debugger for a while in case you want any values from it).

Other than that, I haven't seen any issues on the JS/XPCOM/DOM front.

Comment 37

[Andrew McCreight [:mccr8]]

Bug 825326 fixes the same assertion, but was fixed before this.  Anyways, this is a JS assertion, so I'll move it over there...

Comment 38

[Ryan VanderMeulen [:RyanVM]]

None of the asserts in this bug exist anymore AFAICT. Closing this as incomplete at this point since it seems unlikely that anything useful is going to come out of this bug at this point. Feel free to reopen if there's something to do here still.