551077 - Crash in [@ js_ConcatStrings ] on http://joris.kluivers.nl/Bingo/

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Marcia Knous [:marcia]]

Seen while running Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a3pre) Gecko/20100308 Minefield/3.7a3pre. Confirmed crash on Windows 7 trunk build as well.

STR:
1. Load site in URL.
2. Crash.

https://crash-stats.mozilla.com/report/index/63e2f3c6-34fe-48f5-9484-3d1422100304 is my report. 

Frame  	Module  	Signature [Expand]  	Source
0 		        @0xffff0f14 	
1 	libmozjs.dylib 	js_ConcatStrings 	js/src/jsstr.cpp:171
2 	libmozjs.dylib 	js_Interpret 	        js/src/jsops.cpp:1004
3 	libmozjs.dylib 	js_Invoke 	        js/src/jsinterp.cpp:1378
4 	libmozjs.dylib 	js_InternalInvoke 	js/src/jsinterp.cpp:1435
5 	libmozjs.dylib 	JS_CallFunctionValue 	js/src/jsapi.cpp:4951
6 	XUL 	nsJSContext::CallEventHandler 	dom/base/nsJSEnvironment.cpp:2164
7 	XUL 	nsGlobalWindow::RunTimeout 	dom/base/nsGlobalWindow.cpp:8339
8 	XUL 	nsGlobalWindow::TimerCallback 	dom/base/nsGlobalWindow.cpp:8683
9 	XUL 	nsTimerImpl::Fire 	        xpcom/threads/nsTimerImpl.cpp:427
10 	XUL 	nsTimerEvent::Run 	        xpcom/threads/nsTimerImpl.cpp:519
11 	XUL 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:527
12 	XUL 	NS_ProcessPendingEvents_P 	nsThreadUtils.cpp:200
13 	XUL 	nsBaseAppShell::NativeEventCallback 	widget/src/xpwidgets/nsBaseAppShell.cpp:125
14 	XUL 	nsAppShell::ProcessGeckoEvents 	widget/src/cocoa/nsAppShell.mm:510
15 	CoreFoundation 	CFRunLoopRunSpecific 	
16 	CoreFoundation 	CFRunLoopRunInMode 	
17 	HIToolbox 	RunCurrentEventLoopInMode 	
18 	HIToolbox 	ReceiveNextEventCommon 	
19 	HIToolbox 	BlockUntilNextEventMatchingListInMode 	
20 	AppKit 	_DPSNextEvent 	
21 	AppKit 	-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 	
22 	AppKit 	-[NSApplication run] 	
23 	XUL 	nsAppShell::Run 	widget/src/cocoa/nsAppShell.mm:863
24 	XUL 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:183
25 	XUL 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3494
26 	firefox-bin 	main 	browser/app/nsBrowserApp.cpp:158
27 	firefox-bin 	firefox-bin@0xbf5 	
28 		@0x2

Comment 1

[David Mandelin [:dmandelin]]

The proximate cause is that we evaluate JSOP_ADD with an rval that is a JSObject tagged as a string. This usually indicates a tracing bug, and, in fact, we don't crash if the JIT is turned off. 

Can we get a regression window on this?

Comment 2

[Marcia Knous [:marcia]]

Regression window is as follows:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a2pre) Gecko/20100224 Minefield/3.7a2pre - Works

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a2pre) Gecko/20100225 Minefield/3.7a2pre - Crash

Tinderbox is having difficulties coming up right now so I was not able to look at the checkins that happened between those 2 dates.

Comment 3

[David Mandelin [:dmandelin]]

Cool, thanks. I can narrow it down from there.

Comment 4

[David Mandelin [:dmandelin]]

OK. *This* bug starts from this changeset:


changeset:   38500:6a3c15f34a50
user:        Jeff Walden <[e-mail]>
date:        Tue Jan 26 18:41:30 2010 -0800
summary:     Bug 540985 - Regression in Peacekeeper string benchmarks due to
regexp literals no longer tracing.  A prototype of this patch was by bz. 
r=igor

Comment 5

[Marcia Knous [:marcia]]

Bug 551705 also has this stack noted in its title.

Comment 6

[David Mandelin [:dmandelin]]

(In reply to comment #5)
> Bug 551705 also has this stack noted in its title.

Yes. I originally thought they might be the same, but we have bisected them to different changesets.

Comment 7

[Jeff Walden [:Waldo]]

This isn't crashing TM tip any more, going to debug a touch from the bad changeset a little to see if I can figure out what fixed this...

Comment 8

[Jeff Walden [:Waldo]]

Can't reproduce even with 6a3c15f34a50, marking WFM unless someone else reproduces the problem...

Comment 9

[Martijn Wargers (dead)]

I'm crashing again in current trunk build of 2010-08-08 on that site:
http://crash-stats.mozilla.com/report/index/c35e68a6-3b65-4771-9512-186432100810
0  	mozcrt19.dll  	memcpy  	 memcpy.asm:188
1 	mozjs.dll 	JSString::flatten 	js/src/jsstr.cpp:158
2 	mozjs.dll 	js_ExecuteRegExp 	js/src/jsregexp.cpp:4925
3 	mozjs.dll 	regexp_exec_sub 	js/src/jsregexp.cpp:5707
4 	mozjs.dll 	regexp_test 	js/src/jsregexp.cpp:5733
5 	mozjs.dll 	mozjs.dll@0x6f02f 	
6 	mozjs.dll 	js::InvokeCommon<int > 	js/src/jsinterp.cpp:588
7 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:691
8 		@0x2c0007f 	
9 	mozjs.dll 	JSRegExpStatics::copy 	js/src/jsregexp.cpp:375
10 	mozjs.dll 	JSAutoCrossCompartmentCall::enter 	js/src/jsapi.cpp:1149
etc..

But I guess that's bug 585309.

Comment 10

[Rob Campbell [:rc] (:robcee)]

I just hit this after an uptime of 522637 (seconds?).

Not sure what I was doing at the time. I'd switched back to this instance of Firefox from another Space (in OS X), may have switched to a different tabgroup (with TabCandy), then while idle, crashed.

stack is here:

http://crash-stats.mozilla.com/report/index/bp-709ebd2c-173f-4327-8cb5-c912c2100831

Comment 11

[Martijn Wargers (dead)]

Fwiw, I seem to encounter this a lot in current trunk builds. No testcase, sorry. If wanted, I could try and get one.

Comment 12

[timeless]

apierce, in:
UUID	709ebd2c-173f-4327-8cb5-c912c2100831

222 /*
223 * We need to survive a GC upon failure and in case creating a new
224 * string header triggers a GC, but we've broken the invariant that
225 * rope top nodes always point to freeable JSRopeBufferInfo
226 * objects, so make them point to NULL.
227 */

I like the comment about needing to survive GC / failure :)

228 if (usingLeft)
229 left->nullifyTopNodeBuffer();
230 if (usingRight)
231 right->nullifyTopNodeBuffer();

this vaguely implies that once in a while there might be some null pointers floating around.

233 /*
234 * Try to reuse sourceBuffer. If it's not suitable, free it and create a
235 * suitable buffer.
236 */

237 if (length <= sourceBuffer->capacity) { 

... here sourceBuffer is null.

Comment 13

[Josh Matthews [:jdm]]

http://crash-stats.mozilla.com/report/index/1cfc3d9b-9c8e-4079-8ede-cca292101020 also agrees with timeless' analysis.  I'm going to reopen this, since it's a legitimate issue.

Comment 14

[Scoobidiver (away)]

There is an increase of crashes from b8pre/20101014 build.
It is #16 top crasher in 4.0b8pre for the last week.

More reports at:
http://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=js_ConcatStrings&version=Firefox%3A4.0b8pre

Comment 15

[Chris Morgan]

I've been having this crash on Ubuntu 10.10, 64-bit; narrowing down it seems to be only nodes at portableapps.com that are triggering it, not Gmail or Google Reader or Twitter or portableapps.com/tracker or any other page.

Interestingly, when I upgraded from:
  4.0~b8~hg20101025r56422+nobinonly-0ubuntu1~umd1~maverick
to:
  4.0~b8~hg20101026r56482+nobinonly-0ubuntu1~umd1~maverick
the [@ js_ConcatStrings] changed to [@ Decompile] - bp-2c574be0-6abf-4ec8-841e-8a07e2101026 etc. It's still crashing in exactly the same way though. I had formerly had a recurring crash in Decompile, but that had been fixed.

Comment 16

[John Ford [:jhford] CET/CEST Berlin Time]

i am hitting this and i do not use portableapps.com and have never been there.  

fedora 12 64bit

Comment 17

[Nicholas Nethercote [inactive]]

The nullifyTopNodeBuffer() calls appear overly aggressive, ie. they are called in cases where they don't need to be -- where the invariant isn't broken.  Having said that, all nullified rope nodes then get converted to interior nodes so this shouldn't matter.  But clearly it does.

Anyway, bug 609440 will get rid of the top node buffer, and simplify various bits of this code in the process, so hopefully that'll fix things.

Comment 18

[David Mandelin [:dmandelin]]

(In reply to comment #17)
> The nullifyTopNodeBuffer() calls appear overly aggressive, ie. they are called
> in cases where they don't need to be -- where the invariant isn't broken. 
> Having said that, all nullified rope nodes then get converted to interior nodes
> so this shouldn't matter.  But clearly it does.
> 
> Anyway, bug 609440 will get rid of the top node buffer, and simplify various
> bits of this code in the process, so hopefully that'll fix things.

That bug is not currently blocking. Are we expecting it to be done for Fx4?

Comment 19

[Nicholas Nethercote [inactive]]

(In reply to comment #18)
> 
> That bug is not currently blocking. Are we expecting it to be done for Fx4?

I hope so, it'll simplify JSString quite a bit.  I think Luke is working on it -- Luke?


> > Having said that, all nullified rope nodes then get converted to interior nodes
> > so this shouldn't matter.  But clearly it does.

Ah, I bet it's a problem if any of the failure (OOM) cases occur.  Eg. if ObtainRopeBuffer() returns NULL, the rope nodes will have been nullified but they won't subsequently be converted to interior nodes.  Bug 609440 will hopefully fix this!

Comment 20

[Luke Wagner [:luke]]

Blocking a blocker and a 2% SS speedup?  On it.

Comment 21

[Marcia Knous [:marcia]]

Still around on the trunk, but in pretty low volume. Only 3 crashes in the last week.

Comment 22

[Nicholas Nethercote [inactive]]

Luke's getting close to finishing a patch for bug 609440, it simplifies js_ConcatStrings() *greatly*.

Comment 23

[David Mandelin [:dmandelin]]

OK, this bug, which is for a crash on the URL in the URL field, and blocks because it is a regression, WFM in the nightly of Dec 13. Reopen only if that site starts crashing again. 

If any other crash involving js_ConcatStrings occurs, please don't post anything here: open a new bug instead.