Bug 1675240 - Stop using CallNonGenericMethod for RegExp.prototype getters. r=arai

This also implements an improved error message:
> TypeError: RegExp.prototype.global getter called on non-RegExp object: Math
This is similar to Chromes' message:
> RegExp.prototype.global getter called on non-RegExp object

Differential Revision: https://phabricator.services.mozilla.com/D95837

js/public/friend/ErrorNumbers.msg

@@ -606,6 +606,7 @@ MSG_DEF(JSMSG_INVALID_CAPTURE_NAME,        0, JSEXN_SYNTAXERR, "invalid capture
 MSG_DEF(JSMSG_DUPLICATE_CAPTURE_NAME,      0, JSEXN_SYNTAXERR, "duplicate capture group name in regular expression")
 MSG_DEF(JSMSG_INVALID_NAMED_REF,           0, JSEXN_SYNTAXERR, "invalid named reference in regular expression")
 MSG_DEF(JSMSG_INVALID_NAMED_CAPTURE_REF,   0, JSEXN_SYNTAXERR, "invalid named capture reference in regular expression")
+MSG_DEF(JSMSG_INCOMPATIBLE_REGEXP_GETTER, 2, JSEXN_TYPEERR, "RegExp.prototype.{0} getter called on non-RegExp object: {1}")
 
 // Self-hosting
 MSG_DEF(JSMSG_DEFAULT_LOCALE_ERROR,    0, JSEXN_ERR, "internal error getting the default locale")

js/src/builtin/RegExp.cpp

@@ -663,184 +663,133 @@ bool js::regexp_construct_raw_flags(JSContext* cx, unsigned argc, Value* vp) {
   return true;
 }
 
-MOZ_ALWAYS_INLINE bool IsRegExpPrototype(HandleValue v, JSContext* cx) {
+// This is a specialized implementation of "UnwrapAndTypeCheckThis" for RegExp
-  return (v.isObject() &&
+// getters that need to return a special value for same-realm
-          cx->global()->maybeGetRegExpPrototype() == &v.toObject());
+// %RegExp.prototype%.
-}
+template <typename Fn>
-
+static bool RegExpGetter(JSContext* cx, CallArgs& args, const char* methodName,
-// ES 2017 draft 21.2.5.4.
+                         Fn&& fn,
-MOZ_ALWAYS_INLINE bool regexp_global_impl(JSContext* cx, const CallArgs& args) {
+                         HandleValue fallbackValue = UndefinedHandleValue) {
-  MOZ_ASSERT(IsRegExpObject(args.thisv()));
+  JSObject* obj = nullptr;
-
+  if (args.thisv().isObject()) {
-  // Steps 4-6.
+    obj = &args.thisv().toObject();
-  RegExpObject* reObj = &args.thisv().toObject().as<RegExpObject>();
+    if (IsWrapper(obj)) {
-  args.rval().setBoolean(reObj->global());
+      obj = CheckedUnwrapStatic(obj);
-  return true;
+      if (!obj) {
+        ReportAccessDenied(cx);
+        return false;
+      }
+    }
+  }
+
+  if (obj) {
+    // Step 4ff
+    if (obj->is<RegExpObject>()) {
+      return fn(&obj->as<RegExpObject>());
+    }
+
+    // Step 3.a. "If SameValue(R, %RegExp.prototype%) is true, return
+    // undefined."
+    // Or `return "(?:)"` for get RegExp.prototype.source.
+    if (obj == cx->global()->maybeGetRegExpPrototype()) {
+      args.rval().set(fallbackValue);
+      return true;
+    }
+
+    // fall-through
+  }
+
+  // Step 2. and Step 3.b.
+  JS_ReportErrorNumberLatin1(cx, GetErrorMessage, nullptr,
+                             JSMSG_INCOMPATIBLE_REGEXP_GETTER, methodName,
+                             InformalValueTypeName(args.thisv()));
+  return false;
 }
 
+// ES2021 draft rev 0b3a808af87a9123890767152a26599cc8fde161
+// 21.2.5.5 get RegExp.prototype.global
 bool js::regexp_global(JSContext* cx, unsigned argc, JS::Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
-
+  return RegExpGetter(cx, args, "global", [args](RegExpObject* unwrapped) {
-  // Step 3.a.
+    args.rval().setBoolean(unwrapped->global());
-  if (IsRegExpPrototype(args.thisv(), cx)) {
-    args.rval().setUndefined();
     return true;
-  }
+  });
-
-  // Steps 1-3.
-  return CallNonGenericMethod<IsRegExpObject, regexp_global_impl>(cx, args);
-}
-
-// ES 2017 draft 21.2.5.5.
-MOZ_ALWAYS_INLINE bool regexp_ignoreCase_impl(JSContext* cx,
-                                              const CallArgs& args) {
-  MOZ_ASSERT(IsRegExpObject(args.thisv()));
-
-  // Steps 4-6.
-  RegExpObject* reObj = &args.thisv().toObject().as<RegExpObject>();
-  args.rval().setBoolean(reObj->ignoreCase());
-  return true;
 }
 
+// ES2021 draft rev 0b3a808af87a9123890767152a26599cc8fde161
+// 21.2.5.6 get RegExp.prototype.ignoreCase
 bool js::regexp_ignoreCase(JSContext* cx, unsigned argc, JS::Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
-
+  return RegExpGetter(cx, args, "ignoreCase", [args](RegExpObject* unwrapped) {
-  // Step 3.a.
+    args.rval().setBoolean(unwrapped->ignoreCase());
-  if (IsRegExpPrototype(args.thisv(), cx)) {
-    args.rval().setUndefined();
     return true;
-  }
+  });
-
-  // Steps 1-3.
-  return CallNonGenericMethod<IsRegExpObject, regexp_ignoreCase_impl>(cx, args);
-}
-
-// ES 2017 draft 21.2.5.7.
-MOZ_ALWAYS_INLINE bool regexp_multiline_impl(JSContext* cx,
-                                             const CallArgs& args) {
-  MOZ_ASSERT(IsRegExpObject(args.thisv()));
-
-  // Steps 4-6.
-  RegExpObject* reObj = &args.thisv().toObject().as<RegExpObject>();
-  args.rval().setBoolean(reObj->multiline());
-  return true;
 }
 
+// ES2021 draft rev 0b3a808af87a9123890767152a26599cc8fde161
+// 21.2.5.9 get RegExp.prototype.multiline
 bool js::regexp_multiline(JSContext* cx, unsigned argc, JS::Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
-
+  return RegExpGetter(cx, args, "multiline", [args](RegExpObject* unwrapped) {
-  // Step 3.a.
+    args.rval().setBoolean(unwrapped->multiline());
-  if (IsRegExpPrototype(args.thisv(), cx)) {
-    args.rval().setUndefined();
     return true;
-  }
+  });
-
-  // Steps 1-3.
-  return CallNonGenericMethod<IsRegExpObject, regexp_multiline_impl>(cx, args);
-}
-
-// ES 2017 draft 21.2.5.10.
-MOZ_ALWAYS_INLINE bool regexp_source_impl(JSContext* cx, const CallArgs& args) {
-  MOZ_ASSERT(IsRegExpObject(args.thisv()));
-
-  // Step 5.
-  RegExpObject* reObj = &args.thisv().toObject().as<RegExpObject>();
-  RootedAtom src(cx, reObj->getSource());
-  if (!src) {
-    return false;
-  }
-
-  // Step 7.
-  JSString* str = EscapeRegExpPattern(cx, src);
-  if (!str) {
-    return false;
-  }
-
-  args.rval().setString(str);
-  return true;
 }
 
+// ES2021 draft rev 0b3a808af87a9123890767152a26599cc8fde161
+// 21.2.5.12 get RegExp.prototype.source
 static bool regexp_source(JSContext* cx, unsigned argc, JS::Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
+  // Step 3.a. Return "(?:)" for %RegExp.prototype%.
+  RootedValue fallback(cx, StringValue(cx->names().emptyRegExp));
+  return RegExpGetter(
+      cx, args, "source",
+      [cx, args](RegExpObject* unwrapped) {
+        RootedAtom src(cx, unwrapped->getSource());
+        MOZ_ASSERT(src);
+        // Mark potentially cross-compartment JSAtom.
+        cx->markAtom(src);
 
-  // Step 3.a.
+        // Step 7.
-  if (IsRegExpPrototype(args.thisv(), cx)) {
+        JSAtom* escaped = EscapeRegExpPattern(cx, src);
-    args.rval().setString(cx->names().emptyRegExp);
+        if (!escaped) {
-    return true;
+          return false;
-  }
+        }
 
-  // Steps 1-4.
+        args.rval().setString(escaped);
-  return CallNonGenericMethod<IsRegExpObject, regexp_source_impl>(cx, args);
+        return true;
-}
+      },
-
+      fallback);
-// ES 2020 draft 21.2.5.3.
-MOZ_ALWAYS_INLINE bool regexp_dotAll_impl(JSContext* cx, const CallArgs& args) {
-  MOZ_ASSERT(IsRegExpObject(args.thisv()));
-
-  // Steps 4-6.
-  RegExpObject* reObj = &args.thisv().toObject().as<RegExpObject>();
-  args.rval().setBoolean(reObj->dotAll());
-  return true;
 }
 
+// ES2021 draft rev 0b3a808af87a9123890767152a26599cc8fde161
+// 21.2.5.3 get RegExp.prototype.dotAll
 bool js::regexp_dotAll(JSContext* cx, unsigned argc, JS::Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
-
+  return RegExpGetter(cx, args, "dotAll", [args](RegExpObject* unwrapped) {
-  // Step 3.a.
+    args.rval().setBoolean(unwrapped->dotAll());
-  if (IsRegExpPrototype(args.thisv(), cx)) {
-    args.rval().setUndefined();
     return true;
-  }
+  });
-
-  // Steps 1-3.
-  return CallNonGenericMethod<IsRegExpObject, regexp_dotAll_impl>(cx, args);
-}
-
-// ES 2017 draft 21.2.5.12.
-MOZ_ALWAYS_INLINE bool regexp_sticky_impl(JSContext* cx, const CallArgs& args) {
-  MOZ_ASSERT(IsRegExpObject(args.thisv()));
-
-  // Steps 4-6.
-  RegExpObject* reObj = &args.thisv().toObject().as<RegExpObject>();
-  args.rval().setBoolean(reObj->sticky());
-  return true;
 }
 
+// ES2021 draft rev 0b3a808af87a9123890767152a26599cc8fde161
+// 21.2.5.14 get RegExp.prototype.sticky
 bool js::regexp_sticky(JSContext* cx, unsigned argc, JS::Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
-
+  return RegExpGetter(cx, args, "sticky", [args](RegExpObject* unwrapped) {
-  // Step 3.a.
+    args.rval().setBoolean(unwrapped->sticky());
-  if (IsRegExpPrototype(args.thisv(), cx)) {
-    args.rval().setUndefined();
     return true;
-  }
+  });
-
-  // Steps 1-3.
-  return CallNonGenericMethod<IsRegExpObject, regexp_sticky_impl>(cx, args);
-}
-
-// ES 2017 draft 21.2.5.15.
-MOZ_ALWAYS_INLINE bool regexp_unicode_impl(JSContext* cx,
-                                           const CallArgs& args) {
-  MOZ_ASSERT(IsRegExpObject(args.thisv()));
-
-  // Steps 4-6.
-  RegExpObject* reObj = &args.thisv().toObject().as<RegExpObject>();
-  args.rval().setBoolean(reObj->unicode());
-  return true;
 }
 
+// ES2021 draft rev 0b3a808af87a9123890767152a26599cc8fde161
+// 21.2.5.17 get RegExp.prototype.unicode
 bool js::regexp_unicode(JSContext* cx, unsigned argc, JS::Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
-
+  return RegExpGetter(cx, args, "unicode", [args](RegExpObject* unwrapped) {
-  // Step 3.a.
+    args.rval().setBoolean(unwrapped->unicode());
-  if (IsRegExpPrototype(args.thisv(), cx)) {
-    args.rval().setUndefined();
     return true;
-  }
+  });
-
-  // Steps 1-3.
-  return CallNonGenericMethod<IsRegExpObject, regexp_unicode_impl>(cx, args);
 }
 
 const JSPropertySpec js::regexp_properties[] = {

js/src/tests/jstests.list

@@ -401,16 +401,6 @@ skip script test262/annexB/built-ins/Function/createdynfn-no-line-terminator-htm
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1462745
 skip script test262/annexB/language/function-code/block-decl-nested-blocks-with-fun-decl.js
 
-# https://bugzilla.mozilla.org/show_bug.cgi?id=1508683
-# All of these tests pass except with --more-compartments.
-ignore-flag(--more-compartments) script test262/built-ins/RegExp/prototype/multiline/cross-realm.js
-ignore-flag(--more-compartments) script test262/built-ins/RegExp/prototype/global/cross-realm.js
-ignore-flag(--more-compartments) script test262/built-ins/RegExp/prototype/sticky/cross-realm.js
-ignore-flag(--more-compartments) script test262/built-ins/RegExp/prototype/ignoreCase/cross-realm.js
-ignore-flag(--more-compartments) script test262/built-ins/RegExp/prototype/unicode/cross-realm.js
-ignore-flag(--more-compartments) script test262/built-ins/RegExp/prototype/dotAll/cross-realm.js
-ignore-flag(--more-compartments) script test262/built-ins/RegExp/prototype/source/cross-realm.js
-
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1545038
 # All of these tests pass except with --more-compartments.
 ignore-flag(--more-compartments) script test262/built-ins/String/prototype/valueOf/non-generic-realm.js

js/src/tests/non262/RegExp/cross-compartment-getter.js

@@ -0,0 +1,43 @@
+const otherGlobal = newGlobal({newCompartment: true});
+
+let regExp = otherGlobal.eval("/a(b|c)/iy");
+
+function get(name) {
+    const descriptor = Object.getOwnPropertyDescriptor(RegExp.prototype, name);
+    return descriptor.get.call(regExp);
+}
+
+assertEq(get("flags"), "iy");
+assertEq(get("global"), false);
+assertEq(get("ignoreCase"), true);
+assertEq(get("multiline"), false);
+assertEq(get("dotAll"), false);
+assertEq(get("source"), "a(b|c)");
+assertEq(get("sticky"), true);
+assertEq(get("unicode"), false);
+
+regExp = otherGlobal.eval("new RegExp('', 'gu')");
+
+assertEq(get("flags"), "gu");
+assertEq(get("global"), true);
+assertEq(get("ignoreCase"), false);
+assertEq(get("multiline"), false);
+assertEq(get("dotAll"), false);
+assertEq(get("source"), "(?:)");
+assertEq(get("sticky"), false);
+assertEq(get("unicode"), true);
+
+// Trigger escaping
+regExp = otherGlobal.eval("new RegExp('a/b', '')");
+
+assertEq(get("flags"), "");
+assertEq(get("global"), false);
+assertEq(get("ignoreCase"), false);
+assertEq(get("multiline"), false);
+assertEq(get("dotAll"), false);
+assertEq(get("source"), "a\\/b");
+assertEq(get("sticky"), false);
+assertEq(get("unicode"), false);
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);