Sorting array of arrays can crash due to GC

VERIFIED FIXED in mozilla1.8beta4

Status

()

Core
JavaScript Engine
P1
critical
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: bz, Assigned: brendan)

Tracking

({js1.5, verified1.7.13, verified1.8})

Trunk
mozilla1.8beta4
js1.5, verified1.7.13, verified1.8
Points:
---
Bug Flags:
blocking1.8b4 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

BUILD:  Current trunk debug build

STEPS TO REPRODUCE:

1)  Load http://www.cloud9sales.com/test/raining_NSMozFail.htm
2)  Click the "All Products" link

NOTE:  What the page is doing is:

prodRainTapThr[0] = new
Array("Airedale","","ATPAIR.jpg",125,151,"Multi","RcP","$59.95",339);
...
prodRainTapThr[58] = new
Array("Yorkie","","ATPYOR.jpg",150,114,"Multi","RcL","$59.95",396);
prodRainTapThr.sort();

We crash inside Array.sort like so:

(gdb) frame 0
#0  0xb7e6fb09 in js_CompareStrings (str1=0xb32460e0, str2=0x83ae120)
    at ../../../mozilla/js/src/jsstr.c:2800
2800        s1 = JSSTRING_CHARS(str1),  s2 = JSSTRING_CHARS(str2);
(gdb) p *str1
$9 = {length = 3671775962, chars = 0xdadadada}

The problem is the code at
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsarray.c&rev=3.60&mark=816,817#812
-- astr can get GCed when we create bstr.  I've verified that if I lock astr
around the creation of bstr the crash goes away.
At brendan's prodding, I broke in js_GC -- it's being called of a DOM branch
callback with this stack:

#4  0xb6884975 in nsJSContext::DOMBranchCallback (cx=0x88010a0, script=0x0)
    at ../../../../mozilla/dom/src/base/nsJSEnvironment.cpp:516
#5  0xb7e35636 in js_EnterSharpObject (cx=0x88010a0, obj=0xb29bcbd0, idap=0x0, 
    sp=0xbfffd9d0) at ../../../mozilla/js/src/jsobj.c:507
#6  0xb7de2c69 in array_join_sub (cx=0x88010a0, obj=0xb29bcbd0, sep=0xb7e9ce44, 
    literalize=0, rval=0xbfffdb40, localeString=0)
    at ../../../mozilla/js/src/jsarray.c:369
#7  0xb7de3319 in array_toString (cx=0x88010a0, obj=0xb29bcbd0, argc=0,
argv=0xb2949d2c, 
    rval=0xbfffdb40) at ../../../mozilla/js/src/jsarray.c:545
#8  0xb7e14d4f in js_Invoke (cx=0x88010a0, argc=0, flags=2)
    at ../../../mozilla/js/src/jsinterp.c:1174
#9  0xb7e15182 in js_InternalInvoke (cx=0x88010a0, obj=0xb29bcbd0,
fval=-1259988056, 
    flags=0, argc=0, argv=0x0, rval=0xbfffdd10)
    at ../../../mozilla/js/src/jsinterp.c:1271
#10 0xb7e3f5e4 in js_TryMethod (cx=0x88010a0, obj=0xb29bcbd0, atom=0x81f6620,
argc=0, 
    argv=0x0, rval=0xbfffdd10) at ../../../mozilla/js/src/jsobj.c:3890
#11 0xb7e3d58e in js_DefaultValue (cx=0x88010a0, obj=0xb29bcbd0,
hint=JSTYPE_STRING, 
    vp=0xbfffdd54) at ../../../mozilla/js/src/jsobj.c:3217
#12 0xb7e6f78a in js_ValueToString (cx=0x88010a0, v=-1298412592)
    at ../../../mozilla/js/src/jsstr.c:2739
#13 0xb7de3bda in sort_compare (a=0xb29c2b98, b=0x
(Assignee)

Comment 2

13 years ago
Created attachment 194628 [details] [diff] [review]
fix

Fixes the bug bz identified, plus an old bug where failure from
js_ValueToNumber was not propagated.

/be
Attachment #194628 - Flags: superreview?(shaver)
Attachment #194628 - Flags: review?(mrbkap)
(Assignee)

Comment 3

13 years ago
Easy crash bug to fix.

/be
Assignee: general → brendan
Flags: blocking1.8b4+
Keywords: js1.5
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta4
Comment on attachment 194628 [details] [diff] [review]
fix

r=mrbkap
Attachment #194628 - Flags: review?(mrbkap) → review+
Comment on attachment 194628 [details] [diff] [review]
fix

sr=shaver
Attachment #194628 - Flags: superreview?(shaver) → superreview+
(Assignee)

Comment 6

13 years ago
Fixed on trunk and 1.8 branch.

/be
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Verified FIXED using trunk build 2005-09-04-06 on Windows XP SeaMonkey trunk by
loading http://www.cloud9sales.com/test/raining_NSMozFail.htm and clicking "All
Products" a few times.  No crash.
Status: RESOLVED → VERIFIED
Checking in regress-306788.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-306788.js,v  <--  regress-306788.js
initial revision: 1.1
done
Flags: testcase+
Fix checked into the 1.7 branch as part of my bug 311497 checkin.
Keywords: fixed-aviary1.0.8, fixed1.7.13
hmmm, not crashing. But "all products" page never finishes loading *endless throbber spinning*. Is that expected?

Comment 11

12 years ago
v.fixed on 1.0.1 Aviary branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060215 Firefox/1.0.8, seeing the same as Tracy, but no crash.  Looks like that is a test page, so not sure how to answer Tracy's question, but if anyone knows, please share.
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
js1_5/Regress/regress-306788.js 
verified on winxp/linux/mac, firefox 1.7.13, 1.8.0.1, 1.8, 1.9a1
verified on winxp, mozilla 1.7.13

note that I don't show a crash in the tests for firefox 1.0 or 1.0.7 on any platform for this test and don't crash in either 1.0 or 1.0.7 for the testcase when clicking the all products link.
Keywords: fixed1.7.13, fixed1.8 → verified1.7.13, verified1.8
You need to log in before you can comment on or make changes to this bug.