632358 - Assertion failure: invokeSegment == currentSegment

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: partially reduced testcase

Attached testcase asserts xpcshell (with both -m and -j, I forgot which one was unnecessary) at Assertion failure: invokeSegment == currentSegment, on TM changeset db8be4e3f373.

s-s because gc is on the backtrace. The testcase is still being reduced (painfully) as it takes 1-2 minutes to crash, but attaching what I have as a gzip'ed testcase to let the devs take a first look.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: more information

gdb stacktrace

Comment 2

[Luke Wagner [:luke]]

Is this by any chance an opt build with #define DEBUG ?

Comment 3

[Luke Wagner [:luke]]

It looks like this test depends on GC happening at a specific call, so it may help to add gczeal when trying to bisect.  I'm also trying to repro.

Comment 4

[Luke Wagner [:luke]]

Attachment: patch to assert earlier, hopefully

Alternatively, gc could not be causing the problem, but rather be observing it.  This patch checks the failing assertion at more points and thus may also be useful to reduce.

Comment 5

[Luke Wagner [:luke]]

Muahaha, its not an optimized build, setCurrentRegs actually does call resetCompartment... in the middle of what should be an atomic "push a stack frame and stack segment" operation.

Gary, I think this pins down the problem, so probably don't need a bisect as long as you can verify the fix.

Comment 6

[Luke Wagner [:luke]]

Attachment: fix

Gary, does this fix the crash for you (I was unable to repro to begin with).

The fix is just to make sure that the stack is GC-able by the time we call resetCompartment.  Doing resetCompartment in setCurrentRegs is creepy since that is a hell of an effect for what seems to be a simple asserting setter.  Instead, I put it closer to the "source" in JSContext operations.  (I also lightly twiddled where we set initialVarObj b/c its current placement is ad hoc and weird.)

Comment 7

[Luke Wagner [:luke]]

Not s-s since all that will happen is StackSpace::mark will skip over the segment.  Non-exploitable and extremely rare so non-blocking status makes sense.

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Luke, so should I ignore the patch in comment 4 and immediately test the patch in comment 6?

Comment 9

[Luke Wagner [:luke]]

Gary: yes, sorry about the confusion.

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #6)
> Created attachment 510805 [details] [diff] [review]
> fix
> 
> Gary, does this fix the crash for you (I was unable to repro to begin with).

Yes, this does!

Comment 11

[Jeff Walden [:Waldo]]

Comment on attachment 510805 [details] [diff] [review]
fix

>+    /* Officialy push the segment/frame on the stack. */

...but fix this typo please.

Comment 12

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/bce779f4d16b

Comment 14

[Luke Wagner [:luke]]

This fixes top-crashes in nightlies (bug 633802 and bug 633803) and TM isn't ready to merge yet, so landed early:

http://hg.mozilla.org/mozilla-central/rev/c46f36d6b1fe

Comment 16

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 510661 [details] [diff] [review]
patch to assert earlier, hopefully

Obsoleted per comment 8.