running js1_5/Array/regress-311515.js in the browser with WAY_TOO_MUCH_GC STATUS: Array.sort should skip holes and undefined during sort Assertion failure: offsetInArena < GC_THINGS_SIZE, at /work/mozilla/builds/ff/trunk-too-much-gc/mozilla/js/src/jsgc.c:493 I couldn't reproduce on Windows. marking security sensitive just in case.

#0 JS_Assert (s=0x28b338 "offsetInArena < GC_THINGS_SIZE", file=0x28b1c8 "mozilla/js/src/jsgc.c", ln=493) at mozilla/js/src/jsutil.c:63 #1 0x001dfd95 in js_GetGCThingFlags (thing=0xa035d90) at mozilla/js/src/jsgc.c:493 #2 0x001e2928 in js_MarkGCThing (cx=0xa035f58, thing=0xa035d90) at mozilla/js/src/jsgc.c:2445 #3 0x001e396e in js_GC (cx=0xa035f58, gckind=GC_LAST_DITCH) at mozilla/js/src/jsgc.c:2924 #4 0x001e12ff in js_NewGCThing (cx=0xa035f58, flags=1, nbytes=8) at mozilla/js/src/jsgc.c:1420 #5 0x00262126 in js_NewString (cx=0xa035f58, chars=0xa213328, length=1, gcflag=0) at mozilla/js/src/jsstr.c:2425 #6 0x001a7fb7 in JS_NewStringCopyZ (cx=0xa035f58, s=0xbfdbe506 "1") at mozilla/js/src/jsapi.c:4465 #7 0x00211fad in js_NumberToString (cx=0xa035f58, d=1) at mozilla/js/src/jsnum.c:719 #8 0x00262d6f in js_ValueToString (cx=0xa035f58, v=3) at mozilla/js/src/jsstr.c:2670 #9 0x001abd24 in sort_compare (arg=0xbfdbe6a4, a=0x99fd930, b=0x99fd934, result=0xbfdbe624) at mozilla/js/src/jsarray.c:997 #10 0x001aba44 in js_MergeSort (src=0x99fd930, nel=2, elsize=4, cmp=0x1abc62 <sort_compare>, arg=0xbfdbe6a4, tmp=0x99fd938) at mozilla/js/src/jsarray.c:912 #11 0x001ac29e in array_sort (cx=0xa035f58, obj=0xa2f6b78, argc=0, argv=0xa31d768, rval=0xbfdbe790) at mozilla/js/src/jsarray.c:1167 #12 0x001e76ff in js_Invoke (cx=0xa035f58, argc=0, flags=0) at mozilla/js/src/jsinterp.c:1396 #13 0x001fb2ea in js_Interpret (cx=0xa035f58, pc=0xa3f051e ":", result=0xbfdbf450) at mozilla/js/src/jsinterp.c:3948 #14 0x001e824f in js_Execute (cx=0xa035f58, chain=0xa1c5de0, script=0xa3f0470, down=0x0, flags=0, result=0xbfdbf55c) at mozilla/js/src/jsinterp.c:1643 #15 0x001a79d1 in JS_EvaluateUCScriptForPrincipals (cx=0xa035f58, obj=0xa1c5de0, principals=0x9a5dcf4, chars=0xa3ef370, length=2170, filename=0xa19a2f0 "http://test.mozilla.com/tests/mozilla.org/js/js1_5/Array/regress-311515.js", lineno=1, rval=0xbfdbf55c) at mozilla/js/src/jsapi.c:4302 #16 0x017b909f in nsJSContext::EvaluateString (this=0xa03e288, aScript=@0xa19b05c, aScopeObject=0xa1c5de0, aPrincipal=0x9a5dcf0, aURL=0xa19a2f0 "http://test.mozilla.com/tests/mozilla.org/js/js1_5/Array/regress-311515.js", aLineNo=1, aVersion=150, aRetValue=0x0, aIsUndefined=0xbfdbf670) at mozilla/dom/src/base/nsJSEnvironment.cpp:1306 #17 0x015c72d1 in nsScriptLoader::EvaluateScript (this=0x9af8100, aRequest=0xa19b048, aScript=@0xa19b05c) at mozilla/content/base/src/nsScriptLoader.cpp:613 #18 0x015c75a6 in nsScriptLoader::ProcessRequest (this=0x9af8100, aRequest=0xa19b048) at mozilla/content/base/src/nsScriptLoader.cpp:527 #19 0x015c7637 in nsScriptLoader::ProcessPendingRequests (this=0x9af8100) at mozilla/content/base/src/nsScriptLoader.cpp:649

This is a regression from bug 224128.

The patch changes "tvr.count = newlen * 2" from "tvr.count += newlen" as after the loop tvr.count is either newlen or newlen + 1 iff the last element was hole or undefined. In that case before the change tvr.count would be 2 * newlen + 1 and GC would read vec[newlen + 1] which is a junk value.

(In reply to comment #3) > Created an attachment (id=245716) [edit] > Fix > > The patch changes "tvr.count = newlen * 2" from "tvr.count += newlen" as after > the loop tvr.count is either newlen or newlen + 1 iff the last element was hole > or undefined. In that case before the change tvr.count would be 2 * newlen + 1 > and GC would read vec[newlen + 1] which is a junk value. ^^^^^^^^^^^^^^^^ I meant vec[newlen * 2] which is a junk value.

Comment on attachment 245716 [details] [diff] [review] Fix r=brendan@mozilla.org /be

Here is a test case that crashes on Linux without way_too_much_gc: var a = Array(3); a[0] = 1; a[1] = 2; a.sort(function () { gc(); return 1; }); Let me know if does not crash on Windows.

crashes trunk debug shell without WAY_TOO_MUCH_GC on winxp at js_GetGCThingFlags(void * thing=0xdadadad8) Line 492

It turned out that the previous test case may not crash on Linux, so here is better one: var N = 1000; // Make an array with a hole at the end var a = Array(N); for (i = 0; i < N - 1; ++i) a[i] = 1; // array_sort due to the bug for array with N elements with a hole at the end // allocates a temporary vector with 2*N+1 words. Lets create strings that on // 32 and 64 bit CPU cause allocation of the same amount of memory for their // char arrays to cause malloc in array_sort to reuse char arrays memory for // the temporary vector after we GC strings (assuming a reasonable malloc // implementation ). Then the bug triggers reinterpretation of 0xFFF0FFF0 as GC // thing. var str1 = Array(2*(2*N + 1) + 1).join(String.fromCharCode(0xFFF0)); var str2 = Array(4*(2*N + 1) + 1).join(String.fromCharCode(0xFFF0)); gc(); str1 = str2 = null; gc(); var firstCall = true; a.sort(function (a, b) { if (firstCall) { firstCall = false; gc(); } return a - b; });

I committed the patch from comment 3 to the trunk: Checking in jsarray.c; /cvsroot/mozilla/js/src/jsarray.c,v <-- jsarray.c new revision: 3.100; previous revision: 3.99 done

Test case with proper explanation in comments: var N = 1000; // Make an array with a hole at the end var a = Array(N); for (i = 0; i < N - 1; ++i) a[i] = 1; // array_sort due for array with N elements with allocates a temporary vector // with 2*N. Lets create strings that on 32 and 64 bit CPU cause allocation // of the same amount of memory + 1 word for their char arrays. After we GC // strings with a reasonable malloc implementation that memory will be most // likely reused in array_sort for the temporary vector. Then the bug causes // accessing the one-beyond-the-aloocation word and re-interpretation of // 0xFFF0FFF0 as GC thing. var str1 = Array(2*(2*N + 1) + 1).join(String.fromCharCode(0xFFF0)); var str2 = Array(4*(2*N + 1) + 1).join(String.fromCharCode(0xFFF0)); gc(); str1 = str2 = null; gc(); var firstCall = true; a.sort(function (a, b) { if (firstCall) { firstCall = false; gc(); } return a - b; });

Note these tests do not require WAY_TOO_MUCH_GC. Thanks Igor!

verified fixed 1.9 20061121 windows/linux

Given that this was a regression from bug 224128, it's not an issue on the branch.

We usually mark regressions as "blocking" the causing bug rather than "depending on" -- that way should we take the original bug on a branch in the future we will see that a real fix also requires patches from its dependencies

/cvsroot/mozilla/js/tests/js1_5/Array/regress-360681-01.js,v <-- regress-360681-01.js initial revision: 1.1 /cvsroot/mozilla/js/tests/js1_5/Array/regress-360681-02.js,v <-- regress-360681-02.js initial revision: 1.1