Bug 1055472 - Part 8: Make the RegExp constructor properly subclassable. (r=Waldo)
authorEric Faust <efaustbmo@gmail.com>
Fri, 13 Nov 2015 18:22:21 -0800
changeset 275715 e38d42f7ba2bee86e49b7dab94f6951c1752efa0
parent 275714 e22cd35d3c39d5719a98a6602f75496f1d9672e8
child 275716 ed1209c6d7e3f2e7dcdc430c53b1d83c323c47a7
push id16582
push usercbook@mozilla.com
push dateMon, 07 Dec 2015 13:55:38 +0000
treeherderfx-team@59bc3c7a83de [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1055472
milestone45.0a1
Bug 1055472 - Part 8: Make the RegExp constructor properly subclassable. (r=Waldo)
js/src/builtin/RegExp.cpp
js/src/tests/ecma_6/Class/extendBuiltinConstructors.js
js/src/tests/ecma_6/RegExp/constructor-ordering-2.js
js/src/tests/ecma_6/RegExp/constructor-ordering.js
js/src/vm/RegExpObject.cpp
js/src/vm/RegExpObject.h
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -302,21 +302,21 @@ js::regexp_construct(JSContext* cx, unsi
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     // Steps 1-2.
     bool patternIsRegExp;
     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());
 
             // Steps 4b.i-ii.
             RootedValue patternConstructor(cx);
             if (!GetProperty(cx, patternObj, patternObj, cx->names().constructor, &patternConstructor))
                 return false;
@@ -336,48 +336,52 @@ js::regexp_construct(JSContext* cx, unsi
     if (!GetClassOfValue(cx, patternValue, &cls))
         return false;
     if (cls == ESClass_RegExp) {
         // Beware!  |patternObj| might be a proxy into another compartment, so
         // don't assume |patternObj.is<RegExpObject>()|.  For the same reason,
         // don't reuse the RegExpShared below.
         RootedObject patternObj(cx, &patternValue.toObject());
 
+        // Step 5
         RootedAtom sourceAtom(cx);
         RegExpFlag flags;
         {
             // Step 5.a.
             RegExpGuard g(cx);
             if (!RegExpToShared(cx, patternObj, &g))
                 return false;
             sourceAtom = g->getSource();
 
             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<CanGC>(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<RegExpObject*> regexp(cx, RegExpAlloc(cx));
+        RootedObject proto(cx);
+        if (!GetPrototypeFromCallableConstructor(cx, args, &proto))
+            return false;
+
+        Rooted<RegExpObject*> 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<CanGC>(cx, args[1]));
+            if (!flagStr)
+                return false;
+            if (!ParseRegExpFlags(cx, flagStr, &flags))
+                return false;
+        }
+
         if (!RegExpObject::initFromAtom(cx, regexp, sourceAtom, flags))
             return false;
 
         args.rval().setObject(*regexp);
         return true;
     }
 
     RootedValue P(cx);
@@ -399,17 +403,21 @@ js::regexp_construct(JSContext* cx, unsi
         }
     } else {
         // Steps 7a-b.
         P = patternValue;
         F = args.get(1);
     }
 
     // Steps 8-9.
-    Rooted<RegExpObject*> regexp(cx, RegExpAlloc(cx));
+    RootedObject proto(cx);
+    if (!GetPrototypeFromCallableConstructor(cx, args, &proto))
+        return false;
+
+    Rooted<RegExpObject*> regexp(cx, RegExpAlloc(cx, proto));
     if (!regexp)
         return false;
 
     // Step 10.
     if (!RegExpInitialize(cx, regexp, P, F, UseRegExpStatics))
         return false;
 
     args.rval().setObject(*regexp);
--- a/js/src/tests/ecma_6/Class/extendBuiltinConstructors.js
+++ b/js/src/tests/ecma_6/Class/extendBuiltinConstructors.js
@@ -24,16 +24,19 @@ testBuiltin(RangeError);
 testBuiltin(ReferenceError);
 testBuiltin(SyntaxError);
 testBuiltin(TypeError);
 testBuiltin(URIError);
 testBuiltin(Number);
 testBuiltin(Date);
 testBuiltin(Date, 5);
 testBuiltin(Date, 5, 10);
+testBuiltin(RegExp);
+testBuiltin(RegExp, /Regexp Argument/);
+testBuiltin(RegExp, "String Argument");
 
 `;
 
 if (classesEnabled())
     eval(test);
 
 if (typeof reportCompare === 'function')
     reportCompare(0,0,"OK");
new file mode 100644
--- /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");
new file mode 100644
--- /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");
--- a/js/src/vm/RegExpObject.cpp
+++ b/js/src/vm/RegExpObject.cpp
@@ -36,24 +36,23 @@ using js::frontend::TokenStream;
 using JS::AutoCheckCannotGC;
 
 JS_STATIC_ASSERT(IgnoreCaseFlag == JSREG_FOLD);
 JS_STATIC_ASSERT(GlobalFlag == JSREG_GLOB);
 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<RegExpObject>(cx, TenuredObject);
+    RegExpObject* regexp = NewObjectWithClassProto<RegExpObject>(cx, proto, TenuredObject);
     if (!regexp)
         return nullptr;
-
     regexp->initPrivate(nullptr);
     return regexp;
 }
 
 /* MatchPairs */
 
 bool
 MatchPairs::initArrayFrom(MatchPairs& copyFrom)
--- a/js/src/vm/RegExpObject.h
+++ b/js/src/vm/RegExpObject.h
@@ -59,17 +59,17 @@ enum RegExpFlag
 enum RegExpRunStatus
 {
     RegExpRunStatus_Error,
     RegExpRunStatus_Success,
     RegExpRunStatus_Success_NotFound
 };
 
 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*
 CloneRegExpObject(JSContext* cx, JSObject* regexp);
 
 extern JSObject*
 CreateRegExpPrototype(JSContext* cx, JSProtoKey key);