crash in js::jit::JitFrameIterator::script

RESOLVED FIXED in Firefox 47

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Jukka Jylänki, Assigned: jandem)

Tracking

({crash})

Trunk
mozilla47
x86_64
Windows 10
crash
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
This bug was filed from the Socorro interface and is 
report bp-e70c0399-acf3-47d7-8698-6728e2160205.
=============================================================

Difficult to reproduce, got this once on a page load of asm.js content, but several subsequent runs loaded up fine. Still, a crash is a crash so marking this down :\
(Assignee)

Comment 1

2 years ago
Interesting. It looks like we end up in InvokeInterruptCallback, where we do:

    if (cb(cx)) {
        // Debugger treats invoking the interrupt callback as a "step", so
        // invoke the onStep handler.
        if (cx->compartment()->isDebuggee()) {
            ScriptFrameIter iter(cx);
            if (iter.script()->stepModeEnabled()) {

But from your stack trace, the JS stack should be empty here, so iter.script() will indeed crash in weird ways.
Component: JavaScript Engine: JIT → JavaScript Engine
(Assignee)

Comment 2

2 years ago
Probably a regression from bug 1243242, although it might be possible to trigger this bug (calling CheckForInterrupt while we're debugging and there are no JS frames on the stack) in other ways.
(Assignee)

Comment 3

2 years ago
It's easy to fix this crash by adding a !iter.done() check, but should we also fix InvokeInterruptCallback to not call the interrupt callback if there are no activations on the stack?
Flags: needinfo?(luke)

Comment 4

2 years ago
Probably best to keep the callback call since it may be used to schedule work "some time later" (like we do inside InvokeInterruptCallback.
Flags: needinfo?(luke)
(Assignee)

Comment 5

2 years ago
Created attachment 8716347 [details] [diff] [review]
Patch

Do nothing if the compartment doesn't have any scripts on the stack.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Attachment #8716347 - Flags: review?(shu)

Comment 6

2 years ago
Comment on attachment 8716347 [details] [diff] [review]
Patch

Review of attachment 8716347 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine by me, but I thought it was an invariant that we only invoke the interrupt callback with JS on the stack. What is it used for/how does it even get invoked when there's no JS running?
Attachment #8716347 - Flags: review?(shu) → review+

Comment 7

2 years ago
The embedding can use it as a sortof cross-thread setTimeout().  I might be wrong, but I think workers use this sometimes to send control messages.
(Assignee)

Comment 8

2 years ago
(In reply to Shu-yu Guo [:shu] from comment #6)
> This is fine by me, but I thought it was an invariant that we only invoke
> the interrupt callback with JS on the stack. What is it used for/how does it
> even get invoked when there's no JS running?

According to the crash report we can call structured clone code without JS on the stack, and bug 1243242 added an interrupt check there.
Depends on: 1243242

Comment 9

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1c3cf6f0f7a

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d1c3cf6f0f7a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.