875790 - crash in js::CloneScript on startup when adblock plus is enabled

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Octoploid]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130524154306

Steps to reproduce:

Start Firefox with Adblock Plus 2.2.3558 enabled.


Actual results:

Firefox crashes on startup:
...
[1369408455099] ABP timeline: Async action FilterStorageRead done                                                (2168/2608)
[1369408455099] ABP timeline: FilterStorage.loadFromDisk() read callback                                         (first event)
[1369408455099] ABP timeline: * Initializing data done, triggering observers                                     (0)
[1369408456306] ABP timeline: * Entered ElemHide.apply()                                                         (1207)
[1369408456307] ABP timeline: * * start grouping selectors                                                       (1)
[1369408456452] ABP timeline: * * done grouping selectors                                                        (145)
[1369408456455] ABP timeline: * ElemHide.apply() done (async action pending)                                     (3)
[1369408456455] ABP timeline: FilterStorage.loadFromDisk() read callback done                                    (0)
ABP timeline: Total time elapsed: 1356
[New Thread 0x7fffa52ff700 (LWP 26147)]

Program received signal SIGSEGV, Segmentation fault.
js::CloneScript (cx=0x17086b0, newKind=js::GenericObject, enclosingScope=..., fun=..., src=...) at /home/markus/mozilla-central/js/src/jsscript.cpp:2258
2258        uint32_t nobjects  = src->hasObjects()  ? src->objects()->length  : 0;
(gdb) bt
#0  js::CloneScript (cx=0x17086b0, newKind=js::GenericObject, enclosingScope=..., fun=..., src=...) at /home/markus/mozilla-central/js/src/jsscript.cpp:2258
#1  0x00007ffff58b4c90 in JS_ExecuteScript (cx=0x17086b0, objArg=<optimized out>, scriptArg=<optimized out>, rval=0x7fffffffc658)
    at /home/markus/mozilla-central/js/src/jsapi.cpp:5595
#2  0x00007ffff48325df in nsJSContext::ExecuteScript (this=0x1708640, aScriptObject_=0x7fffd167de70, aScopeObject_=<optimized out>)
    at /home/markus/mozilla-central/dom/base/nsJSEnvironment.cpp:1398
#3  0x00007ffff48122df in mozilla::dom::XULDocument::ExecuteScript (this=0x17c7350, aScript=0x1872a60)
    at /home/markus/mozilla-central/content/xul/document/src/XULDocument.cpp:3678
#4  0x00007ffff4812ec1 in mozilla::dom::XULDocument::OnStreamComplete (this=0x17c7350, aLoader=<optimized out>, 
    context=0x7ffff623e502 <_ZZN2js2gcL19MapAllocToTraceKindENS0_9AllocKindEE3map+66>, aStatus=NS_OK, stringLen=3417, 
    string=0x177a200 "// -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-\n\n/* This Source Code Form is subject to the terms of the Mozilla Public\n * License, v. 2.0. If a copy of the MPL was not d"...) at /home/markus/mozilla-central/content/xul/document/src/XULDocument.cpp:3556
#5  0x00007ffff402cfe1 in nsStreamLoader::OnStopRequest (this=0x192ca00, request=0x192c5d0, ctxt=<optimized out>, aStatus=NS_OK)
    at /home/markus/mozilla-central/netwerk/base/src/nsStreamLoader.cpp:100
#6  0x00007ffff3ff3671 in nsBaseChannel::OnStopRequest (this=0x192c580, request=<optimized out>, ctxt=<optimized out>, status=<optimized out>)
    at /home/markus/mozilla-central/netwerk/base/src/nsBaseChannel.cpp:737
#7  0x00007ffff3ff3719 in non-virtual thunk to nsBaseChannel::OnStopRequest(nsIRequest*, nsISupports*, tag_nsresult) (this=<optimized out>, request=0x8, 
    ctxt=0x7ffff623e502 <_ZZN2js2gcL19MapAllocToTraceKindENS0_9AllocKindEE3map+66>, status=-14848) at /home/markus/mozilla-central/netwerk/base/src/nsBaseChannel.cpp:751
#8  0x00007ffff4000b49 in nsInputStreamPump::OnStateStop (this=0x192c420) at /home/markus/mozilla-central/netwerk/base/src/nsInputStreamPump.cpp:555
#9  0x00007ffff400050b in nsInputStreamPump::OnInputStreamReady (this=0x192c420, stream=0x8) at /home/markus/mozilla-central/netwerk/base/src/nsInputStreamPump.cpp:375
#10 0x00007ffff5453157 in nsInputStreamReadyEvent::Run (this=0x1923730) at /home/markus/mozilla-central/xpcom/io/nsStreamUtils.cpp:82
#11 0x00007ffff546b0b4 in nsThread::ProcessNextEvent (this=0x4a4d00, mayWait=false, result=0x7fffffffca17) at /home/markus/mozilla-central/xpcom/threads/nsThread.cpp:627
#12 0x00007ffff541f55a in NS_ProcessNextEvent (thread=<optimized out>, mayWait=false) at nsThreadUtils.cpp:238
#13 0x00007ffff50108a7 in mozilla::ipc::MessagePump::Run (this=0x4a3140, aDelegate=0x4a4170) at /home/markus/mozilla-central/ipc/glue/MessagePump.cpp:82
#14 0x00007ffff54a04a4 in MessageLoop::RunInternal (this=0x4a4170) at /home/markus/mozilla-central/ipc/chromium/src/base/message_loop.cc:219
#15 0x00007ffff54a03e2 in RunHandler (this=0xdadadadadadadada) at /home/markus/mozilla-central/ipc/chromium/src/base/message_loop.cc:212
#16 MessageLoop::Run (this=0xdadadadadadadada) at /home/markus/mozilla-central/ipc/chromium/src/base/message_loop.cc:186
#17 0x00007ffff4f5db8d in nsBaseAppShell::Run (this=0x844760) at /home/markus/mozilla-central/widget/xpwidgets/nsBaseAppShell.cpp:163
#18 0x00007ffff4d7f674 in nsAppStartup::Run (this=0x7fd1b0) at /home/markus/mozilla-central/toolkit/components/startup/nsAppStartup.cpp:269
#19 0x00007ffff3f9a93f in XREMain::XRE_mainRun (this=<optimized out>) at /home/markus/mozilla-central/toolkit/xre/nsAppRunner.cpp:3872
#20 0x00007ffff3f9ae14 in XREMain::XRE_main (this=0x7fffffffcda8, argc=<optimized out>, argv=0x7fffffffe278, aAppData=<optimized out>)
    at /home/markus/mozilla-central/toolkit/xre/nsAppRunner.cpp:3939
#21 0x00007ffff3f9b308 in XRE_main (argc=8, argv=0x7ffff623e502 <_ZZN2js2gcL19MapAllocToTraceKindENS0_9AllocKindEE3map+66>, aAppData=0x7fffffffc600, aFlags=<optimized out>)
    at /home/markus/mozilla-central/toolkit/xre/nsAppRunner.cpp:4140
#22 0x00000000004036ac in do_main (xreDirectory=0x418010, argc=<optimized out>, argv=<optimized out>) at /home/markus/mozilla-central/browser/app/nsBrowserApp.cpp:272
#23 main (argc=<optimized out>, argv=<optimized out>) at /home/markus/mozilla-central/browser/app/nsBrowserApp.cpp:632

Comment 1

[Scoobidiver (away)]

The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=00b264c7cced&tochange=df526497d949

There are about 250 crashes an hour.

More reports at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3ACloneScript%28JSContext*%2C+JS%3A%3AHandle%3CJSObject*%3E%2C+JS%3A%3AHandle%3CJSFunction*%3E%2C+JS%3A%3AHandle%3CJSScript*%3E%2C+js%3A%3ANewObjectKind%29

Comment 4

[Boris Zbarsky [:bzbarsky]]

Note that bug 875791 has much more useful steps to reproduce (in particular, the specific filter to use).

Comment 5

[Brian Carpenter [:geeknik]]

Someone in bug 875791 said this might be a user after free issue. If that's true, someone might want to touch the security flag. I would, but I want someone else to make the determination first.

Comment 6

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

(In reply to Scoobidiver from comment #1)
> The regression range is:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=00b264c7cced&tochange=df526497d949
> 
> There are about 250 crashes an hour.

In this same regression range, bug 864218 is also present, which seems to also cause another topcrash in bug 875757.

Comment 7

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

Bug 864218 has been backed out in:

https://hg.mozilla.org/mozilla-central/rev/7a2f7a45819a

Let's keep a lookout to see if this bug still occurs.

Comment 8

[Octoploid]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #7)
> Bug 864218 has been backed out in:
> 
> https://hg.mozilla.org/mozilla-central/rev/7a2f7a45819a
> 
> Let's keep a lookout to see if this bug still occurs.

Yes, unfortunately the bug still occurs even with your backout.

Comment 9

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

(In reply to Scoobidiver from comment #1)
> The regression range is:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=00b264c7cced&tochange=df526497d949

(In reply to Octoploid from comment #8)
> Yes, unfortunately the bug still occurs even with your backout.

:( In that case, someone will have to bisect that regression range so we can backout another bug.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Why not debug the actual crash instead?

Comment 11

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

(In reply to Boris Zbarsky (:bz) from comment #10)
> Why not debug the actual crash instead?

If someone would like to do that and land a fix, sure. :)

Comment 12

[Bobby Holley (:bholley)]

The STR in bug 875791 don't seem to work for me.

Note that if bug 868130 or bug 868110 turn out to be the culprit, please consider something surgical and targeted rather than a full backout, because the patches are huge and any potential backout has probably already bitrotted. The changesets should all be bisectable. I'm on PTO, but feel free to send me mail asking for any clarifications on what's going on.

Comment 13

[Brian Carpenter [:geeknik]]

Sorry if this is obvious, but if you set javascript.options.baselinejit.chrome to false, the crash goes away, at least in the latest hourly build.

Comment 14

[Bobby Holley (:bholley)]

(In reply to Brian Carpenter [:geeknik] from comment #13)
> Sorry if this is obvious, but if you set
> javascript.options.baselinejit.chrome to false, the crash goes away, at
> least in the latest hourly build.

Ah, ok. Then bug 868130 and bug 868110 are unlikely to be the culprits. :-)

Comment 15

[bhavana bajaj [:bajaj]]

Adding regression window wanted to see if QA can help with bisection

Comment 16

[Octoploid]

(In reply to Bobby Holley (:bholley) (PTO thurs/fri) from comment #14)
> (In reply to Brian Carpenter [:geeknik] from comment #13)
> > Sorry if this is obvious, but if you set
> > javascript.options.baselinejit.chrome to false, the crash goes away, at
> > least in the latest hourly build.
> 
> Ah, ok. Then bug 868130 and bug 868110 are unlikely to be the culprits. :-)

No. Bug 868130 _is_ the culprit:
7793aa8a3e98be9acd3e6186b9ffd3adbdf1f4c4 is the first bad commit
commit 7793aa8a3e98be9acd3e6186b9ffd3adbdf1f4c4
Author: Bobby Holley <[e-mail]>
Date:   Wed May 22 10:05:23 2013 -0600

    Bug 868130 - Make sure mContext is stack-top in nsJSContext::CompileScript. r=gabor

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

During bisection it caused:
*** Error in `./firefox': malloc(): smallbin double linked list corrupted: 0x000000000167db50 ***

Comment 17

[Brian Carpenter [:geeknik]]

Sorry, that should be javascript.options.baselinejit.content and NOT javascript.options.baselinejit.chrome. Copy and paste failure.

Comment 18

[Bobby Holley (:bholley)]

(In reply to Octoploid from comment #16)
> (In reply to Bobby Holley (:bholley) (PTO thurs/fri) from comment #14)
> > (In reply to Brian Carpenter [:geeknik] from comment #13)
> > > Sorry if this is obvious, but if you set
> > > javascript.options.baselinejit.chrome to false, the crash goes away, at
> > > least in the latest hourly build.
> > 
> > Ah, ok. Then bug 868130 and bug 868110 are unlikely to be the culprits. :-)
> 
> No. Bug 868130 _is_ the culprit:
> 7793aa8a3e98be9acd3e6186b9ffd3adbdf1f4c4 is the first bad commit
> commit 7793aa8a3e98be9acd3e6186b9ffd3adbdf1f4c4
> Author: Bobby Holley <[e-mail]>
> Date:   Wed May 22 10:05:23 2013 -0600
> 
>     Bug 868130 - Make sure mContext is stack-top in
> nsJSContext::CompileScript. r=gabor
> 
> http://hg.mozilla.org/mozilla-central/rev/a0e74ae591bd
> 
> During bisection it caused:
> *** Error in `./firefox': malloc(): smallbin double linked list corrupted:
> 0x000000000167db50 ***

Huh, really? That is...baffling. The only impact of that patch should be that we potentially push a JSContext with the AutoPushJSContext (everything else is just a rename that should be invisible to the compiler).

Does the corruption disappear if you replace:
AutoPushJSContext cx(mContext);
with
JSContext* cx = mContext;

?

This patch is very correct AFAICT, so if it's the culprit, we probably have deeper memory corruption issues going on that need to be investigated.

Comment 19

[Octoploid]

(In reply to Bobby Holley (:bholley) (PTO thurs/fri) from comment #18)
> 
> Huh, really? That is...baffling. The only impact of that patch should be
> that we potentially push a JSContext with the AutoPushJSContext (everything
> else is just a rename that should be invisible to the compiler).
> 
> Does the corruption disappear if you replace:
> AutoPushJSContext cx(mContext);
> with
> JSContext* cx = mContext;
> 
> ?

Yes. The bug disappears with this replacement...

Comment 20

[Scoobidiver (away)]

When is bug 868130 backed out? There are about 6000 crashes per build!

Comment 21

[Octoploid]

Attachment: Patch to fix crash

The attached patch fixes the issue for now.
Maybe it should be applied until Bobby gets back from holiday.

Comment 22

[Andrew McCreight [:mccr8]]

Comment on attachment 754213 [details] [diff] [review]
Patch to fix crash

Maybe Gabor or Bobby are around to review the patch and make sure it is okay?  Unfortunately I don't understand enough to review it myself.

Comment 23

[Andrew McCreight [:mccr8]]

try run for the patch:  https://tbpl.mozilla.org/?tree=Try&rev=55b0556d328f

Comment 24

[Andrew McCreight [:mccr8]]

Comment on attachment 754213 [details] [diff] [review]
Patch to fix crash

Well, the try run is quite orange, so that patch isn't right somehow. :)

Comment 25

[Octoploid]

Attachment: alternative fix

Looks like the "try run" still crashes. 
Here's an alternative patch (based on Bobby's comment 18).
Could you give it another try?
Thanks.

Comment 26

[Andrew McCreight [:mccr8]]

https://tbpl.mozilla.org/?tree=Try&rev=3c799f408511

Comment 27

[Octoploid]

(In reply to Andrew McCreight [:mccr8] from comment #26)
> https://tbpl.mozilla.org/?tree=Try&rev=3c799f408511

Well, at least the optimized builds are fine now.

Comment 28

[Bobby Holley (:bholley)]

If you stop pushing the JSContext, you'll also need a JSAutoRequest ar(cx);

Comment 29

[Octoploid]

Attachment: Updated fix

(In reply to Bobby Holley (:bholley) (PTO thurs/fri) from comment #28)
> If you stop pushing the JSContext, you'll also need a JSAutoRequest ar(cx);

OK, thanks. I've updated the patch accordingly.

Andrew: Can you start another try run?

Comment 30

[Andrew McCreight [:mccr8]]

https://tbpl.mozilla.org/?tree=Try&rev=339d5f15afe2

Comment 31

[Octoploid]

Comment on attachment 754265 [details] [diff] [review]
Updated fix

(In reply to Andrew McCreight [:mccr8] from comment #30)
> https://tbpl.mozilla.org/?tree=Try&rev=339d5f15afe2

Everything's green this time.

Comment 32

[Bobby Holley (:bholley)]

Comment on attachment 754265 [details] [diff] [review]
Updated fix

r=me in the interim I guess.

But when I'm back on tuesday I'm going to need to undo this, and will need a way to reproduce this bug if I'm to get to the bottom of the crashes. Can you make some reproducible STR?

Comment 34

[Alice0775 White]

bp-7bc88d59-38a3-4c8a-baa4-5e5cc2130527
I can reproduce the crash on ubuntu12.04LTS 32bit
http://hg.mozilla.org/mozilla-central/rev/0fed3377c839
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20130526 Firefox/24.0 ID:20130526031046

STR(Bug 875791)
1. Cleate New profile and start Nightly
2. Install Adblock Plus https://addons.mozilla.org/en-US/firefox/addon/adblock-plus/
3. Install certain filter
3-1. Click AdblockPlus Icon and Choose "Filter Preferences"
3-2. Click "Back and Restore" button at bottom and Choose "restore own backup"
3-3. Open patterns-backup5.ini and OK on confirmation dialog,
3-4. Close Filter Preferences dialog
4. Restart Nightly

Actual Results:
Clashes at start up

Comment 35

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/mozilla-central/rev/7c7524e8638c

Comment 36

[:Irving Reid (No longer working on Firefox)]

This is crashing reliably in my default profile (https://crash-stats.mozilla.com/report/index/bp-8111730a-dc70-4268-8abc-1b3272130527, for example). Let me know if you want me to run it under a debugger.

Comment 37

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8508340fabb7

Comment 38

[Bobby Holley (:bholley)]

Comment 37 was actually bug 879649, landed with the wrong bug number.

Comment 39

[Ada [:adalucinet]]

Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:26.0) Gecko/20100101 Firefox/26.0

Reproduced this issue with Nightly 24.0a1(Build ID: 20130526031046), but with different signatures: 
https://crash-stats.mozilla.com/report/index/bp-c5435061-bce1-4cfb-8320-b97e52130820
https://crash-stats.mozilla.com/report/index/d830a7f8-8614-4be7-8b6f-73a392130820

Verified as fixed with Firefox 24 beta 4 (Build ID: 20130605070403) and latest Nightly (Build ID: 20130820030206) when using STR from comment 34: no crashes at start up. 
In Socorro there are some crashes with this signature in the latest builds:
http://goo.gl/3X6dtZ and http://goo.gl/VFe5c2

Are those related? Any thoughts?

Comment 40

[Ada [:adalucinet]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0

Verified as fixed with Firefox 24 beta 8 and latest Nightly (2013-09-02): with STR from comment 34: 0 crashes at start up.
Reports from Scorro:
- 1st signature: 9 crashes with beta 7: http://goo.gl/hQ2IL6
- 2nd signature: 1 crash with beta 7: http://goo.gl/n35Isd