From 47d95b18a246d23db83b6dab68e431ec8ad546e0 Mon Sep 17 00:00:00 2001 From: Eric Faust Date: Fri, 13 Nov 2015 18:22:21 -0800 Subject: [PATCH] Bug 1055472 - Part 8: Make the RegExp constructor properly subclassable. (r=Waldo) --- js/src/builtin/RegExp.cpp | 42 +++++++++++-------- .../ecma_6/Class/extendBuiltinConstructors.js | 3 ++ .../ecma_6/RegExp/constructor-ordering-2.js | 21 ++++++++++ .../ecma_6/RegExp/constructor-ordering.js | 16 +++++++ js/src/vm/RegExpObject.cpp | 5 +-- js/src/vm/RegExpObject.h | 2 +- 6 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 js/src/tests/ecma_6/RegExp/constructor-ordering-2.js create mode 100644 js/src/tests/ecma_6/RegExp/constructor-ordering.js diff --git a/js/src/builtin/RegExp.cpp b/js/src/builtin/RegExp.cpp index 5399998a1764..8d6c4a1b7c31 100644 --- a/js/src/builtin/RegExp.cpp +++ b/js/src/builtin/RegExp.cpp @@ -307,11 +307,11 @@ js::regexp_construct(JSContext* cx, unsigned argc, Value* vp) if (!IsRegExp(cx, args.get(0), &patternIsRegExp)) return false; - if (args.isConstructing()) { - // XXX Step 3! - } else { - // XXX Step 4a + // We can delay step 3 and step 4a until later, during + // GetPrototypeFromCallableConstructor calls. Accessing the new.target + // and the callee from the stack is unobservable. + if (!args.isConstructing()) { // Step 4b. if (patternIsRegExp && !args.hasDefined(1)) { RootedObject patternObj(cx, &args[0].toObject()); @@ -341,6 +341,7 @@ js::regexp_construct(JSContext* cx, unsigned argc, Value* vp) // don't reuse the RegExpShared below. RootedObject patternObj(cx, &patternValue.toObject()); + // Step 5 RootedAtom sourceAtom(cx); RegExpFlag flags; { @@ -353,26 +354,29 @@ js::regexp_construct(JSContext* cx, unsigned argc, Value* vp) if (!args.hasDefined(1)) { // Step 5b. flags = g->getFlags(); - } else { - // Step 5c. - // XXX We shouldn't be converting to string yet! This must - // come *after* the .constructor access in step 8. - flags = RegExpFlag(0); - RootedString flagStr(cx, ToString(cx, args[1])); - if (!flagStr) - return false; - if (!ParseRegExpFlags(cx, flagStr, &flags)) - return false; } } // Steps 8-9. - // XXX Note bug in step 5c, with respect to step 8. - Rooted regexp(cx, RegExpAlloc(cx)); + RootedObject proto(cx); + if (!GetPrototypeFromCallableConstructor(cx, args, &proto)) + return false; + + Rooted regexp(cx, RegExpAlloc(cx, proto)); if (!regexp) return false; // Step 10. + if (args.hasDefined(1)) { + // Step 5c / 21.2.3.2.2 RegExpInitialize step 5. + flags = RegExpFlag(0); + RootedString flagStr(cx, ToString(cx, args[1])); + if (!flagStr) + return false; + if (!ParseRegExpFlags(cx, flagStr, &flags)) + return false; + } + if (!RegExpObject::initFromAtom(cx, regexp, sourceAtom, flags)) return false; @@ -404,7 +408,11 @@ js::regexp_construct(JSContext* cx, unsigned argc, Value* vp) } // Steps 8-9. - Rooted regexp(cx, RegExpAlloc(cx)); + RootedObject proto(cx); + if (!GetPrototypeFromCallableConstructor(cx, args, &proto)) + return false; + + Rooted regexp(cx, RegExpAlloc(cx, proto)); if (!regexp) return false; diff --git a/js/src/tests/ecma_6/Class/extendBuiltinConstructors.js b/js/src/tests/ecma_6/Class/extendBuiltinConstructors.js index d6d3843094bf..6c9db2eb0e50 100644 --- a/js/src/tests/ecma_6/Class/extendBuiltinConstructors.js +++ b/js/src/tests/ecma_6/Class/extendBuiltinConstructors.js @@ -29,6 +29,9 @@ testBuiltin(Number); testBuiltin(Date); testBuiltin(Date, 5); testBuiltin(Date, 5, 10); +testBuiltin(RegExp); +testBuiltin(RegExp, /Regexp Argument/); +testBuiltin(RegExp, "String Argument"); `; diff --git a/js/src/tests/ecma_6/RegExp/constructor-ordering-2.js b/js/src/tests/ecma_6/RegExp/constructor-ordering-2.js new file mode 100644 index 000000000000..21a6bbeca745 --- /dev/null +++ b/js/src/tests/ecma_6/RegExp/constructor-ordering-2.js @@ -0,0 +1,21 @@ +// Make sure that we don't ToString the second argument until /after/ doing +// the appropriate subclassing lookups + +var didLookup = false; + +var re = /a/; +var flags = { toString() { assertEq(didLookup, true); return "g"; } }; +var newRe = Reflect.construct(RegExp, [re, flags], + Object.defineProperty(function(){}.bind(null), "prototype", { + get() { + didLookup = true; + return RegExp.prototype; + } +})); + +assertEq(Object.getPrototypeOf(newRe), RegExp.prototype); +assertEq(didLookup, true); + + +if (typeof reportCompare === 'function') + reportCompare(0,0,"OK"); diff --git a/js/src/tests/ecma_6/RegExp/constructor-ordering.js b/js/src/tests/ecma_6/RegExp/constructor-ordering.js new file mode 100644 index 000000000000..3e3a9b695b31 --- /dev/null +++ b/js/src/tests/ecma_6/RegExp/constructor-ordering.js @@ -0,0 +1,16 @@ +// Make sure that we don't misorder subclassing accesses with respect to +// accessing regex arg internal slots +// +// Test credit André Bargull. + +var re = /a/; +var newRe = Reflect.construct(RegExp, [re], Object.defineProperty(function(){}.bind(null), "prototype", { + get() { + re.compile("b"); + return RegExp.prototype; + } +})); +assertEq(newRe.source, "a"); + +if (typeof reportCompare === 'function') + reportCompare(0,0,"OK"); diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp index 8c219b7a6a3c..89b3bf6ac345 100644 --- a/js/src/vm/RegExpObject.cpp +++ b/js/src/vm/RegExpObject.cpp @@ -41,14 +41,13 @@ JS_STATIC_ASSERT(MultilineFlag == JSREG_MULTILINE); JS_STATIC_ASSERT(StickyFlag == JSREG_STICKY); RegExpObject* -js::RegExpAlloc(ExclusiveContext* cx) +js::RegExpAlloc(ExclusiveContext* cx, HandleObject proto /* = nullptr */) { // Note: RegExp objects are always allocated in the tenured heap. This is // not strictly required, but simplifies embedding them in jitcode. - RegExpObject* regexp = NewBuiltinClassInstance(cx, TenuredObject); + RegExpObject* regexp = NewObjectWithClassProto(cx, proto, TenuredObject); if (!regexp) return nullptr; - regexp->initPrivate(nullptr); return regexp; } diff --git a/js/src/vm/RegExpObject.h b/js/src/vm/RegExpObject.h index 44012001f9d1..ad118914d9d0 100644 --- a/js/src/vm/RegExpObject.h +++ b/js/src/vm/RegExpObject.h @@ -64,7 +64,7 @@ enum RegExpRunStatus }; extern RegExpObject* -RegExpAlloc(ExclusiveContext* cx); +RegExpAlloc(ExclusiveContext* cx, HandleObject proto = nullptr); // |regexp| is under-typed because this function's used in the JIT. extern JSObject*