Closed Bug 632358 Opened 9 years ago Closed 9 years ago

Assertion failure: invokeSegment == currentSegment

Categories

(Core :: JavaScript Engine, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

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

People

(Reporter: gkw, Assigned: luke)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [ccbr][fixed-in-tracemonkey][hardblocker])

Attachments

(3 files, 1 obsolete file)

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.
Attached file more information
gdb stacktrace
Summary: TM/JM?: "Assertion failure: invokeSegment == currentSegment," → TM/JM?: "Assertion failure: invokeSegment == currentSegment," with xpcshell
Assignee: general → lw
Is this by any chance an opt build with #define DEBUG ?
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.
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.
blocking2.0: ? → .x
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.
Attached patch fixSplinter Review
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.)
Attachment #510805 - Flags: review?(jwalden+bmo)
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.
Group: core-security
Whiteboard: [ccbr][sg:critical?] → [ccbr]
Summary: TM/JM?: "Assertion failure: invokeSegment == currentSegment," with xpcshell → Assertion failure: invokeSegment == currentSegment
Luke, so should I ignore the patch in comment 4 and immediately test the patch in comment 6?
Gary: yes, sorry about the confusion.
(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!
Attachment #510805 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 510805 [details] [diff] [review]
fix

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

...but fix this typo please.
http://hg.mozilla.org/tracemonkey/rev/bce779f4d16b
Whiteboard: [ccbr] → [ccbr][fixed-in-tracemonkey]
Blocks: 633802
Blocks: 633803
Duplicate of this bug: 633803
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
Status: NEW → RESOLVED
blocking2.0: .x → betaN+
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [ccbr][fixed-in-tracemonkey] → [ccbr][fixed-in-tracemonkey][hardblocker]
No longer blocks: 633802
Duplicate of this bug: 633802
No longer blocks: 633803
Comment on attachment 510661 [details] [diff] [review]
patch to assert earlier, hopefully

Obsoleted per comment 8.
Attachment #510661 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.