Issue #2083 - Part 3: Fix RegExpShared rooting hazards now it's a GC thing.

Based on Mozilla bug 1345177.
Changes from the original bug's patch:

- The original patch didn't have a dotAll for a call to irregexp::ParsePattern,
  so let's make our dotAll a member of the MutableHandleRegExpShared re.
- Make RegExpShared::initializeNamedCaptures, introduced in Issue #1285, static.
  This resolves a build bustage where static RegExpShared::compile was trying to
  use a member function.
This commit is contained in:
Job Bautista 2023-01-26 15:20:34 +08:00 • committed by roytam1
commit 2ac60a27c7
7 changed files with 90 additions and 74 deletions

View file

@ -164,10 +164,11 @@ CreateRegExpSearchResult(JSContext* cx, const MatchPairs& matches)
* steps 3, 9-14, except 12.a.i, 12.c.i.1. * steps 3, 9-14, except 12.a.i, 12.c.i.1.
*/ */
static RegExpRunStatus static RegExpRunStatus
ExecuteRegExpImpl(JSContext* cx, RegExpStatics* res, RegExpShared& re, HandleLinearString input, ExecuteRegExpImpl(JSContext* cx, RegExpStatics* res, MutableHandleRegExpShared re,
size_t searchIndex, MatchPairs* matches, size_t* endIndex) HandleLinearString input, size_t searchIndex, MatchPairs* matches,
size_t* endIndex)
{ {
RegExpRunStatus status = re.execute(cx, input, searchIndex, matches, endIndex); RegExpRunStatus status = RegExpShared::execute(cx, re, input, searchIndex, matches, endIndex);
/* Out of spec: Update RegExpStatics. */ /* Out of spec: Update RegExpStatics. */
if (status == RegExpRunStatus_Success && res) { if (status == RegExpRunStatus_Success && res) {
@ -175,7 +176,7 @@ ExecuteRegExpImpl(JSContext* cx, RegExpStatics* res, RegExpShared& re, HandleLin
if (!res->updateFromMatchPairs(cx, input, *matches)) if (!res->updateFromMatchPairs(cx, input, *matches))
return RegExpRunStatus_Error; return RegExpRunStatus_Error;
} else { } else {
res->updateLazily(cx, input, &re, searchIndex); res->updateLazily(cx, input, re, searchIndex);
} }
} }
return status; return status;
@ -193,7 +194,7 @@ js::ExecuteRegExpLegacy(JSContext* cx, RegExpStatics* res, Handle<RegExpObject*>
ScopedMatchPairs matches(&cx->tempLifoAlloc()); ScopedMatchPairs matches(&cx->tempLifoAlloc());
RegExpRunStatus status = ExecuteRegExpImpl(cx, res, *shared, input, *lastIndex, RegExpRunStatus status = ExecuteRegExpImpl(cx, res, &shared, input, *lastIndex,
&matches, nullptr); &matches, nullptr);
if (status == RegExpRunStatus_Error) if (status == RegExpRunStatus_Error)
return false; return false;
@ -1036,7 +1037,7 @@ ExecuteRegExp(JSContext* cx, HandleObject regexp, HandleString string,
} }
/* Steps 3, 11-14, except 12.a.i, 12.c.i.1. */ /* Steps 3, 11-14, except 12.a.i, 12.c.i.1. */
RegExpRunStatus status = ExecuteRegExpImpl(cx, res, *re, input, lastIndex, matches, endIndex); RegExpRunStatus status = ExecuteRegExpImpl(cx, res, &re, input, lastIndex, matches, endIndex);
if (status == RegExpRunStatus_Error) if (status == RegExpRunStatus_Error)
return RegExpRunStatus_Error; return RegExpRunStatus_Error;

View file

@ -1259,7 +1259,7 @@ IsNativeRegExpEnabled(JSContext* cx)
} }
RegExpCode RegExpCode
irregexp::CompilePattern(JSContext* cx, RegExpShared* shared, RegExpCompileData* data, irregexp::CompilePattern(JSContext* cx, HandleRegExpShared shared, RegExpCompileData* data,
HandleLinearString sample, bool is_global, bool ignore_case, HandleLinearString sample, bool is_global, bool ignore_case,
bool is_ascii, bool match_only, bool force_bytecode, bool sticky, bool is_ascii, bool match_only, bool force_bytecode, bool sticky,
bool unicode) bool unicode)

View file

@ -103,7 +103,7 @@ struct RegExpCode
}; };
RegExpCode RegExpCode
CompilePattern(JSContext* cx, RegExpShared* shared, RegExpCompileData* data, CompilePattern(JSContext* cx, HandleRegExpShared shared, RegExpCompileData* data,
HandleLinearString sample, bool is_global, bool ignore_case, HandleLinearString sample, bool is_global, bool ignore_case,
bool is_ascii, bool match_only, bool force_bytecode, bool sticky, bool is_ascii, bool match_only, bool force_bytecode, bool sticky,
bool unicode); bool unicode);

View file

@ -448,7 +448,8 @@ CrossCompartmentWrapper::regexp_toShared(JSContext* cx, HandleObject wrapper,
} }
// Get an equivalent RegExpShared associated with the current compartment. // Get an equivalent RegExpShared associated with the current compartment.
return cx->compartment()->regExps.get(cx, re->getSource(), re->getFlags(), shared); RootedAtom source(cx, re->getSource());
return cx->compartment()->regExps.get(cx, source, re->getFlags(), shared);
} }
bool bool

View file

@ -283,7 +283,8 @@ RegExpObject::createShared(JSContext* cx, Handle<RegExpObject*> regexp,
MutableHandleRegExpShared shared) MutableHandleRegExpShared shared)
{ {
MOZ_ASSERT(!regexp->hasShared()); MOZ_ASSERT(!regexp->hasShared());
if (!cx->compartment()->regExps.get(cx, regexp->getSource(), regexp->getFlags(), shared)) RootedAtom source(cx, regexp->getSource());
if (!cx->compartment()->regExps.get(cx, source, regexp->getFlags(), shared))
return false; return false;
regexp->setShared(*shared); regexp->setShared(*shared);
@ -507,14 +508,15 @@ RegExpObject::toString(JSContext* cx) const
} }
#ifdef DEBUG #ifdef DEBUG
bool /* static */ bool
RegExpShared::dumpBytecode(JSContext* cx, bool match_only, HandleLinearString input) RegExpShared::dumpBytecode(JSContext* cx, MutableHandleRegExpShared re, bool match_only,
HandleLinearString input)
{ {
CompilationMode mode = match_only ? MatchOnly : Normal; CompilationMode mode = match_only ? MatchOnly : Normal;
if (!compileIfNecessary(cx, input, mode, ForceByteCode)) if (!RegExpShared::compileIfNecessary(cx, re, input, mode, ForceByteCode))
return false; return false;
const uint8_t* byteCode = compilation(mode, input->hasLatin1Chars()).byteCode; const uint8_t* byteCode = re->compilation(mode, input->hasLatin1Chars()).byteCode;
const uint8_t* pc = byteCode; const uint8_t* pc = byteCode;
auto Load32Aligned = [](const uint8_t* pc) -> int32_t { auto Load32Aligned = [](const uint8_t* pc) -> int32_t {
@ -897,7 +899,7 @@ RegExpObject::dumpBytecode(JSContext* cx, Handle<RegExpObject*> regexp,
if (!getShared(cx, regexp, &shared)) if (!getShared(cx, regexp, &shared))
return false; return false;
return shared->dumpBytecode(cx, match_only, input); return RegExpShared::dumpBytecode(cx, &shared, match_only, input);
} }
#endif #endif
@ -976,21 +978,23 @@ RegExpShared::discardJitCode()
comp.jitCode = nullptr; comp.jitCode = nullptr;
} }
bool /* static */ bool
RegExpShared::compile(JSContext* cx, HandleLinearString input, RegExpShared::compile(JSContext* cx, MutableHandleRegExpShared re, HandleLinearString input,
CompilationMode mode, ForceByteCodeEnum force) CompilationMode mode, ForceByteCodeEnum force)
{ {
TraceLoggerThread* logger = TraceLoggerForMainThread(cx->runtime()); TraceLoggerThread* logger = TraceLoggerForMainThread(cx->runtime());
AutoTraceLog logCompile(logger, TraceLogger_IrregexpCompile); AutoTraceLog logCompile(logger, TraceLogger_IrregexpCompile);
RootedAtom pattern(cx, source); RootedAtom pattern(cx, re->source);
return compile(cx, pattern, input, mode, force); return compile(cx, re, pattern, input, mode, force);
} }
bool /* static */ bool
RegExpShared::initializeNamedCaptures(JSContext* cx, irregexp::CharacterVectorVector* names, irregexp::IntegerVector* indices) RegExpShared::initializeNamedCaptures(JSContext* cx, HandleRegExpShared re,
irregexp::CharacterVectorVector* names,
irregexp::IntegerVector* indices)
{ {
MOZ_ASSERT(!groupsTemplate_); MOZ_ASSERT(!re->groupsTemplate_);
MOZ_ASSERT(names); MOZ_ASSERT(names);
MOZ_ASSERT(indices); MOZ_ASSERT(indices);
MOZ_ASSERT(names->length() == indices->length()); MOZ_ASSERT(names->length() == indices->length());
@ -1032,17 +1036,17 @@ RegExpShared::initializeNamedCaptures(JSContext* cx, irregexp::CharacterVectorVe
AddTypePropertyId(cx, templateObject, id, TypeSet::Int32Type()); AddTypePropertyId(cx, templateObject, id, TypeSet::Int32Type());
} }
groupsTemplate_ = templateObject; re->groupsTemplate_ = templateObject;
numNamedCaptures_ = numNamedCaptures; re->numNamedCaptures_ = numNamedCaptures;
return true; return true;
} }
bool /* static */ bool
RegExpShared::compile(JSContext* cx, HandleAtom pattern, HandleLinearString input, RegExpShared::compile(JSContext* cx, MutableHandleRegExpShared re, HandleAtom pattern,
CompilationMode mode, ForceByteCodeEnum force) HandleLinearString input, CompilationMode mode, ForceByteCodeEnum force)
{ {
if (!ignoreCase() && !StringHasRegExpMetaChars(pattern)) if (!re->ignoreCase() && !StringHasRegExpMetaChars(pattern))
canStringMatch = true; re->canStringMatch = true;
CompileOptions options(cx); CompileOptions options(cx);
TokenStream dummyTokenStream(cx, options, nullptr, 0, nullptr); TokenStream dummyTokenStream(cx, options, nullptr, 0, nullptr);
@ -1052,34 +1056,36 @@ RegExpShared::compile(JSContext* cx, HandleAtom pattern, HandleLinearString inpu
/* Parse the pattern. */ /* Parse the pattern. */
irregexp::RegExpCompileData data; irregexp::RegExpCompileData data;
if (!irregexp::ParsePattern(dummyTokenStream, cx->tempLifoAlloc(), pattern, if (!irregexp::ParsePattern(dummyTokenStream, cx->tempLifoAlloc(), pattern,
multiline(), mode == MatchOnly, unicode(), ignoreCase(), re->multiline(), mode == MatchOnly, re->unicode(),
global(), sticky(), dotAll(), &data)) re->ignoreCase(), re->global(), re->sticky(),
re->dotAll(), &data))
{ {
return false; return false;
} }
this->parenCount = data.capture_count; re->parenCount = data.capture_count;
if (data.capture_name_list) { if (data.capture_name_list) {
// convert LifoAlloc'd named capture info to NativeObject // convert LifoAlloc'd named capture info to NativeObject
if (!initializeNamedCaptures(cx, data.capture_name_list, data.capture_index_list)) { if (!initializeNamedCaptures(cx, re, data.capture_name_list, data.capture_index_list)) {
return false; return false;
} }
} }
irregexp::RegExpCode code = irregexp::CompilePattern(cx, this, &data, input, irregexp::RegExpCode code = irregexp::CompilePattern(cx, re, &data, input,
false /* global() */, false /* global() */,
ignoreCase(), re->ignoreCase(),
input->hasLatin1Chars(), input->hasLatin1Chars(),
mode == MatchOnly, mode == MatchOnly,
force == ForceByteCode, force == ForceByteCode,
sticky(), unicode()); re->sticky(),
re->unicode());
if (code.empty()) if (code.empty())
return false; return false;
MOZ_ASSERT(!code.jitCode || !code.byteCode); MOZ_ASSERT(!code.jitCode || !code.byteCode);
MOZ_ASSERT_IF(force == ForceByteCode, code.byteCode); MOZ_ASSERT_IF(force == ForceByteCode, code.byteCode);
RegExpCompilation& compilation = this->compilation(mode, input->hasLatin1Chars()); RegExpCompilation& compilation = re->compilation(mode, input->hasLatin1Chars());
if (code.jitCode) if (code.jitCode)
compilation.jitCode = code.jitCode; compilation.jitCode = code.jitCode;
else if (code.byteCode) else if (code.byteCode)
@ -1088,18 +1094,19 @@ RegExpShared::compile(JSContext* cx, HandleAtom pattern, HandleLinearString inpu
return true; return true;
} }
bool /* static */ bool
RegExpShared::compileIfNecessary(JSContext* cx, HandleLinearString input, RegExpShared::compileIfNecessary(JSContext* cx, MutableHandleRegExpShared re,
CompilationMode mode, ForceByteCodeEnum force) HandleLinearString input, CompilationMode mode,
ForceByteCodeEnum force)
{ {
if (isCompiled(mode, input->hasLatin1Chars(), force)) if (re->isCompiled(mode, input->hasLatin1Chars(), force))
return true; return true;
return compile(cx, input, mode, force); return compile(cx, re, input, mode, force);
} }
RegExpRunStatus /* static */ RegExpRunStatus
RegExpShared::execute(JSContext* cx, HandleLinearString input, size_t start, RegExpShared::execute(JSContext* cx, MutableHandleRegExpShared re, HandleLinearString input,
MatchPairs* matches, size_t* endIndex) size_t start, MatchPairs* matches, size_t* endIndex)
{ {
MOZ_ASSERT_IF(matches, !endIndex); MOZ_ASSERT_IF(matches, !endIndex);
MOZ_ASSERT_IF(!matches, endIndex); MOZ_ASSERT_IF(!matches, endIndex);
@ -1108,14 +1115,14 @@ RegExpShared::execute(JSContext* cx, HandleLinearString input, size_t start,
CompilationMode mode = matches ? Normal : MatchOnly; CompilationMode mode = matches ? Normal : MatchOnly;
/* Compile the code at point-of-use. */ /* Compile the code at point-of-use. */
if (!compileIfNecessary(cx, input, mode, DontForceByteCode)) if (!compileIfNecessary(cx, re, input, mode, DontForceByteCode))
return RegExpRunStatus_Error; return RegExpRunStatus_Error;
/* /*
* Ensure sufficient memory for output vector. * Ensure sufficient memory for output vector.
* No need to initialize it. The RegExp engine fills them in on a match. * No need to initialize it. The RegExp engine fills them in on a match.
*/ */
if (matches && !matches->allocOrExpandArray(pairCount())) { if (matches && !matches->allocOrExpandArray(re->pairCount())) {
ReportOutOfMemory(cx); ReportOutOfMemory(cx);
return RegExpRunStatus_Error; return RegExpRunStatus_Error;
} }
@ -1125,14 +1132,14 @@ RegExpShared::execute(JSContext* cx, HandleLinearString input, size_t start,
// Reset the Irregexp backtrack stack if it grows during execution. // Reset the Irregexp backtrack stack if it grows during execution.
irregexp::RegExpStackScope stackScope(cx->runtime()); irregexp::RegExpStackScope stackScope(cx->runtime());
if (canStringMatch) { if (re->canStringMatch) {
MOZ_ASSERT(pairCount() == 1); MOZ_ASSERT(re->pairCount() == 1);
size_t sourceLength = source->length(); size_t sourceLength = re->source->length();
if (sticky()) { if (re->sticky()) {
// First part checks size_t overflow. // First part checks size_t overflow.
if (sourceLength + start < sourceLength || sourceLength + start > length) if (sourceLength + start < sourceLength || sourceLength + start > length)
return RegExpRunStatus_Success_NotFound; return RegExpRunStatus_Success_NotFound;
if (!HasSubstringAt(input, source, start)) if (!HasSubstringAt(input, re->source, start))
return RegExpRunStatus_Success_NotFound; return RegExpRunStatus_Success_NotFound;
if (matches) { if (matches) {
@ -1146,7 +1153,7 @@ RegExpShared::execute(JSContext* cx, HandleLinearString input, size_t start,
return RegExpRunStatus_Success; return RegExpRunStatus_Success;
} }
int res = StringFindPattern(input, source, start); int res = StringFindPattern(input, re->source, start);
if (res == -1) if (res == -1)
return RegExpRunStatus_Success_NotFound; return RegExpRunStatus_Success_NotFound;
@ -1162,7 +1169,7 @@ RegExpShared::execute(JSContext* cx, HandleLinearString input, size_t start,
} }
do { do {
jit::JitCode* code = compilation(mode, input->hasLatin1Chars()).jitCode; jit::JitCode* code = re->compilation(mode, input->hasLatin1Chars()).jitCode;
if (!code) if (!code)
break; break;
@ -1201,10 +1208,10 @@ RegExpShared::execute(JSContext* cx, HandleLinearString input, size_t start,
} while (false); } while (false);
// Compile bytecode for the RegExp if necessary. // Compile bytecode for the RegExp if necessary.
if (!compileIfNecessary(cx, input, mode, ForceByteCode)) if (!compileIfNecessary(cx, re, input, mode, ForceByteCode))
return RegExpRunStatus_Error; return RegExpRunStatus_Error;
uint8_t* byteCode = compilation(mode, input->hasLatin1Chars()).byteCode; uint8_t* byteCode = re->compilation(mode, input->hasLatin1Chars()).byteCode;
AutoTraceLog logInterpreter(logger, TraceLogger_IrregexpExecute); AutoTraceLog logInterpreter(logger, TraceLogger_IrregexpExecute);
AutoStableStringChars inputChars(cx); AutoStableStringChars inputChars(cx);
@ -1353,7 +1360,7 @@ RegExpCompartment::sweep(JSRuntime* rt)
} }
bool bool
RegExpCompartment::get(JSContext* cx, JSAtom* source, RegExpFlag flags, RegExpCompartment::get(JSContext* cx, HandleAtom source, RegExpFlag flags,
MutableHandleRegExpShared result) MutableHandleRegExpShared result)
{ {
DependentAddPtr<Set> p(cx, set_.get(), Key(source, flags)); DependentAddPtr<Set> p(cx, set_.get(), Key(source, flags));

View file

@ -42,6 +42,10 @@ class MatchPairs;
class RegExpShared; class RegExpShared;
class RegExpStatics; class RegExpStatics;
using RootedRegExpShared = JS::Rooted<RegExpShared*>;
using HandleRegExpShared = JS::Handle<RegExpShared*>;
using MutableHandleRegExpShared = JS::MutableHandle<RegExpShared*>;
namespace frontend { class TokenStream; } namespace frontend { class TokenStream; }
enum RegExpFlag : uint8_t enum RegExpFlag : uint8_t
@ -150,13 +154,14 @@ class RegExpShared : public gc::TenuredCell
/* Internal functions. */ /* Internal functions. */
RegExpShared(JSAtom* source, RegExpFlag flags); RegExpShared(JSAtom* source, RegExpFlag flags);
bool compile(JSContext* cx, HandleLinearString input, static bool compile(JSContext* cx, MutableHandleRegExpShared res, HandleLinearString input,
CompilationMode mode, ForceByteCodeEnum force); CompilationMode mode, ForceByteCodeEnum force);
bool compile(JSContext* cx, HandleAtom pattern, HandleLinearString input, static bool compile(JSContext* cx, MutableHandleRegExpShared res, HandleAtom pattern,
CompilationMode mode, ForceByteCodeEnum force); HandleLinearString input, CompilationMode mode, ForceByteCodeEnum force);
bool compileIfNecessary(JSContext* cx, HandleLinearString input, static bool compileIfNecessary(JSContext* cx, MutableHandleRegExpShared res,
CompilationMode mode, ForceByteCodeEnum force); HandleLinearString input, CompilationMode mode,
ForceByteCodeEnum force);
const RegExpCompilation& compilation(CompilationMode mode, bool latin1) const { const RegExpCompilation& compilation(CompilationMode mode, bool latin1) const {
return compilationArray[CompilationIndex(mode, latin1)]; return compilationArray[CompilationIndex(mode, latin1)];
@ -171,8 +176,9 @@ class RegExpShared : public gc::TenuredCell
// Execute this RegExp on input starting from searchIndex, filling in // Execute this RegExp on input starting from searchIndex, filling in
// matches if specified and otherwise only determining if there is a match. // matches if specified and otherwise only determining if there is a match.
RegExpRunStatus execute(JSContext* cx, HandleLinearString input, size_t searchIndex, static RegExpRunStatus execute(JSContext* cx, MutableHandleRegExpShared res,
MatchPairs* matches, size_t* endIndex); HandleLinearString input, size_t searchIndex,
MatchPairs* matches, size_t* endIndex);
// Register a table with this RegExpShared, and take ownership. // Register a table with this RegExpShared, and take ownership.
bool addTable(uint8_t* table) { bool addTable(uint8_t* table) {
@ -190,7 +196,9 @@ class RegExpShared : public gc::TenuredCell
size_t pairCount() const { return getParenCount() + 1; } size_t pairCount() const { return getParenCount() + 1; }
// not public due to circular inclusion problems // not public due to circular inclusion problems
bool initializeNamedCaptures(JSContext* cx, irregexp::CharacterVectorVector* names, irregexp::IntegerVector* indices); static bool initializeNamedCaptures(JSContext* cx, HandleRegExpShared re,
irregexp::CharacterVectorVector* names,
irregexp::IntegerVector* indices);
PlainObject* getGroupsTemplate() { return groupsTemplate_; } PlainObject* getGroupsTemplate() { return groupsTemplate_; }
uint32_t numNamedCaptures() const { return numNamedCaptures_; } uint32_t numNamedCaptures() const { return numNamedCaptures_; }
@ -245,14 +253,11 @@ class RegExpShared : public gc::TenuredCell
size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf); size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf);
#ifdef DEBUG #ifdef DEBUG
bool dumpBytecode(JSContext* cx, bool match_only, HandleLinearString input); static bool dumpBytecode(JSContext* cx, MutableHandleRegExpShared res, bool match_only,
HandleLinearString input);
#endif #endif
}; };
using RootedRegExpShared = JS::Rooted<RegExpShared*>;
using HandleRegExpShared = JS::Handle<RegExpShared*>;
using MutableHandleRegExpShared = JS::MutableHandle<RegExpShared*>;
class RegExpCompartment class RegExpCompartment
{ {
struct Key { struct Key {
@ -323,7 +328,7 @@ class RegExpCompartment
bool empty() { return set_.empty(); } bool empty() { return set_.empty(); }
bool get(JSContext* cx, JSAtom* source, RegExpFlag flags, MutableHandleRegExpShared shared); bool get(JSContext* cx, HandleAtom source, RegExpFlag flags, MutableHandleRegExpShared shared);
/* Like 'get', but compile 'maybeOpt' (if non-null). */ /* Like 'get', but compile 'maybeOpt' (if non-null). */
bool get(JSContext* cx, HandleAtom source, JSString* maybeOpt, bool get(JSContext* cx, HandleAtom source, JSString* maybeOpt,

View file

@ -81,7 +81,8 @@ RegExpStatics::executeLazy(JSContext* cx)
/* Retrieve or create the RegExpShared in this compartment. */ /* Retrieve or create the RegExpShared in this compartment. */
RootedRegExpShared shared(cx); RootedRegExpShared shared(cx);
if (!cx->compartment()->regExps.get(cx, lazySource, lazyFlags, &shared)) RootedAtom source(cx, lazySource);
if (!cx->compartment()->regExps.get(cx, source, lazyFlags, &shared))
return false; return false;
/* /*
@ -91,7 +92,8 @@ RegExpStatics::executeLazy(JSContext* cx)
/* Execute the full regular expression. */ /* Execute the full regular expression. */
RootedLinearString input(cx, matchesInput); RootedLinearString input(cx, matchesInput);
RegExpRunStatus status = shared->execute(cx, input, lazyIndex, &this->matches, nullptr); RegExpRunStatus status = RegExpShared::execute(cx, &shared, input, lazyIndex, &this->matches,
nullptr);
if (status == RegExpRunStatus_Error) if (status == RegExpRunStatus_Error)
return false; return false;