crash js::analyze::ScriptAnalysis::addTypeBarrier

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: past, Assigned: jorendorff)

Tracking

({crash})

Other Branch
mozilla15
All
Mac OS X
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-kilimanjaro:+)

Details

(Whiteboard: [paladin] [chrome-debug][k9o:p1:fx15], crash signature, URL)

Attachments

(2 attachments, 2 obsolete attachments)

This bug was filed from the Socorro interface and is 
report bp-dc200b1e-1f69-4e06-b741-603b62120209 .
============================================================= 

User comment:

Set devtools.debug.enable to true. Selected "Script Debugger" from the menu while on a page with a WebGL demo running. Boom.
Assignee: nobody → general
Component: Developer Tools: Debugger → JavaScript Engine
Product: Firefox → Core
QA Contact: developer.tools.debugger → general
This is mine.  It happened while trying to load the sprite demo at index.html here:

https://github.com/dmose/gladius/tree/sprites/example/sprites
Whiteboard: [paladin]
I set devtools.debug.enable to true, but I don't have "Script Debugger" in my Tools/Web Developer menu. Do I need a special build?
My mistake; I meant devtools.debugger.enable; it's only in today's nightly build.
How do I conveniently load the demo? I thought

  https://raw.github.com/dmose/gladius/sprites/example/sprites/index.html

would work, but I just see the HTML source.
I just pushed it to my gh-pages branch.  This should do the trick: 

<http://dmose.github.com/gladius/example/sprites/index.html>
(Assignee)

Comment 6

6 years ago
I can reproduce this.
Assignee: general → jorendorff
(Assignee)

Comment 7

6 years ago
Created attachment 595897 [details] [diff] [review]
v1

I can't get this to reproduce in the shell, so no reduced testcase. :-P

Brian, I can (in the shell) create a new compartment, run some code in it, turn on the debugger, then run some more code. So it seems like I *should* be able to reproduce this bug if I can get the right kind of analysis to occur. Any suggestions?
Attachment #595897 - Flags: review?(bhackett1024)
Comment on attachment 595897 [details] [diff] [review]
v1

Which situation are you trying to repro in the shell?  Not sure what chain of events is causing this crash.
Attachment #595897 - Flags: review?(bhackett1024) → review+
(Assignee)

Comment 9

6 years ago
Well, I don't either. That's the problem. The chain of events is *like* this:

1. Make some analysis happen in compartment X.

2. Turn on debug mode for compartment X, e.g. by doing |new Debugger(X)|
   from some other compartment.
   This causes JSCompartment::updateForDebugMode to run, see patch.

   At this point analysis is in some kind of inconsistent state.

3. Run more code in compartment X. This causes a crash reading null:
   https://crash-stats.mozilla.com/report/index/dc200b1e-1f69-4e06-b741-603b62120209

So the test would look like this (run with -m -n):

    var g = newGlobal('new-compartment');
    g.eval(???);                   // step 1
    var dbg = new Debugger(g);     // step 2
    g.eval(???);                   // step 3

It might be really simple. I just don't know what kind of code triggers the creation of the particular kind of data structure that updateForDebugMode screws up.

Comment 10

6 years ago
Try run for 65cd443e128f is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=65cd443e128f
Results (out of 190 total builds):
    exception: 3
    success: 146
    warnings: 38
    failure: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jorendorff@mozilla.com-65cd443e128f
(Assignee)

Comment 11

6 years ago
This patch causes some Linux-only orange. I don't know what's wrong. Last time I looked I couldn't reproduce it, but I'll try again next week.
(Assignee)

Comment 12

6 years ago
I'll try again next week. We really need this fixed.
Assignee: jorendorff → jhpedemonte
Component: JavaScript Engine → Java to XPCOM Bridge
QA Contact: general → xpcom-bridge
Target Milestone: --- → mozilla13
Version: Trunk → Other Branch
I'm pretty the switch to the Java xpcom-bridge component & owner was inadvertent; putting it back.
Assignee: jhpedemonte → general
Component: Java to XPCOM Bridge → JavaScript Engine
QA Contact: xpcom-bridge → general
(Assignee)

Updated

6 years ago
Whiteboard: [paladin] → [paladin] [chrome-debug]
I am facing this crash quite often, any estimate on when this could be fixed?
Thanks!
Honza
adding blocking-k9o request. This prevents shipping.

Any luck on the orange, Jason?
blocking-kilimanjaro: --- → ?
Blocks: 676586
(Assignee)

Comment 16

5 years ago
The patch has rotted. The "FreeOp" work makes this harder to hack around in this way. I'll try to fix it some other way.

I managed to produce a shell test case by error and error. (It's like trial and error, but with more errors.)

var g = newGlobal('new-compartment');
g.eval(
    "function f(x) { return {q: x}; }\n" +
    "var n = f('').q;\n");
var dbg = new Debugger(g);
g.eval("f(1000)");
(Assignee)

Comment 17

5 years ago
Forgot to mention: the shell test case in comment 16 requires -m -n -a.
(Assignee)

Comment 18

5 years ago
Created attachment 617231 [details] [diff] [review]
v2
Assignee: general → jorendorff
Attachment #595897 - Attachment is obsolete: true
(Assignee)

Comment 19

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=79f757e8a473

The orange is apparently due to running tests with --valgrind when we didn't configure with --enable-valgrind. (That means we don't have valgrind-readable annotations in the executable about the conservative stack scanning, which reads uninitialized stack locations. So valgrind has no reason to consider the GC sane.) But given that we have that (ridiculous) bug in the Makefile, I don't understand why we do not have huge amounts of orange all the time. Anyone?
(Assignee)

Comment 20

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=e94f19888bda

Fixed the valgrind thing. It looks like now I'm tripping an assertion that happens at the end of GC:

  Assertion failure: it < end, at ../../../js/src/jsgcinlines.h:402
 1  libxul.so!js::gc::GCCompartmentsIter::GCCompartmentsIter [jsgcinlines.h
 2  libxul.so!AutoGCSession::~AutoGCSession [jsgc.cpp : 3316 + 0x10]
 3  libxul.so!GCCycle [jsgc.cpp : 3556 + 0xd]
 4  libxul.so!Collect [jsgc.cpp : 3689 + 0x17]
 5  libxul.so!js::GC [jsgc.cpp : 3710 + 0x24]
 6  libxul.so!JSCompartment::updateForDebugMode [jscompartment.cpp : 718 + 0x17]

GCCompartmentsIter asserts that at least one compartment isCollecting(). The only way I see that this could be false is if the compartment itself is being destroyed during GC. 

I will fix this using a kung fu death grip (a.k.a. AutoHoldCompatment), just to make sure my understanding of the problem is correct, but I *hope* the right answer is to remove the assertion, and billm will tell me that on review...
(Assignee)

Comment 21

5 years ago
Created attachment 618389 [details] [diff] [review]
prologue: use the jit_test.py --valgrind option only if we have valgrind support
Attachment #618389 - Flags: review?(jimb)
(Assignee)

Comment 22

5 years ago
Created attachment 618390 [details] [diff] [review]
fix: v3
Attachment #617231 - Attachment is obsolete: true
Attachment #618390 - Flags: review?(wmccloskey)
Comment on attachment 618390 [details] [diff] [review]
fix: v3

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

::: js/src/jscompartment.cpp
@@ +713,5 @@
> +    // compartment. Because !hasScriptsOnStack(), it suffices to do a garbage
> +    // collection cycle or to finish the ongoing GC cycle. The necessary
> +    // cleanup happens in JSCompartment::sweep.
> +    if (!rt->gcRunning) {
> +        scheduleGC();

Could you change this to PrepareCompartmentForGC(this)?
Attachment #618390 - Flags: review?(wmccloskey) → review+
(Assignee)

Comment 24

5 years ago
Review ping. Should be pretty trivial.
Whiteboard: [paladin] [chrome-debug] → [paladin] [chrome-debug][k9o:p1:fx15]
blocking-kilimanjaro: ? → +

Updated

5 years ago
Attachment #618389 - Flags: review?(jimb) → review+
Patches have r+. Is this ready to go in?
https://hg.mozilla.org/mozilla-central/rev/4e5d09c8e1a1
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: mozilla13 → mozilla15
And

https://hg.mozilla.org/mozilla-central/rev/c40f450c8b9c

Please note the inbound changesets in the bug.
Depends on: 754201
You need to log in before you can comment on or make changes to this bug.