Closed Bug 875790 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: JavaScript Engine, defect, blocker)

24 Branch
defect
Not set
blocker

Tracking

()

VERIFIED FIXED
mozilla24
Tracking Status
firefox23 --- unaffected
firefox24 + verified

People

(Reporter: octoploid, Assigned: octoploid)

References

Details

(4 keywords, Whiteboard: See bug 875791 or comment 34 for steps to reproduce)

Crash Data

Attachments

(1 file, 2 obsolete files)

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
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
Assignee: nobody → general
Severity: normal → blocker
Status: UNCONFIRMED → NEW
Crash Signature: [@ js::CloneScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSFunction*>, JS::Handle<JSScript*>, js::NewObjectKind) ]
Component: Untriaged → JavaScript Engine
Ever confirmed: true
OS: Linux → All
Product: Firefox → Core
Hardware: x86_64 → All
Duplicate of this bug: 875791
Note that bug 875791 has much more useful steps to reproduce (in particular, the specific filter to use).
Whiteboard: See bug 875791 for steps to reproduce
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.
(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.
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.
(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.
(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.
Why not debug the actual crash instead?
(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. :)
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.
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.
(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. :-)
Adding regression window wanted to see if QA can help with bisection
(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 <bobbyholley@gmail.com>
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 ***
Sorry, that should be javascript.options.baselinejit.content and NOT javascript.options.baselinejit.chrome. Copy and paste failure.
(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 <bobbyholley@gmail.com>
> 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.
(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...
Blocks: 868130
Keywords: reproducible
When is bug 868130 backed out? There are about 6000 crashes per build!
Crash Signature: [@ js::CloneScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSFunction*>, JS::Handle<JSScript*>, js::NewObjectKind) ] → [@ js::CloneScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSFunction*>, JS::Handle<JSScript*>, js::NewObjectKind) ] [@ js::ScriptSourceObject::create(JSContext*, js::ScriptSource*) ]
Attached patch Patch to fix crash (obsolete) — Splinter Review
The attached patch fixes the issue for now.
Maybe it should be applied until Bobby gets back from holiday.
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.
Attachment #754213 - Flags: review?(gkrizsanits)
Attachment #754213 - Flags: review?(bobbyholley+bmo)
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. :)
Attachment #754213 - Flags: review?(gkrizsanits)
Attachment #754213 - Flags: review?(bobbyholley+bmo)
Attached patch alternative fix (obsolete) — Splinter Review
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.
Attachment #754213 - Attachment is obsolete: true
(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.
If you stop pushing the JSContext, you'll also need a JSAutoRequest ar(cx);
Attached patch Updated fixSplinter Review
(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?
Attachment #754241 - Attachment is obsolete: true
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.
Attachment #754265 - Flags: review?(bobbyholley+bmo)
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?
Attachment #754265 - Flags: review?(bobbyholley+bmo) → review+
Duplicate of this bug: 876269
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
Whiteboard: See bug 875791 for steps to reproduce → See bug 875791 or comment 34 for steps to reproduce
https://hg.mozilla.org/mozilla-central/rev/7c7524e8638c
Assignee: general → octoploid
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
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.
Depends on: 879649
Comment 37 was actually bug 879649, landed with the wrong bug number.
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?
Flags: needinfo?
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
Status: RESOLVED → VERIFIED
Flags: needinfo?
You need to log in before you can comment on or make changes to this bug.