Bug 1147817 - Part 2: Use IsRegExp in RegExp constructor. r=till
authorTooru Fujisawa <arai_a@mac.com>
Fri, 10 Apr 2015 23:49:16 +0900
changeset 238532 ad1cd598bb350306283e0ae63e4da6a7770b69a8
parent 238531 224db47e1e20984aae2a30227d78feddb9bbf40f
child 238533 39f5e5456b5f0830bcbe618d8d9896cc116b9ad9
push id58254
push userarai_a@mac.com
push dateFri, 10 Apr 2015 14:51:46 +0000
treeherdermozilla-inbound@ad1cd598bb35 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill
bugs1147817
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1147817 - Part 2: Use IsRegExp in RegExp constructor. r=till
js/src/builtin/RegExp.cpp
js/src/jit-test/tests/basic/testCrossCompartmentTransparency.js
js/src/tests/ecma_6/RegExp/constructor-IsRegExp.js
js/src/tests/ecma_6/RegExp/constructor-constructor.js
js/src/vm/CommonPropertyNames.h
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -187,91 +187,142 @@ RegExpInitialize(JSContext* cx, RegExpOb
         return false;
 
     /* Step 16. */
     result.set(reobj);
     return true;
 }
 
 /*
+ * ES6 draft rc4 21.2.3.1 steps 5-10.
+ * ES6 draft rc4 B.2.5.1 steps 3-5.
  * Compile a new |RegExpShared| for the |RegExpObject|.
  */
 static bool
 CompileRegExpObject(JSContext* cx, RegExpObjectBuilder& builder, CallArgs args,
-                    RegExpCreationMode creationMode)
+                    RegExpCreationMode creationMode, bool patternIsRegExp=false)
 {
     if (args.length() == 0) {
+        /*
+         * 21.2.3.1 step 10.
+         * B.2.5.1 step 5.
+         */
         RegExpStatics* res = cx->global()->getRegExpStatics(cx);
         if (!res)
             return false;
-        Rooted<JSAtom*> empty(cx, cx->runtime()->emptyString);
+
+        RootedAtom empty(cx, cx->runtime()->emptyString);
         RegExpObject* reobj = builder.build(empty, res->getFlags());
         if (!reobj)
             return false;
+
         args.rval().setObject(*reobj);
         return true;
     }
 
-    RootedValue sourceValue(cx, args[0]);
+    RootedValue patternValue(cx, args.get(0));
 
-    if (IsObjectWithClass(sourceValue, ESClass_RegExp, cx)) {
+    /*
+     * 21.2.3.1 step 5
+     * B.2.5.1 step 3.
+     */
+    if (IsObjectWithClass(patternValue, ESClass_RegExp, cx)) {
         /*
-         * For RegExp.prototype.compile, if the first argument is a RegExp object,
-         * the second argument must be undefined. Otherwise, throw a TypeError.
+         * B.2.5.1 step 3.a.
          */
         if (args.hasDefined(1) && creationMode == CreateForCompile) {
             JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NEWREGEXP_FLAGGED);
             return false;
         }
 
         /*
-         * Beware, sourceObj may be a (transparent) proxy to a RegExp, so only
-         * use generic (proxyable) operations on sourceObj that do not assume
-         * sourceObj.is<RegExpObject>().
+         * Beware, patternObj may be a (transparent) proxy to a RegExp, so only
+         * use generic (proxyable) operations on patternObj that do not assume
+         * patternObj.is<RegExpObject>().
          */
-        RootedObject sourceObj(cx, &sourceValue.toObject());
+        RootedObject patternObj(cx, &patternValue.toObject());
 
         RootedAtom sourceAtom(cx);
         RegExpFlag flags;
         {
             /*
-             * Extract the 'source' from sourceObj; do not reuse the RegExpShared
-             * since it may be from a different compartment.
+             * 21.2.3.1 step 5.a.
+             * B.2.5.1 step 3.a.
+             * Extract the 'source' from patternObj; do not reuse the
+             * RegExpShared since it may be from a different compartment.
              */
             RegExpGuard g(cx);
-            if (!RegExpToShared(cx, sourceObj, &g))
+            if (!RegExpToShared(cx, patternObj, &g))
                 return false;
             sourceAtom = g->getSource();
 
-            /*
-             * If args[1] is not undefined, then parse the 'flags' from args[1].
-             * Otherwise, extract the 'flags' from sourceObj.
-             */
             if (args.hasDefined(1)) {
+                /* 21.2.3.1 step 5.c. */
                 flags = RegExpFlag(0);
                 RootedString flagStr(cx, ToString<CanGC>(cx, args[1]));
                 if (!flagStr)
                     return false;
                 if (!ParseRegExpFlags(cx, flagStr, &flags))
                     return false;
             } else {
+                /*
+                 * 21.2.3.1 step 5.b.
+                 * B.2.5.1 step 3.c.
+                 */
                 flags = g->getFlags();
             }
         }
 
+        /*
+         * 21.2.3.1 steps 8-10.
+         * B.2.5.1 step 5.
+         */
         RegExpObject* reobj = builder.build(sourceAtom, flags);
         if (!reobj)
             return false;
 
         args.rval().setObject(*reobj);
         return true;
     }
 
+    RootedValue P(cx);
+    RootedValue F(cx);
+    /* 21.2.3.1 step 6. */
+    if (patternIsRegExp) {
+        MOZ_ASSERT(creationMode == CreateForConstruct);
+        RootedObject patternObj(cx, &patternValue.toObject());
+
+        /* 21.2.3.1 steps 6.a-b. */
+        if (!GetProperty(cx, patternObj, patternObj, cx->names().source, &P))
+            return false;
+
+        /* 21.2.3.1 step 6.c. */
+        if (!args.hasDefined(1)) {
+            /* 21.2.3.1 steps 6.c.i-ii. */
+            if (!GetProperty(cx, patternObj, patternObj, cx->names().flags, &F))
+                return false;
+        } else {
+            /* 21.2.3.1 steps 6.d. */
+            F = args[1];
+        }
+    } else {
+        /*
+         * 21.2.3.1 steps 7.a-b.
+         * B.2.5.1 steps 4.a-b.
+         */
+        P = patternValue;
+        F = args.get(1);
+    }
+
+    /*
+     * 21.2.3.1 steps 8-10.
+     * B.2.5.1 step 5.
+     */
     RootedObject reobj(cx);
-    if (!RegExpInitialize(cx, builder, sourceValue, args.get(1), UseRegExpStatics, &reobj))
+    if (!RegExpInitialize(cx, builder, P, F, UseRegExpStatics, &reobj))
         return false;
 
     args.rval().setObject(*reobj);
     return true;
 }
 
 MOZ_ALWAYS_INLINE bool
 IsRegExpObject(HandleValue v)
@@ -302,65 +353,84 @@ js::IsRegExp(JSContext* cx, HandleValue 
         return true;
     }
 
     /* Steps 5-6. */
     *result = IsObjectWithClass(value, ESClass_RegExp, cx);
     return true;
 }
 
+/* ES6 draft rc4 B.2.5.1. */
 MOZ_ALWAYS_INLINE bool
 regexp_compile_impl(JSContext* cx, CallArgs args)
 {
     MOZ_ASSERT(IsRegExpObject(args.thisv()));
+
+    /* Steps 3-5. */
     RegExpObjectBuilder builder(cx, &args.thisv().toObject().as<RegExpObject>());
     return CompileRegExpObject(cx, builder, args, CreateForCompile);
 }
 
 static bool
 regexp_compile(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
+
+    /* Steps 1-2. */
     return CallNonGenericMethod<IsRegExpObject, regexp_compile_impl>(cx, args);
 }
 
+/* ES6 draft rc4 21.2.3.1. */
 bool
 js::regexp_construct(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
+    /* Steps 1-2. */
+    bool patternIsRegExp;
+    if (!IsRegExp(cx, args.get(0), &patternIsRegExp))
+        return false;
+
+    /* Step 4. */
     if (!args.isConstructing()) {
-        /*
-         * If first arg is regexp and no flags are given, just return the arg.
-         * Otherwise, delegate to the standard constructor.
-         * See ECMAv5 15.10.3.1.
-         */
-        if (args.hasDefined(0) &&
-            IsObjectWithClass(args[0], ESClass_RegExp, cx) &&
-            !args.hasDefined(1))
-        {
-            args.rval().set(args[0]);
-            return true;
+        /* Step 4.b. */
+        if (patternIsRegExp && !args.hasDefined(1)) {
+            RootedObject patternObj(cx, &args[0].toObject());
+
+            /* Steps 4.b.i-ii. */
+            RootedValue patternConstructor(cx);
+            if (!GetProperty(cx, patternObj, patternObj, cx->names().constructor, &patternConstructor))
+                return false;
+
+            /* Step 4.b.iii. */
+            if (patternConstructor.isObject() && patternConstructor.toObject() == args.callee()) {
+                args.rval().set(args[0]);
+                return true;
+            }
         }
     }
 
+    /* Steps 5-10. */
     RegExpObjectBuilder builder(cx);
-    return CompileRegExpObject(cx, builder, args, CreateForConstruct);
+    return CompileRegExpObject(cx, builder, args, CreateForConstruct, patternIsRegExp);
 }
 
 bool
 js::regexp_construct_no_statics(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     MOZ_ASSERT(args.length() == 1 || args.length() == 2);
     MOZ_ASSERT(args[0].isString());
     MOZ_ASSERT_IF(args.length() == 2, args[1].isString());
     MOZ_ASSERT(!args.isConstructing());
 
+    /* Steps 1-6 are not required since pattern is always string. */
+
+    /* Steps 7-10. */
     RegExpObjectBuilder builder(cx);
     RootedObject reobj(cx);
     if (!RegExpInitialize(cx, builder, args[0], args.get(1), DontUseRegExpStatics, &reobj))
         return false;
 
     args.rval().setObject(*reobj);
     return true;
 }
--- a/js/src/jit-test/tests/basic/testCrossCompartmentTransparency.js
+++ b/js/src/jit-test/tests/basic/testCrossCompartmentTransparency.js
@@ -71,17 +71,16 @@ test("new String('one')", s => String.pr
 test("new String('one')", s => String.prototype.toString.call(s));
 test("new RegExp('1')", r => RegExp.prototype.exec.call(r, '1').toString());
 test("new RegExp('1')", r => RegExp.prototype.test.call(r, '1'));
 test("new RegExp('1')", r => RegExp.prototype.compile.call(r, '1').toString());
 test("new RegExp('1')", r => assertEq("a1".search(r), 1));
 test("new RegExp('1')", r => assertEq("a1".match(r)[0], '1'));
 test("new RegExp('1')", r => assertEq("a1".replace(r, 'A'), 'aA'));
 test("new RegExp('1')", r => assertEq(String("a1b".split(r)), "a,b"));
-test("new RegExp('1')", r => assertEq(r, RegExp(r)));
 test("new RegExp('1')", r => assertEq(String(new RegExp(r)), String(r)));
 test("new RegExp('1')", r => assertEq(String(/x/.compile(r)), String(r)));
 test("new WeakMap()", w => WeakMap.prototype.has.call(w, {}));
 test("new WeakMap()", w => WeakMap.prototype.get.call(w, {}));
 test("new WeakMap()", w => WeakMap.prototype.delete.call(w, {}));
 test("new WeakMap()", w => WeakMap.prototype.set.call(w, {}).toString());
 test("new Map()", w => Map.prototype.has.call(w, {}));
 test("new Map()", w => Map.prototype.get.call(w, {}));
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/RegExp/constructor-IsRegExp.js
@@ -0,0 +1,86 @@
+var BUGNUMBER = 1147817;
+var summary = "RegExp constructor with pattern with @@match.";
+
+print(BUGNUMBER + ": " + summary);
+
+var matchValue;
+var constructorValue;
+
+var matchGet;
+var constructorGet;
+var sourceGet;
+var flagsGet;
+function reset() {
+  matchGet = false;
+  constructorGet = false;
+  sourceGet = false;
+  flagsGet = false;
+}
+var obj = {
+  get [Symbol.match]() {
+    matchGet = true;
+    return matchValue;
+  },
+  get constructor() {
+    constructorGet = true;
+    return constructorValue;
+  },
+  get source() {
+    sourceGet = true;
+    return "foo";
+  },
+  get flags() {
+    flagsGet = true;
+    return "i";
+  },
+  toString() {
+    return "bar";
+  }
+};
+
+matchValue = true;
+constructorValue = function() {};
+
+reset();
+assertEq(RegExp(obj).toString(), "/foo/i");
+assertEq(matchGet, true);
+assertEq(constructorGet, true);
+assertEq(sourceGet, true);
+assertEq(flagsGet, true);
+
+reset();
+assertEq(RegExp(obj, "g").toString(), "/foo/g");
+assertEq(matchGet, true);
+assertEq(constructorGet, false);
+assertEq(sourceGet, true);
+assertEq(flagsGet, false);
+
+matchValue = false;
+constructorValue = function() {};
+
+reset();
+assertEq(RegExp(obj).toString(), "/bar/");
+assertEq(matchGet, true);
+assertEq(constructorGet, false);
+assertEq(sourceGet, false);
+assertEq(flagsGet, false);
+
+reset();
+assertEq(RegExp(obj, "g").toString(), "/bar/g");
+assertEq(matchGet, true);
+assertEq(constructorGet, false);
+assertEq(sourceGet, false);
+assertEq(flagsGet, false);
+
+matchValue = true;
+constructorValue = RegExp;
+
+reset();
+assertEq(RegExp(obj), obj);
+assertEq(matchGet, true);
+assertEq(constructorGet, true);
+assertEq(sourceGet, false);
+assertEq(flagsGet, false);
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/RegExp/constructor-constructor.js
@@ -0,0 +1,60 @@
+var BUGNUMBER = 1147817;
+var summary = "RegExp constructor should check pattern.constructor.";
+
+print(BUGNUMBER + ": " + summary);
+
+var g = newGlobal();
+
+var re = /foo/;
+assertEq(RegExp(re), re);
+re.constructor = 10;
+assertEq(RegExp(re) === re, false);
+assertEq(RegExp(re).toString(), re.toString());
+
+// If pattern comes from different global, RegExp shouldn't return it.
+re = g.eval(`var re = /foo/; re;`);
+assertEq(RegExp(re) === re, false);
+assertEq(RegExp(re).toString(), re.toString());
+g.eval(`re.constructor = 10;`);
+assertEq(RegExp(re) === re, false);
+assertEq(RegExp(re).toString(), re.toString());
+
+
+re = new Proxy(/a/, {
+  get(that, name) {
+    return that[name];
+  }
+});
+assertEq(RegExp(re) === re, false);
+
+re = new Proxy(g.eval(`/a/`), {
+  get(that, name) {
+    return that[name];
+  }
+});
+assertEq(RegExp(re) === re, false);
+
+re = g.eval(`new Proxy(/a/, {
+  get(that, name) {
+    return that[name];
+  }
+});`);
+assertEq(RegExp(re) === re, false);
+
+
+var obj = { [Symbol.match]: true, source: "foo", flags: "gi" };
+assertEq(RegExp(obj) === obj, false);
+assertEq(RegExp(obj).toString(), "/foo/gi");
+obj.constructor = RegExp;
+assertEq(RegExp(obj), obj);
+
+obj = g.eval(`var obj = { [Symbol.match]: true, source: "foo", flags: "gi" }; obj;`);
+assertEq(RegExp(obj) === obj, false);
+assertEq(RegExp(obj).toString(), "/foo/gi");
+g.eval(`obj.constructor = RegExp`);
+assertEq(RegExp(obj) === obj, false);
+obj.constructor = RegExp;
+assertEq(RegExp(obj), obj);
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);
--- a/js/src/vm/CommonPropertyNames.h
+++ b/js/src/vm/CommonPropertyNames.h
@@ -74,16 +74,17 @@
     macro(enumerate, enumerate, "enumerate") \
     macro(escape, escape, "escape") \
     macro(eval, eval, "eval") \
     macro(false, false_, "false") \
     macro(fieldOffsets, fieldOffsets, "fieldOffsets") \
     macro(fieldTypes, fieldTypes, "fieldTypes") \
     macro(fileName, fileName, "fileName") \
     macro(fix, fix, "fix") \
+    macro(flags, flags, "flags") \
     macro(float32, float32, "float32") \
     macro(float32x4, float32x4, "float32x4") \
     macro(float64, float64, "float64") \
     macro(float64x2, float64x2, "float64x2") \
     macro(forceInterpreter, forceInterpreter, "forceInterpreter") \
     macro(format, format, "format") \
     macro(frame, frame, "frame") \
     macro(from, from, "from") \