From c813e63e73a46094bbdc3aad7eb6663b5b9826c6 Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Tue, 23 Nov 2010 14:40:55 +0100 Subject: [PATCH 01/42] bug 613516 - xpcshell dump doesn't print newlines. r=jorendorff --- js/src/xpconnect/shell/xpcshell.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/js/src/xpconnect/shell/xpcshell.cpp b/js/src/xpconnect/shell/xpcshell.cpp index 85ee59f2c23..277d3c4a204 100644 --- a/js/src/xpconnect/shell/xpcshell.cpp +++ b/js/src/xpconnect/shell/xpcshell.cpp @@ -449,7 +449,11 @@ Dump(JSContext *cx, uintN argc, jsval *vp) if (!str) return JS_FALSE; - JS_FileEscapedString(gOutFile, str, 0); + JSAutoByteString bytes(cx, str); + if (!bytes) + return JS_FALSE; + + fputs(bytes.ptr(), gOutFile); fflush(gOutFile); return JS_TRUE; } From 9d7887368df24ff1f3bfb29b6b77c01a715886b0 Mon Sep 17 00:00:00 2001 From: Reed Loden Date: Mon, 29 Nov 2010 03:52:27 -0800 Subject: [PATCH 02/42] Bug 615173 - Rerun 'make genservercert' in build/pgo and commit the resulting certs to fix expired test server certs and make the tree green. a=closedtree=orange:timebomb --- build/pgo/certs/cert8.db | Bin 65536 -> 65536 bytes build/pgo/certs/key3.db | Bin 61440 -> 61440 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/build/pgo/certs/cert8.db b/build/pgo/certs/cert8.db index ed2ea44e15ad0dfa41741bbb945c140299423bc2..0fe4acb0462274872abe0dfbf54e8feb7cd7a79c 100644 GIT binary patch delta 348 zcmV-i0i*tafCPYm1h6824y87LHeWW}HElIlGwCw3Fn|>=lbo`X95FC4F)}$ZF*PtS zGFlf6F)=YQGC46ZH83zTT9a>b7?T-#9e=gwjZ@iG;!fG6)h}iVJW&v#A9Y!59@6S;}h0T$I=Pq>xhhYE@?bynGb%OE6{$Py=NfZUObO zw5b7q6kY*$4^I@h?>*#^qR4HBKy2SupDGt7AeqLy3?}KtiG`2>NRuhHkF!RvRs*xQwn<5Ee4J$f delta 349 zcmV-j0iynZfCPYm1h6824nQ`OGKw-~GDI@LF;X$JFn|>=vtYAV1CvFslN>NPF)=bZ zFf}qbGcj5h4KXk=F)}$YH8MCeFd+ z`i13u)9~+2Q%7ozsnlyaZN*VZ4VJV>=C%#A09@B5(vpc+MlZa<>03Fo5n;e zfgU=0{-%mb$7Z$rR-RcbmgQ#ekI?2>MLa@?fz4sC6?E?vn?!l5_chk^-dRCp%awwc zW@iDh6kEE-~buHO^gZ)fghYk9-Ip1s#6lP2$_!hNc2r_S6!Nfv<#KIu6(b zp*EP_{#hQ8FjjBbzDlN0S5fhq>A;-Y+zQWAi`+EkdnW)AZY#ZIOb>YE#1n$QA%8!b zLYOx>PyMPA$<=&dMq^rIylJpw^!ug#Z>M$A4~3?Gg~e?hRLRFMN7mQ&P4>>MufvyE zHKXDZUrs_bRsG)Yph4*EA$$!}@tFtOCuID^oPgw#DeH!CsdaI)d5N;#<_Udr{ zSQlVYw2Ap0p1RVWRY$AODJy`)10Azu&?cP$2F(xT^h<-RkXU@xJA_#LqeVO0vpk^E z@GKpR7kxK`0C$tQ#}3*_#nBa>Ap94hcU?q3`A$R(PH&0^84?_Y*beD?tAC~3K4Q{* zn&l!my>mmq*vvZBVK_RzDq>&T&9&~d7XB2+wW!aX9Bd~3IMGq1gLjVxWTROHcj%&>Id~4YsasE3 z>Xygf!elBs%h?CjTK!xc_J8>k=Or9c2VJuIrw4JR=<_d8(z3j%0kBFeZX6%NKMYjR z%<~KI76E=a&BOdD(&oO5r~NsCJ1}c`em!ryM@c>Imu$j?&b(|pKv{iP%AqABzZ*Z2 zZrX~OCVQ=t9~KZO9AlKm+}^9Sdz~gIJ>#flWlyU6KJuct(TOx7rhi2IB_-a=KR0Yi za~XM)wxN|DJJ8N;vc4H+Q+1jjf%vuOjZ@iG;!fG6)h}iVJW&v#A9Y!66tG7g@?`#$1%w`lOIj_-a*V{=9q?^Gh&h2~Y!N8*cTJKUN#F HfDg(HzzoU^vjH%`43l2w8?)t18>1l-4*>uG From a1b63424024ea04209d454f1a625794784799fe9 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Mon, 29 Nov 2010 12:50:07 -0600 Subject: [PATCH 03/42] JS_DeepFreezeObject does not actually do anything. Bug 609024, r=Waldo. --- js/src/jsapi-tests/testDeepFreeze.cpp | 46 +++++++++++++++++++++++++-- js/src/jsapi.cpp | 2 +- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/js/src/jsapi-tests/testDeepFreeze.cpp b/js/src/jsapi-tests/testDeepFreeze.cpp index 70fb2831630..3c2d3544934 100644 --- a/js/src/jsapi-tests/testDeepFreeze.cpp +++ b/js/src/jsapi-tests/testDeepFreeze.cpp @@ -6,9 +6,49 @@ BEGIN_TEST(testDeepFreeze_bug535703) { - JSObject *obj = JS_NewObject(cx, NULL, NULL, NULL); - CHECK(obj); - JS_DeepFreezeObject(cx, obj); // don't crash + jsval v; + EVAL("var x = {}; x;", &v); + CHECK(JS_DeepFreezeObject(cx, JSVAL_TO_OBJECT(v))); // don't crash + EVAL("Object.isFrozen(x)", &v); + CHECK_SAME(v, JSVAL_TRUE); return true; } END_TEST(testDeepFreeze_bug535703) + +BEGIN_TEST(testDeepFreeze_deep) +{ + jsval a, o; + EXEC("var a = {}, o = a;\n" + "for (var i = 0; i < 10000; i++)\n" + " a = {x: a, y: a};\n"); + EVAL("a", &a); + EVAL("o", &o); + + CHECK(JS_DeepFreezeObject(cx, JSVAL_TO_OBJECT(a))); + + jsval b; + EVAL("Object.isFrozen(a)", &b); + CHECK_SAME(b, JSVAL_TRUE); + EVAL("Object.isFrozen(o)", &b); + CHECK_SAME(b, JSVAL_TRUE); + return true; +} +END_TEST(testDeepFreeze_deep) + +BEGIN_TEST(testDeepFreeze_loop) +{ + jsval x, y; + EXEC("var x = [], y = {x: x}; y.y = y; x.push(x, y);"); + EVAL("x", &x); + EVAL("y", &y); + + CHECK(JS_DeepFreezeObject(cx, JSVAL_TO_OBJECT(x))); + + jsval b; + EVAL("Object.isFrozen(x)", &b); + CHECK_SAME(b, JSVAL_TRUE); + EVAL("Object.isFrozen(y)", &b); + CHECK_SAME(b, JSVAL_TRUE); + return true; +} +END_TEST(testDeepFreeze_loop) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 253061fa02e..d27f3bcb876 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -3051,7 +3051,7 @@ JS_DeepFreezeObject(JSContext *cx, JSObject *obj) assertSameCompartment(cx, obj); /* Assume that non-extensible objects are already deep-frozen, to avoid divergence. */ - if (obj->isExtensible()) + if (!obj->isExtensible()) return true; if (!obj->freeze(cx)) From b996b5475a5fe4b98756a8e3829cd7897ec26a29 Mon Sep 17 00:00:00 2001 From: David Mandelin Date: Mon, 29 Nov 2010 13:29:44 -0800 Subject: [PATCH 04/42] Bug 554338: Show correct timezone abbreviation on OSX/Linux, r=jorendorff --- js/src/prmjtime.cpp | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/js/src/prmjtime.cpp b/js/src/prmjtime.cpp index 995d898f667..d0114e8bf94 100644 --- a/js/src/prmjtime.cpp +++ b/js/src/prmjtime.cpp @@ -583,19 +583,6 @@ PRMJ_FormatTime(char *buf, int buflen, const char *fmt, PRMJTime *prtm) int oldReportMode; #endif - /* Zero out the tm struct. Linux, SunOS 4 struct tm has extra members int - * tm_gmtoff, char *tm_zone; when tm_zone is garbage, strftime gets - * confused and dumps core. NSPR20 prtime.c attempts to fill these in by - * calling mktime on the partially filled struct, but this doesn't seem to - * work as well; the result string has "can't get timezone" for ECMA-valid - * years. Might still make sense to use this, but find the range of years - * for which valid tz information exists, and map (per ECMA hint) from the - * given year into that range. - - * N.B. This hasn't been tested with anything that actually _uses_ - * tm_gmtoff; zero might be the wrong thing to set it to if you really need - * to format a time. This fix is for jsdate.c, which only uses - * JS_FormatTime to get a string representing the time zone. */ memset(&a, 0, sizeof(struct tm)); a.tm_sec = prtm->tm_sec; @@ -605,11 +592,33 @@ PRMJ_FormatTime(char *buf, int buflen, const char *fmt, PRMJTime *prtm) a.tm_mon = prtm->tm_mon; a.tm_wday = prtm->tm_wday; + /* + * On systems where |struct tm| has members tm_gmtoff and tm_zone, we + * must fill in those values, or else strftime will return wrong results + * (e.g., bug 511726, bug 554338). + */ #if defined(HAVE_LOCALTIME_R) && defined(HAVE_TM_ZONE_TM_GMTOFF) { + /* + * Fill out |td| to the time represented by |prtm|, leaving the + * timezone fields zeroed out. localtime_r will then fill in the + * timezone fields for that local time according to the system's + * timezone parameters. + */ struct tm td; - time_t bogus = 0; - localtime_r(&bogus, &td); + memset(&td, 0, sizeof(td)); + td.tm_sec = prtm->tm_sec; + td.tm_min = prtm->tm_min; + td.tm_hour = prtm->tm_hour; + td.tm_mday = prtm->tm_mday; + td.tm_mon = prtm->tm_mon; + td.tm_wday = prtm->tm_wday; + td.tm_year = prtm->tm_year - 1900; + td.tm_yday = prtm->tm_yday; + td.tm_isdst = prtm->tm_isdst; + time_t t = mktime(&td); + localtime_r(&t, &td); + a.tm_gmtoff = td.tm_gmtoff; a.tm_zone = td.tm_zone; } From 6110e0823df1bdf4a8d088769209f8e23d02f4de Mon Sep 17 00:00:00 2001 From: David Mandelin Date: Mon, 29 Nov 2010 13:33:17 -0800 Subject: [PATCH 05/42] Bug 610480 last part: fix MSVC warnings, r=jorendorff --- js/src/jsscript.cpp | 9 ++++++--- js/src/jsscript.h | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/js/src/jsscript.cpp b/js/src/jsscript.cpp index 230ab56ef2a..ceb680f97a6 100644 --- a/js/src/jsscript.cpp +++ b/js/src/jsscript.cpp @@ -910,7 +910,7 @@ JSScript * JSScript::NewScript(JSContext *cx, uint32 length, uint32 nsrcnotes, uint32 natoms, uint32 nobjects, uint32 nupvars, uint32 nregexps, uint32 ntrynotes, uint32 nconsts, uint32 nglobals, - uint32 nClosedArgs, uint32 nClosedVars) + uint16 nClosedArgs, uint16 nClosedVars) { size_t size, vectorSize; JSScript *script; @@ -1160,12 +1160,15 @@ JSScript::NewScriptFromCG(JSContext *cx, JSCodeGenerator *cg) skip_empty: CG_COUNT_FINAL_SRCNOTES(cg, nsrcnotes); + uint16 nClosedArgs = uint16(cg->closedArgs.length()); + JS_ASSERT(nClosedArgs == cg->closedArgs.length()); + uint16 nClosedVars = uint16(cg->closedVars.length()); + JS_ASSERT(nClosedVars == cg->closedVars.length()); script = NewScript(cx, prologLength + mainLength, nsrcnotes, cg->atomList.count, cg->objectList.length, cg->upvarList.count, cg->regexpList.length, cg->ntrynotes, cg->constList.length(), - cg->globalUses.length(), cg->closedArgs.length(), - cg->closedVars.length()); + cg->globalUses.length(), nClosedArgs, nClosedVars); if (!script) return NULL; diff --git a/js/src/jsscript.h b/js/src/jsscript.h index e080a3cf715..d4d6d11e17f 100644 --- a/js/src/jsscript.h +++ b/js/src/jsscript.h @@ -208,7 +208,7 @@ struct JSScript { static JSScript *NewScript(JSContext *cx, uint32 length, uint32 nsrcnotes, uint32 natoms, uint32 nobjects, uint32 nupvars, uint32 nregexps, uint32 ntrynotes, uint32 nconsts, uint32 nglobals, - uint32 nClosedArgs, uint32 nClosedVars); + uint16 nClosedArgs, uint16 nClosedVars); static JSScript *NewScriptFromCG(JSContext *cx, JSCodeGenerator *cg); From cff611e9ece5257114abd3a6dbc7ef85064bcb5a Mon Sep 17 00:00:00 2001 From: Leon Sha Date: Tue, 30 Nov 2010 11:19:17 +0800 Subject: [PATCH 06/42] Bug 609222 - JM: Fix call mechanism and recompilation. r=dvander. Patch to make Sun Studio on X86 work (typedef int32). --- js/src/jsotypes.h | 8 ++++++++ js/src/methodjit/MethodJIT.h | 2 +- js/src/methodjit/TrampolineCompiler.cpp | 2 +- js/src/methodjit/TrampolineSUNWX86.s | 10 +++++----- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/js/src/jsotypes.h b/js/src/jsotypes.h index a79044f29aa..bfc85ccc236 100644 --- a/js/src/jsotypes.h +++ b/js/src/jsotypes.h @@ -91,6 +91,14 @@ typedef JSIntn intn; */ #if defined(AIX) && defined(HAVE_SYS_INTTYPES_H) #include +#elif defined(__SUNPRO_C) || defined(__SUNPRO_CC) +typedef JSInt64 int64; + +/* Explicit signed keyword for bitfield types is required. */ +/* Some compilers may treat them as unsigned without it. */ +typedef signed int int32; +typedef signed short int16; +typedef signed char int8; #else typedef JSInt64 int64; diff --git a/js/src/methodjit/MethodJIT.h b/js/src/methodjit/MethodJIT.h index 0773445f74e..f5b43470fba 100644 --- a/js/src/methodjit/MethodJIT.h +++ b/js/src/methodjit/MethodJIT.h @@ -87,7 +87,7 @@ struct VMFrame # ifdef JS_NO_FASTCALL inline void** returnAddressLocation() { - return reinterpret_cast(this) - 3; + return reinterpret_cast(this) - 5; } # else inline void** returnAddressLocation() { diff --git a/js/src/methodjit/TrampolineCompiler.cpp b/js/src/methodjit/TrampolineCompiler.cpp index fb266e5f42a..24531a6e019 100644 --- a/js/src/methodjit/TrampolineCompiler.cpp +++ b/js/src/methodjit/TrampolineCompiler.cpp @@ -146,7 +146,7 @@ TrampolineCompiler::generateForceReturnFast(Assembler &masm) #else // In case of no fast call, when we change the return address, // we need to make sure add esp by 8. - masm.addPtr(Imm32(8), Registers::StackPointer); + masm.addPtr(Imm32(16), Registers::StackPointer); #endif return generateForceReturn(masm); } diff --git a/js/src/methodjit/TrampolineSUNWX86.s b/js/src/methodjit/TrampolineSUNWX86.s index 16ee60fcb6b..dbc75a49106 100644 --- a/js/src/methodjit/TrampolineSUNWX86.s +++ b/js/src/methodjit/TrampolineSUNWX86.s @@ -1,4 +1,4 @@ -/ -*- Mode: C++/ tab-width: 4/ indent-tabs-mode: nil/ c-basic-offset: 4 -*- +/ -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- / ***** BEGIN LICENSE BLOCK ***** / Version: MPL 1.1/GPL 2.0/LGPL 2.1 / @@ -92,8 +92,8 @@ JaegerTrampolineReturn: .type JaegerThrowpoline, @function JaegerThrowpoline: /* For Sun Studio there is no fast call. */ - /* We add the stack by 8 before. */ - addl $0x8, %esp + /* We add the stack by 16 before. */ + addl $0x10, %esp /* Align the stack to 16 bytes. */ pushl %esp pushl (%esp) @@ -127,8 +127,8 @@ InjectJaegerReturn: movl 0x1C(%ebx), %ecx /* fp->rval_ type */ movl 0x14(%ebx), %eax /* fp->ncode_ */ /* For Sun Studio there is no fast call. */ - /* We add the stack by 8 before. */ - addl $0x8, %esp + /* We add the stack by 16 before. */ + addl $0x10, %esp /* Restore frame regs. */ movl 0x1C(%esp), %ebx /* f.fp */ jmp *%eax From 2068ab2523f422dc3cc0b49d4a5b223ffc9b2385 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Tue, 30 Nov 2010 09:34:21 -0600 Subject: [PATCH 07/42] Fix two tests that were failing in the shell for no good reason. (This will not affect how the tests work in the browser either way; one is skipped in the browser and the other is expected to fail in the browser.) no_r=me. --HG-- extra : rebase_source : 956a8358113f4626beaf32fac3810fb1038d06cb --- js/src/tests/js1_5/extensions/regress-336410-1.js | 2 +- js/src/tests/js1_8_5/regress/regress-595230-1.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/js/src/tests/js1_5/extensions/regress-336410-1.js b/js/src/tests/js1_5/extensions/regress-336410-1.js index 16ea12d8381..7165bc6b451 100644 --- a/js/src/tests/js1_5/extensions/regress-336410-1.js +++ b/js/src/tests/js1_5/extensions/regress-336410-1.js @@ -73,7 +73,7 @@ try } catch(ex) { - expect = 'InternalError: script stack space quota is exhausted'; + expect = 'InternalError: allocation size overflow'; actual = ex + ''; print(actual); } diff --git a/js/src/tests/js1_8_5/regress/regress-595230-1.js b/js/src/tests/js1_8_5/regress/regress-595230-1.js index 87c776c8f24..e88562bf2a8 100644 --- a/js/src/tests/js1_8_5/regress/regress-595230-1.js +++ b/js/src/tests/js1_8_5/regress/regress-595230-1.js @@ -11,7 +11,8 @@ var src = ' *\n' + '} catch(e) {}\n' + 'default xml namespace = x\n' + - 'for (let b in [0, 0]) \n'; + 'for (let b in [0, 0]) \n' + + '0\n'; evalcx(src, box); From df6686eb087ebc9e13b1984391a75e80cd5f40c5 Mon Sep 17 00:00:00 2001 From: "timeless@mozdev.org" Date: Tue, 30 Nov 2010 10:08:03 -0600 Subject: [PATCH 08/42] Bug 614928 PropertyTree::insertChild returns without unlocking cx->runtime when hash->add fails. r=jorendorff. --- js/src/jspropertytree.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/js/src/jspropertytree.cpp b/js/src/jspropertytree.cpp index 63cfe20749b..628942a40c4 100644 --- a/js/src/jspropertytree.cpp +++ b/js/src/jspropertytree.cpp @@ -140,7 +140,7 @@ KidsChunk::destroy(JSContext *cx, KidsChunk *chunk) /* * NB: Called with cx->runtime->gcLock held, always. - * On failure, return null after unlocking the GC and reporting out of memory. + * On failure, return false after unlocking the GC and reporting out of memory. */ bool PropertyTree::insertChild(JSContext *cx, Shape *parent, Shape *child) @@ -219,8 +219,11 @@ PropertyTree::insertChild(JSContext *cx, Shape *parent, Shape *child) KidsHash *hash = kidp->toHash(); KidsHash::AddPtr addPtr = hash->lookupForAdd(child); if (!addPtr) { - if (!hash->add(addPtr, child)) + if (!hash->add(addPtr, child)) { + JS_UNLOCK_GC(cx->runtime); + JS_ReportOutOfMemory(cx); return false; + } } else { // FIXME ignore duplicate child case here, going thread-local soon! } From 9520a5b1a41d515c161dbc6bd73a99501552ca0c Mon Sep 17 00:00:00 2001 From: "timeless@mozdev.org" Date: Tue, 30 Nov 2010 10:16:21 -0600 Subject: [PATCH 09/42] Bug 615068 obj is only used ifdef DEBUG in JS_NextProperty. r=jorendorff. --- js/src/jsapi.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index d27f3bcb876..1166ec2add2 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -3956,7 +3956,6 @@ JS_PUBLIC_API(JSBool) JS_NextProperty(JSContext *cx, JSObject *iterobj, jsid *idp) { jsint i; - JSObject *obj; const Shape *shape; JSIdArray *ida; @@ -3965,15 +3964,9 @@ JS_NextProperty(JSContext *cx, JSObject *iterobj, jsid *idp) i = iterobj->getSlot(JSSLOT_ITER_INDEX).toInt32(); if (i < 0) { /* Native case: private data is a property tree node pointer. */ - obj = iterobj->getParent(); - JS_ASSERT(obj->isNative()); + JS_ASSERT(iterobj->getParent()->isNative()); shape = (Shape *) iterobj->getPrivate(); - /* - * If the next property mapped by obj in the property tree ancestor - * line is not enumerable, or it's an alias, skip it and keep on trying - * to find an enumerable property that is still in obj. - */ while (shape->previous() && (!shape->enumerable() || shape->isAlias())) shape = shape->previous(); From dcc8dc67e213dd3b49ac865fd87f1b90f74f5d87 Mon Sep 17 00:00:00 2001 From: Tom Schuster Date: Tue, 30 Nov 2010 10:22:18 -0800 Subject: [PATCH 10/42] Bug 591172: make typeof comparisons faster in JM, r=dvander, a=sayrer --- .../assembler/MacroAssemblerX86_64.h | 6 +++ js/src/methodjit/FastOps.cpp | 47 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/js/src/assembler/assembler/MacroAssemblerX86_64.h b/js/src/assembler/assembler/MacroAssemblerX86_64.h index a772ea4f06a..2a13206c831 100644 --- a/js/src/assembler/assembler/MacroAssemblerX86_64.h +++ b/js/src/assembler/assembler/MacroAssemblerX86_64.h @@ -380,6 +380,12 @@ public: m_assembler.movzbl_rr(dest, dest); } + void setPtr(Condition cond, RegisterID left, ImmPtr right, RegisterID dest) + { + move(right, scratchRegister); + setPtr(cond, left, scratchRegister, dest); + } + Jump branchPtr(Condition cond, RegisterID left, RegisterID right) { m_assembler.cmpq_rr(right, left); diff --git a/js/src/methodjit/FastOps.cpp b/js/src/methodjit/FastOps.cpp index 5c6b129ad05..785f0473dd8 100644 --- a/js/src/methodjit/FastOps.cpp +++ b/js/src/methodjit/FastOps.cpp @@ -914,6 +914,53 @@ mjit::Compiler::jsop_typeof() } } + JSOp fused = JSOp(PC[JSOP_TYPEOF_LENGTH]); + if (fused == JSOP_STRING && !fe->isTypeKnown()) { + JSOp op = JSOp(PC[JSOP_TYPEOF_LENGTH + JSOP_STRING_LENGTH]); + + if (op == JSOP_STRICTEQ || op == JSOP_EQ || op == JSOP_STRICTNE || op == JSOP_NE) { + JSAtom *atom = script->getAtom(fullAtomIndex(PC + JSOP_TYPEOF_LENGTH)); + JSRuntime *rt = cx->runtime; + JSValueType type = JSVAL_TYPE_UNINITIALIZED; + Assembler::Condition cond = (op == JSOP_STRICTEQ || op == JSOP_EQ) + ? Assembler::Equal + : Assembler::NotEqual; + + if (atom == rt->atomState.typeAtoms[JSTYPE_VOID]) { + type = JSVAL_TYPE_UNDEFINED; + } else if (atom == rt->atomState.typeAtoms[JSTYPE_STRING]) { + type = JSVAL_TYPE_STRING; + } else if (atom == rt->atomState.typeAtoms[JSTYPE_BOOLEAN]) { + type = JSVAL_TYPE_BOOLEAN; + } else if (atom == rt->atomState.typeAtoms[JSTYPE_NUMBER]) { + type = JSVAL_TYPE_INT32; + + /* JSVAL_TYPE_DOUBLE is 0x0 and JSVAL_TYPE_INT32 is 0x1, use <= or > to match both */ + cond = (cond == Assembler::Equal) ? Assembler::BelowOrEqual : Assembler::Above; + } + + if (type != JSVAL_TYPE_UNINITIALIZED) { + PC += JSOP_STRING_LENGTH;; + PC += JSOP_EQ_LENGTH; + + RegisterID result = frame.allocReg(Registers::SingleByteRegs); + +#if defined JS_NUNBOX32 + if (frame.shouldAvoidTypeRemat(fe)) + masm.set32(cond, masm.tagOf(frame.addressOf(fe)), ImmType(type), result); + else + masm.set32(cond, frame.tempRegForType(fe), ImmType(type), result); +#elif defined JS_PUNBOX64 + masm.setPtr(cond, frame.tempRegForType(fe), ImmType(type), result); +#endif + + frame.pop(); + frame.pushTypedPayload(JSVAL_TYPE_BOOLEAN, result); + return; + } + } + } + prepareStubCall(Uses(1)); INLINE_STUBCALL(stubs::TypeOf); frame.pop(); From bf0a0f2c33a1b7238aa1fcd5b0ef42a05f687c46 Mon Sep 17 00:00:00 2001 From: David Mandelin Date: Tue, 30 Nov 2010 10:25:07 -0800 Subject: [PATCH 11/42] Bug 614915: set return value correctly when sorting a list containing only undefined values, r=cdleary --- js/src/jit-test/tests/basic/bug614915.js | 2 ++ js/src/jsarray.cpp | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 js/src/jit-test/tests/basic/bug614915.js diff --git a/js/src/jit-test/tests/basic/bug614915.js b/js/src/jit-test/tests/basic/bug614915.js new file mode 100644 index 00000000000..b4ab9491ae4 --- /dev/null +++ b/js/src/jit-test/tests/basic/bug614915.js @@ -0,0 +1,2 @@ +var s = [undefined, undefined].sort(); +assertEq(s.length, 2); diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 57d8350dcb0..57aa75b5bde 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -1843,8 +1843,10 @@ js::array_sort(JSContext *cx, uintN argc, Value *vp) ++newlen; } - if (newlen == 0) + if (newlen == 0) { + vp->setObject(*obj); return true; /* The array has only holes and undefs. */ + } /* * The first newlen elements of vec are copied from the array object From e067f6da9108f2b29ddd57fb023464d4dfaf110a Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 30 Nov 2010 17:11:02 -0800 Subject: [PATCH 12/42] Setting debug mode should purge call ICs (bug 612640, r=bhackett). --- js/src/jsdbgapi.cpp | 28 ++++++++++++++++++++++++++++ js/src/methodjit/MethodJIT.h | 1 + js/src/methodjit/MonoIC.cpp | 20 ++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index 19f9184b79c..111d30fffd0 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -115,6 +115,28 @@ JS_SetRuntimeDebugMode(JSRuntime *rt, JSBool debug) rt->debugMode = debug; } +static void +PurgeCallICs(JSContext *cx, JSScript *start) +{ +#ifdef JS_METHODJIT + for (JSScript *script = start; + &script->links != &cx->compartment->scripts; + script = (JSScript *)script->links.next) + { + // Debug mode does not use call ICs. + if (script->debugMode) + continue; + + JS_ASSERT(!IsScriptLive(cx, script)); + + if (script->jitNormal) + script->jitNormal->nukeScriptDependentICs(); + if (script->jitCtor) + script->jitCtor->nukeScriptDependentICs(); + } +#endif +} + JS_FRIEND_API(JSBool) js_SetDebugMode(JSContext *cx, JSBool debug) { @@ -134,6 +156,12 @@ js_SetDebugMode(JSContext *cx, JSBool debug) */ js::mjit::Recompiler recompiler(cx, script); if (!recompiler.recompile()) { + /* + * If recompilation failed, we could be in a state where + * remaining compiled scripts hold call IC references that + * have been destroyed by recompilation. Clear those ICs now. + */ + PurgeCallICs(cx, script); cx->compartment->debugMode = JS_FALSE; return JS_FALSE; } diff --git a/js/src/methodjit/MethodJIT.h b/js/src/methodjit/MethodJIT.h index f5b43470fba..12903c2ff37 100644 --- a/js/src/methodjit/MethodJIT.h +++ b/js/src/methodjit/MethodJIT.h @@ -332,6 +332,7 @@ struct JITScript { return jcheck >= jitcode && jcheck < jitcode + code.m_size; } + void nukeScriptDependentICs(); void sweepCallICs(); void purgeMICs(); void purgePICs(); diff --git a/js/src/methodjit/MonoIC.cpp b/js/src/methodjit/MonoIC.cpp index 02dda5cfb66..d17b42b1bee 100644 --- a/js/src/methodjit/MonoIC.cpp +++ b/js/src/methodjit/MonoIC.cpp @@ -1097,6 +1097,26 @@ ic::PurgeMICs(JSContext *cx, JSScript *script) script->jitCtor->purgeMICs(); } +void +JITScript::nukeScriptDependentICs() +{ + if (!nCallICs) + return; + + Repatcher repatcher(this); + + for (uint32 i = 0; i < nCallICs; i++) { + ic::CallICInfo &ic = callICs[i]; + if (!ic.fastGuardedObject) + continue; + repatcher.repatch(ic.funGuard, NULL); + repatcher.relink(ic.funJump, ic.slowPathStart); + ic.releasePool(CallICInfo::Pool_ClosureStub); + ic.fastGuardedObject = NULL; + ic.hasJsFunCheck = false; + } +} + void JITScript::sweepCallICs() { From 0c7d481977c6af9cd674e78ba861acde47d97bc4 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 30 Nov 2010 17:14:01 -0800 Subject: [PATCH 13/42] Fix prototype guards on array hole ICs (bug 615440, r=dmandelin). --- js/src/jit-test/tests/jaeger/bug615440.js | 5 ++++ js/src/methodjit/PolyIC.cpp | 36 ++++++++++------------- 2 files changed, 21 insertions(+), 20 deletions(-) create mode 100644 js/src/jit-test/tests/jaeger/bug615440.js diff --git a/js/src/jit-test/tests/jaeger/bug615440.js b/js/src/jit-test/tests/jaeger/bug615440.js new file mode 100644 index 00000000000..1ed4cd1c043 --- /dev/null +++ b/js/src/jit-test/tests/jaeger/bug615440.js @@ -0,0 +1,5 @@ +Array.prototype.__proto__ = null; +for (var r = 0; r < 3; ++r) [][0] = 1; + +// Don't crash. + diff --git a/js/src/methodjit/PolyIC.cpp b/js/src/methodjit/PolyIC.cpp index 90892ea1567..e959742c457 100644 --- a/js/src/methodjit/PolyIC.cpp +++ b/js/src/methodjit/PolyIC.cpp @@ -2373,24 +2373,21 @@ SetElementIC::attachHoleStub(JSContext *cx, JSObject *obj, int32 keyval) Assembler masm; - // Test for indexed properties in Array.prototype. It is safe to bake in - // this pointer because changing __proto__ will slowify. - JSObject *arrayProto = obj->getProto(); - masm.move(ImmPtr(arrayProto), objReg); - Jump extendedArray = masm.branchTest32(Assembler::NonZero, - Address(objReg, offsetof(JSObject, flags)), - Imm32(JSObject::INDEXED)); + Vector fails(cx); - // Text for indexed properties in Object.prototype. Guard that - // Array.prototype doesn't change, too. - JSObject *objProto = arrayProto->getProto(); - Jump sameProto = masm.branchPtr(Assembler::NotEqual, - Address(objReg, offsetof(JSObject, proto)), - ImmPtr(objProto)); - masm.move(ImmPtr(objProto), objReg); - Jump extendedObject = masm.branchTest32(Assembler::NonZero, - Address(objReg, offsetof(JSObject, flags)), - Imm32(JSObject::INDEXED)); + // Test for indexed properties in Array.prototype. We test each shape + // along the proto chain. This affords us two optimizations: + // 1) Loading the prototype can be avoided because the shape would change; + // instead we can bake in their identities. + // 2) We only have to test the shape, rather than INDEXED. + for (JSObject *pobj = obj->getProto(); pobj; pobj = pobj->getProto()) { + if (!pobj->isNative()) + return disable(cx, "non-native array prototype"); + masm.move(ImmPtr(pobj), objReg); + Jump j = masm.guardShape(objReg, pobj); + if (!fails.append(j)) + return error(cx); + } // Restore |obj|. masm.rematPayload(StateRemat::FromInt32(objRemat), objReg); @@ -2438,9 +2435,8 @@ SetElementIC::attachHoleStub(JSContext *cx, JSObject *obj, int32 keyval) return disable(cx, "code memory is out of range"); // Patch all guards. - buffer.link(extendedArray, slowPathStart); - buffer.link(sameProto, slowPathStart); - buffer.link(extendedObject, slowPathStart); + for (size_t i = 0; i < fails.length(); i++) + buffer.link(fails[i], slowPathStart); buffer.link(done, fastPathRejoin); CodeLocationLabel cs = buffer.finalize(); From 1b9203534fb9eec99253485bc78275411101eb18 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 1 Dec 2010 14:23:44 -0800 Subject: [PATCH 14/42] Bug 580515 - TM: LIR_cmovd mishandled with X86_FORCE_SSE2=no. r=edwsmith. --HG-- extra : convert_revision : 4effe362e918583ec7b98b08da24f02c0833d306 --- js/src/nanojit/NativeX64.cpp | 125 ++++++++++++++++++---------------- js/src/nanojit/NativeX64.h | 9 ++- js/src/nanojit/Nativei386.cpp | 107 +++++++++++++++++------------ js/src/nanojit/Nativei386.h | 9 +-- 4 files changed, 142 insertions(+), 108 deletions(-) diff --git a/js/src/nanojit/NativeX64.cpp b/js/src/nanojit/NativeX64.cpp index e33fe748a0a..8b0e7761623 100644 --- a/js/src/nanojit/NativeX64.cpp +++ b/js/src/nanojit/NativeX64.cpp @@ -1173,20 +1173,24 @@ namespace nanojit Register rf = findRegFor(iffalse, allow & ~rmask(rr)); if (ins->isop(LIR_cmovd)) { + // See Nativei386.cpp:asm_cmov() for an explanation of the subtleties here. NIns* target = _nIns; asm_nongp_copy(rr, rf); - asm_branch(false, cond, target); + asm_branch_helper(false, cond, target); // If 'iftrue' isn't in a register, it can be clobbered by 'ins'. Register rt = iftrue->isInReg() ? iftrue->getReg() : rr; if (rr != rt) asm_nongp_copy(rr, rt); + freeResourcesOf(ins); if (!iftrue->isInReg()) { NanoAssert(rt == rr); findSpecificRegForUnallocated(iftrue, rr); } + + asm_cmp(cond); return; } @@ -1194,8 +1198,8 @@ namespace nanojit Register rt = iftrue->isInReg() ? iftrue->getReg() : rr; // WARNING: We cannot generate any code that affects the condition - // codes between the MRcc generation here and the asm_cmp() call - // below. See asm_cmp() for more details. + // codes between the MRcc generation here and the asm_cmpi() call + // below. See asm_cmpi() for more details. LOpcode condop = cond->opcode(); if (ins->isop(LIR_cmovi)) { switch (condop) { @@ -1234,30 +1238,36 @@ namespace nanojit findSpecificRegForUnallocated(iftrue, rr); } - asm_cmp(cond); + asm_cmpi(cond); } - NIns* Assembler::asm_branch(bool onFalse, LIns *cond, NIns *target) { - NanoAssert(cond->isCmp()); - LOpcode condop = cond->opcode(); + NIns* Assembler::asm_branch(bool onFalse, LIns* cond, NIns* target) { + NIns* patch = asm_branch_helper(onFalse, cond, target); + asm_cmp(cond); + return patch; + } + NIns* Assembler::asm_branch_helper(bool onFalse, LIns *cond, NIns *target) { if (target && !isTargetWithinS32(target)) { - // conditional jumps beyond 32bit range, so invert the branch/compare - // and emit an unconditional jump to the target + // A conditional jump beyond 32-bit range, so invert the + // branch/compare and emit an unconditional jump to the target: // j(inverted) B1 // jmp target // B1: NIns* shortTarget = _nIns; JMP(target); target = shortTarget; - onFalse = !onFalse; } - if (isCmpDOpcode(condop)) - return asm_branchd(onFalse, cond, target); + return isCmpDOpcode(cond->opcode()) + ? asm_branchd_helper(onFalse, cond, target) + : asm_branchi_helper(onFalse, cond, target); + } + NIns* Assembler::asm_branchi_helper(bool onFalse, LIns *cond, NIns *target) { // We must ensure there's room for the instruction before calculating // the offset. And the offset determines the opcode (8bit or 32bit). + LOpcode condop = cond->opcode(); if (target && isTargetWithinS8(target)) { if (onFalse) { switch (condop) { @@ -1315,9 +1325,7 @@ namespace nanojit } } } - NIns *patch = _nIns; // address of instruction to patch - asm_cmp(cond); - return patch; + return _nIns; // address of instruction to patch } NIns* Assembler::asm_branch_ov(LOpcode, NIns* target) { @@ -1334,13 +1342,17 @@ namespace nanojit return _nIns; } + void Assembler::asm_cmp(LIns *cond) { + isCmpDOpcode(cond->opcode()) ? asm_cmpd(cond) : asm_cmpi(cond); + } + // WARNING: this function cannot generate code that will affect the // condition codes prior to the generation of the test/cmp. See - // Nativei386.cpp:asm_cmp() for details. - void Assembler::asm_cmp(LIns *cond) { + // Nativei386.cpp:asm_cmpi() for details. + void Assembler::asm_cmpi(LIns *cond) { LIns *b = cond->oprnd2(); if (isImm32(b)) { - asm_cmp_imm(cond); + asm_cmpi_imm(cond); return; } LIns *a = cond->oprnd1(); @@ -1361,7 +1373,7 @@ namespace nanojit } } - void Assembler::asm_cmp_imm(LIns *cond) { + void Assembler::asm_cmpi_imm(LIns *cond) { LOpcode condop = cond->opcode(); LIns *a = cond->oprnd1(); LIns *b = cond->oprnd2(); @@ -1399,11 +1411,9 @@ namespace nanojit // LIR_jt jae ja swap+jae swap+ja jp over je // LIR_jf jb jbe swap+jb swap+jbe jne+jp - NIns* Assembler::asm_branchd(bool onFalse, LIns *cond, NIns *target) { + NIns* Assembler::asm_branchd_helper(bool onFalse, LIns *cond, NIns *target) { LOpcode condop = cond->opcode(); NIns *patch; - LIns *a = cond->oprnd1(); - LIns *b = cond->oprnd2(); if (condop == LIR_eqd) { if (onFalse) { // branch if unordered or != @@ -1422,34 +1432,23 @@ namespace nanojit } } else { - if (condop == LIR_ltd) { - condop = LIR_gtd; - LIns *t = a; a = b; b = t; - } else if (condop == LIR_led) { - condop = LIR_ged; - LIns *t = a; a = b; b = t; - } - if (condop == LIR_gtd) { - if (onFalse) - JBE(8, target); - else - JA(8, target); - } else { // LIR_ged - if (onFalse) - JB(8, target); - else - JAE(8, target); + // LIR_ltd and LIR_gtd are handled by the same case because + // asm_cmpd() converts LIR_ltd(a,b) to LIR_gtd(b,a). Likewise for + // LIR_led/LIR_ged. + switch (condop) { + case LIR_ltd: + case LIR_gtd: if (onFalse) JBE(8, target); else JA(8, target); break; + case LIR_led: + case LIR_ged: if (onFalse) JB(8, target); else JAE(8, target); break; + default: NanoAssert(0); break; } patch = _nIns; } - asm_cmpd(a, b); return patch; } void Assembler::asm_condd(LIns *ins) { LOpcode op = ins->opcode(); - LIns *a = ins->oprnd1(); - LIns *b = ins->oprnd2(); if (op == LIR_eqd) { // result = ZF & !PF, must do logic on flags // r = al|bl|cl|dl, can only use rh without rex prefix @@ -1460,30 +1459,40 @@ namespace nanojit X86_SETNP(r); // setnp rh rh = !PF X86_SETE(r); // sete rl rl = ZF } else { - if (op == LIR_ltd) { - op = LIR_gtd; - LIns *t = a; a = b; b = t; - } else if (op == LIR_led) { - op = LIR_ged; - LIns *t = a; a = b; b = t; - } + // LIR_ltd and LIR_gtd are handled by the same case because + // asm_cmpd() converts LIR_ltd(a,b) to LIR_gtd(b,a). Likewise for + // LIR_led/LIR_ged. Register r = prepareResultReg(ins, GpRegs); // x64 can use any GPR as setcc target MOVZX8(r, r); - if (op == LIR_gtd) - SETA(r); - else - SETAE(r); + switch (op) { + case LIR_ltd: + case LIR_gtd: SETA(r); break; + case LIR_led: + case LIR_ged: SETAE(r); break; + default: NanoAssert(0); break; + } } freeResourcesOf(ins); - asm_cmpd(a, b); + asm_cmpd(ins); } // WARNING: This function cannot generate any code that will affect the - // condition codes prior to the generation of the ucomisd. See asm_cmp() + // condition codes prior to the generation of the ucomisd. See asm_cmpi() // for more details. - void Assembler::asm_cmpd(LIns *a, LIns *b) { + void Assembler::asm_cmpd(LIns *cond) { + LOpcode opcode = cond->opcode(); + LIns* a = cond->oprnd1(); + LIns* b = cond->oprnd2(); + // First, we convert (a < b) into (b > a), and (a <= b) into (b >= a). + if (opcode == LIR_ltd) { + opcode = LIR_gtd; + LIns* t = a; a = b; b = t; + } else if (opcode == LIR_led) { + opcode = LIR_ged; + LIns* t = a; a = b; b = t; + } Register ra, rb; findRegFor2(FpRegs, a, ra, FpRegs, b, rb); UCOMISD(ra, rb); @@ -1518,7 +1527,7 @@ namespace nanojit } // WARNING: the code generated by this function must not affect the - // condition codes. See asm_cmp() for details. + // condition codes. See asm_cmpi() for details. void Assembler::asm_restore(LIns *ins, Register r) { if (ins->isop(LIR_allocp)) { int d = arDisp(ins); @@ -1587,7 +1596,7 @@ namespace nanojit } freeResourcesOf(ins); - asm_cmp(ins); + asm_cmpi(ins); } void Assembler::asm_ret(LIns *ins) { diff --git a/js/src/nanojit/NativeX64.h b/js/src/nanojit/NativeX64.h index 37dbc5de4a9..872b5f20ee1 100644 --- a/js/src/nanojit/NativeX64.h +++ b/js/src/nanojit/NativeX64.h @@ -423,9 +423,12 @@ namespace nanojit void endLoadRegs(LIns *ins);\ void dis(NIns *p, int bytes);\ void asm_cmp(LIns*);\ - void asm_cmp_imm(LIns*);\ - void asm_cmpd(LIns*, LIns*);\ - NIns* asm_branchd(bool, LIns*, NIns*);\ + void asm_cmpi(LIns*);\ + void asm_cmpi_imm(LIns*);\ + void asm_cmpd(LIns*);\ + NIns* asm_branch_helper(bool, LIns*, NIns*);\ + NIns* asm_branchi_helper(bool, LIns*, NIns*);\ + NIns* asm_branchd_helper(bool, LIns*, NIns*);\ void asm_div(LIns *ins);\ void asm_div_mod(LIns *ins);\ int max_stk_used;\ diff --git a/js/src/nanojit/Nativei386.cpp b/js/src/nanojit/Nativei386.cpp index 289a1f75261..d5df6de7bb3 100644 --- a/js/src/nanojit/Nativei386.cpp +++ b/js/src/nanojit/Nativei386.cpp @@ -854,8 +854,6 @@ namespace nanojit inline void Assembler::FLD1() { count_fpu(); FPUc(0xd9e8); asm_output("fld1"); fpu_push(); } inline void Assembler::FLDZ() { count_fpu(); FPUc(0xd9ee); asm_output("fldz"); fpu_push(); } - inline void Assembler::FFREE(R r) { count_fpu(); FPU(0xddc0, r); asm_output("ffree %s",gpn(r)); } - inline void Assembler::FST32(bool p, I32 d, R b){ count_stq(); FPUm(0xd902|(p?1:0), d, b); asm_output("fst%s32 %d(%s)", (p?"p":""), d, gpn(b)); if (p) fpu_pop(); } inline void Assembler::FSTQ(bool p, I32 d, R b) { count_stq(); FPUm(0xdd02|(p?1:0), d, b); asm_output("fst%sq %d(%s)", (p?"p":""), d, gpn(b)); if (p) fpu_pop(); } @@ -894,8 +892,6 @@ namespace nanojit inline void Assembler::FMULdm( const double* dm) { count_ldq(); FPUdm(0xdc01, dm); asm_output("fmul (%p)", (void*)dm); } inline void Assembler::FDIVRdm(const double* dm) { count_ldq(); FPUdm(0xdc07, dm); asm_output("fdivr (%p)", (void*)dm); } - inline void Assembler::FINCSTP() { count_fpu(); FPUc(0xd9f7); asm_output("fincstp"); fpu_pop(); } - inline void Assembler::FCOMP() { count_fpu(); FPUc(0xD8D9); asm_output("fcomp"); fpu_pop();} inline void Assembler::FCOMPP() { count_fpu(); FPUc(0xDED9); asm_output("fcompp"); fpu_pop();fpu_pop();} inline void Assembler::FLDr(R r) { count_ldq(); FPU(0xd9c0, r); asm_output("fld %s", gpn(r)); fpu_push(); } @@ -1208,7 +1204,7 @@ namespace nanojit } // WARNING: the code generated by this function must not affect the - // condition codes. See asm_cmp(). + // condition codes. See asm_cmpi(). void Assembler::asm_restore(LIns* ins, Register r) { NanoAssert(ins->getReg() == r); @@ -1521,19 +1517,18 @@ namespace nanojit } } - NIns* Assembler::asm_branch(bool branchOnFalse, LIns* cond, NIns* targ) + NIns* Assembler::asm_branch_helper(bool branchOnFalse, LIns* cond, NIns* targ) { - LOpcode condop = cond->opcode(); - NanoAssert(cond->isCmp()); - - // Handle float conditions separately. - if (isCmpDOpcode(condop)) { - return asm_branchd(branchOnFalse, cond, targ); - } + return isCmpDOpcode(cond->opcode()) + ? asm_branchd_helper(branchOnFalse, cond, targ) + : asm_branchi_helper(branchOnFalse, cond, targ); + } + NIns* Assembler::asm_branchi_helper(bool branchOnFalse, LIns* cond, NIns* targ) + { if (branchOnFalse) { // op == LIR_xf/LIR_jf - switch (condop) { + switch (cond->opcode()) { case LIR_eqi: JNE(targ); break; case LIR_lti: JNL(targ); break; case LIR_lei: JNLE(targ); break; @@ -1547,7 +1542,7 @@ namespace nanojit } } else { // op == LIR_xt/LIR_jt - switch (condop) { + switch (cond->opcode()) { case LIR_eqi: JE(targ); break; case LIR_lti: JL(targ); break; case LIR_lei: JLE(targ); break; @@ -1560,7 +1555,12 @@ namespace nanojit default: NanoAssert(0); break; } } - NIns* at = _nIns; + return _nIns; + } + + NIns* Assembler::asm_branch(bool branchOnFalse, LIns* cond, NIns* targ) + { + NIns* at = asm_branch_helper(branchOnFalse, cond, targ); asm_cmp(cond); return at; } @@ -1584,6 +1584,11 @@ namespace nanojit JMP_indexed(indexreg, 2, table); } + void Assembler::asm_cmp(LIns *cond) + { + isCmpDOpcode(cond->opcode()) ? asm_cmpd(cond) : asm_cmpi(cond); + } + // This generates a 'test' or 'cmp' instruction for a condition, which // causes the condition codes to be set appropriately. It's used with // conditional branches, conditional moves, and when generating @@ -1623,7 +1628,7 @@ namespace nanojit // asm_restore(), that means that asm_restore() cannot generate code which // affects the condition codes. // - void Assembler::asm_cmp(LIns *cond) + void Assembler::asm_cmpi(LIns *cond) { LIns* lhs = cond->oprnd1(); LIns* rhs = cond->oprnd2(); @@ -1734,7 +1739,7 @@ namespace nanojit freeResourcesOf(ins); - asm_cmp(ins); + asm_cmpi(ins); } // Two example cases for "ins = add lhs, rhs". '*' lines are those @@ -2051,11 +2056,10 @@ namespace nanojit (ins->isop(LIR_cmovd) && iftrue->isD() && iffalse->isD())); if (!_config.i386_sse2 && ins->isop(LIR_cmovd)) { + // See the SSE2 case below for an explanation of the subtleties here. debug_only( Register rr = ) prepareResultReg(ins, x87Regs); NanoAssert(FST0 == rr); - NanoAssert(!iftrue->isInReg() || iftrue->getReg() == FST0); - - NanoAssert(!iffalse->isInReg()); + NanoAssert(!iftrue->isInReg() && !iffalse->isInReg()); NIns* target = _nIns; @@ -2065,52 +2069,73 @@ namespace nanojit int df = findMemFor(iffalse); FLDQ(df, FP); } + FSTP(FST0); // pop the stack + asm_branch_helper(false, condval, target); - FINCSTP(); - // Its not sufficient to merely decrement the FP stack pointer, we have to - // also free FST0, otherwise the load above fails. - FFREE(FST0); - asm_branch(false, condval, target); - + NanoAssert(ins->getReg() == rr); freeResourcesOf(ins); if (!iftrue->isInReg()) findSpecificRegForUnallocated(iftrue, FST0); + asm_cmp(condval); + return; } RegisterMask allow = ins->isD() ? XmmRegs : GpRegs; - Register rr = prepareResultReg(ins, allow); - Register rf = findRegFor(iffalse, allow & ~rmask(rr)); if (ins->isop(LIR_cmovd)) { + // The obvious way to handle this is as follows: + // + // mov rr, rt # only needed if rt is live afterwards + // do comparison + // jt end + // mov rr, rf + // end: + // + // The problem with this is that doing the comparison can cause + // registers to be evicted, possibly including 'rr', which holds + // 'ins'. And that screws things up. So instead we do this: + // + // do comparison + // mov rr, rt # only needed if rt is live afterwards + // jt end + // mov rr, rf + // end: + // + // Putting the 'mov' between the comparison and the jump is ok + // because move instructions don't modify the condition codes. + // NIns* target = _nIns; asm_nongp_copy(rr, rf); - asm_branch(false, condval, target); + asm_branch_helper(false, condval, target); // If 'iftrue' isn't in a register, it can be clobbered by 'ins'. Register rt = iftrue->isInReg() ? iftrue->getReg() : rr; if (rr != rt) asm_nongp_copy(rr, rt); + + NanoAssert(ins->getReg() == rr); freeResourcesOf(ins); if (!iftrue->isInReg()) { NanoAssert(rt == rr); findSpecificRegForUnallocated(iftrue, rr); } + + asm_cmp(condval); return; } // If 'iftrue' isn't in a register, it can be clobbered by 'ins'. Register rt = iftrue->isInReg() ? iftrue->getReg() : rr; - NanoAssert(ins->isop(LIR_cmovi)); // WARNING: We cannot generate any code that affects the condition - // codes between the MRcc generation here and the asm_cmp() call - // below. See asm_cmp() for more details. + // codes between the MRcc generation here and the asm_cmpi() call + // below. See asm_cmpi() for more details. switch (condval->opcode()) { // Note that these are all opposites... case LIR_eqi: MRNE(rr, rf); break; @@ -2128,6 +2153,7 @@ namespace nanojit if (rr != rt) MR(rr, rt); + NanoAssert(ins->getReg() == rr); freeResourcesOf(ins); if (!iftrue->isInReg()) { NanoAssert(rt == rr); @@ -2614,7 +2640,7 @@ namespace nanojit } } - NIns* Assembler::asm_branchd(bool branchOnFalse, LIns *cond, NIns *targ) + NIns* Assembler::asm_branchd_helper(bool branchOnFalse, LIns* cond, NIns *targ) { NIns* at = 0; LOpcode opcode = cond->opcode(); @@ -2673,14 +2699,13 @@ namespace nanojit if (!at) at = _nIns; - asm_cmpd(cond); return at; } // WARNING: This function cannot generate any code that will affect the // condition codes prior to the generation of the - // ucomisd/fcompp/fcmop/fcom. See asm_cmp() for more details. + // ucomisd/fcompp/fcmop/fcom. See asm_cmpi() for more details. void Assembler::asm_cmpd(LIns *cond) { LOpcode condop = cond->opcode(); @@ -2699,14 +2724,13 @@ namespace nanojit LIns* t = lhs; lhs = rhs; rhs = t; } - // LIR_eqd, if lhs == rhs: // ucomisd ZPC outcome (SETNP/JNP succeeds if P==0) // ------- --- ------- // UNORDERED 111 SETNP/JNP fails // EQUAL 100 SETNP/JNP succeeds // - // LIR_eqd, if lsh != rhs; + // LIR_eqd, if lhs != rhs; // ucomisd ZPC outcome (SETP/JP succeeds if P==0, // SETE/JE succeeds if Z==0) // ------- --- ------- @@ -2810,13 +2834,10 @@ namespace nanojit } else { TEST_AH(mask); FNSTSW_AX(); // requires rEAX to be free - if (rhs->isImmD()) - { + if (rhs->isImmD()) { const uint64_t* p = findImmDFromPool(rhs->immDasQ()); FCOMdm(pop, (const double*)p); - } - else - { + } else { int d = findMemFor(rhs); FCOM(pop, d, FP); } diff --git a/js/src/nanojit/Nativei386.h b/js/src/nanojit/Nativei386.h index f3ca5006f20..a88dbf1fdd9 100644 --- a/js/src/nanojit/Nativei386.h +++ b/js/src/nanojit/Nativei386.h @@ -199,9 +199,12 @@ namespace nanojit void asm_farg(LIns*, int32_t& stkd);\ void asm_arg(ArgType ty, LIns* p, Register r, int32_t& stkd);\ void asm_pusharg(LIns*);\ - void asm_cmpd(LIns *cond);\ - NIns* asm_branchd(bool, LIns*, NIns*);\ void asm_cmp(LIns *cond); \ + void asm_cmpi(LIns *cond); \ + void asm_cmpd(LIns *cond);\ + NIns* asm_branch_helper(bool, LIns* cond, NIns*);\ + NIns* asm_branchi_helper(bool, LIns* cond, NIns*);\ + NIns* asm_branchd_helper(bool, LIns* cond, NIns*);\ void asm_div_mod(LIns *cond); \ void asm_load(int d, Register r); \ void asm_immd(Register r, uint64_t q, double d, bool canClobberCCs); \ @@ -429,7 +432,6 @@ namespace nanojit void FCHS(); \ void FLD1(); \ void FLDZ(); \ - void FFREE(Register r); \ void FST32(bool p, int32_t d, Register b); \ void FSTQ(bool p, int32_t d, Register b); \ void FSTPQ(int32_t d, Register b); \ @@ -451,7 +453,6 @@ namespace nanojit void FSUBRdm(const double* dm); \ void FMULdm( const double* dm); \ void FDIVRdm(const double* dm); \ - void FINCSTP(); \ void FSTP(Register r) { \ count_fpu(); \ FPU(0xddd8, r); \ From bb4d273591b951e841d48932c137b6c2c87d2057 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 1 Dec 2010 14:58:24 -0800 Subject: [PATCH 15/42] Update nanojit-import-rev stamp. --- js/src/nanojit-import-rev | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/nanojit-import-rev b/js/src/nanojit-import-rev index 8a25fe0c5f4..81d9e8008f2 100644 --- a/js/src/nanojit-import-rev +++ b/js/src/nanojit-import-rev @@ -1 +1 @@ -1f90e61950c44193ea5a1800c06d7dba8240cfd9 +4effe362e918583ec7b98b08da24f02c0833d306 From 71dc2197902cf51302b0189e7e12b86df6aeaabf Mon Sep 17 00:00:00 2001 From: Chris Leary Date: Wed, 1 Dec 2010 16:33:49 -0800 Subject: [PATCH 16/42] Fix regexp match pair end-index == -1 assumption. (r=dmandelin, b=605754) --- .../jit-test/tests/basic/bug605754-regexp.js | 2 ++ js/src/jsregexp.h | 31 ++++++++++++------- js/src/jsregexpinlines.h | 18 ++++++----- 3 files changed, 31 insertions(+), 20 deletions(-) create mode 100644 js/src/jit-test/tests/basic/bug605754-regexp.js diff --git a/js/src/jit-test/tests/basic/bug605754-regexp.js b/js/src/jit-test/tests/basic/bug605754-regexp.js new file mode 100644 index 00000000000..9acd3143779 --- /dev/null +++ b/js/src/jit-test/tests/basic/bug605754-regexp.js @@ -0,0 +1,2 @@ +var result = "foobarbaz".replace(/foo(bar)?bar/, "$1"); +assertEq(result, "baz"); diff --git a/js/src/jsregexp.h b/js/src/jsregexp.h index ba0653cb567..0fdcab4b5f6 100644 --- a/js/src/jsregexp.h +++ b/js/src/jsregexp.h @@ -124,7 +124,9 @@ class RegExpStatics JS_ASSERT(matchPairsInput); size_t mpiLen = matchPairsInput->length(); + /* Both members of the first pair must be non-negative. */ JS_ASSERT(pairIsPresent(0)); + JS_ASSERT(get(0, 1) >= 0); /* Present pairs must be valid. */ for (size_t i = 0; i < pairCount(); ++i) { @@ -137,6 +139,19 @@ class RegExpStatics #endif } + bool pairIsPresent(size_t pairNum) const { + return getCrash(pairNum, 0) >= 0; + } + + /* Precondition: paren is present. */ + size_t getParenLength(size_t parenNum) const { + size_t pairNum = parenNum + 1; + if (pairCountCrash() <= pairNum) + return 0; + JS_CRASH_UNLESS(pairIsPresent(pairNum)); + return getCrash(pairNum, 1) - getCrash(pairNum, 0); + } + int get(size_t pairNum, bool which) const { JS_ASSERT(pairNum < pairCount()); return matchPairs[2 * pairNum + which]; @@ -168,10 +183,6 @@ class RegExpStatics /* Mutators. */ - /* - * The inputOffset parameter is added to the present (i.e. non-negative) match items to emulate - * sticky mode. - */ bool updateFromMatch(JSContext *cx, JSString *input, int *buf, size_t matchItemCount) { aboutToWrite(); pendingInput = input; @@ -204,8 +215,6 @@ class RegExpStatics matchPairs.clear(); } - bool pairIsPresent(size_t pairNum) { return get(0, 0) != -1; } - /* Corresponds to JSAPI functionality to set the pending RegExp input. */ void reset(JSString *newInput, bool newMultiline) { aboutToWrite(); @@ -238,12 +247,15 @@ class RegExpStatics return size_t(limit); } + /* Returns whether results for a non-empty match are present. */ bool matched() const { JS_ASSERT(pairCount() > 0); + JS_ASSERT_IF(get(0, 1) == -1, get(1, 1) == -1); return get(0, 1) - get(0, 0) > 0; } size_t getParenCount() const { + /* The first pair is the whole match. */ JS_ASSERT(pairCount() > 0); return pairCount() - 1; } @@ -255,12 +267,6 @@ class RegExpStatics JS_CALL_STRING_TRACER(trc, matchPairsInput, "res->matchPairsInput"); } - size_t getParenLength(size_t parenNum) const { - if (pairCountCrash() <= parenNum + 1) - return 0; - return getCrash(parenNum + 1, 1) - getCrash(parenNum + 1, 0); - } - /* Value creators. */ bool createPendingInput(JSContext *cx, Value *out) const; @@ -275,6 +281,7 @@ class RegExpStatics /* Substring creators. */ + /* @param num Zero-indexed paren number (i.e. $1 is 0). */ void getParen(size_t num, JSSubString *out) const; void getLastMatch(JSSubString *out) const; void getLastParen(JSSubString *out) const; diff --git a/js/src/jsregexpinlines.h b/js/src/jsregexpinlines.h index 17cb1e5c809..5c0f650fc84 100644 --- a/js/src/jsregexpinlines.h +++ b/js/src/jsregexpinlines.h @@ -265,7 +265,6 @@ RegExp::createResult(JSContext *cx, JSString *input, int *buf, size_t matchItemC } else { /* Missing parenthesized match. */ JS_ASSERT(i != 0); /* Since we had a match, first pair must be present. */ - JS_ASSERT(start == end && end == -1); if (!builder.append(INT_TO_JSID(i / 2), UndefinedValue())) return NULL; } @@ -582,11 +581,11 @@ RegExpStatics::createLastParen(JSContext *cx, Value *out) const int start = get(num, 0); int end = get(num, 1); if (start == -1) { - JS_ASSERT(end == -1); out->setString(cx->runtime->emptyString); return true; } JS_ASSERT(start >= 0 && end >= 0); + JS_ASSERT(end >= start); return createDependent(cx, start, end, out); } @@ -621,7 +620,12 @@ RegExpStatics::createRightContext(JSContext *cx, Value *out) const inline void RegExpStatics::getParen(size_t num, JSSubString *out) const { - out->chars = matchPairsInput->chars() + getCrash(num + 1, 0); + size_t pairNum = num + 1; + if (!pairIsPresent(pairNum)) { + *out = js_EmptySubString; + return; + } + out->chars = matchPairsInput->chars() + getCrash(pairNum, 0); out->length = getParenLength(num); } @@ -641,14 +645,12 @@ RegExpStatics::getLastMatch(JSSubString *out) const inline void RegExpStatics::getLastParen(JSSubString *out) const { - if (!pairCountCrash()) { + size_t parenCount = getParenCount(); + if (!parenCount) { *out = js_EmptySubString; return; } - size_t num = pairCount() - 1; - out->chars = matchPairsInput->chars() + getCrash(num, 0); - JS_CRASH_UNLESS(getCrash(num, 1) >= get(num, 0)); - out->length = get(num, 1) - get(num, 0); + getParen(parenCount - 1, out); } inline void From 8d40137fd188fb878ec817625a3af0678817879a Mon Sep 17 00:00:00 2001 From: Chris Leary Date: Wed, 1 Dec 2010 16:34:10 -0800 Subject: [PATCH 17/42] Make paren indexing uniform. (r=dmandelin, b=605754) --- js/src/jsregexp.cpp | 18 +++++------ js/src/jsregexp.h | 69 ++++++++++++++++++++++++++-------------- js/src/jsregexpinlines.h | 15 +++++---- js/src/jsstr.cpp | 27 ++++++++-------- 4 files changed, 76 insertions(+), 53 deletions(-) diff --git a/js/src/jsregexp.cpp b/js/src/jsregexp.cpp index cd6e86d2b52..ce5344d70f4 100644 --- a/js/src/jsregexp.cpp +++ b/js/src/jsregexp.cpp @@ -401,15 +401,15 @@ DEFINE_STATIC_GETTER(static_lastParen_getter, return res->createLastParen(cx, DEFINE_STATIC_GETTER(static_leftContext_getter, return res->createLeftContext(cx, Valueify(vp))) DEFINE_STATIC_GETTER(static_rightContext_getter, return res->createRightContext(cx, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren1_getter, return res->createParen(cx, 0, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren2_getter, return res->createParen(cx, 1, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren3_getter, return res->createParen(cx, 2, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren4_getter, return res->createParen(cx, 3, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren5_getter, return res->createParen(cx, 4, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren6_getter, return res->createParen(cx, 5, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren7_getter, return res->createParen(cx, 6, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren8_getter, return res->createParen(cx, 7, Valueify(vp))) -DEFINE_STATIC_GETTER(static_paren9_getter, return res->createParen(cx, 8, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren1_getter, return res->createParen(cx, 1, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren2_getter, return res->createParen(cx, 2, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren3_getter, return res->createParen(cx, 3, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren4_getter, return res->createParen(cx, 4, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren5_getter, return res->createParen(cx, 5, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren6_getter, return res->createParen(cx, 6, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren7_getter, return res->createParen(cx, 7, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren8_getter, return res->createParen(cx, 8, Valueify(vp))) +DEFINE_STATIC_GETTER(static_paren9_getter, return res->createParen(cx, 9, Valueify(vp))) #define DEFINE_STATIC_SETTER(name, code) \ static JSBool \ diff --git a/js/src/jsregexp.h b/js/src/jsregexp.h index 0fdcab4b5f6..7f6fae02435 100644 --- a/js/src/jsregexp.h +++ b/js/src/jsregexp.h @@ -70,16 +70,6 @@ class RegExpStatics bool createDependent(JSContext *cx, size_t start, size_t end, Value *out) const; - size_t pairCount() const { - JS_ASSERT(matchPairs.length() % 2 == 0); - return matchPairs.length() / 2; - } - - size_t pairCountCrash() const { - JS_CRASH_UNLESS(matchPairs.length() % 2 == 0); - return pairCount(); - } - void copyTo(RegExpStatics &dst) { dst.matchPairs.clear(); /* 'save' has already reserved space in matchPairs */ @@ -139,15 +129,22 @@ class RegExpStatics #endif } + /* + * Since the first pair indicates the whole match, the paren pair numbers have to be in the + * range [1, pairCount). + */ + void checkParenNum(size_t pairNum) const { + JS_CRASH_UNLESS(1 <= pairNum); + JS_CRASH_UNLESS(pairNum < pairCount()); + } + bool pairIsPresent(size_t pairNum) const { return getCrash(pairNum, 0) >= 0; } /* Precondition: paren is present. */ - size_t getParenLength(size_t parenNum) const { - size_t pairNum = parenNum + 1; - if (pairCountCrash() <= pairNum) - return 0; + size_t getParenLength(size_t pairNum) const { + checkParenNum(pairNum); JS_CRASH_UNLESS(pairIsPresent(pairNum)); return getCrash(pairNum, 1) - getCrash(pairNum, 0); } @@ -231,6 +228,31 @@ class RegExpStatics /* Accessors. */ + /* + * When there is a match present, the pairCount is at least 1 for the whole + * match. There is one additional pair per parenthesis. + * + * Getting a parenCount requires there to be a match result as a precondition. + */ + + private: + size_t pairCount() const { + JS_ASSERT(matchPairs.length() % 2 == 0); + return matchPairs.length() / 2; + } + + size_t pairCountCrash() const { + JS_CRASH_UNLESS(matchPairs.length() % 2 == 0); + return pairCount(); + } + + public: + size_t parenCount() const { + size_t pc = pairCount(); + JS_CRASH_UNLESS(pc); + return pc - 1; + } + JSString *getPendingInput() const { return pendingInput; } uintN getFlags() const { return flags; } bool multiline() const { return flags & JSREG_MULTILINE; } @@ -254,12 +276,6 @@ class RegExpStatics return get(0, 1) - get(0, 0) > 0; } - size_t getParenCount() const { - /* The first pair is the whole match. */ - JS_ASSERT(pairCount() > 0); - return pairCount() - 1; - } - void mark(JSTracer *trc) const { if (pendingInput) JS_CALL_STRING_TRACER(trc, pendingInput, "res->pendingInput"); @@ -275,14 +291,19 @@ class RegExpStatics bool createLeftContext(JSContext *cx, Value *out) const; bool createRightContext(JSContext *cx, Value *out) const; - bool createParen(JSContext *cx, size_t parenNum, Value *out) const { - return makeMatch(cx, (parenNum + 1) * 2, parenNum + 1, out); + /* @param pairNum Any number >= 1. */ + bool createParen(JSContext *cx, size_t pairNum, Value *out) const { + JS_CRASH_UNLESS(pairNum >= 1); + if (pairNum >= pairCount()) { + out->setString(cx->runtime->emptyString); + return true; + } + return makeMatch(cx, pairNum * 2, pairNum, out); } /* Substring creators. */ - /* @param num Zero-indexed paren number (i.e. $1 is 0). */ - void getParen(size_t num, JSSubString *out) const; + void getParen(size_t pairNum, JSSubString *out) const; void getLastMatch(JSSubString *out) const; void getLastParen(JSSubString *out) const; void getLeftContext(JSSubString *out) const; diff --git a/js/src/jsregexpinlines.h b/js/src/jsregexpinlines.h index 5c0f650fc84..ff015c7fbcf 100644 --- a/js/src/jsregexpinlines.h +++ b/js/src/jsregexpinlines.h @@ -244,7 +244,7 @@ RegExp::createResult(JSContext *cx, JSString *input, int *buf, size_t matchItemC /* * Create the result array for a match. Array contents: * 0: matched string - * 1..parenCount: paren matches + * 1..pairCount-1: paren matches */ JSObject *array = js_NewSlowArrayObject(cx); if (!array) @@ -618,15 +618,15 @@ RegExpStatics::createRightContext(JSContext *cx, Value *out) const } inline void -RegExpStatics::getParen(size_t num, JSSubString *out) const +RegExpStatics::getParen(size_t pairNum, JSSubString *out) const { - size_t pairNum = num + 1; + checkParenNum(pairNum); if (!pairIsPresent(pairNum)) { *out = js_EmptySubString; return; } out->chars = matchPairsInput->chars() + getCrash(pairNum, 0); - out->length = getParenLength(num); + out->length = getParenLength(pairNum); } inline void @@ -645,12 +645,13 @@ RegExpStatics::getLastMatch(JSSubString *out) const inline void RegExpStatics::getLastParen(JSSubString *out) const { - size_t parenCount = getParenCount(); - if (!parenCount) { + size_t pairCount = pairCountCrash(); + /* Note: the first pair is the whole match. */ + if (pairCount <= 1) { *out = js_EmptySubString; return; } - getParen(parenCount - 1, out); + getParen(pairCount - 1, out); } inline void diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp index baf8a9f0449..7f72461c206 100644 --- a/js/src/jsstr.cpp +++ b/js/src/jsstr.cpp @@ -2003,13 +2003,13 @@ InterpretDollar(JSContext *cx, RegExpStatics *res, jschar *dp, jschar *ep, Repla if (JS7_ISDEC(dc)) { /* ECMA-262 Edition 3: 1-9 or 01-99 */ uintN num = JS7_UNDEC(dc); - if (num > res->getParenCount()) + if (num > res->parenCount()) return false; jschar *cp = dp + 2; if (cp < ep && (dc = *cp, JS7_ISDEC(dc))) { uintN tmp = 10 * num + JS7_UNDEC(dc); - if (tmp <= res->getParenCount()) { + if (tmp <= res->parenCount()) { cp++; num = tmp; } @@ -2017,13 +2017,14 @@ InterpretDollar(JSContext *cx, RegExpStatics *res, jschar *dp, jschar *ep, Repla if (num == 0) return false; - /* Adjust num from 1 $n-origin to 0 array-index-origin. */ - num--; *skip = cp - dp; - if (num < res->getParenCount()) - res->getParen(num, out); - else - *out = js_EmptySubString; + + JS_CRASH_UNLESS(num <= res->parenCount()); + /* + * Note: we index to get the paren with the (1-indexed) pair + * number, as opposed to a (0-indexed) paren number. + */ + res->getParen(num, out); return true; } @@ -2120,7 +2121,7 @@ FindReplaceLength(JSContext *cx, RegExpStatics *res, ReplaceData &rdata, size_t * For $&, etc., we must create string jsvals from cx->regExpStatics. * We grab up stack space to keep the newborn strings GC-rooted. */ - uintN p = res->getParenCount(); + uintN p = res->parenCount(); uintN argc = 1 + p + 2; InvokeSessionGuard &session = rdata.session; @@ -2139,8 +2140,8 @@ FindReplaceLength(JSContext *cx, RegExpStatics *res, ReplaceData &rdata, size_t if (!res->createLastMatch(cx, &session[argi++])) return false; - for (size_t i = 0; i < res->getParenCount(); ++i) { - if (!res->createParen(cx, i, &session[argi++])) + for (size_t i = 0; i < res->parenCount(); ++i) { + if (!res->createParen(cx, i + 1, &session[argi++])) return false; } @@ -2758,11 +2759,11 @@ str_split(JSContext *cx, uintN argc, Value *vp) * substring that was delimited. */ if (re && sep->chars) { - for (uintN num = 0; num < res->getParenCount(); num++) { + for (uintN num = 0; num < res->parenCount(); num++) { if (limited && len >= limit) break; JSSubString parsub; - res->getParen(num, &parsub); + res->getParen(num + 1, &parsub); sub = js_NewStringCopyN(cx, parsub.chars, parsub.length); if (!sub || !splits.append(StringValue(sub))) return false; From 2e7f4be85c648af62336c140f8072060b6646be6 Mon Sep 17 00:00:00 2001 From: Chris Leary Date: Wed, 1 Dec 2010 16:34:42 -0800 Subject: [PATCH 18/42] Additional regexp DoReplace diagnostics. (r=dmandelin, b=605754) --- js/src/jscntxt.h | 16 +++++++++++++++- js/src/jsstr.cpp | 29 +++++++++++++++++++++++++++-- js/src/yarr/yarr/RegexJIT.cpp | 2 +- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 64a58320c58..881d9053fba 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -2369,10 +2369,24 @@ struct JSContext DOLLAR_AMP, DOLLAR_PLUS, DOLLAR_TICK, - DOLLAR_QUOT + DOLLAR_QUOT, + DOLLAR_EMPTY, + DOLLAR_1, + DOLLAR_2, + DOLLAR_3, + DOLLAR_4, + DOLLAR_5, + DOLLAR_OTHER }; +#ifdef XP_WIN volatile DollarPath *dollarPath; + volatile JSSubString *sub; volatile jschar *blackBox; + volatile jschar **repstrChars; + volatile jschar **repstrDollar; + volatile jschar **repstrDollarEnd; + volatile size_t *peekLen; +#endif private: diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp index 7f72461c206..15b3e100697 100644 --- a/js/src/jsstr.cpp +++ b/js/src/jsstr.cpp @@ -2020,6 +2020,16 @@ InterpretDollar(JSContext *cx, RegExpStatics *res, jschar *dp, jschar *ep, Repla *skip = cp - dp; JS_CRASH_UNLESS(num <= res->parenCount()); + + switch (num) { + case 1: *path = JSContext::DOLLAR_1; break; + case 2: *path = JSContext::DOLLAR_2; break; + case 3: *path = JSContext::DOLLAR_3; break; + case 4: *path = JSContext::DOLLAR_4; break; + case 5: *path = JSContext::DOLLAR_5; break; + default: *path = JSContext::DOLLAR_OTHER; + } + /* * Note: we index to get the paren with the (1-indexed) pair * number, as opposed to a (0-indexed) paren number. @@ -2185,9 +2195,11 @@ DoReplace(JSContext *cx, RegExpStatics *res, ReplaceData &rdata, jschar *chars) jschar *cp; jschar *bp = cp = repstr->chars(); volatile JSContext::DollarPath path; +#ifdef XP_WIN cx->dollarPath = &path; jschar sourceBuf[128]; cx->blackBox = sourceBuf; +#endif for (jschar *dp = rdata.dollar, *ep = rdata.dollarEnd; dp; dp = js_strchr_limit(dp, '$', ep)) { size_t len = dp - cp; @@ -2198,11 +2210,24 @@ DoReplace(JSContext *cx, RegExpStatics *res, ReplaceData &rdata, jschar *chars) JSSubString sub; size_t skip; if (InterpretDollar(cx, res, dp, ep, rdata, &sub, &skip, &path)) { +#ifdef XP_WIN if (((size_t(sub.chars) & 0xfffffU) + sub.length) > 0x100000U) { /* Going to cross a 0xffffe address, so take a gander at the replace value. */ - size_t peekLen = JS_MIN(rdata.dollarEnd - rdata.dollar, 128); - js_strncpy(sourceBuf, rdata.dollar, peekLen); + volatile JSSubString vsub = sub; + volatile jschar *repstrChars = rdata.repstr->chars(); + volatile jschar *repstrDollar = rdata.dollar; + volatile jschar *repstrDollarEnd = rdata.dollarEnd; + cx->sub = &vsub; + cx->repstrChars = &repstrChars; + cx->repstrDollar = &repstrDollar; + cx->repstrDollarEnd = &repstrDollarEnd; + ptrdiff_t dollarDistance = rdata.dollarEnd - rdata.dollar; + JS_CRASH_UNLESS(dollarDistance >= 0); + volatile size_t peekLen = JS_MIN(rdata.repstr->length(), 128); + cx->peekLen = &peekLen; + js_strncpy(sourceBuf, rdata.repstr->chars(), peekLen); } +#endif len = sub.length; js_strncpy(chars, sub.chars, len); diff --git a/js/src/yarr/yarr/RegexJIT.cpp b/js/src/yarr/yarr/RegexJIT.cpp index e53a885dd09..d12350075af 100644 --- a/js/src/yarr/yarr/RegexJIT.cpp +++ b/js/src/yarr/yarr/RegexJIT.cpp @@ -994,7 +994,7 @@ class RegexGenerator : private MacroAssembler { parenthesesState.linkAlternativeBacktracks(this); if (term.invertOrCapture) { store32(Imm32(-1), Address(output, (term.parentheses.subpatternId << 1) * sizeof(int))); -#if DEBUG +#if 0 store32(Imm32(-1), Address(output, ((term.parentheses.subpatternId << 1) + 1) * sizeof(int))); #endif } From 473a8118a101f565ecf2e55ff423b959ca7efeb7 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 1 Dec 2010 17:02:15 -0800 Subject: [PATCH 19/42] Fix constructors that return objects in catch blocks (bug 604381, r=dmandelin). --- js/src/jit-test/tests/jaeger/bug604381.js | 14 ++++++++++++++ js/src/methodjit/Compiler.cpp | 10 +++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 js/src/jit-test/tests/jaeger/bug604381.js diff --git a/js/src/jit-test/tests/jaeger/bug604381.js b/js/src/jit-test/tests/jaeger/bug604381.js new file mode 100644 index 00000000000..365c9e3d2b1 --- /dev/null +++ b/js/src/jit-test/tests/jaeger/bug604381.js @@ -0,0 +1,14 @@ +// vim: set ts=4 sw=4 tw=99 et: + +function F() { + var T = { }; + try { + throw 12; + } catch (e) { + T.x = 5; + return T; + } +} + +assertEq((new F()).x, 5); + diff --git a/js/src/methodjit/Compiler.cpp b/js/src/methodjit/Compiler.cpp index d4591c27de2..af3435302c5 100644 --- a/js/src/methodjit/Compiler.cpp +++ b/js/src/methodjit/Compiler.cpp @@ -2177,8 +2177,12 @@ mjit::Compiler::fixPrimitiveReturn(Assembler *masm, FrameEntry *fe) bool ool = (masm != &this->masm); Address thisv(JSFrameReg, JSStackFrame::offsetOfThis(fun)); - // Easy cases - no return value, or known primitive, so just return thisv. - if (!fe || (fe->isTypeKnown() && fe->getKnownType() != JSVAL_TYPE_OBJECT)) { + // We can just load |thisv| if either of the following is true: + // (1) There is no explicit return value, AND fp->rval is not used. + // (2) There is an explicit return value, and it's known to be primitive. + if ((!fe && !analysis->usesReturnValue()) || + (fe && fe->isTypeKnown() && fe->getKnownType() != JSVAL_TYPE_OBJECT)) + { if (ool) masm->loadValueAsComponents(thisv, JSReturnReg_Type, JSReturnReg_Data); else @@ -2187,7 +2191,7 @@ mjit::Compiler::fixPrimitiveReturn(Assembler *masm, FrameEntry *fe) } // If the type is known to be an object, just load the return value as normal. - if (fe->isTypeKnown() && fe->getKnownType() == JSVAL_TYPE_OBJECT) { + if (fe && fe->isTypeKnown() && fe->getKnownType() == JSVAL_TYPE_OBJECT) { loadReturnValue(masm, fe); return; } From 96c95689e64addc3f28a503c9a47fb7ba1b1596e Mon Sep 17 00:00:00 2001 From: Jacob Bramley Date: Wed, 1 Dec 2010 17:38:23 -0800 Subject: [PATCH 20/42] Link the shape guard in bindname to the exit sequence (bug 614907, r=dvander). --- js/src/methodjit/PolyIC.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/src/methodjit/PolyIC.cpp b/js/src/methodjit/PolyIC.cpp index e959742c457..40f25d53e48 100644 --- a/js/src/methodjit/PolyIC.cpp +++ b/js/src/methodjit/PolyIC.cpp @@ -1551,6 +1551,8 @@ class BindNameCompiler : public PICStubCompiler masm.loadShape(pic.objReg, pic.shapeReg); Jump shapeTest = masm.branch32(Assembler::NotEqual, pic.shapeReg, Imm32(tobj->shape())); + if (!fails.append(shapeTest)) + return error(); tobj = tobj->getParent(); } if (tobj != obj) From f110aa58b77da9520c01ad60670da69df932fccc Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Thu, 14 Oct 2010 16:12:19 +0200 Subject: [PATCH 21/42] bug 603318 - make dense array slow during array growth, not during the GC. r=bhackett --- .../tests/basic/testDenseToSlowArray.js | 185 ++++++++++++ js/src/jsarray.cpp | 282 +++++++++++------- js/src/jsarray.h | 43 ++- js/src/jsemit.cpp | 8 +- js/src/jsgc.cpp | 13 - js/src/jsgc.h | 4 - js/src/jsinterp.cpp | 3 +- js/src/jsobj.h | 17 +- js/src/jsobjinlines.h | 7 - 9 files changed, 414 insertions(+), 148 deletions(-) create mode 100644 js/src/jit-test/tests/basic/testDenseToSlowArray.js diff --git a/js/src/jit-test/tests/basic/testDenseToSlowArray.js b/js/src/jit-test/tests/basic/testDenseToSlowArray.js new file mode 100644 index 00000000000..e4b6b5d2dde --- /dev/null +++ b/js/src/jit-test/tests/basic/testDenseToSlowArray.js @@ -0,0 +1,185 @@ +// test dense -> slow array transitions during the recording and on trace +// for various array functions and property accessors + +function test_set_elem() { + + function f() { + var bag = []; + for (var i = 0; i != 100; ++i) { + var a = [0]; + a[100*100] = i; + bag.push(a); + } + + for (var i = 0; i != 100; ++i) { + var a = [0]; + a[200 + i] = i; + bag.push(a); + } + return bag; + } + + var bag = f(); + + for (var i = 0; i != 100; ++i) { + var a = bag[i]; + assertEq(a.length, 100 * 100 + 1); + assertEq(a[100*100], i); + assertEq(a[0], 0); + assertEq(1 + i in a, false); + } + + for (var i = 0; i != 100; ++i) { + var a = bag[100 + i]; + assertEq(a.length, 200 + i + 1); + assertEq(a[200 + i], i); + assertEq(a[0], 0); + assertEq(1 + i in a, false); + } +} + +function test_reverse() { + + function prepare_arays() { + var bag = []; + var base_index = 245; + for (var i = 0; i != 50; ++i) { + var a = [1, 2, 3, 4, 5]; + a.length = i + base_index; + bag.push(a); + } + return bag; + } + + function test(bag) { + for (var i = 0; i != bag.length; ++i) { + var a = bag[i]; + a.reverse(); + a[0] = 1; + } + } + + var bag = prepare_arays(); + test(bag); + for (var i = 0; i != bag.length; ++i) { + var a = bag[i]; + assertEq(a[0], 1); + for (var j = 1; j <= 5; ++j) { + assertEq(a[a.length - j], j); + } + for (var j = 1; j < a.length - 5; ++j) { + assertEq(j in a, false); + } + } + +} + +function test_push() { + + function prepare_arays() { + var bag = []; + var base_index = 245; + for (var i = 0; i != 50; ++i) { + var a = [0]; + a.length = i + base_index; + bag.push(a); + } + return bag; + } + + function test(bag) { + for (var i = 0; i != bag.length; ++i) { + var a = bag[i]; + a.push(2); + a[0] = 1; + } + } + + var bag = prepare_arays(); + test(bag); + for (var i = 0; i != bag.length; ++i) { + var a = bag[i]; + assertEq(a[0], 1); + assertEq(a[a.length - 1], 2); + for (var j = 1; j < a.length - 1; ++j) { + assertEq(j in a, false); + } + } +} + +function test_unshift() { + + function prepare_arays() { + var bag = []; + var base_index = 245; + for (var i = 0; i != 50; ++i) { + var a = [0]; + a.length = i + base_index; + bag.push(a); + } + return bag; + } + + function test(bag) { + for (var i = 0; i != bag.length; ++i) { + var a = bag[i]; + a.unshift(1); + a[2] = 2; + } + } + + var bag = prepare_arays(); + test(bag); + for (var i = 0; i != bag.length; ++i) { + var a = bag[i]; + assertEq(a[0], 1); + assertEq(a[1], 0); + assertEq(a[2], 2); + for (var j = 3; j < a.length; ++j) { + assertEq(j in a, false); + } + } +} + +function test_splice() { + + function prepare_arays() { + var bag = []; + var base_index = 245; + for (var i = 0; i != 50; ++i) { + var a = [1, 2]; + a.length = i + base_index; + bag.push(a); + } + return bag; + } + + function test(bag) { + for (var i = 0; i != bag.length; ++i) { + var a = bag[i]; + a.splice(1, 0, "a", "b", "c"); + a[2] = 100; + } + } + + var bag = prepare_arays(); + test(bag); + for (var i = 0; i != bag.length; ++i) { + var a = bag[i]; + assertEq(a[0], 1); + assertEq(a[1], "a"); + assertEq(a[2], 100); + assertEq(a[3], "c"); + assertEq(a[4], 2); + for (var j = 5; j < a.length; ++j) { + assertEq(j in a, false); + } + } +} + +test_set_elem(); +test_reverse(); +test_push(); +test_unshift(); +test_splice(); + diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 57aa75b5bde..e36a9ea6d28 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -62,9 +62,9 @@ * * Arrays are converted to use js_SlowArrayClass when any of these conditions * are met: - * - the load factor (COUNT / capacity) is less than 0.25, and there are - * more than MIN_SPARSE_INDEX slots total - * - a property is set that is not indexed (and not "length"); or + * - there are more than MIN_SPARSE_INDEX slots total + * - the load factor (COUNT / capacity) is less than 0.25 + * - a property is set that is not indexed (and not "length") * - a property is defined that has non-default property attributes. * * Dense arrays do not track property creation order, so unlike other native @@ -115,30 +115,6 @@ using namespace js::gc; #define MAXINDEX 4294967295u #define MAXSTR "4294967295" -/* - * Use the limit on number of object slots for sanity and consistency (see the - * assertion in JSObject::makeDenseArraySlow). - */ -static inline bool -INDEX_TOO_BIG(jsuint index) -{ - return index >= JSObject::NSLOTS_LIMIT; -} - -static inline bool -INDEX_TOO_SPARSE(JSObject *array, jsuint index) -{ - /* Small arrays with less than 256 elements are dense, no matter what. */ - if (index < 256) - return false; - - /* - * Otherwise if the index becomes too large or is more than 256 past - * the current capacity, we have to slowify. - */ - return INDEX_TOO_BIG(index) || (index > array->getDenseArrayCapacity() + 256); -} - static inline bool ENSURE_SLOW_ARRAY(JSContext *cx, JSObject *obj) { @@ -310,6 +286,34 @@ BigIndexToId(JSContext *cx, JSObject *obj, jsuint index, JSBool createAtom, return JS_TRUE; } +bool +JSObject::willBeSparseDenseArray(uintN requiredCapacity, uintN newElementsHint) +{ + JS_ASSERT(isDenseArray()); + JS_ASSERT(requiredCapacity > MIN_SPARSE_INDEX); + + uintN cap = numSlots(); + JS_ASSERT(requiredCapacity >= cap); + + if (requiredCapacity >= JSObject::NSLOTS_LIMIT) + return true; + + uintN minimalDenseCount = requiredCapacity / 4; + if (newElementsHint >= minimalDenseCount) + return false; + minimalDenseCount -= newElementsHint; + + if (minimalDenseCount > cap) + return true; + + Value *elems = getDenseArrayElements(); + for (uintN i = 0; i < cap; i++) { + if (!elems[i].isMagic(JS_ARRAY_HOLE) && !--minimalDenseCount) + return false; + } + return true; +} + static bool ReallyBigIndexToId(JSContext* cx, jsdouble index, jsid* idp) { @@ -439,19 +443,23 @@ SetArrayElement(JSContext *cx, JSObject *obj, jsdouble index, const Value &v) if (obj->isDenseArray()) { /* Predicted/prefetched code should favor the remains-dense case. */ - if (index <= jsuint(-1)) { + JSObject::EnsureDenseResult result = JSObject::ED_SPARSE; + do { + if (index > jsuint(-1)) + break; jsuint idx = jsuint(index); - if (!INDEX_TOO_SPARSE(obj, idx)) { - JS_ASSERT(idx + 1 > idx); - if (!obj->ensureDenseArrayElements(cx, idx + 1)) - return JS_FALSE; - if (idx >= obj->getArrayLength()) - obj->setArrayLength(idx + 1); - obj->setDenseArrayElement(idx, v); - return JS_TRUE; - } - } + result = obj->ensureDenseArrayElements(cx, idx, 1); + if (result != JSObject::ED_OK) + break; + if (idx >= obj->getArrayLength()) + obj->setArrayLength(idx + 1); + obj->setDenseArrayElement(idx, v); + return true; + } while (false); + if (result == JSObject::ED_FAILED) + return false; + JS_ASSERT(result == JSObject::ED_SPARSE); if (!obj->makeDenseArraySlow(cx)) return JS_FALSE; } @@ -474,13 +482,7 @@ js_EnsureDenseArrayCapacity(JSContext *cx, JSObject *obj, jsint i) Class *origObjClasp = obj->clasp; #endif jsuint u = jsuint(i); - jsuint capacity = obj->getDenseArrayCapacity(); - if (u < capacity) - return true; - if (INDEX_TOO_SPARSE(obj, u)) - return false; - - JSBool ret = obj->ensureDenseArrayElements(cx, u + 1); + JSBool ret = (obj->ensureDenseArrayElements(cx, u, 1) == JSObject::ED_OK); /* Partially check the CallInfo's storeAccSet is correct. */ JS_ASSERT(obj->clasp == origObjClasp); @@ -801,20 +803,29 @@ array_setProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp, JSBool stric if (!obj->isDenseArray()) return js_SetProperty(cx, obj, id, vp, strict); - if (!js_IdIsIndex(id, &i) || js_PrototypeHasIndexedProperties(cx, obj) || - INDEX_TOO_SPARSE(obj, i)) { - if (!obj->makeDenseArraySlow(cx)) - return false; - return js_SetProperty(cx, obj, id, vp, strict); - } + do { + if (!js_IdIsIndex(id, &i)) + break; + if (js_PrototypeHasIndexedProperties(cx, obj)) + break; - if (!obj->ensureDenseArrayElements(cx, i + 1)) + JSObject::EnsureDenseResult result = obj->ensureDenseArrayElements(cx, i, 1); + if (result != JSObject::ED_OK) { + if (result == JSObject::ED_FAILED) + return false; + JS_ASSERT(result == JSObject::ED_SPARSE); + break; + } + + if (i >= obj->getArrayLength()) + obj->setArrayLength(i + 1); + obj->setDenseArrayElement(i, *vp); + return true; + } while (false); + + if (!obj->makeDenseArraySlow(cx)) return false; - - if (i >= obj->getArrayLength()) - obj->setArrayLength(i + 1); - obj->setDenseArrayElement(i, *vp); - return true; + return js_SetProperty(cx, obj, id, vp, strict); } static JSBool @@ -861,7 +872,7 @@ array_defineProperty(JSContext *cx, JSObject *obj, jsid id, const Value *value, return JS_TRUE; isIndex = js_IdIsIndex(id, &i); - if (!isIndex || attrs != JSPROP_ENUMERATE || !obj->isDenseArray() || INDEX_TOO_SPARSE(obj, i)) { + if (!isIndex || attrs != JSPROP_ENUMERATE) { if (!ENSURE_SLOW_ARRAY(cx, obj)) return JS_FALSE; return js_DefineProperty(cx, obj, id, value, getter, setter, attrs); @@ -915,20 +926,9 @@ array_trace(JSTracer *trc, JSObject *obj) { JS_ASSERT(obj->isDenseArray()); - size_t holes = 0; uint32 capacity = obj->getDenseArrayCapacity(); - for (uint32 i = 0; i < capacity; i++) { - Value v = obj->getDenseArrayElement(i); - if (v.isMagic(JS_ARRAY_HOLE)) - ++holes; - else - MarkValue(trc, obj->getDenseArrayElement(i), "dense_array_elems"); - } - - if (IS_GC_MARKING_TRACER(trc) && holes > MIN_SPARSE_INDEX && holes > capacity / 4 * 3) { - /* This might fail, in which case we don't slowify it. */ - static_cast(trc)->arraysToSlowify.append(obj); - } + for (uint32 i = 0; i < capacity; i++) + MarkValue(trc, obj->getDenseArrayElement(i), "dense_array_elems"); } static JSBool @@ -1379,22 +1379,28 @@ InitArrayElements(JSContext *cx, JSObject *obj, jsuint start, jsuint count, Valu * Optimize for dense arrays so long as adding the given set of elements * wouldn't otherwise make the array slow. */ - if (obj->isDenseArray() && !js_PrototypeHasIndexedProperties(cx, obj) && - start <= MAXINDEX - count && !INDEX_TOO_BIG(start + count)) { + do { + if (!obj->isDenseArray()) + break; + if (js_PrototypeHasIndexedProperties(cx, obj)) + break; + JSObject::EnsureDenseResult result = obj->ensureDenseArrayElements(cx, start, count); + if (result != JSObject::ED_OK) { + if (result == JSObject::ED_FAILED) + return false; + JS_ASSERT(result == JSObject::ED_SPARSE); + break; + } jsuint newlen = start + count; - JS_ASSERT(jsdouble(start) + count == jsdouble(newlen)); - if (!obj->ensureDenseArrayElements(cx, newlen)) - return JS_FALSE; - if (newlen > obj->getArrayLength()) obj->setArrayLength(newlen); JS_ASSERT(count < uint32(-1) / sizeof(Value)); memcpy(obj->getDenseArrayElements() + start, vector, sizeof(jsval) * count); JS_ASSERT_IF(count != 0, !obj->getDenseArrayElement(newlen - 1).isMagic(JS_ARRAY_HOLE)); - return JS_TRUE; - } + return true; + } while (false); Value* end = vector + count; while (vector != end && start < MAXINDEX) { @@ -1436,7 +1442,9 @@ InitArrayObject(JSContext *cx, JSObject *obj, jsuint length, const Value *vector obj->setArrayLength(length); if (!vector || !length) return true; - if (!obj->ensureDenseArrayElements(cx, length)) + + /* Avoid ensureDenseArrayElements to skip sparse array checks there. */ + if (!obj->ensureSlots(cx, length)) return false; memcpy(obj->getDenseArrayElements(), vector, length * sizeof(Value)); return true; @@ -1470,7 +1478,12 @@ array_reverse(JSContext *cx, uintN argc, Value *vp) return JS_FALSE; vp->setObject(*obj); - if (obj->isDenseArray() && !js_PrototypeHasIndexedProperties(cx, obj)) { + do { + if (!obj->isDenseArray()) + break; + if (js_PrototypeHasIndexedProperties(cx, obj)) + break; + /* An empty array or an array with no elements is already reversed. */ if (len == 0 || obj->getDenseArrayCapacity() == 0) return JS_TRUE; @@ -1484,8 +1497,13 @@ array_reverse(JSContext *cx, uintN argc, Value *vp) * holes in the array at its start) and ensure that the capacity is * sufficient to hold all the elements in the array if it were full. */ - if (!obj->ensureDenseArrayElements(cx, len)) - return JS_FALSE; + JSObject::EnsureDenseResult result = obj->ensureDenseArrayElements(cx, len, 0); + if (result != JSObject::ED_OK) { + if (result == JSObject::ED_FAILED) + return false; + JS_ASSERT(result == JSObject::ED_SPARSE); + break; + } uint32 lo = 0, hi = len - 1; for (; lo < hi; lo++, hi--) { @@ -1500,7 +1518,7 @@ array_reverse(JSContext *cx, uintN argc, Value *vp) * holes). */ return JS_TRUE; - } + } while (false); AutoValueRooter tvr(cx); for (jsuint i = 0, half = len / 2; i < half; i++) { @@ -2005,21 +2023,27 @@ static JSBool array_push1_dense(JSContext* cx, JSObject* obj, const Value &v, Value *rval) { uint32 length = obj->getArrayLength(); - if (INDEX_TOO_SPARSE(obj, length)) { - if (!obj->makeDenseArraySlow(cx)) - return JS_FALSE; - Value tmp = v; - return array_push_slowly(cx, obj, 1, &tmp, rval); - } + do { + JSObject::EnsureDenseResult result = obj->ensureDenseArrayElements(cx, length, 1); + if (result != JSObject::ED_OK) { + if (result == JSObject::ED_FAILED) + return false; + JS_ASSERT(result == JSObject::ED_SPARSE); + break; + } - if (!obj->ensureDenseArrayElements(cx, length + 1)) - return JS_FALSE; - obj->setArrayLength(length + 1); + obj->setArrayLength(length + 1); - JS_ASSERT(obj->getDenseArrayElement(length).isMagic(JS_ARRAY_HOLE)); - obj->setDenseArrayElement(length, v); - rval->setNumber(obj->getArrayLength()); - return JS_TRUE; + JS_ASSERT(obj->getDenseArrayElement(length).isMagic(JS_ARRAY_HOLE)); + obj->setDenseArrayElement(length, v); + rval->setNumber(obj->getArrayLength()); + return true; + } while (false); + + if (!obj->makeDenseArraySlow(cx)) + return false; + Value tmp = v; + return array_push_slowly(cx, obj, 1, &tmp, rval); } JS_ALWAYS_INLINE JSBool @@ -2036,8 +2060,13 @@ ArrayCompPushImpl(JSContext *cx, JSObject *obj, const Value &v) return JS_FALSE; } - if (!obj->ensureDenseArrayElements(cx, length + 1)) - return JS_FALSE; + /* + * Array comprehension cannot add holes to the array and never leaks + * the array before it is fully initialized. So we can use ensureSlots + * instead of ensureDenseArrayElements. + */ + if (!obj->ensureSlots(cx, length + 1)) + return false; } obj->setArrayLength(length + 1); obj->setDenseArrayElement(length, v); @@ -2195,16 +2224,27 @@ array_unshift(JSContext *cx, uintN argc, Value *vp) /* Slide up the array to make room for argc at the bottom. */ argv = JS_ARGV(cx, vp); if (length > 0) { - if (obj->isDenseArray() && !js_PrototypeHasIndexedProperties(cx, obj) && - !INDEX_TOO_SPARSE(obj, unsigned(newlen + argc))) { - JS_ASSERT(newlen + argc == length + argc); - if (!obj->ensureDenseArrayElements(cx, length + argc)) - return JS_FALSE; + bool optimized = false; + do { + if (!obj->isDenseArray()) + break; + if (js_PrototypeHasIndexedProperties(cx, obj)) + break; + JSObject::EnsureDenseResult result = obj->ensureDenseArrayElements(cx, length, argc); + if (result != JSObject::ED_OK) { + if (result == JSObject::ED_FAILED) + return false; + JS_ASSERT(result == JSObject::ED_SPARSE); + break; + } Value *elems = obj->getDenseArrayElements(); memmove(elems + argc, elems, length * sizeof(jsval)); for (uint32 i = 0; i < argc; i++) obj->setDenseArrayElement(i, MagicValue(JS_ARRAY_HOLE)); - } else { + optimized = true; + } while (false); + + if (!optimized) { last = length; jsdouble upperIndex = last + argc; AutoValueRooter tvr(cx); @@ -2324,12 +2364,23 @@ array_splice(JSContext *cx, uintN argc, Value *vp) if (argc > count) { delta = (jsuint)argc - count; last = length; - if (obj->isDenseArray() && !js_PrototypeHasIndexedProperties(cx, obj) && - length <= obj->getDenseArrayCapacity() && - (length == 0 || !obj->getDenseArrayElement(length - 1).isMagic(JS_ARRAY_HOLE))) { - if (!obj->ensureDenseArrayElements(cx, length + delta)) - return JS_FALSE; - + bool optimized = false; + do { + if (!obj->isDenseArray()) + break; + if (js_PrototypeHasIndexedProperties(cx, obj)) + break; + if (length > obj->getDenseArrayCapacity()) + break; + if (length != 0 && obj->getDenseArrayElement(length - 1).isMagic(JS_ARRAY_HOLE)) + break; + JSObject::EnsureDenseResult result = obj->ensureDenseArrayElements(cx, length, delta); + if (result != JSObject::ED_OK) { + if (result == JSObject::ED_FAILED) + return false; + JS_ASSERT(result == JSObject::ED_SPARSE); + break; + } Value *arraybeg = obj->getDenseArrayElements(); Value *srcbeg = arraybeg + last - 1; Value *srcend = arraybeg + end - 1; @@ -2338,7 +2389,10 @@ array_splice(JSContext *cx, uintN argc, Value *vp) *dst = *src; obj->setArrayLength(obj->getArrayLength() + delta); - } else { + optimized = true; + } while (false); + + if (!optimized) { /* (uint) end could be 0, so we can't use a vanilla >= test. */ while (last-- > end) { if (!JS_CHECK_OPERATION_LIMIT(cx) || @@ -2979,7 +3033,9 @@ js_NewPreallocatedArray(JSContext* cx, JSObject* proto, int32 len) JSObject *obj = js_NewEmptyArray(cx, proto, len); if (!obj) return NULL; - if (!obj->ensureDenseArrayElements(cx, len)) + + /* Avoid ensureDenseArrayElements to skip sparse array checks there. */ + if (!obj->ensureSlots(cx, len)) return NULL; return obj; } diff --git a/js/src/jsarray.h b/js/src/jsarray.h index 7a7cfc64918..36141ca2606 100644 --- a/js/src/jsarray.h +++ b/js/src/jsarray.h @@ -46,6 +46,46 @@ #include "jspubtd.h" #include "jsobj.h" +/* Small arrays are dense, no matter what. */ +const uintN MIN_SPARSE_INDEX = 256; + +inline JSObject::EnsureDenseResult +JSObject::ensureDenseArrayElements(JSContext *cx, uintN index, uintN extra) +{ + JS_ASSERT(isDenseArray()); + uintN currentCapacity = numSlots(); + + uintN requiredCapacity; + if (extra == 1) { + /* Optimize for the common case. */ + if (index < currentCapacity) + return ED_OK; + requiredCapacity = index + 1; + if (requiredCapacity == 0) { + /* Overflow. */ + return ED_SPARSE; + } + } else { + requiredCapacity = index + extra; + if (requiredCapacity < index) { + /* Overflow. */ + return ED_SPARSE; + } + if (requiredCapacity <= currentCapacity) + return ED_OK; + } + + /* + * We use the extra argument also as a hint about number of non-hole + * elements to be inserted. + */ + if (requiredCapacity > MIN_SPARSE_INDEX && + willBeSparseDenseArray(requiredCapacity, extra)) { + return ED_SPARSE; + } + return growSlots(cx, requiredCapacity) ? ED_OK : ED_FAILED; +} + extern JSBool js_StringIsIndex(JSString *str, jsuint *indexp); @@ -144,9 +184,6 @@ js_NewArrayObject(JSContext *cx, jsuint length, const js::Value *vector); extern JSObject * js_NewSlowArrayObject(JSContext *cx); -/* Minimum size at which a dense array can be made sparse. */ -const uint32 MIN_SPARSE_INDEX = 256; - extern JSBool js_GetLengthProperty(JSContext *cx, JSObject *obj, jsuint *lengthp); diff --git a/js/src/jsemit.cpp b/js/src/jsemit.cpp index f9e12cabb53..af7a30ae13c 100644 --- a/js/src/jsemit.cpp +++ b/js/src/jsemit.cpp @@ -6809,12 +6809,8 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn) } #endif /* JS_HAS_GENERATORS */ - /* - * Use the slower NEWINIT for arrays in scripts containing sharps, and when - * the array length exceeds MIN_SPARSE_INDEX and can be slowified during GC. - * :FIXME: bug 607825 handle slowify case. - */ - if (cg->hasSharps() || pn->pn_count >= MIN_SPARSE_INDEX) { + /* Use the slower NEWINIT for arrays in scripts containing sharps. */ + if (cg->hasSharps()) { if (!EmitNewInit(cx, cg, JSProto_Array, pn, sharpnum)) return JS_FALSE; } else { diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 9f03b183b42..d90227292e9 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -1374,16 +1374,6 @@ GCMarker::markDelayedChildren() JS_ASSERT(!unmarkedArenaStackTop); } -void -GCMarker::slowifyArrays() -{ - while (!arraysToSlowify.empty()) { - JSObject *obj = arraysToSlowify.back(); - arraysToSlowify.popBack(); - if (obj->isMarked()) - obj->makeDenseArraySlow(context); - } -} } /* namespace js */ static void @@ -2243,9 +2233,6 @@ MarkAndSweep(JSContext *cx, JSGCInvocationKind gckind GCTIMER_PARAM) */ js_SweepScriptFilenames(rt); - /* Slowify arrays we have accumulated. */ - gcmarker.slowifyArrays(); - /* * Destroy arenas after we finished the sweeping so finalizers can safely * use js_IsAboutToBeFinalized(). diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 2108ede567e..99838cd66fd 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -983,8 +983,6 @@ struct GCMarker : public JSTracer { void dumpConservativeRoots(); #endif - js::Vector arraysToSlowify; - public: explicit GCMarker(JSContext *cx); ~GCMarker(); @@ -1005,8 +1003,6 @@ struct GCMarker : public JSTracer { void delayMarkingChildren(void *thing); JS_FRIEND_API(void) markDelayedChildren(); - - void slowifyArrays(); }; void diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index ac1d60b979c..3627e4030f6 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -5911,7 +5911,8 @@ BEGIN_CASE(JSOP_NEWARRAY) unsigned count = GET_UINT24(regs.pc); JSObject *obj = js_NewArrayObject(cx, count, NULL); - if (!obj || !obj->ensureDenseArrayElements(cx, count)) + /* Avoid ensureDenseArrayElements to skip sparse array checks there. */ + if (!obj || !obj->ensureSlots(cx, count)) goto error; PUSH_OBJECT(*obj); diff --git a/js/src/jsobj.h b/js/src/jsobj.h index b187c3109c2..3c802bdca78 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -750,9 +750,24 @@ struct JSObject : js::gc::Cell { inline const js::Value &getDenseArrayElement(uintN idx); inline js::Value* addressOfDenseArrayElement(uintN idx); inline void setDenseArrayElement(uintN idx, const js::Value &val); - inline bool ensureDenseArrayElements(JSContext *cx, uintN cap); inline void shrinkDenseArrayElements(JSContext *cx, uintN cap); + /* + * ensureDenseArrayElements ensures that the dense array can hold at least + * index + extra elements. It returns ED_OK on success, ED_FAILED on + * failure to grow the array, ED_SPARSE when the array is too sparse to + * grow (this includes the case of index + extra overflow). In the last + * two cases the array is kept intact. + */ + enum EnsureDenseResult { ED_OK, ED_FAILED, ED_SPARSE }; + inline EnsureDenseResult ensureDenseArrayElements(JSContext *cx, uintN index, uintN extra); + + /* + * Check if after growing the dense array will be too sparse. + * newElementsHint is an estimated number of elements to be added. + */ + bool willBeSparseDenseArray(uintN requiredCapacity, uintN newElementsHint); + JSBool makeDenseArraySlow(JSContext *cx); /* diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index 63456182510..fbddae97f05 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -329,13 +329,6 @@ JSObject::setDenseArrayElement(uintN idx, const js::Value &val) setSlot(idx, val); } -inline bool -JSObject::ensureDenseArrayElements(JSContext *cx, uintN cap) -{ - JS_ASSERT(isDenseArray()); - return ensureSlots(cx, cap); -} - inline void JSObject::shrinkDenseArrayElements(JSContext *cx, uintN cap) { From 439ca83a7b204bbc0e9046b3d68261e222d0336f Mon Sep 17 00:00:00 2001 From: Blake Kaplan Date: Mon, 15 Nov 2010 17:21:25 -0800 Subject: [PATCH 22/42] bug 601803 - Support adopting a node cross-compartment. r=gal/jst --- content/base/src/nsNodeUtils.cpp | 31 ++- content/base/test/test_bug601803.html | 2 +- dom/base/nsGlobalWindow.cpp | 2 +- js/src/js.msg | 1 + js/src/jsapi-tests/testBug604087.cpp | 4 +- js/src/jsapi.cpp | 46 ++-- js/src/jsapi.h | 2 +- js/src/jsobj.cpp | 207 +++++++++++++++--- js/src/jsobj.h | 6 +- js/src/jsobjinlines.h | 10 +- js/src/jswrapper.cpp | 3 +- js/src/jswrapper.h | 8 + js/src/xpconnect/src/nsXPConnect.cpp | 22 +- js/src/xpconnect/src/xpcpublic.h | 4 + js/src/xpconnect/src/xpcwrappednative.cpp | 62 +++++- js/src/xpconnect/tests/chrome/Makefile.in | 1 + .../xpconnect/tests/chrome/test_bug601803.xul | 40 ++++ js/src/xpconnect/wrappers/WrapperFactory.h | 10 +- 18 files changed, 374 insertions(+), 87 deletions(-) create mode 100644 js/src/xpconnect/tests/chrome/test_bug601803.xul diff --git a/content/base/src/nsNodeUtils.cpp b/content/base/src/nsNodeUtils.cpp index c9d7736ccf5..3bf4b07fdc8 100644 --- a/content/base/src/nsNodeUtils.cpp +++ b/content/base/src/nsNodeUtils.cpp @@ -1,4 +1,5 @@ /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=2 sw=2 et tw=99: */ /* ***** BEGIN LICENSE BLOCK ***** * Version: MPL 1.1/GPL 2.0/LGPL 2.1 * @@ -64,6 +65,7 @@ #include "nsImageLoadingContent.h" #include "jsobj.h" #include "jsgc.h" +#include "xpcpublic.h" using namespace mozilla::dom; @@ -461,6 +463,11 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, PRBool aClone, PRBool aDeep, // attributes and children). nsresult rv; + if (aCx) { + rv = xpc_MorphSlimWrapper(aCx, aNode); + NS_ENSURE_SUCCESS(rv, rv); + } + nsNodeInfoManager *nodeInfoManager = aNewNodeInfoManager; // aNode. @@ -516,11 +523,6 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, PRBool aClone, PRBool aDeep, } } else if (nodeInfoManager) { - // FIXME Bug 601803 Need to support adopting a node cross-compartment - if (aCx && aOldScope->compartment() != aNewScope->compartment()) { - return NS_ERROR_DOM_NOT_SUPPORTED_ERR; - } - nsIDocument* oldDoc = aNode->GetOwnerDoc(); PRBool wasRegistered = PR_FALSE; if (oldDoc && aNode->IsElement()) { @@ -583,9 +585,28 @@ nsNodeUtils::CloneAndAdopt(nsINode *aNode, PRBool aClone, PRBool aDeep, if (aCx) { nsIXPConnect *xpc = nsContentUtils::XPConnect(); if (xpc) { + nsWrapperCache *cache; + CallQueryInterface(aNode, &cache); + JSObject *preservedWrapper = nsnull; + + // If reparenting moves us to a new compartment, preserving causes + // problems. In that case, we release ourselves and re-preserve after + // reparenting so we're sure to have the right JS object preserved. + // We use a JSObject stack copy of the wrapper to protect it from GC + // under ReparentWrappedNativeIfFound. + if (cache && cache->PreservingWrapper()) { + preservedWrapper = cache->GetWrapper(); + nsContentUtils::ReleaseWrapper(aNode, cache); + } + nsCOMPtr oldWrapper; rv = xpc->ReparentWrappedNativeIfFound(aCx, aOldScope, aNewScope, aNode, getter_AddRefs(oldWrapper)); + + if (preservedWrapper) { + nsContentUtils::PreserveWrapper(aNode, cache); + } + if (NS_FAILED(rv)) { aNode->mNodeInfo.swap(nodeInfo); diff --git a/content/base/test/test_bug601803.html b/content/base/test/test_bug601803.html index 18744efa223..9f1451975a7 100644 --- a/content/base/test/test_bug601803.html +++ b/content/base/test/test_bug601803.html @@ -22,7 +22,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=601803 SimpleTest.waitForExplicitFinish(); window.onmessage = function (event) { - todo(event.data == "false", "Shouldn't throw when adopting a node cross-compartment"); + is(event.data, "false", "Shouldn't throw when adopting a node cross-compartment"); SimpleTest.finish(); } diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index ad2f3ce6850..50f971da991 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -2050,7 +2050,7 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument, return NS_ERROR_FAILURE; } - outerObject = JS_TransplantWrapper(cx, mJSObject, outerObject); + outerObject = JS_TransplantObject(cx, mJSObject, outerObject); if (!outerObject) { NS_ERROR("unable to transplant wrappers, probably OOM"); return NS_ERROR_FAILURE; diff --git a/js/src/js.msg b/js/src/js.msg index c93a7b0634e..031bea8e207 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -344,3 +344,4 @@ MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE, 261, 0, JSEXN_TYPEERR, "unsupported type f MSG_DEF(JSMSG_SC_RECURSION, 262, 0, JSEXN_INTERNALERR, "recursive object") MSG_DEF(JSMSG_CANT_WRAP_XML_OBJECT, 263, 0, JSEXN_TYPEERR, "can't wrap XML objects") MSG_DEF(JSMSG_BAD_CLONE_VERSION, 264, 0, JSEXN_ERR, "unsupported structured clone version") +MSG_DEF(JSMSG_CANT_CLONE_OBJECT, 265, 0, JSEXN_TYPEERR, "can't clone object") diff --git a/js/src/jsapi-tests/testBug604087.cpp b/js/src/jsapi-tests/testBug604087.cpp index 8bb5c1a28cc..402c362a81e 100644 --- a/js/src/jsapi-tests/testBug604087.cpp +++ b/js/src/jsapi-tests/testBug604087.cpp @@ -1,7 +1,7 @@ /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- * vim: set ts=8 sw=4 et tw=99: * - * Tests JS_TransplantWrappers + * Tests JS_TransplantObject */ #include "tests.h" @@ -78,7 +78,7 @@ BEGIN_TEST(testBug604087) } JS_SetWrapObjectCallbacks(JS_GetRuntime(cx), Wrap, PreWrap); - CHECK(JS_TransplantWrapper(cx, outerObj, next)); + CHECK(JS_TransplantObject(cx, outerObj, next)); return true; } END_TEST(testBug604087) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 1166ec2add2..e48027accbc 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -1217,31 +1217,29 @@ JS_WrapValue(JSContext *cx, jsval *vp) } JS_PUBLIC_API(JSObject *) -JS_TransplantWrapper(JSContext *cx, JSObject *wrapper, JSObject *target) +JS_TransplantObject(JSContext *cx, JSObject *origobj, JSObject *target) { - JS_ASSERT(wrapper->isWrapper()); - - /* - * This function is called when a window is navigating. In that case, we - * need to "move" the window from wrapper's compartment to target's - * compartment. - */ + // This function is called when an object moves between two different + // compartments. In that case, we need to "move" the window from origobj's + // compartment to target's compartment. JSCompartment *destination = target->getCompartment(); - if (wrapper->getCompartment() == destination) { - // If the wrapper is in the same compartment as the destination, then - // we know that we won't find wrapper in the destination's cross - // compartment map and that the same object will continue to work. - if (!wrapper->swap(cx, target)) + if (origobj->getCompartment() == destination) { + // If the original object is in the same compartment as the + // destination, then we know that we won't find wrapper in the + // destination's cross compartment map and that the same object + // will continue to work. + if (!origobj->swap(cx, target)) return NULL; - return wrapper; + return origobj; } JSObject *obj; WrapperMap &map = destination->crossCompartmentWrappers; - Value wrapperv = ObjectValue(*wrapper); + Value origv = ObjectValue(*origobj); - // There might already be a wrapper for the window in the new compartment. - if (WrapperMap::Ptr p = map.lookup(wrapperv)) { + // There might already be a wrapper for the original object in the new + // compartment. + if (WrapperMap::Ptr p = map.lookup(origv)) { // If there is, make it the primary outer window proxy around the // inner (accomplished by swapping target's innards with the old, // possibly security wrapper, innards). @@ -1266,7 +1264,7 @@ JS_TransplantWrapper(JSContext *cx, JSObject *wrapper, JSObject *target) for (JSCompartment **p = vector.begin(), **end = vector.end(); p != end; ++p) { WrapperMap &pmap = (*p)->crossCompartmentWrappers; - if (WrapperMap::Ptr wp = pmap.lookup(wrapperv)) { + if (WrapperMap::Ptr wp = pmap.lookup(origv)) { // We found a wrapper. Remember and root it. toTransplant.append(wp->value); } @@ -1276,8 +1274,8 @@ JS_TransplantWrapper(JSContext *cx, JSObject *wrapper, JSObject *target) JSObject *wobj = &begin->toObject(); JSCompartment *wcompartment = wobj->compartment(); WrapperMap &pmap = wcompartment->crossCompartmentWrappers; - JS_ASSERT(pmap.lookup(wrapperv)); - pmap.remove(wrapperv); + JS_ASSERT(pmap.lookup(origv)); + pmap.remove(origv); // First, we wrap it in the new compartment. This will return a // new wrapper. @@ -1296,15 +1294,15 @@ JS_TransplantWrapper(JSContext *cx, JSObject *wrapper, JSObject *target) pmap.put(targetv, ObjectValue(*wobj)); } - // Lastly, update the old outer window proxy to point to the new one. + // Lastly, update the original object to point to the new one. { - AutoCompartment ac(cx, wrapper); + AutoCompartment ac(cx, origobj); JSObject *tobj = obj; if (!ac.enter() || !JS_WrapObject(cx, &tobj)) return NULL; - if (!wrapper->swap(cx, tobj)) + if (!origobj->swap(cx, tobj)) return NULL; - wrapper->getCompartment()->crossCompartmentWrappers.put(targetv, wrapperv); + origobj->getCompartment()->crossCompartmentWrappers.put(targetv, origv); } return obj; diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 136ade29465..0c283a89dd8 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -965,7 +965,7 @@ extern JS_PUBLIC_API(JSBool) JS_WrapValue(JSContext *cx, jsval *vp); extern JS_PUBLIC_API(JSObject *) -JS_TransplantWrapper(JSContext *cx, JSObject *wrapper, JSObject *target); +JS_TransplantObject(JSContext *cx, JSObject *origobj, JSObject *target); #ifdef __cplusplus JS_END_EXTERN_C diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index d88edad0b2c..3ed2f044390 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -77,6 +77,7 @@ #include "jstracer.h" #include "jsdbgapi.h" #include "json.h" +#include "jswrapper.h" #include "jsinterpinlines.h" #include "jsscopeinlines.h" @@ -3296,6 +3297,155 @@ GetObjectSize(JSObject *obj) : sizeof(JSObject) + sizeof(js::Value) * obj->numFixedSlots(); } +bool +JSObject::copyPropertiesFrom(JSContext *cx, JSObject *obj) +{ + // If we're not native, then we cannot copy properties. + JS_ASSERT(isNative() == obj->isNative()); + if (!isNative()) + return true; + + Vector shapes(cx); + for (Shape::Range r(obj->lastProperty()); !r.empty(); r.popFront()) { + if (!shapes.append(&r.front())) + return false; + } + + size_t n = shapes.length(); + while (n > 0) { + const Shape *shape = shapes[--n]; + uintN attrs = shape->attributes(); + PropertyOp getter = shape->getter(); + if ((attrs & JSPROP_GETTER) && !cx->compartment->wrap(cx, &getter)) + return false; + PropertyOp setter = shape->setter(); + if ((attrs & JSPROP_SETTER) && !cx->compartment->wrap(cx, &setter)) + return false; + Value v = shape->hasSlot() ? obj->getSlot(shape->slot) : UndefinedValue(); + if (!cx->compartment->wrap(cx, &v)) + return false; + if (!defineProperty(cx, shape->id, v, getter, setter, attrs)) + return false; + } + return true; +} + +static bool +CopySlots(JSContext *cx, JSObject *from, JSObject *to) +{ + JS_ASSERT(!from->isNative() && !to->isNative()); + size_t nslots = from->numSlots(); + if (to->ensureSlots(cx, nslots)) + return false; + + size_t n = 0; + if (to->isWrapper() && + (JSWrapper::wrapperHandler(to)->flags() & JSWrapper::CROSS_COMPARTMENT)) { + to->slots[0] = from->slots[0]; + to->slots[1] = from->slots[1]; + n = 2; + } + + for (; n < nslots; ++n) { + Value v = from->slots[n]; + if (!cx->compartment->wrap(cx, &v)) + return false; + to->slots[n] = v; + } + return true; +} + +JSObject * +JSObject::clone(JSContext *cx, JSObject *proto, JSObject *parent) +{ + /* + * We can only clone native objects and proxies. Dense arrays are slowified if + * we try to clone them. + */ + if (!isNative()) { + if (isDenseArray()) { + if (!makeDenseArraySlow(cx)) + return NULL; + } else if (!isProxy()) { + JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, + JSMSG_CANT_CLONE_OBJECT); + return NULL; + } + } + JSObject *clone = NewObject(cx, getClass(), + proto, parent, + gc::FinalizeKind(finalizeKind())); + if (!clone) + return NULL; + if (isNative()) { + if (clone->isFunction() && (compartment() != clone->compartment())) { + JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, + JSMSG_CANT_CLONE_OBJECT); + return NULL; + } + + if (getClass()->flags & JSCLASS_HAS_PRIVATE) + clone->setPrivate(getPrivate()); + } else { + JS_ASSERT(isProxy()); + if (!CopySlots(cx, this, clone)) + return NULL; + } + return clone; +} + +static void +TradeGuts(JSObject *a, JSObject *b) +{ + JS_ASSERT(a->compartment() == b->compartment()); + JS_ASSERT(a->isFunction() == b->isFunction()); + + bool aInline = !a->hasSlotsArray(); + bool bInline = !b->hasSlotsArray(); + + /* Trade the guts of the objects. */ + const size_t size = GetObjectSize(a); + if (size == GetObjectSize(b)) { + /* + * If the objects are the same size, then we make no assumptions about + * whether they have dynamically allocated slots and instead just copy + * them over wholesale. + */ + char tmp[tl::Max::result]; + JS_ASSERT(size <= sizeof(tmp)); + + memcpy(tmp, a, size); + memcpy(a, b, size); + memcpy(b, tmp, size); + + /* Fixup pointers for inline slots on the objects. */ + if (aInline) + b->slots = b->fixedSlots(); + if (bInline) + a->slots = a->fixedSlots(); + } else { + /* + * If the objects are of differing sizes, then we only copy over the + * JSObject portion (things like class, etc.) and leave it to + * JSObject::clone to copy over the dynamic slots for us. + */ + if (a->isFunction()) { + JSFunction tmp; + memcpy(&tmp, a, sizeof tmp); + memcpy(a, b, sizeof tmp); + memcpy(b, &tmp, sizeof tmp); + } else { + JSObject tmp; + memcpy(&tmp, a, sizeof tmp); + memcpy(a, b, sizeof tmp); + memcpy(b, &tmp, sizeof tmp); + } + + JS_ASSERT(!aInline); + JS_ASSERT(!bInline); + } +} + /* * Use this method with extreme caution. It trades the guts of two objects and updates * scope ownership. This operation is not thread-safe, just as fast array to slow array @@ -3305,17 +3455,11 @@ GetObjectSize(JSObject *obj) bool JSObject::swap(JSContext *cx, JSObject *other) { - size_t size = GetObjectSize(this); - - if (size != GetObjectSize(other)) { - /* - * Objects with different numbers of fixed slots can be swapped only if they - * are both shapeless non-natives, to preserve the invariant that objects with the - * same shape have the same number of fixed slots. Use a dynamic array for both. - */ - JS_ASSERT(!isNative()); - JS_ASSERT(!other->isNative()); - size = sizeof(JSObject); + /* + * If we are swapping objects with a different number of builtin slots, force + * both to not use their inline slots. + */ + if (GetObjectSize(this) != GetObjectSize(other)) { if (!hasSlotsArray()) { if (!allocSlots(cx, numSlots())) return false; @@ -3326,24 +3470,31 @@ JSObject::swap(JSContext *cx, JSObject *other) } } - bool thisInline = !hasSlotsArray(); - bool otherInline = !other->hasSlotsArray(); + if (this->compartment() == other->compartment()) { + TradeGuts(this, other); + return true; + } - JS_STATIC_ASSERT(FINALIZE_OBJECT_LAST == FINALIZE_OBJECT16); - - char tmp[tl::Max::result]; - JS_ASSERT(size <= sizeof(tmp)); - - /* Trade the guts of the objects. */ - memcpy(tmp, this, size); - memcpy(this, other, size); - memcpy(other, tmp, size); - - /* Fixup pointers for inline slots on the objects. */ - if (thisInline) - other->slots = other->fixedSlots(); - if (otherInline) - this->slots = this->fixedSlots(); + JSObject *thisClone; + JSObject *otherClone; + { + AutoCompartment ac(cx, other); + if (!ac.enter()) + return false; + thisClone = this->clone(cx, other->getProto(), other->getParent()); + if (!thisClone || !thisClone->copyPropertiesFrom(cx, this)) + return false; + } + { + AutoCompartment ac(cx, this); + if (!ac.enter()) + return false; + otherClone = other->clone(cx, other->getProto(), other->getParent()); + if (!otherClone || !otherClone->copyPropertiesFrom(cx, other)) + return false; + } + TradeGuts(this, otherClone); + TradeGuts(other, thisClone); return true; } diff --git a/js/src/jsobj.h b/js/src/jsobj.h index 3c802bdca78..7a1365f96b1 100644 --- a/js/src/jsobj.h +++ b/js/src/jsobj.h @@ -552,6 +552,8 @@ struct JSObject : js::gc::Cell { inline bool hasPropertyTable() const; + /* gc::FinalizeKind */ unsigned finalizeKind() const; + uint32 numSlots() const { return capacity; } size_t slotsAndStructSize(uint32 nslots) const; @@ -1146,7 +1148,9 @@ struct JSObject : js::gc::Cell { inline JSObject *getThrowTypeError() const; - bool swap(JSContext *cx, JSObject *obj); + JSObject *clone(JSContext *cx, JSObject *proto, JSObject *parent); + bool copyPropertiesFrom(JSContext *cx, JSObject *obj); + bool swap(JSContext *cx, JSObject *other); const js::Shape *defineBlockVariable(JSContext *cx, jsid id, intN index); diff --git a/js/src/jsobjinlines.h b/js/src/jsobjinlines.h index fbddae97f05..13c78d79f80 100644 --- a/js/src/jsobjinlines.h +++ b/js/src/jsobjinlines.h @@ -252,10 +252,10 @@ JSObject::setPrimitiveThis(const js::Value &pthis) setSlot(JSSLOT_PRIMITIVE_THIS, pthis); } -inline js::gc::FinalizeKind -GetObjectFinalizeKind(const JSObject *obj) +inline /* gc::FinalizeKind */ unsigned +JSObject::finalizeKind() const { - return js::gc::FinalizeKind(obj->arena()->header()->thingKind); + return js::gc::FinalizeKind(arena()->header()->thingKind); } inline size_t @@ -265,7 +265,7 @@ JSObject::numFixedSlots() const return JSObject::FUN_CLASS_RESERVED_SLOTS; if (!hasSlotsArray()) return capacity; - return js::gc::GetGCKindSlots(GetObjectFinalizeKind(this)); + return js::gc::GetGCKindSlots(js::gc::FinalizeKind(finalizeKind())); } inline size_t @@ -1063,7 +1063,7 @@ CopyInitializerObject(JSContext *cx, JSObject *baseobj) JS_ASSERT(baseobj->getClass() == &js_ObjectClass); JS_ASSERT(!baseobj->inDictionaryMode()); - gc::FinalizeKind kind = GetObjectFinalizeKind(baseobj); + gc::FinalizeKind kind = gc::FinalizeKind(baseobj->finalizeKind()); JSObject *obj = NewBuiltinClassInstance(cx, &js_ObjectClass, kind); if (!obj || !obj->ensureSlots(cx, baseobj->numSlots())) diff --git a/js/src/jswrapper.cpp b/js/src/jswrapper.cpp index 53e6b95e3ed..ceb0432b170 100644 --- a/js/src/jswrapper.cpp +++ b/js/src/jswrapper.cpp @@ -383,7 +383,8 @@ AutoCompartment::leave() /* Cross compartment wrappers. */ -JSCrossCompartmentWrapper::JSCrossCompartmentWrapper(uintN flags) : JSWrapper(flags) +JSCrossCompartmentWrapper::JSCrossCompartmentWrapper(uintN flags) + : JSWrapper(CROSS_COMPARTMENT | flags) { } diff --git a/js/src/jswrapper.h b/js/src/jswrapper.h index 6d0732ecda8..fce9cc80714 100644 --- a/js/src/jswrapper.h +++ b/js/src/jswrapper.h @@ -103,6 +103,14 @@ class JS_FRIEND_API(JSWrapper) : public js::JSProxyHandler { static inline JSObject *wrappedObject(const JSObject *wrapper) { return wrapper->getProxyPrivate().toObjectOrNull(); } + static inline JSWrapper *wrapperHandler(const JSObject *wrapper) { + return static_cast(wrapper->getProxyHandler()); + } + + enum { + CROSS_COMPARTMENT = 1 << 0, + LAST_USED_FLAG = CROSS_COMPARTMENT + }; static void *getWrapperFamily(); }; diff --git a/js/src/xpconnect/src/nsXPConnect.cpp b/js/src/xpconnect/src/nsXPConnect.cpp index f00a7961680..4d8da0389f9 100644 --- a/js/src/xpconnect/src/nsXPConnect.cpp +++ b/js/src/xpconnect/src/nsXPConnect.cpp @@ -1182,6 +1182,20 @@ nsXPConnect::InitClassesWithNewWrappedGlobal(JSContext * aJSContext, return NS_OK; } +nsresult +xpc_MorphSlimWrapper(JSContext *cx, nsISupports *tomorph) +{ + nsWrapperCache *cache; + CallQueryInterface(tomorph, &cache); + if(!cache) + return NS_OK; + + JSObject *obj = cache->GetWrapper(); + if(!obj || !IS_SLIM_WRAPPER(obj)) + return NS_OK; + return MorphSlimWrapper(cx, obj); +} + static nsresult NativeInterface2JSObject(XPCLazyCallContext & lccx, JSObject * aScope, @@ -1483,10 +1497,14 @@ static JSDHashOperator MoveableWrapperFinder(JSDHashTable *table, JSDHashEntryHdr *hdr, uint32 number, void *arg) { - // Every element counts. nsTArray > *array = static_cast > *>(arg); - array->AppendElement(((Native2WrappedNativeMap::Entry*)hdr)->value); + XPCWrappedNative *wn = ((Native2WrappedNativeMap::Entry*)hdr)->value; + + // If a wrapper is expired, then there are no references to it from JS, so + // we don't have to move it. + if(!wn->IsWrapperExpired()) + array->AppendElement(wn); return JS_DHASH_NEXT; } diff --git a/js/src/xpconnect/src/xpcpublic.h b/js/src/xpconnect/src/xpcpublic.h index 6b0cfa57f4b..27bca6e36d2 100644 --- a/js/src/xpconnect/src/xpcpublic.h +++ b/js/src/xpconnect/src/xpcpublic.h @@ -41,6 +41,7 @@ #define xpcpublic_h #include "jsapi.h" +#include "nsISupports.h" #include "jsobj.h" #include "nsAString.h" #include "nsIPrincipal.h" @@ -59,6 +60,9 @@ xpc_CreateMTGlobalObject(JSContext *cx, JSClass *clasp, nsISupports *ptr, JSObject **global, JSCompartment **compartment); +nsresult +xpc_MorphSlimWrapper(JSContext *cx, nsISupports *tomorph); + extern JSBool XPC_WN_Equality(JSContext *cx, JSObject *obj, const jsval *v, JSBool *bp); diff --git a/js/src/xpconnect/src/xpcwrappednative.cpp b/js/src/xpconnect/src/xpcwrappednative.cpp index 8257dd6704b..f75b73a7a3b 100644 --- a/js/src/xpconnect/src/xpcwrappednative.cpp +++ b/js/src/xpconnect/src/xpcwrappednative.cpp @@ -519,8 +519,8 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx, nsCOMPtr wrappedjs(do_QueryInterface(Object)); JSObject *obj; wrappedjs->GetJSObject(&obj); - if(xpc::AccessCheck::isChrome(obj->getCompartment()) && - !xpc::AccessCheck::isChrome(Scope->GetGlobalJSObject()->getCompartment())) + if(xpc::AccessCheck::isChrome(obj->compartment()) && + !xpc::AccessCheck::isChrome(Scope->GetGlobalJSObject()->compartment())) { needsCOW = JS_TRUE; } @@ -1508,6 +1508,16 @@ XPCWrappedNative::ReparentWrapperIfFound(XPCCallContext& ccx, return NS_OK; } + bool crosscompartment = aOldScope->GetGlobalJSObject()->compartment() != + aNewScope->GetGlobalJSObject()->compartment(); +#ifdef DEBUG + if(crosscompartment) + { + NS_ASSERTION(aNewParent, "won't be able to find the new parent"); + NS_ASSERTION(wrapper, "can't transplant slim wrappers"); + } +#endif + // ReparentWrapperIfFound is really only meant to be called from DOM code // which must happen only on the main thread. Bail if we're on some other // thread or have a non-main-thread-only wrapper. @@ -1519,6 +1529,10 @@ XPCWrappedNative::ReparentWrapperIfFound(XPCCallContext& ccx, return NS_ERROR_FAILURE; } + JSAutoEnterCompartment ac; + if(!ac.enter(ccx, aNewScope->GetGlobalJSObject())) + return NS_ERROR_FAILURE; + if(aOldScope != aNewScope) { // Oh, so now we need to move the wrapper to a different scope. @@ -1590,20 +1604,46 @@ XPCWrappedNative::ReparentWrapperIfFound(XPCCallContext& ccx, // We only try to fixup the __proto__ JSObject if the wrapper // is directly using that of its XPCWrappedNativeProto. - if(wrapper->HasProto() && - flat->getProto() == oldProto->GetJSProtoObject()) + if(crosscompartment) { - if(!JS_SetPrototype(ccx, flat, newProto->GetJSProtoObject())) - { - // this is bad, very bad - NS_ERROR("JS_SetPrototype failed"); + JSObject *newobj = flat->clone(ccx, newProto->GetJSProtoObject(), + aNewParent); + if(!newobj) + return NS_ERROR_FAILURE; + + JS_SetPrivate(ccx, flat, nsnull); + + JSObject *propertyHolder = + JS_NewObjectWithGivenProto(ccx, NULL, NULL, aNewParent); + if(!propertyHolder || !propertyHolder->copyPropertiesFrom(ccx, flat)) + return NS_ERROR_OUT_OF_MEMORY; + + flat = JS_TransplantObject(ccx, flat, newobj); + if(!flat) + return NS_ERROR_FAILURE; + wrapper->mFlatJSObject = flat; + if(cache) + cache->SetWrapper(flat); + if (!flat->copyPropertiesFrom(ccx, propertyHolder)) return NS_ERROR_FAILURE; - } } else { - NS_WARNING("Moving XPConnect wrappedNative to new scope, " - "but can't fixup __proto__"); + if(wrapper->HasProto() && + flat->getProto() == oldProto->GetJSProtoObject()) + { + if(!JS_SetPrototype(ccx, flat, newProto->GetJSProtoObject())) + { + // this is bad, very bad + NS_ERROR("JS_SetPrototype failed"); + return NS_ERROR_FAILURE; + } + } + else + { + NS_WARNING("Moving XPConnect wrappedNative to new scope, " + "but can't fixup __proto__"); + } } } else diff --git a/js/src/xpconnect/tests/chrome/Makefile.in b/js/src/xpconnect/tests/chrome/Makefile.in index 9a9bca37f8b..aad74660e21 100644 --- a/js/src/xpconnect/tests/chrome/Makefile.in +++ b/js/src/xpconnect/tests/chrome/Makefile.in @@ -57,6 +57,7 @@ _CHROME_FILES = \ test_cows.xul \ test_bug517163.xul \ test_bug571849.xul \ + test_bug601803.xul \ $(NULL) # Disabled until this test gets updated to test the new proxy based diff --git a/js/src/xpconnect/tests/chrome/test_bug601803.xul b/js/src/xpconnect/tests/chrome/test_bug601803.xul new file mode 100644 index 00000000000..a6c8e230620 --- /dev/null +++ b/js/src/xpconnect/tests/chrome/test_bug601803.xul @@ -0,0 +1,40 @@ + + + + + + + + + + + Mozilla Bug 601803 + + + + + + + + + diff --git a/js/src/xpconnect/wrappers/WrapperFactory.cpp b/js/src/xpconnect/wrappers/WrapperFactory.cpp index a8d82461af1..286f6095cf1 100644 --- a/js/src/xpconnect/wrappers/WrapperFactory.cpp +++ b/js/src/xpconnect/wrappers/WrapperFactory.cpp @@ -48,6 +48,7 @@ #include "XPCWrapper.h" #include "xpcprivate.h" +#include "xpcmaps.h" namespace xpc { @@ -307,10 +308,33 @@ WrapperFactory::WaiveXrayAndWrap(JSContext *cx, jsval *vp) obj = GetCurrentOuter(cx, obj); { - js::SwitchToCompartment sc(cx, obj->compartment()); - obj = JSWrapper::New(cx, obj, NULL, obj->getGlobal(), &WaiveXrayWrapperWrapper); - if (!obj) - return false; + // See if we already have a waiver wrapper for this object. + CompartmentPrivate *priv = + (CompartmentPrivate *)JS_GetCompartmentPrivate(cx, obj->compartment()); + JSObject *wobj = nsnull; + if (priv && priv->waiverWrapperMap) + wobj = priv->waiverWrapperMap->Find(obj); + + // No wrapper yet, make one. + if (!wobj) { + js::SwitchToCompartment sc(cx, obj->compartment()); + wobj = JSWrapper::New(cx, obj, NULL, obj->getGlobal(), &WaiveXrayWrapperWrapper); + if (!wobj) + return false; + + // Add the new wrapper so we find it next time. + if (priv) { + if (!priv->waiverWrapperMap) { + priv->waiverWrapperMap = JSObject2JSObjectMap::newMap(XPC_WRAPPER_MAP_SIZE); + if (!priv->waiverWrapperMap) + return false; + } + if (!priv->waiverWrapperMap->Add(obj, wobj)) + return false; + } + } + + obj = wobj; } *vp = OBJECT_TO_JSVAL(obj); From 1fd342421957f0364662518933cd6d6522dab050 Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Sat, 4 Dec 2010 17:04:10 +0100 Subject: [PATCH 41/42] bug 590533 - InvokeOperationCallback should yield when the is cancelled. r=gal --- js/src/jsapi-tests/Makefile.in | 3 +- js/src/jsapi-tests/testThreadGC.cpp | 195 ++++++++++++++++++++++++++++ js/src/jscntxt.cpp | 28 ++-- 3 files changed, 212 insertions(+), 14 deletions(-) create mode 100644 js/src/jsapi-tests/testThreadGC.cpp diff --git a/js/src/jsapi-tests/Makefile.in b/js/src/jsapi-tests/Makefile.in index 5bcea7f7dac..e70c7adf24d 100644 --- a/js/src/jsapi-tests/Makefile.in +++ b/js/src/jsapi-tests/Makefile.in @@ -49,6 +49,7 @@ PROGRAM = jsapi-tests$(BIN_SUFFIX) CPPSRCS = \ tests.cpp \ selfTest.cpp \ + testBug604087.cpp \ testClassGetter.cpp \ testCloneScript.cpp \ testConservativeGC.cpp \ @@ -69,7 +70,7 @@ CPPSRCS = \ testSameValue.cpp \ testScriptObject.cpp \ testSetPropertyWithNativeGetterStubSetter.cpp \ - testBug604087.cpp \ + testThreadGC.cpp \ testThreads.cpp \ testTrap.cpp \ testUTF8.cpp \ diff --git a/js/src/jsapi-tests/testThreadGC.cpp b/js/src/jsapi-tests/testThreadGC.cpp new file mode 100644 index 00000000000..fadbf97187d --- /dev/null +++ b/js/src/jsapi-tests/testThreadGC.cpp @@ -0,0 +1,195 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * vim: set ts=8 sw=4 et tw=99: + */ + +#ifdef JS_THREADSAFE + +#include "tests.h" +#include "prthread.h" + +#include "jscntxt.h" + +/* + * We test that if a GC callback cancels the GC on a child thread the GC can + * still proceed on the main thread even if the child thread continue to + * run uninterrupted. + */ + +struct SharedData { + enum ChildState { + CHILD_STARTING, + CHILD_RUNNING, + CHILD_DONE, + CHILD_ERROR + }; + + JSRuntime *const runtime; + PRThread *const mainThread; + PRLock *const lock; + PRCondVar *const signal; + ChildState childState; + bool childShouldStop; + JSContext *childContext; + + SharedData(JSRuntime *rt, bool *ok) + : runtime(rt), + mainThread(PR_GetCurrentThread()), + lock(PR_NewLock()), + signal(lock ? PR_NewCondVar(lock) : NULL), + childState(CHILD_STARTING), + childShouldStop(false), + childContext(NULL) + { + JS_ASSERT(!*ok); + *ok = !!signal; + } + + ~SharedData() { + if (signal) + PR_DestroyCondVar(signal); + if (lock) + PR_DestroyLock(lock); + } +}; + +static SharedData *shared; + +static JSBool +CancelNonMainThreadGCCallback(JSContext *cx, JSGCStatus status) +{ + return status != JSGC_BEGIN || PR_GetCurrentThread() == shared->mainThread; +} + +static JSBool +StopChildOperationCallback(JSContext *cx) +{ + bool shouldStop; + PR_Lock(shared->lock); + shouldStop = shared->childShouldStop; + PR_Unlock(shared->lock); + return !shouldStop; +} + +static JSBool +NotifyMainThreadAboutBusyLoop(JSContext *cx, uintN argc, jsval *vp) +{ + PR_Lock(shared->lock); + JS_ASSERT(shared->childState == SharedData::CHILD_STARTING); + shared->childState = SharedData::CHILD_RUNNING; + shared->childContext = cx; + PR_NotifyCondVar(shared->signal); + PR_Unlock(shared->lock); + + return true; +} + +static void +ChildThreadMain(void *arg) +{ + JS_ASSERT(!arg); + bool error = true; + JSContext *cx = JS_NewContext(shared->runtime, 8192); + if (cx) { + JS_SetOperationCallback(cx, StopChildOperationCallback); + JSAutoRequest ar(cx); + JSObject *global = JS_NewCompartmentAndGlobalObject(cx, JSAPITest::basicGlobalClass(), + NULL); + if (global) { + JS_SetGlobalObject(cx, global); + if (JS_InitStandardClasses(cx, global) && + JS_DefineFunction(cx, global, "notify", NotifyMainThreadAboutBusyLoop, 0, 0)) { + + jsval rval; + static const char code[] = "var i = 0; notify(); for (var i = 0; ; ++i);"; + JSBool ok = JS_EvaluateScript(cx, global, code, strlen(code), + __FILE__, __LINE__, &rval); + if (!ok && !JS_IsExceptionPending(cx)) { + /* Evaluate should only return via the callback cancellation. */ + error = false; + } + } + } + } + + PR_Lock(shared->lock); + shared->childState = error ? SharedData::CHILD_DONE : SharedData::CHILD_ERROR; + shared->childContext = NULL; + PR_NotifyCondVar(shared->signal); + PR_Unlock(shared->lock); + + if (cx) + JS_DestroyContextNoGC(cx); +} + +BEGIN_TEST(testThreadGC_bug590533) + { + /* + * Test the child thread busy running while the current thread calls + * the GC both with JSRuntime->gcIsNeeded set and unset. + */ + bool ok = TestChildThread(true); + CHECK(ok); + ok = TestChildThread(false); + CHECK(ok); + return ok; + } + + bool TestChildThread(bool setGCIsNeeded) + { + bool ok = false; + shared = new SharedData(rt, &ok); + CHECK(ok); + + JSGCCallback oldGCCallback = JS_SetGCCallback(cx, CancelNonMainThreadGCCallback); + + PRThread *thread = + PR_CreateThread(PR_USER_THREAD, ChildThreadMain, NULL, + PR_PRIORITY_NORMAL, PR_LOCAL_THREAD, PR_JOINABLE_THREAD, 0); + if (!thread) + return false; + + PR_Lock(shared->lock); + while (shared->childState == SharedData::CHILD_STARTING) + PR_WaitCondVar(shared->signal, PR_INTERVAL_NO_TIMEOUT); + JS_ASSERT(shared->childState != SharedData::CHILD_DONE); + ok = (shared->childState == SharedData::CHILD_RUNNING); + PR_Unlock(shared->lock); + + CHECK(ok); + + if (setGCIsNeeded) { + /* + * Use JS internal API to set the GC trigger flag after we know + * that the child is in a request and is about to run an infinite + * loop. Then run the GC with JSRuntime->gcIsNeeded flag set. + */ + js::AutoLockGC lock(rt); + js::TriggerGC(rt); + } + + JS_GC(cx); + + PR_Lock(shared->lock); + shared->childShouldStop = true; + while (shared->childState == SharedData::CHILD_RUNNING) { + JS_TriggerOperationCallback(shared->childContext); + PR_WaitCondVar(shared->signal, PR_INTERVAL_NO_TIMEOUT); + } + JS_ASSERT(shared->childState != SharedData::CHILD_STARTING); + ok = (shared->childState == SharedData::CHILD_DONE); + PR_Unlock(shared->lock); + + JS_SetGCCallback(cx, oldGCCallback); + + PR_JoinThread(thread); + + delete shared; + shared = NULL; + + return true; + } + + +END_TEST(testThreadGC_bug590533) + +#endif diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index cf4f986ca0b..5d6b92c67b2 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -1845,9 +1845,9 @@ js_InvokeOperationCallback(JSContext *cx) JS_ASSERT(td->interruptFlags != 0); /* - * Reset the callback counter first, then yield. If another thread is racing - * us here we will accumulate another callback request which will be - * serviced at the next opportunity. + * Reset the callback counter first, then run GC and yield. If another + * thread is racing us here we will accumulate another callback request + * which will be serviced at the next opportunity. */ JS_LOCK_GC(rt); td->interruptFlags = 0; @@ -1856,13 +1856,6 @@ js_InvokeOperationCallback(JSContext *cx) #endif JS_UNLOCK_GC(rt); - /* - * Unless we are going to run the GC, we automatically yield the current - * context every time the operation callback is hit since we might be - * called as a result of an impending GC, which would deadlock if we do - * not yield. Operation callbacks are supposed to happen rarely (seconds, - * not milliseconds) so it is acceptable to yield at every callback. - */ if (rt->gcIsNeeded) { js_GC(cx, GC_NORMAL); @@ -1879,10 +1872,19 @@ js_InvokeOperationCallback(JSContext *cx) return false; } } + #ifdef JS_THREADSAFE - else { - JS_YieldRequest(cx); - } + /* + * We automatically yield the current context every time the operation + * callback is hit since we might be called as a result of an impending + * GC on another thread, which would deadlock if we do not yield. + * Operation callbacks are supposed to happen rarely (seconds, not + * milliseconds) so it is acceptable to yield at every callback. + * + * As the GC can be canceled before it does any request checks we yield + * even if rt->gcIsNeeded was true above. See bug 590533. + */ + JS_YieldRequest(cx); #endif JSOperationCallback cb = cx->operationCallback; From 2331ad205ec29ff0ce4ca5c0478b060ff80288e1 Mon Sep 17 00:00:00 2001 From: Jacob Bramley Date: Mon, 6 Dec 2010 11:07:37 +0000 Subject: [PATCH 42/42] Extend the IC protection introduced by bug 614323. [Bug 615875] [r=cdleary] --- js/src/assembler/assembler/ARMAssembler.h | 7 +++++++ .../AssemblerBufferWithConstantPool.h | 18 ++++++++++++++++++ js/src/assembler/assembler/MacroAssemblerARM.h | 7 +++++++ js/src/methodjit/BaseCompiler.h | 17 ++++++++++++++++- 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/js/src/assembler/assembler/ARMAssembler.h b/js/src/assembler/assembler/ARMAssembler.h index 2ac35740b04..1d5a66190d0 100644 --- a/js/src/assembler/assembler/ARMAssembler.h +++ b/js/src/assembler/assembler/ARMAssembler.h @@ -932,6 +932,13 @@ namespace JSC { return m_buffer.sizeOfConstantPool(); } +#ifdef DEBUG + void allowPoolFlush(bool allowFlush) + { + m_buffer.allowPoolFlush(allowFlush); + } +#endif + JmpDst label() { JmpDst label(m_buffer.size()); diff --git a/js/src/assembler/assembler/AssemblerBufferWithConstantPool.h b/js/src/assembler/assembler/AssemblerBufferWithConstantPool.h index 66c927a2655..8b853a16149 100644 --- a/js/src/assembler/assembler/AssemblerBufferWithConstantPool.h +++ b/js/src/assembler/assembler/AssemblerBufferWithConstantPool.h @@ -37,6 +37,7 @@ #include "AssemblerBuffer.h" #include "assembler/wtf/SegmentedVector.h" +#include "assembler/wtf/Assertions.h" #define ASSEMBLER_HAS_CONSTANT_POOL 1 @@ -103,6 +104,9 @@ public: , m_numConsts(0) , m_maxDistance(maxPoolSize) , m_lastConstDelta(0) +#ifdef DEBUG + , m_allowFlush(true) +#endif { m_pool = static_cast(malloc(maxPoolSize)); m_mask = static_cast(malloc(maxPoolSize / sizeof(uint32_t))); @@ -235,6 +239,15 @@ public: return m_numConsts; } +#ifdef DEBUG + // Guard constant pool flushes to ensure that they don't occur during + // regions where offsets into the code have to be maintained (such as PICs). + void allowPoolFlush(bool allowFlush) + { + m_allowFlush = allowFlush; + } +#endif + private: void correctDeltas(int insnSize) { @@ -254,6 +267,7 @@ private: void flushConstantPool(bool useBarrier = true) { + ASSERT(m_allowFlush); if (m_numConsts == 0) return; int alignPool = (AssemblerBuffer::size() + (useBarrier ? barrierSize : 0)) & (sizeof(uint64_t) - 1); @@ -313,6 +327,10 @@ private: int m_numConsts; int m_maxDistance; int m_lastConstDelta; + +#ifdef DEBUG + bool m_allowFlush; +#endif }; } // namespace JSC diff --git a/js/src/assembler/assembler/MacroAssemblerARM.h b/js/src/assembler/assembler/MacroAssemblerARM.h index 0f614850d1c..67539e1b1b4 100644 --- a/js/src/assembler/assembler/MacroAssemblerARM.h +++ b/js/src/assembler/assembler/MacroAssemblerARM.h @@ -1078,6 +1078,13 @@ public: m_assembler.forceFlushConstantPool(); } +#ifdef DEBUG + void allowPoolFlush(bool allowFlush) + { + m_assembler.allowPoolFlush(allowFlush); + } +#endif + protected: ARMAssembler::Condition ARMCondition(Condition cond) { diff --git a/js/src/methodjit/BaseCompiler.h b/js/src/methodjit/BaseCompiler.h index 8b10814f0d5..6717e93ed31 100644 --- a/js/src/methodjit/BaseCompiler.h +++ b/js/src/methodjit/BaseCompiler.h @@ -207,7 +207,7 @@ class Repatcher : public JSC::RepatchBuffer #ifdef JS_CPU_ARM class AutoReserveICSpace { typedef Assembler::Label Label; - static const size_t reservedSpace = 64; + static const size_t reservedSpace = 68; Assembler &masm; #ifdef DEBUG @@ -219,6 +219,11 @@ class AutoReserveICSpace { masm.ensureSpace(reservedSpace); #ifdef DEBUG startLabel = masm.label(); + + /* Assert that the constant pool is not flushed until we reach a safe point. */ + masm.allowPoolFlush(false); + + JaegerSpew(JSpew_Insns, " -- BEGIN CONSTANT-POOL-FREE REGION -- \n"); #endif } @@ -226,8 +231,18 @@ class AutoReserveICSpace { #ifdef DEBUG Label endLabel = masm.label(); int spaceUsed = masm.differenceBetween(startLabel, endLabel); + + /* Spew the space used, to help tuning of reservedSpace. */ + JaegerSpew(JSpew_Insns, + " -- END CONSTANT-POOL-FREE REGION: %u bytes used of %u reserved. -- \n", + spaceUsed, reservedSpace); + + /* Assert that we didn't emit more code than we protected. */ JS_ASSERT(spaceUsed >= 0); JS_ASSERT(size_t(spaceUsed) <= reservedSpace); + + /* Allow the pool to be flushed. */ + masm.allowPoolFlush(true); #endif } };