Last Comment Bug 467499 - crash using array.splice on an array with some non-set elements [@ js_GetGCThingTraceKind ]
: crash using array.splice on an array with some non-set elements [@ js_GetGCTh...
Status: VERIFIED FIXED
: [sg:critical?] fixed-in-tracemonkey, ...
: crash, regression, testcase, topcrash, verified1.9.0.7, verified1.9.1
Product: Core
Classification: Components
Component: JavaScript Engine
: Trunk
: All All
: -- critical (vote)
: mozilla1.9.2a1
Assigned To: Blake Kaplan (:mrbkap)
: general
:
:
:
  Show dependency treegraph
 
Reported: 2008-12-01 23:10 PST by Timothee Groleau
Modified: 2009-03-15 08:23 PDT (History)
10 users (show)
sayrer: blocking1.9.2-
sayrer: blocking1.9.1+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x-
hskupin: in‑testsuite?
See Also:
Crash Signature:
[@ js_GetGCThingTraceKind ]


Attachments
reporter's testcase (113 bytes, text/html)
2008-12-02 07:58 PST, Ria Klaassen (not reading all bugmail)
no flags Details
modified testcase (410 bytes, text/html)
2009-01-25 15:42 PST, Mukunda
no flags Details
Fix (968 bytes, patch)
2009-01-25 23:43 PST, Blake Kaplan (:mrbkap)
crowderbt: review+
shaver: review+
shaver: approval1.9.1+
Details | Diff | Splinter Review
Patch for the 1.9.0 branch (977 bytes, patch)
2009-01-26 17:33 PST, Blake Kaplan (:mrbkap)
mrbkap: review+
dveditz: approval1.9.0.7+
Details | Diff | Splinter Review

Summon comment box

Description Timothee Groleau 2008-12-01 23:10:28 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4) Gecko/2008111419 Gentoo Firefox/3.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.4) Gecko/2008111419 Gentoo Firefox/3.0.4

using this simple javascript lines crash FF3.0.4:

var a = new Array(999);
a[6] = "toto";
a.splice(6, 1);

Reproducible: Always

Steps to Reproduce:
1) set a simple html document with the following code (intentionally left bare for brevity, bug occurs with proper doctype set, etc.)
======
<html>
<body>
<script>
var a = new Array(999);
a[6] = "toto";
a.splice(6, 1);
</script>
</body>
</html>
======

2) open the page in FF and wait

3) browser will crash within a few seconds
Actual Results:  
crash, sometimes with some delay of 5-10 seconds or more (!)

Expected Results:  
no crash
array should contain no element but have length 998

It seems the script does work before the crash, so I'm not sure what is going wrong. Adding an alert shows that the length has been updated, as expected:

var a = new Array(999);
a[6] = "toto";
a.splice(6, 1);
alert(a.length); // <- shows 998, as expected


Crash happens on FF windows as well
Comment 1 Ria Klaassen (not reading all bugmail) 2008-12-02 07:20:47 PST
Confirmed with trunk, Shiretoko, Firefox 3.0.4 on Windows XP. 
It is not that easy to reproduce. (Many) reloads and opening/closing error console seems to trigger it.
Comment 3 Ria Klaassen (not reading all bugmail) 2008-12-02 07:58:32 PST
Created attachment 350976 [details]
reporter's testcase

STR:
- load page
- click 10x reload
- click tools 
- repeat the last two steps
Comment 4 Daniel Veditz 2008-12-05 11:36:33 PST
Doesn't sound like a branch blocker if it's as flaky as comment 1 and 3 say, but when this is fixed on the trunk we'll happily look at branch approval requests.
Comment 5 Henrik Skupin (:whimboo) 2009-01-25 13:50:58 PST
This bug is on #64 in the top crasher list for 1.9.1 and 1.9.2a1pre.

Don't we really wanna have this fixed for 1.9.2 and even 1.9.1?
Comment 6 Mukunda 2009-01-25 14:01:15 PST
looks similar to a bug we're seeing at deviantART.com which is generating a lot of reports from our users ... I'm trying to make another test case.
Comment 7 Brendan Eich [:brendan] 2009-01-25 14:21:49 PST
Re-nominating in light of frequency. This also should be considered on memory safety grounds -- without more analysis it has to be presumed exploitable. Sayrer can you help get it assigned? Thanks,

/be
Comment 8 Mukunda 2009-01-25 15:19:00 PST
I have not been able to reproduce this in 3.1 beta 2 but it's definitely happening in 3.0.5

It also seems to be more easily reproducible with firebug enabled :(
Comment 9 Mukunda 2009-01-25 15:32:31 PST
I was wrong, it does happen in 3.1 beta - http://crash-stats.mozilla.com/report/index/34cc7139-14d0-4e40-91c1-3c9112090125?p=1
Comment 10 Henrik Skupin (:whimboo) 2009-01-25 15:34:01 PST
Please don't remove the blocking flag. It's highly reproducible here with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090124 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090124020525
Comment 11 Henrik Skupin (:whimboo) 2009-01-25 15:39:11 PST
Opening the testcase with the above mentioned build gives a bit different stack:

0  	libmozjs.dylib  JS_CallTracer  	js/src/jsgc.cpp:1085
1 	libmozjs.dylib 	array_trace 	js/src/jsarray.cpp:1069
2 	libmozjs.dylib 	JS_CallTracer 	js/src/jsgc.cpp:2692
3 	libmozjs.dylib 	js_TraceObject 	js/src/jsobj.cpp:5322
4 	libmozjs.dylib 	JS_CallTracer 	js/src/jsgc.cpp:2692
5 	libmozjs.dylib 	js_TraceObject 	js/src/jsobj.cpp:5322
Comment 12 Mukunda 2009-01-25 15:40:11 PST
sorry didn't mean to remove the flag... not sure how that happened.

The quickest way I've found to reproduce it is setting a breakpoint in the testcase code and calling that repeatedly from a loop, then just stepping through the code in firebug and boom, crashes right away.
Comment 13 Mukunda 2009-01-25 15:42:02 PST
Created attachment 358770 [details]
modified testcase

my modified testcase, calls the original testcase code in a loop...
Comment 14 Blake Kaplan (:mrbkap) 2009-01-25 23:43:04 PST
Created attachment 358806 [details] [review]
Fix

The actual bug fix here is passing |oldsize| to ResizeSlots, which was failing to initialize the newly-allocated spaces. I've also avoided calling ResizeSlots in the case that we're not going to be shrinking the slots anyway.

I'm not sure who's doing reviews in this code nowadays, so the first of crowder and shaver to get to this should take it!
Comment 15 Brian Crowder 2009-01-26 08:05:21 PST
Comment on attachment 358806 [details] [review]
Fix

Hm.  I approve of the additional length read, if your debugging shows it to be necessary, but is it actually necessary that we don't shrink the array here?  If we can, I think we should.
Comment 16 Mike Shaver (:shaver) 2009-01-26 14:33:09 PST
Comment on attachment 358806 [details] [review]
Fix

Did I not make ResizeSlots early-out if it didn't really need to resize? Bad shaver. :(
Comment 17 Blake Kaplan (:mrbkap) 2009-01-26 16:22:11 PST
http://hg.mozilla.org/tracemonkey/rev/081bfaae5839
Comment 18 Blake Kaplan (:mrbkap) 2009-01-26 16:23:21 PST
Comment on attachment 358806 [details] [review]
Fix

I'm not sure if this is going to get blocking+ status, but IMO the severity and frequency warrant a 1.9.1 landing.
Comment 19 Mike Shaver (:shaver) 2009-01-26 17:16:47 PST
Comment on attachment 358806 [details] [review]
Fix

yeah, and we don't want to fix it on 1.9.0.x without having fixed in 1.9.1. a=shaver
Comment 20 Blake Kaplan (:mrbkap) 2009-01-26 17:33:02 PST
Created attachment 358965 [details] [review]
Patch for the 1.9.0 branch

This is the patch with the filename fixed up.
Comment 22 Henrik Skupin (:whimboo) 2009-01-29 09:10:56 PST
No crash anymore with the attached testcases on trunk. Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090128 Minefield/3.2a1pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090129 Minefield/3.2a1pre
Comment 23 Samuel Sidler (old account; do not CC) 2009-02-02 07:58:27 PST
When is this landing on 1.9.1?
Comment 24 Blake Kaplan (:mrbkap) 2009-02-02 14:18:26 PST
Now: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d320b89f6f92
Comment 25 Daniel Veditz 2009-02-02 15:32:30 PST
Comment on attachment 358965 [details] [review]
Patch for the 1.9.0 branch

Approved for 1.9.0.7, a=dveditz for release-drivers.
Comment 26 Blake Kaplan (:mrbkap) 2009-02-03 15:22:40 PST
Fixed on the 1.9.0 branch.
Comment 27 Henrik Skupin (:whimboo) 2009-02-04 07:29:08 PST
Verified fixed on the 1.9.1 and 1.9.0 branch with:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204020327

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.7pre) Gecko/2009020405 GranParadiso/3.0.7pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.7pre) Gecko/2009020404 GranParadiso/3.0.7pre

Mukunda, can you please cross-verify with one of the latest nightly builds? Thanks! You can download here: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/02/

Note You need to log in before you can comment on or make changes to this bug.