From 47ef1701b3682a18b3640eb9c070c928fe3ec24f Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 8 Nov 2024 18:22:22 +0100 Subject: [PATCH 1/6] Issue #2305 Part 1: Add support for hasIndices to RegExpFlags (/d) Adds recognition and parsing of the /d RegExp flag. Drive-by correction: add missing "s" for dotAll to RegExpObject::toString. Tag #1284 and #2008 --- js/src/builtin/RegExp.cpp | 36 ++++++++++++++++++++++++++++- js/src/builtin/RegExp.h | 6 +++-- js/src/builtin/RegExp.js | 36 +++++++++++++++++------------ js/src/builtin/SelfHostingDefines.h | 1 + js/src/frontend/TokenStream.cpp | 4 +++- js/src/jsapi.h | 13 ++++++----- js/src/vm/CommonPropertyNames.h | 1 + js/src/vm/RegExpObject.cpp | 25 +++++++++++++++----- js/src/vm/RegExpObject.h | 20 +++++++++------- 9 files changed, 103 insertions(+), 39 deletions(-) diff --git a/js/src/builtin/RegExp.cpp b/js/src/builtin/RegExp.cpp index f3d34762f6..198610438c 100644 --- a/js/src/builtin/RegExp.cpp +++ b/js/src/builtin/RegExp.cpp @@ -818,15 +818,42 @@ js::regexp_unicode(JSContext* cx, unsigned argc, JS::Value* vp) return CallNonGenericMethod(cx, args); } +// ES 2022 22.2.5.6. +MOZ_ALWAYS_INLINE bool +regexp_hasIndices_impl(JSContext* cx, const CallArgs& args) +{ + MOZ_ASSERT(IsRegExpInstanceOrPrototype(args.thisv())); + + // Step 2.a. + if (!IsRegExpObject(args.thisv())) { + args.rval().setUndefined(); + return true; + } + + // Steps 3-5. + Rooted reObj(cx, &args.thisv().toObject().as()); + args.rval().setBoolean(reObj->hasIndices()); + return true; +} + +bool +js::regexp_hasIndices(JSContext* cx, unsigned argc, JS::Value* vp) +{ + // Steps 1-3. + CallArgs args = CallArgsFromVp(argc, vp); + return CallNonGenericMethod(cx, args); +} + const JSPropertySpec js::regexp_properties[] = { JS_SELF_HOSTED_GET("flags", "RegExpFlagsGetter", 0), + JS_PSG("hasIndices", regexp_hasIndices, 0), JS_PSG("global", regexp_global, 0), JS_PSG("ignoreCase", regexp_ignoreCase, 0), JS_PSG("multiline", regexp_multiline, 0), + JS_PSG("dotAll", regexp_dotAll, 0), JS_PSG("source", regexp_source, 0), JS_PSG("sticky", regexp_sticky, 0), JS_PSG("unicode", regexp_unicode, 0), - JS_PSG("dotAll", regexp_dotAll, 0), JS_PS_END }; @@ -1802,6 +1829,13 @@ js::RegExpPrototypeOptimizableRaw(JSContext* cx, JSObject* proto) if (!IsSelfHostedFunctionWithName(flagsGetter, cx->names().RegExpFlagsGetter)) return false; + JSNative hasIndicesGetter; + if (!GetOwnNativeGetterPure(cx, proto, NameToId(cx->names().hasIndices), &hasIndicesGetter)) + return false; + + if (hasIndicesGetter != regexp_hasIndices) + return false; + JSNative globalGetter; if (!GetOwnNativeGetterPure(cx, proto, NameToId(cx->names().global), &globalGetter)) return false; diff --git a/js/src/builtin/RegExp.h b/js/src/builtin/RegExp.h index c0a7d59f77..b15c3d82f4 100644 --- a/js/src/builtin/RegExp.h +++ b/js/src/builtin/RegExp.h @@ -124,17 +124,19 @@ extern const JSFunctionSpec regexp_methods[]; // Used in RegExpObject::isOriginalFlagGetter. extern MOZ_MUST_USE bool +regexp_hasIndices(JSContext* cx, unsigned argc, JS::Value* vp); +extern MOZ_MUST_USE bool regexp_global(JSContext* cx, unsigned argc, JS::Value* vp); extern MOZ_MUST_USE bool regexp_ignoreCase(JSContext* cx, unsigned argc, JS::Value* vp); extern MOZ_MUST_USE bool regexp_multiline(JSContext* cx, unsigned argc, JS::Value* vp); extern MOZ_MUST_USE bool +regexp_dotAll(JSContext* cx, unsigned argc, JS::Value* vp); +extern MOZ_MUST_USE bool regexp_sticky(JSContext* cx, unsigned argc, JS::Value* vp); extern MOZ_MUST_USE bool regexp_unicode(JSContext* cx, unsigned argc, JS::Value* vp); -extern MOZ_MUST_USE bool -regexp_dotAll(JSContext* cx, unsigned argc, JS::Value* vp); } /* namespace js */ diff --git a/js/src/builtin/RegExp.js b/js/src/builtin/RegExp.js index ab4d76f4ca..536b0dac4b 100644 --- a/js/src/builtin/RegExp.js +++ b/js/src/builtin/RegExp.js @@ -2,8 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -// ES6 draft rev34 (2015/02/20) 21.2.5.3 get RegExp.prototype.flags -// Updated for ES2018 /s (dotAll) +// ES2022 22.2.5.4 get RegExp.prototype.flags function RegExpFlagsGetter() { // Steps 1-2. var R = this; @@ -13,31 +12,36 @@ function RegExpFlagsGetter() { // Step 3. var result = ""; - // Steps 4-6. + // Steps 4-5. + if (R.hasIndices) + result += "d"; + + // Steps 6-7. if (R.global) result += "g"; - // Steps 7-9. + // Steps 8-9. if (R.ignoreCase) result += "i"; - // Steps 10-12. + // Steps 10-11. if (R.multiline) result += "m"; - // Steps 13-15. - if (R.unicode) - result += "u"; - - // Steps 16-18. - if (R.sticky) - result += "y"; - - // ES2018 + // Steps 12-13. if (R.dotAll) result += "s"; - // Step 19. + // Steps 14-15. + if (R.unicode) + result += "u"; + + // Steps 16-17. + if (R.sticky) + result += "y"; + + + // Step 18. return result; } _SetCanonicalName(RegExpFlagsGetter, "get flags"); @@ -227,9 +231,11 @@ function RegExpGlobalMatchOpt(rx, S, fullUnicode) { // Checks if following properties and getters are not modified, and accessing // them not observed by content script: // * flags +// * hasIndices // * global // * ignoreCase // * multiline +// * dotAll // * sticky // * unicode // * exec diff --git a/js/src/builtin/SelfHostingDefines.h b/js/src/builtin/SelfHostingDefines.h index 9a93a2c795..2745fa754b 100644 --- a/js/src/builtin/SelfHostingDefines.h +++ b/js/src/builtin/SelfHostingDefines.h @@ -85,6 +85,7 @@ #define REGEXP_STICKY_FLAG 0x08 #define REGEXP_UNICODE_FLAG 0x10 #define REGEXP_DOTALL_FLAG 0x20 +#define REGEXP_HASINDICES_FLAG 0x40 #define REGEXP_STRING_ITERATOR_REGEXP_SLOT 0 #define REGEXP_STRING_ITERATOR_STRING_SLOT 1 diff --git a/js/src/frontend/TokenStream.cpp b/js/src/frontend/TokenStream.cpp index 0130b20abc..5402727cc7 100644 --- a/js/src/frontend/TokenStream.cpp +++ b/js/src/frontend/TokenStream.cpp @@ -2083,7 +2083,9 @@ TokenStream::getTokenInternal(TokenKind* ttp, Modifier modifier) while (true) { if (!peekChar(&c)) goto error; - if (c == 'g' && !(reflags & GlobalFlag)) + if (c == 'd' && !(reflags & HasIndicesFlag)) + reflags = RegExpFlag(reflags | HasIndicesFlag); + else if (c == 'g' && !(reflags & GlobalFlag)) reflags = RegExpFlag(reflags | GlobalFlag); else if (c == 'i' && !(reflags & IgnoreCaseFlag)) reflags = RegExpFlag(reflags | IgnoreCaseFlag); diff --git a/js/src/jsapi.h b/js/src/jsapi.h index f1e71f9e7a..68e852e23f 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -5824,12 +5824,13 @@ JS_ObjectIsDate(JSContext* cx, JS::HandleObject obj, bool* isDate); /* * Regular Expressions. */ -#define JSREG_FOLD 0x01u /* fold uppercase to lowercase */ -#define JSREG_GLOB 0x02u /* global exec, creates array of matches */ -#define JSREG_MULTILINE 0x04u /* treat ^ and $ as begin and end of line */ -#define JSREG_STICKY 0x08u /* only match starting at lastIndex */ -#define JSREG_UNICODE 0x10u /* unicode */ -#define JSREG_DOTALL 0x20u /* match . to everything including newlines */ +#define JSREG_FOLD 0x01u /* fold uppercase to lowercase */ +#define JSREG_GLOB 0x02u /* global exec, creates array of matches */ +#define JSREG_MULTILINE 0x04u /* treat ^ and $ as begin and end of line */ +#define JSREG_STICKY 0x08u /* only match starting at lastIndex */ +#define JSREG_UNICODE 0x10u /* unicode */ +#define JSREG_DOTALL 0x20u /* match . to everything including newlines */ +#define JSREG_HASINDICES 0x40u /* add .indices property to the match result */ extern JS_PUBLIC_API(JSObject*) JS_NewRegExpObject(JSContext* cx, const char* bytes, size_t length, unsigned flags); diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h index ebfad89fa0..809990c0fc 100644 --- a/js/src/vm/CommonPropertyNames.h +++ b/js/src/vm/CommonPropertyNames.h @@ -183,6 +183,7 @@ macro(groups, groups, "groups") \ macro(Handle, Handle, "Handle") \ macro(has, has, "has") \ + macro(hasIndices, hasIndices, "hasIndices") \ macro(hasOwn, hasOwn, "hasOwn") \ macro(hasOwnProperty, hasOwnProperty, "hasOwnProperty") \ macro(highWaterMark, highWaterMark, "highWaterMark") \ diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp index 1d88754084..129ed2492f 100644 --- a/js/src/vm/RegExpObject.cpp +++ b/js/src/vm/RegExpObject.cpp @@ -51,6 +51,7 @@ JS_STATIC_ASSERT(MultilineFlag == JSREG_MULTILINE); JS_STATIC_ASSERT(StickyFlag == JSREG_STICKY); JS_STATIC_ASSERT(UnicodeFlag == JSREG_UNICODE); JS_STATIC_ASSERT(DotAllFlag == JSREG_DOTALL); +JS_STATIC_ASSERT(HasIndicesFlag == JSREG_HASINDICES); RegExpObject* js::RegExpAlloc(ExclusiveContext* cx, HandleObject proto /* = nullptr */) @@ -136,6 +137,10 @@ RegExpObject::getShared(JSContext* cx, Handle regexp, /* static */ bool RegExpObject::isOriginalFlagGetter(JSNative native, RegExpFlag* mask) { + if (native == regexp_hasIndices) { + *mask = HasIndicesFlag; + return true; + } if (native == regexp_global) { *mask = GlobalFlag; return true; @@ -148,6 +153,10 @@ RegExpObject::isOriginalFlagGetter(JSNative native, RegExpFlag* mask) *mask = MultilineFlag; return true; } + if (native == regexp_dotAll) { + *mask = DotAllFlag; + return true; + } if (native == regexp_sticky) { *mask = StickyFlag; return true; @@ -156,10 +165,6 @@ RegExpObject::isOriginalFlagGetter(JSNative native, RegExpFlag* mask) *mask = UnicodeFlag; return true; } - if (native == regexp_dotAll) { - *mask = DotAllFlag; - return true; - } return false; } @@ -488,12 +493,16 @@ RegExpObject::toString(JSContext* cx) const sb.infallibleAppend('/'); // Steps 5-7. + if (hasIndices() && !sb.append('d')) + return nullptr; if (global() && !sb.append('g')) return nullptr; if (ignoreCase() && !sb.append('i')) return nullptr; if (multiline() && !sb.append('m')) return nullptr; + if (dotAll() && !sb.append('s')) + return nullptr; if (unicode() && !sb.append('u')) return nullptr; if (sticky() && !sb.append('y')) @@ -1483,14 +1492,18 @@ ParseRegExpFlags(const CharT* chars, size_t length, RegExpFlag* flagsOut, char16 for (size_t i = 0; i < length; i++) { *lastParsedOut = chars[i]; switch (chars[i]) { - case 'i': - if (!HandleRegExpFlag(IgnoreCaseFlag, flagsOut)) + case 'd': + if (!HandleRegExpFlag(HasIndicesFlag, flagsOut)) return false; break; case 'g': if (!HandleRegExpFlag(GlobalFlag, flagsOut)) return false; break; + case 'i': + if (!HandleRegExpFlag(IgnoreCaseFlag, flagsOut)) + return false; + break; case 'm': if (!HandleRegExpFlag(MultilineFlag, flagsOut)) return false; diff --git a/js/src/vm/RegExpObject.h b/js/src/vm/RegExpObject.h index 5247731112..b088315dea 100644 --- a/js/src/vm/RegExpObject.h +++ b/js/src/vm/RegExpObject.h @@ -56,9 +56,10 @@ enum RegExpFlag : uint8_t StickyFlag = 0x08, UnicodeFlag = 0x10, DotAllFlag = 0x20, + HasIndicesFlag = 0x40, NoFlags = 0x00, - AllFlags = 0x3f + AllFlags = 0x7f }; static_assert(IgnoreCaseFlag == REGEXP_IGNORECASE_FLAG && @@ -66,7 +67,8 @@ static_assert(IgnoreCaseFlag == REGEXP_IGNORECASE_FLAG && MultilineFlag == REGEXP_MULTILINE_FLAG && StickyFlag == REGEXP_STICKY_FLAG && UnicodeFlag == REGEXP_UNICODE_FLAG && - DotAllFlag == REGEXP_DOTALL_FLAG, + DotAllFlag == REGEXP_DOTALL_FLAG && + HasIndicesFlag == REGEXP_HASINDICES_FLAG, "Flag values should be in sync with self-hosted JS"); enum RegExpRunStatus @@ -206,12 +208,13 @@ class RegExpShared : public gc::TenuredCell JSAtom* getSource() const { return source; } RegExpFlag getFlags() const { return flags; } - bool ignoreCase() const { return flags & IgnoreCaseFlag; } + bool hasIndices() const { return flags & HasIndicesFlag; } bool global() const { return flags & GlobalFlag; } + bool ignoreCase() const { return flags & IgnoreCaseFlag; } bool multiline() const { return flags & MultilineFlag; } - bool sticky() const { return flags & StickyFlag; } - bool unicode() const { return flags & UnicodeFlag; } bool dotAll() const { return flags & DotAllFlag; } + bool unicode() const { return flags & UnicodeFlag; } + bool sticky() const { return flags & StickyFlag; } bool isCompiled(CompilationMode mode, bool latin1, ForceByteCodeEnum force = DontForceByteCode) const { @@ -451,12 +454,13 @@ class RegExpObject : public NativeObject setSlot(FLAGS_SLOT, Int32Value(flags)); } - bool ignoreCase() const { return getFlags() & IgnoreCaseFlag; } + bool hasIndices() const { return getFlags() & HasIndicesFlag; } bool global() const { return getFlags() & GlobalFlag; } + bool ignoreCase() const { return getFlags() & IgnoreCaseFlag; } bool multiline() const { return getFlags() & MultilineFlag; } - bool sticky() const { return getFlags() & StickyFlag; } - bool unicode() const { return getFlags() & UnicodeFlag; } bool dotAll() const { return getFlags() & DotAllFlag; } + bool unicode() const { return getFlags() & UnicodeFlag; } + bool sticky() const { return getFlags() & StickyFlag; } static bool isOriginalFlagGetter(JSNative native, RegExpFlag* mask); From 845d36d582190144444cfdc2c8efe07b23a0acde Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 9 Nov 2024 18:51:54 +0100 Subject: [PATCH 2/6] Issue #2305 Part 2: Support .indices in match result template objects Before match indices, all match results had the same shape. (If there are no named capture groups, `.groups` exists but is undefined.) Match results with `.indices` have a distinct shape, so we need a distinct template object. We also need a template object for `.indices` itself. --- js/src/vm/CommonPropertyNames.h | 1 + js/src/vm/RegExpObject.cpp | 72 ++++++++++++++++++++------------- js/src/vm/RegExpObject.h | 33 ++++++++++----- 3 files changed, 68 insertions(+), 38 deletions(-) diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h index 809990c0fc..8489648d6b 100644 --- a/js/src/vm/CommonPropertyNames.h +++ b/js/src/vm/CommonPropertyNames.h @@ -198,6 +198,7 @@ macro(includes, includes, "includes") \ macro(incumbentGlobal, incumbentGlobal, "incumbentGlobal") \ macro(index, index, "index") \ + macro(indices, indices, "indices") \ macro(infinity, infinity, "infinity") \ macro(Infinity, Infinity, "Infinity") \ macro(InitializeCollator, InitializeCollator, "InitializeCollator") \ diff --git a/js/src/vm/RegExpObject.cpp b/js/src/vm/RegExpObject.cpp index 129ed2492f..c4493de28e 100644 --- a/js/src/vm/RegExpObject.cpp +++ b/js/src/vm/RegExpObject.cpp @@ -1297,10 +1297,13 @@ RegExpShared::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) RegExpCompartment::RegExpCompartment(Zone* zone) : set_(zone, Set(zone->runtimeFromMainThread())), - matchResultTemplateObject_(nullptr), optimizableRegExpPrototypeShape_(nullptr), optimizableRegExpInstanceShape_(nullptr) -{} +{ + for (auto& templateObj : matchResultTemplateObjects_) { + templateObj = nullptr; + } +} RegExpCompartment::~RegExpCompartment() { @@ -1308,38 +1311,53 @@ RegExpCompartment::~RegExpCompartment() } ArrayObject* -RegExpCompartment::createMatchResultTemplateObject(JSContext* cx) +RegExpCompartment::createMatchResultTemplateObject(JSContext* cx, ResultTemplateKind kind) { - MOZ_ASSERT(!matchResultTemplateObject_); + MOZ_ASSERT(!matchResultTemplateObjects_[kind]); /* Create template array object */ RootedArrayObject templateObject(cx, NewDenseUnallocatedArray(cx, RegExpObject::MaxPairCount, nullptr, TenuredObject)); - if (!templateObject) - return matchResultTemplateObject_; // = nullptr + if (!templateObject) { + return nullptr; + } // Create a new group for the template. Rooted proto(cx, templateObject->taggedProto()); ObjectGroup* group = ObjectGroupCompartment::makeGroup(cx, templateObject->getClass(), proto); if (!group) - return matchResultTemplateObject_; // = nullptr + return nullptr; templateObject->setGroup(group); + if (kind == ResultTemplateKind::Indices) { + /* The |indices| array only has a |groups| property. */ + RootedValue groupsVal(cx, UndefinedValue()); + if (!NativeDefineProperty(cx, templateObject, cx->names().groups, + groupsVal, nullptr, nullptr, JSPROP_ENUMERATE)) { + return nullptr; + } + AddTypePropertyId(cx, templateObject, NameToId(cx->names().groups), TypeSet::AnyObjectType()); + matchResultTemplateObjects_[kind].set(templateObject); + return matchResultTemplateObjects_[kind]; + } + /* Set dummy index property */ RootedValue index(cx, Int32Value(0)); if (!NativeDefineProperty(cx, templateObject, cx->names().index, index, nullptr, nullptr, JSPROP_ENUMERATE)) { - return matchResultTemplateObject_; // = nullptr + return nullptr; } + AddTypePropertyId(cx, templateObject, JSID_VOID, TypeSet::UndefinedType()); /* Set dummy input property */ RootedValue inputVal(cx, StringValue(cx->runtime()->emptyString)); if (!NativeDefineProperty(cx, templateObject, cx->names().input, inputVal, nullptr, nullptr, JSPROP_ENUMERATE)) { - return matchResultTemplateObject_; // = nullptr + return nullptr; } + AddTypePropertyId(cx, templateObject, JSID_VOID, TypeSet::StringType()); /* Set dummy groups property */ RootedValue groupsVal(cx, UndefinedValue()); @@ -1347,25 +1365,21 @@ RegExpCompartment::createMatchResultTemplateObject(JSContext* cx) cx, templateObject, cx->names().groups, groupsVal, nullptr, nullptr, JSPROP_ENUMERATE)) { return nullptr; } - - // Make sure that the properties are in the right slots. - DebugOnly shape = templateObject->lastProperty(); - MOZ_ASSERT(shape->slot() == 2 && - shape->propidRef() == NameToId(cx->names().groups)); - MOZ_ASSERT(shape->previous()->slot() == 1 && - shape->previous()->propidRef() == NameToId(cx->names().input)); - MOZ_ASSERT(shape->previous()->previous()->slot() == 0 && - shape->previous()->previous()->propidRef() == NameToId(cx->names().index)); - - // Make sure type information reflects the indexed properties which might - // be added. - AddTypePropertyId(cx, templateObject, JSID_VOID, TypeSet::StringType()); - AddTypePropertyId(cx, templateObject, JSID_VOID, TypeSet::UndefinedType()); AddTypePropertyId(cx, templateObject, NameToId(cx->names().groups), TypeSet::AnyObjectType()); - matchResultTemplateObject_.set(templateObject); + if (kind == ResultTemplateKind::WithIndices) { + /* Set dummy indices property */ + RootedValue indicesVal(cx, UndefinedValue()); + if (!NativeDefineProperty(cx, templateObject, cx->names().indices, + indicesVal, nullptr, nullptr, JSPROP_ENUMERATE)) { + return nullptr; + } + AddTypePropertyId(cx, templateObject, NameToId(cx->names().indices), TypeSet::AnyObjectType()); + } - return matchResultTemplateObject_; + matchResultTemplateObjects_[kind].set(templateObject); + + return matchResultTemplateObjects_[kind]; } bool @@ -1383,10 +1397,10 @@ RegExpCompartment::init(JSContext* cx) void RegExpCompartment::sweep(JSRuntime* rt) { - if (matchResultTemplateObject_ && - IsAboutToBeFinalized(&matchResultTemplateObject_)) - { - matchResultTemplateObject_.set(nullptr); + for (auto& templateObject : matchResultTemplateObjects_) { + if (templateObject && IsAboutToBeFinalized(&templateObject)) { + templateObject.set(nullptr); + } } if (optimizableRegExpPrototypeShape_ && diff --git a/js/src/vm/RegExpObject.h b/js/src/vm/RegExpObject.h index b088315dea..1f97f7e47f 100644 --- a/js/src/vm/RegExpObject.h +++ b/js/src/vm/RegExpObject.h @@ -298,12 +298,27 @@ class RegExpCompartment using Set = GCHashSet, Key, RuntimeAllocPolicy>; JS::WeakCache set_; +public: + enum ResultTemplateKind { Normal, WithIndices, Indices, NumKinds }; + +private: /* - * This is the template object where the result of re.exec() is based on, - * if there is a result. This is used in CreateRegExpMatchResult to set - * the input/index properties faster. + * The template objects that the result of re.exec() is based on, if + * there is a result. These are used in CreateRegExpMatchResult. + * There are three template objects, each of which is an ArrayObject + * with some additional properties. We decide which to use based on + * the |hasIndices| (/d) flag. + * + * Normal: Has |index|, |input|, and |groups| properties. + * Used for the result object if |hasIndices| is not set. + * + * WithIndices: Has |index|, |input|, |groups|, and |indices| properties. + * Used for the result object if |hasIndices| is set. + * + * Indices: Has a |groups| property. If |hasIndices| is set, used + * for the |.indices| property of the result object. */ - ReadBarriered matchResultTemplateObject_; + ReadBarriered matchResultTemplateObjects_[ResultTemplateKind::NumKinds]; /* * The shape of RegExp.prototype object that satisfies following: @@ -326,7 +341,7 @@ class RegExpCompartment */ ReadBarriered optimizableRegExpInstanceShape_; - ArrayObject* createMatchResultTemplateObject(JSContext* cx); + ArrayObject* createMatchResultTemplateObject(JSContext* cx, ResultTemplateKind kind); public: explicit RegExpCompartment(Zone* zone); @@ -344,10 +359,10 @@ class RegExpCompartment MutableHandleRegExpShared shared); /* Get or create template object used to base the result of .exec() on. */ - ArrayObject* getOrCreateMatchResultTemplateObject(JSContext* cx) { - if (matchResultTemplateObject_) - return matchResultTemplateObject_; - return createMatchResultTemplateObject(cx); + ArrayObject* getOrCreateMatchResultTemplateObject(JSContext* cx, ResultTemplateKind kind = ResultTemplateKind::Normal) { + if (matchResultTemplateObjects_[kind]) + return matchResultTemplateObjects_[kind]; + return createMatchResultTemplateObject(cx, kind); } Shape* getOptimizableRegExpPrototypeShape() { From 2d8696c4fea958d199b3b92f29d685ec5be7ed45 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 9 Nov 2024 19:17:11 +0100 Subject: [PATCH 3/6] Issue #2305 Part 3a: Pass ResultTemplateKind through CreateRegExpMatchResult This sets up the MatchResult with the proper structure to contain indices when necessary. --- js/src/builtin/RegExp.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/js/src/builtin/RegExp.cpp b/js/src/builtin/RegExp.cpp index 198610438c..590010b868 100644 --- a/js/src/builtin/RegExp.cpp +++ b/js/src/builtin/RegExp.cpp @@ -32,7 +32,7 @@ using mozilla::Maybe; using CapturesVector = GCVector; /* - * ES 2021 draft 21.2.5.2.2: Steps 16-28 + * Implements RegExpBuiltinExec: Steps 18-35 * https://tc39.es/ecma262/#sec-regexpbuiltinexec */ bool @@ -51,10 +51,16 @@ js::CreateRegExpMatchResult(JSContext* cx, RegExpShared& re, * input: input string * index: start index for the match * groups: named capture groups for the match + * indices: capture indices for the match, if required */ + bool hasIndices = re.hasIndices(); + /* Get the templateObject that defines the shape and type of the output object */ - JSObject* templateObject = cx->compartment()->regExps.getOrCreateMatchResultTemplateObject(cx); + RegExpCompartment::ResultTemplateKind kind = + hasIndices ? RegExpCompartment::ResultTemplateKind::WithIndices + : RegExpCompartment::ResultTemplateKind::Normal; + JSObject* templateObject = cx->compartment()->regExps.getOrCreateMatchResultTemplateObject(cx, kind); if (!templateObject) return false; From 6dbe2de6dba7ef7c967a5a944f01bfb4c2edb79d Mon Sep 17 00:00:00 2001 From: Martok Date: Sun, 10 Nov 2024 17:05:24 +0100 Subject: [PATCH 4/6] Issue #2305 Part 3b: Create .indices and .indices.groups --- js/src/builtin/RegExp.cpp | 122 +++++++++++++++++++++++++++++------ js/src/jit/CodeGenerator.cpp | 5 ++ 2 files changed, 109 insertions(+), 18 deletions(-) diff --git a/js/src/builtin/RegExp.cpp b/js/src/builtin/RegExp.cpp index 590010b868..be6f3671e9 100644 --- a/js/src/builtin/RegExp.cpp +++ b/js/src/builtin/RegExp.cpp @@ -31,6 +31,28 @@ using mozilla::Maybe; using CapturesVector = GCVector; + +// Allocate an object for the |.groups| or |.indices.groups| property +// of a regexp match result. +static PlainObject* CreateGroupsObject(JSContext* cx, + HandlePlainObject groupsTemplate) +{ + PlainObject* result = NewObjectWithGivenProto(cx, nullptr); + result->setGroup(groupsTemplate->group()); + + return result; +} + +// Fetch a group index corresponding to a |key| from the template object +static uint32_t GetNamedCaptureIndex(JSContext* cx, HandlePlainObject groupsTemplate, HandleId key) +{ + RootedValue ival(cx); + + if (!NativeGetProperty(cx, groupsTemplate, key, &ival)) + return -1; + return ival.toInt32(); +} + /* * Implements RegExpBuiltinExec: Steps 18-35 * https://tc39.es/ecma262/#sec-regexpbuiltinexec @@ -64,17 +86,17 @@ js::CreateRegExpMatchResult(JSContext* cx, RegExpShared& re, if (!templateObject) return false; - // Step 16 + // Steps 18-19 size_t numPairs = matches.length(); MOZ_ASSERT(numPairs > 0); - /* Step 18-19. */ + // Steps 20-21: Allocate the match result object. RootedArrayObject arr(cx, NewDenseFullyAllocatedArrayWithTemplate(cx, numPairs, templateObject)); if (!arr) return false; - /* Steps 22-23 and 27 a-e - * Store a Value for each pair. */ + // Steps 28-29 and 33 a-d: Initialize the elements of the match result. + // Store a Value for each match pair. for (size_t i = 0; i < numPairs; i++) { const MatchPair& pair = matches[i]; @@ -91,15 +113,66 @@ js::CreateRegExpMatchResult(JSContext* cx, RegExpShared& re, } } - // Step 24 (reordered) + // Step 34a (reordered): Allocate and initialize the indices object if needed. + // This is an inlined implementation of MakeIndicesArray: + // https://tc39.es/ecma262/#sec-makeindicesarray + RootedArrayObject indices(cx); + RootedPlainObject indicesGroups(cx); + if (hasIndices) { + // MakeIndicesArray: step 8 + ArrayObject* indicesTemplate = + cx->compartment()->regExps.getOrCreateMatchResultTemplateObject(cx, RegExpCompartment::ResultTemplateKind::Indices); + indices = NewDenseFullyAllocatedArrayWithTemplate(cx, numPairs, indicesTemplate); + if (!indices) { + return false; + } + + // MakeIndicesArray: steps 10-12 + if (re.numNamedCaptures() > 0) { + RootedPlainObject groupsTemplate(cx, re.getGroupsTemplate()); + indicesGroups = CreateGroupsObject(cx, groupsTemplate); + if (!indicesGroups) { + return false; + } + indices->setSlot(0, ObjectValue(*indicesGroups)); + } else { + indices->setSlot(0, UndefinedValue()); + } + + // FIXME: can we merge this with step 28-29 and 33 a-d above?? + // MakeIndicesArray: step 13 a-d. (Step 13.e is implemented below.) + for (size_t i = 0; i < numPairs; i++) { + const MatchPair& pair = matches[i]; + + if (pair.isUndefined()) { + // Since we had a match, first pair must be present. + MOZ_ASSERT(i != 0); + indices->setDenseInitializedLength(i + 1); + indices->initDenseElement(i, UndefinedValue()); + } else { + RootedArrayObject indexPair(cx, NewDenseFullyAllocatedArray(cx, 2)); + if (!indexPair) { + return false; + } + indexPair->setDenseInitializedLength(2); + indexPair->initDenseElement(0, Int32Value(pair.start)); + indexPair->initDenseElement(1, Int32Value(pair.limit)); + + indices->setDenseInitializedLength(i + 1); + indices->initDenseElement(i, ObjectValue(*indexPair)); + } + } + // /FIXME + } + + // Steps 30-31 (reordered): Allocate the groups object (if needed). RootedPlainObject groups(cx); if (re.numNamedCaptures() > 0) { // construct a new object from the template saved on RegExpShared RootedPlainObject groupsTemplate(cx, re.getGroupsTemplate()); - groups = NewObjectWithGivenProto(cx, nullptr); - groups->setGroup(groupsTemplate->group()); + groups = CreateGroupsObject(cx, groupsTemplate); - // Step 27 f. + // Step 33 e-f // The groups template object stores the names of the named captures in the // the order in which they are defined. // Grab the index into the match vector from the template object and define the @@ -110,33 +183,46 @@ js::CreateRegExpMatchResult(JSContext* cx, RegExpShared& re, } MOZ_ASSERT(keys.length() == re.numNamedCaptures()); RootedId key(cx); - RootedValue ival(cx); RootedValue val(cx); for (size_t i = 0; i < keys.length(); i++) { key = keys[i]; - // fetch the group's match index... - if (!NativeGetProperty(cx, groupsTemplate, key, &ival)) - return false; - // ... and set it on groups - val = arr->getDenseElement(ival.toInt32()); + uint32_t idx = GetNamedCaptureIndex(cx, groupsTemplate, key); + MOZ_ASSERT(idx >= 0); + // Copy the matched string to |.groups| + val = arr->getDenseElement(idx); if (!NativeDefineProperty(cx, groups, key, val, nullptr, nullptr, JSPROP_ENUMERATE)) { return false; } + // MakeIndicesArray: Step 13.e (reordered) + // If we have an |.indices|, copy the information to |.indices.groups| + if (hasIndices) { + val = indices->getDenseElement(idx); + if (!NativeDefineDataProperty(cx, indicesGroups, key, val, + JSPROP_ENUMERATE)) { + return false; + } + } } } - /* Step 20 (reordered). + /* Step 22 (reordered). * Set the |index| property. (TemplateObject positions it in slot 0) */ arr->setSlot(0, Int32Value(matches[0].start)); - /* Step 21 (reordered). + /* Step 23 (reordered). * Set the |input| property. (TemplateObject positions it in slot 1) */ arr->setSlot(1, StringValue(input)); - // Steps 25-26 (reordered) + // Step 32 (reordered) // Set the |groups| property. arr->setSlot(2, groups ? ObjectValue(*groups) : UndefinedValue()); + // Step 34b (reordered) + // Set the |indices| property. + if (hasIndices) { + arr->setSlot(3, ObjectValue(*indices)); + } + #ifdef DEBUG RootedValue test(cx); RootedId id(cx, NameToId(cx->names().index)); @@ -149,7 +235,7 @@ js::CreateRegExpMatchResult(JSContext* cx, RegExpShared& re, MOZ_ASSERT(test == arr->getSlot(1)); #endif - /* Step 25. */ + /* Step 35. */ rval.setObject(*arr); return true; } diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index b2ef808636..b10520252c 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -1545,6 +1545,11 @@ JitCompartment::generateRegExpMatcherStub(JSContext* cx) ImmWord(0), &oolEntry); + // Similarly, if the |hasIndices| flag is set, fall back to the OOL stub. + masm.branchTest32(Assembler::NonZero, + Address(shared, RegExpShared::offsetOfFlags()), + Imm32(int32_t(HasIndicesFlag)), &oolEntry); + // Construct the result. Register object = temp1; Label matchResultFallback, matchResultJoin; From a382c2aa668d030a366d8a6689e2368835106dba Mon Sep 17 00:00:00 2001 From: Martok Date: Sun, 10 Nov 2024 23:35:49 +0100 Subject: [PATCH 5/6] Issue #2305 Part 3c: Merge repeated loops over MatchPairs --- js/src/builtin/RegExp.cpp | 79 ++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/js/src/builtin/RegExp.cpp b/js/src/builtin/RegExp.cpp index be6f3671e9..235d6e8bfc 100644 --- a/js/src/builtin/RegExp.cpp +++ b/js/src/builtin/RegExp.cpp @@ -90,29 +90,6 @@ js::CreateRegExpMatchResult(JSContext* cx, RegExpShared& re, size_t numPairs = matches.length(); MOZ_ASSERT(numPairs > 0); - // Steps 20-21: Allocate the match result object. - RootedArrayObject arr(cx, NewDenseFullyAllocatedArrayWithTemplate(cx, numPairs, templateObject)); - if (!arr) - return false; - - // Steps 28-29 and 33 a-d: Initialize the elements of the match result. - // Store a Value for each match pair. - for (size_t i = 0; i < numPairs; i++) { - const MatchPair& pair = matches[i]; - - if (pair.isUndefined()) { - MOZ_ASSERT(i != 0); /* Since we had a match, first pair must be present. */ - arr->setDenseInitializedLength(i + 1); - arr->initDenseElement(i, UndefinedValue()); - } else { - JSLinearString* str = NewDependentString(cx, input, pair.start, pair.length()); - if (!str) - return false; - arr->setDenseInitializedLength(i + 1); - arr->initDenseElement(i, StringValue(str)); - } - } - // Step 34a (reordered): Allocate and initialize the indices object if needed. // This is an inlined implementation of MakeIndicesArray: // https://tc39.es/ecma262/#sec-makeindicesarray @@ -138,31 +115,47 @@ js::CreateRegExpMatchResult(JSContext* cx, RegExpShared& re, } else { indices->setSlot(0, UndefinedValue()); } + } - // FIXME: can we merge this with step 28-29 and 33 a-d above?? - // MakeIndicesArray: step 13 a-d. (Step 13.e is implemented below.) - for (size_t i = 0; i < numPairs; i++) { + // Steps 20-21: Allocate the match result object. + RootedArrayObject arr(cx, NewDenseFullyAllocatedArrayWithTemplate(cx, numPairs, templateObject)); + if (!arr) + return false; + + // Steps 28-29 and 33 a-d: Initialize the elements of the match result. + // Store a Value for each match pair. + // Merged with MakeIndicesArray: step 13 a-d. (Step 13.e is implemented below.) + for (size_t i = 0; i < numPairs; i++) { const MatchPair& pair = matches[i]; - + if (pair.isUndefined()) { - // Since we had a match, first pair must be present. - MOZ_ASSERT(i != 0); - indices->setDenseInitializedLength(i + 1); - indices->initDenseElement(i, UndefinedValue()); + MOZ_ASSERT(i != 0); /* Since we had a match, first pair must be present. */ + arr->setDenseInitializedLength(i + 1); + arr->initDenseElement(i, UndefinedValue()); + if (hasIndices) { + indices->setDenseInitializedLength(i + 1); + indices->initDenseElement(i, UndefinedValue()); + } } else { - RootedArrayObject indexPair(cx, NewDenseFullyAllocatedArray(cx, 2)); - if (!indexPair) { - return false; - } - indexPair->setDenseInitializedLength(2); - indexPair->initDenseElement(0, Int32Value(pair.start)); - indexPair->initDenseElement(1, Int32Value(pair.limit)); - - indices->setDenseInitializedLength(i + 1); - indices->initDenseElement(i, ObjectValue(*indexPair)); + JSLinearString* str = NewDependentString(cx, input, pair.start, pair.length()); + if (!str) + return false; + arr->setDenseInitializedLength(i + 1); + arr->initDenseElement(i, StringValue(str)); + + if (hasIndices) { + RootedArrayObject indexPair(cx, NewDenseFullyAllocatedArray(cx, 2)); + if (!indexPair) { + return false; + } + indexPair->setDenseInitializedLength(2); + indexPair->initDenseElement(0, Int32Value(pair.start)); + indexPair->initDenseElement(1, Int32Value(pair.limit)); + + indices->setDenseInitializedLength(i + 1); + indices->initDenseElement(i, ObjectValue(*indexPair)); + } } - } - // /FIXME } // Steps 30-31 (reordered): Allocate the groups object (if needed). From 1878228db7e57e478a12753bb36acc9a75638bbf Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 12 Nov 2024 14:30:36 +0100 Subject: [PATCH 6/6] Issue #2305 Part 4: Check for failure when getting RegExp match result template Null deref crash fix. --- js/src/builtin/RegExp.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js/src/builtin/RegExp.cpp b/js/src/builtin/RegExp.cpp index 235d6e8bfc..61382e6aac 100644 --- a/js/src/builtin/RegExp.cpp +++ b/js/src/builtin/RegExp.cpp @@ -99,6 +99,9 @@ js::CreateRegExpMatchResult(JSContext* cx, RegExpShared& re, // MakeIndicesArray: step 8 ArrayObject* indicesTemplate = cx->compartment()->regExps.getOrCreateMatchResultTemplateObject(cx, RegExpCompartment::ResultTemplateKind::Indices); + if (!indicesTemplate) { + return false; + } indices = NewDenseFullyAllocatedArrayWithTemplate(cx, numPairs, indicesTemplate); if (!indices) { return false;