Closed
Bug 467499
Opened 16 years ago
Closed 16 years ago
crash using array.splice on an array with some non-set elements [@ js_GetGCThingTraceKind ]
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
People
(Reporter: timothee, Assigned: mrbkap)
Details
(6 keywords, Whiteboard: [sg:critical?] fixed-in-tracemonkey, post 1.8=branch)
Crash Data
Attachments
(4 files)
113 bytes,
text/html
|
Details | |
410 bytes,
text/html
|
Details | |
968 bytes,
patch
|
crowderbt
:
review+
shaver
:
review+
shaver
:
approval1.9.1+
|
Details | Diff | Splinter Review |
977 bytes,
patch
|
mrbkap
:
review+
dveditz
:
approval1.9.0.7+
|
Details | Diff | Splinter Review |
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•16 years ago
|
||
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Product: Firefox → Core
QA Contact: general → general
Hardware: PC → All
Version: unspecified → Trunk
Comment 2•16 years ago
|
||
Regression range: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2008-02-18+02%3A00&maxdate=2008-02-19+01%3A00
Bug 322889 or bug 418239?
Crash ID: http://crash-stats.mozilla.com/report/index/58f54169-bcfa-4845-a867-f84ca2081202
Assignee: nobody → general
Component: General → JavaScript Engine
QA Contact: general → general
Comment 3•16 years ago
|
||
STR:
- load page
- click 10x reload
- click tools
- repeat the last two steps
Updated•16 years ago
|
Severity: normal → critical
Keywords: crash,
regression
Summary: crash using array.splice on an array with some non-set elements → crash using array.splice on an array with some non-set elements [@ js_GetGCThingTraceKind ]
Updated•16 years ago
|
Flags: blocking1.9.2?
Flags: blocking1.9.1?
Flags: blocking1.9.0.6?
Comment 4•16 years ago
|
||
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.
Flags: blocking1.9.0.6? → wanted1.9.0.x+
Updated•16 years ago
|
Flags: blocking1.9.2?
Flags: blocking1.9.2-
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Comment 5•16 years ago
|
||
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?
Keywords: topcrash
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•16 years ago
|
||
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
Flags: blocking1.9.1- → blocking1.9.1?
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 :(
Flags: blocking1.9.1?
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•16 years ago
|
||
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
Flags: blocking1.9.1?
Comment 11•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
my modified testcase, calls the original testcase code in a loop...
Assignee | ||
Updated•16 years ago
|
Assignee: general → mrbkap
Assignee | ||
Comment 14•16 years ago
|
||
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!
Attachment #358806 -
Flags: review?(shaver)
Attachment #358806 -
Flags: review?(crowder)
Comment 15•16 years ago
|
||
Comment on attachment 358806 [details] [diff] [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.
Attachment #358806 -
Flags: review?(crowder) → review+
Comment 16•16 years ago
|
||
Comment on attachment 358806 [details] [diff] [review]
Fix
Did I not make ResizeSlots early-out if it didn't really need to resize? Bad shaver. :(
Attachment #358806 -
Flags: review?(shaver) → review+
Assignee | ||
Comment 17•16 years ago
|
||
Whiteboard: fixed-in-tracemonkey
Assignee | ||
Comment 18•16 years ago
|
||
Comment on attachment 358806 [details] [diff] [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.
Attachment #358806 -
Flags: approval1.9.1?
Comment 19•16 years ago
|
||
Comment on attachment 358806 [details] [diff] [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
Attachment #358806 -
Flags: approval1.9.1? → approval1.9.1+
Assignee | ||
Comment 20•16 years ago
|
||
This is the patch with the filename fixed up.
Attachment #358965 -
Flags: review+
Attachment #358965 -
Flags: approval1.9.0.7?
Comment 21•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Resolution: --- → FIXED
Updated•16 years ago
|
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey [needs 1.9.1 landing before 1.9.0 approval]
Comment 22•16 years ago
|
||
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
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9.2a1
Comment 23•16 years ago
|
||
When is this landing on 1.9.1?
Assignee | ||
Comment 24•16 years ago
|
||
Keywords: fixed1.9.1
Whiteboard: fixed-in-tracemonkey [needs 1.9.1 landing before 1.9.0 approval] → fixed-in-tracemonkey
Comment 25•16 years ago
|
||
Comment on attachment 358965 [details] [diff] [review]
Patch for the 1.9.0 branch
Approved for 1.9.0.7, a=dveditz for release-drivers.
Attachment #358965 -
Flags: approval1.9.0.7? → approval1.9.0.7+
Comment 27•16 years ago
|
||
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/
Flags: in-testsuite?
Updated•16 years ago
|
Flags: wanted1.8.1.x-
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey, post 1.8=branch
Updated•16 years ago
|
Whiteboard: fixed-in-tracemonkey, post 1.8=branch → [sg:critical?] fixed-in-tracemonkey, post 1.8=branch
Updated•14 years ago
|
Crash Signature: [@ js_GetGCThingTraceKind ]
Updated•12 years ago
|
Flags: in-testsuite? → in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•