Bug 1438310 - Remove ScopedMatchPairs and devirtualize MatchPairs to avoid triggering undefined behavior. r=jwalden
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -115,17 +115,17 @@ CreateRegExpSearchResult(const MatchPair
}
/*
* ES 2017 draft rev 6a13789aa9e7c6de4e96b7d3e24d9e6eba6584ad 21.2.5.2.2
* steps 3, 9-14, except 12.a.i, 12.c.i.1.
*/
static RegExpRunStatus
ExecuteRegExpImpl(JSContext* cx, RegExpStatics* res, MutableHandleRegExpShared re,
- HandleLinearString input, size_t searchIndex, MatchPairs* matches,
+ HandleLinearString input, size_t searchIndex, VectorMatchPairs* matches,
size_t* endIndex)
{
RegExpRunStatus status = RegExpShared::execute(cx, re, input, searchIndex, matches, endIndex);
/* Out of spec: Update RegExpStatics. */
if (status == RegExpRunStatus_Success && res) {
if (matches) {
if (!res->updateFromMatchPairs(cx, input, *matches))
@@ -142,17 +142,17 @@ bool
js::ExecuteRegExpLegacy(JSContext* cx, RegExpStatics* res, Handle<RegExpObject*> reobj,
HandleLinearString input, size_t* lastIndex, bool test,
MutableHandleValue rval)
{
RootedRegExpShared shared(cx, RegExpObject::getShared(cx, reobj));
if (!shared)
return false;
- ScopedMatchPairs matches(&cx->tempLifoAlloc());
+ VectorMatchPairs matches;
RegExpRunStatus status = ExecuteRegExpImpl(cx, res, &shared, input, *lastIndex,
&matches, nullptr);
if (status == RegExpRunStatus_Error)
return false;
if (status == RegExpRunStatus_Success_NotFound) {
/* ExecuteRegExp() previously returned an array or null. */
@@ -899,17 +899,17 @@ IsTrailSurrogateWithLeadSurrogate(Handle
}
/*
* ES 2017 draft rev 6a13789aa9e7c6de4e96b7d3e24d9e6eba6584ad 21.2.5.2.2
* steps 3, 9-14, except 12.a.i, 12.c.i.1.
*/
static RegExpRunStatus
ExecuteRegExp(JSContext* cx, HandleObject regexp, HandleString string, int32_t lastIndex,
- MatchPairs* matches, size_t* endIndex)
+ VectorMatchPairs* matches, size_t* endIndex)
{
/*
* WARNING: Despite the presence of spec step comment numbers, this
* algorithm isn't consistent with any ES6 version, draft or
* otherwise. YOU HAVE BEEN WARNED.
*/
/* Steps 1-2 performed by the caller. */
@@ -974,17 +974,17 @@ ExecuteRegExp(JSContext* cx, HandleObjec
* ES 2017 draft rev 6a13789aa9e7c6de4e96b7d3e24d9e6eba6584ad 21.2.5.2.2
* steps 3, 9-25, except 12.a.i, 12.c.i.1, 15.
*/
static bool
RegExpMatcherImpl(JSContext* cx, HandleObject regexp, HandleString string, int32_t lastIndex,
MutableHandleValue rval)
{
/* Execute regular expression and gather matches. */
- ScopedMatchPairs matches(&cx->tempLifoAlloc());
+ VectorMatchPairs matches;
/* Steps 3, 9-14, except 12.a.i, 12.c.i.1. */
RegExpRunStatus status = ExecuteRegExp(cx, regexp, string, lastIndex, &matches, nullptr);
if (status == RegExpRunStatus_Error)
return false;
/* Steps 12.a, 12.c. */
if (status == RegExpRunStatus_Success_NotFound) {
@@ -1043,17 +1043,17 @@ js::RegExpMatcherRaw(JSContext* cx, Hand
* This code is inlined in CodeGenerator.cpp generateRegExpSearcherStub,
* changes to this code need to get reflected in there too.
*/
static bool
RegExpSearcherImpl(JSContext* cx, HandleObject regexp, HandleString string, int32_t lastIndex,
int32_t* result)
{
/* Execute regular expression and gather matches. */
- ScopedMatchPairs matches(&cx->tempLifoAlloc());
+ VectorMatchPairs matches;
/* Steps 3, 9-14, except 12.a.i, 12.c.i.1. */
RegExpRunStatus status = ExecuteRegExp(cx, regexp, string, lastIndex, &matches, nullptr);
if (status == RegExpRunStatus_Error)
return false;
/* Steps 12.a, 12.c. */
if (status == RegExpRunStatus_Success_NotFound) {
--- a/js/src/vm/MatchPairs.h
+++ b/js/src/vm/MatchPairs.h
@@ -46,17 +46,18 @@ struct MatchPair
inline bool check() const {
MOZ_ASSERT(limit >= start);
MOZ_ASSERT_IF(start < 0, start == -1);
MOZ_ASSERT_IF(limit < 0, limit == -1);
return true;
}
};
-/* Base class for RegExp execution output. */
+// MachPairs is used as base class for VectorMatchPairs but can also be
+// stack-allocated (without a Vector) in JIT code.
class MatchPairs
{
protected:
/* Length of pairs_. */
uint32_t pairCount_;
/* Raw pointer into an allocated MatchPair buffer. */
MatchPair* pairs_;
@@ -67,20 +68,16 @@ class MatchPairs
: pairCount_(0), pairs_(nullptr)
{ }
protected:
/* Functions used by friend classes. */
friend class RegExpShared;
friend class RegExpStatics;
- /* MatchPair buffer allocator: set pairs_ and pairCount_. */
- virtual bool allocOrExpandArray(size_t pairCount) = 0;
-
- bool initArrayFrom(MatchPairs& copyFrom);
void forgetArray() { pairs_ = nullptr; }
void checkAgainst(size_t inputLength) {
#ifdef DEBUG
for (size_t i = 0; i < pairCount_; i++) {
const MatchPair& p = (*this)[i];
MOZ_ASSERT(p.check());
if (p.isUndefined())
@@ -109,44 +106,25 @@ class MatchPairs
return pairs_[i];
}
MatchPair& operator[](size_t i) {
MOZ_ASSERT(i < pairCount_);
return pairs_[i];
}
};
-/* MatchPairs allocated into temporary storage, removed when out of scope. */
-class ScopedMatchPairs : public MatchPairs
-{
- LifoAllocScope lifoScope_;
-
- public:
- /* Constructs an implicit LifoAllocScope. */
- explicit ScopedMatchPairs(LifoAlloc* lifoAlloc)
- : lifoScope_(lifoAlloc)
- { }
-
- protected:
- bool allocOrExpandArray(size_t pairCount) override;
-};
-
-/*
- * MatchPairs allocated into permanent storage, for RegExpStatics.
- * The Vector of MatchPairs is reusable by Vector expansion.
- */
class VectorMatchPairs : public MatchPairs
{
Vector<MatchPair, 10, SystemAllocPolicy> vec_;
- public:
- VectorMatchPairs() {
- vec_.clear();
- }
+ protected:
+ friend class RegExpShared;
+ friend class RegExpStatics;
- protected:
- friend class RegExpStatics;
- bool allocOrExpandArray(size_t pairCount) override;
+ /* MatchPair buffer allocator: set pairs_ and pairCount_. */
+ bool allocOrExpandArray(size_t pairCount);
+
+ bool initArrayFrom(VectorMatchPairs& copyFrom);
};
} /* namespace js */
#endif /* vm_MatchPairs_h */
--- a/js/src/vm/RegExpObject.cpp
+++ b/js/src/vm/RegExpObject.cpp
@@ -65,51 +65,32 @@ js::RegExpAlloc(JSContext* cx, NewObject
RegExpObject::lastIndexSlot());
return regexp;
}
/* MatchPairs */
bool
-MatchPairs::initArrayFrom(MatchPairs& copyFrom)
+VectorMatchPairs::initArrayFrom(VectorMatchPairs& copyFrom)
{
MOZ_ASSERT(copyFrom.pairCount() > 0);
if (!allocOrExpandArray(copyFrom.pairCount()))
return false;
PodCopy(pairs_, copyFrom.pairs_, pairCount_);
return true;
}
bool
-ScopedMatchPairs::allocOrExpandArray(size_t pairCount)
-{
- /* Array expansion is forbidden, but array reuse is acceptable. */
- if (pairCount_) {
- MOZ_ASSERT(pairs_);
- MOZ_ASSERT(pairCount_ == pairCount);
- return true;
- }
-
- MOZ_ASSERT(!pairs_);
- pairs_ = (MatchPair*)lifoScope_.alloc().alloc(sizeof(MatchPair) * pairCount);
- if (!pairs_)
- return false;
-
- pairCount_ = pairCount;
- return true;
-}
-
-bool
VectorMatchPairs::allocOrExpandArray(size_t pairCount)
{
- if (!vec_.resizeUninitialized(sizeof(MatchPair) * pairCount))
+ if (!vec_.resizeUninitialized(pairCount))
return false;
pairs_ = &vec_[0];
pairCount_ = pairCount;
return true;
}
/* RegExpObject */
@@ -1077,17 +1058,17 @@ RegExpShared::compileIfNecessary(JSConte
{
if (re->isCompiled(mode, input->hasLatin1Chars(), force))
return true;
return compile(cx, re, input, mode, force);
}
/* static */ RegExpRunStatus
RegExpShared::execute(JSContext* cx, MutableHandleRegExpShared re, HandleLinearString input,
- size_t start, MatchPairs* matches, size_t* endIndex)
+ size_t start, VectorMatchPairs* matches, size_t* endIndex)
{
MOZ_ASSERT_IF(matches, !endIndex);
MOZ_ASSERT_IF(!matches, endIndex);
TraceLoggerThread* logger = TraceLoggerForCurrentThread(cx);
CompilationMode mode = matches ? Normal : MatchOnly;
/* Compile the code at point-of-use. */
--- a/js/src/vm/RegExpShared.h
+++ b/js/src/vm/RegExpShared.h
@@ -23,20 +23,20 @@
#include "js/UbiNode.h"
#include "js/Vector.h"
#include "vm/ArrayObject.h"
#include "vm/JSAtom.h"
namespace js {
class ArrayObject;
-class MatchPairs;
class RegExpCompartment;
class RegExpShared;
class RegExpStatics;
+class VectorMatchPairs;
using RootedRegExpShared = JS::Rooted<RegExpShared*>;
using HandleRegExpShared = JS::Handle<RegExpShared*>;
using MutableHandleRegExpShared = JS::MutableHandle<RegExpShared*>;
enum RegExpFlag : uint8_t
{
IgnoreCaseFlag = 0x01,
@@ -154,17 +154,17 @@ class RegExpShared : public gc::TenuredC
public:
~RegExpShared() = delete;
// Execute this RegExp on input starting from searchIndex, filling in
// matches if specified and otherwise only determining if there is a match.
static RegExpRunStatus execute(JSContext* cx, MutableHandleRegExpShared res,
HandleLinearString input, size_t searchIndex,
- MatchPairs* matches, size_t* endIndex);
+ VectorMatchPairs* matches, size_t* endIndex);
// Register a table with this RegExpShared, and take ownership.
bool addTable(JitCodeTable table) {
return tables.append(Move(table));
}
/* Accessors */
--- a/js/src/vm/RegExpStatics.h
+++ b/js/src/vm/RegExpStatics.h
@@ -57,17 +57,18 @@ class RegExpStatics
*/
bool makeMatch(JSContext* cx, size_t pairNum, MutableHandleValue out);
bool createDependent(JSContext* cx, size_t start, size_t end, MutableHandleValue out);
public:
/* Mutators. */
inline void updateLazily(JSContext* cx, JSLinearString* input,
RegExpShared* shared, size_t lastIndex);
- inline bool updateFromMatchPairs(JSContext* cx, JSLinearString* input, MatchPairs& newPairs);
+ inline bool updateFromMatchPairs(JSContext* cx, JSLinearString* input,
+ VectorMatchPairs& newPairs);
inline void clear();
/* Corresponds to JSAPI functionality to set the pending RegExp input. */
void reset(JSString* newInput) {
clear();
pendingInput = newInput;
checkInvariants();
@@ -246,17 +247,18 @@ RegExpStatics::updateLazily(JSContext* c
lazySource = shared->source;
lazyFlags = shared->flags;
lazyIndex = lastIndex;
pendingLazyEvaluation = 1;
}
inline bool
-RegExpStatics::updateFromMatchPairs(JSContext* cx, JSLinearString* input, MatchPairs& newPairs)
+RegExpStatics::updateFromMatchPairs(JSContext* cx, JSLinearString* input,
+ VectorMatchPairs& newPairs)
{
MOZ_ASSERT(input);
/* Unset all lazy state. */
pendingLazyEvaluation = false;
this->lazySource = nullptr;
this->lazyIndex = size_t(-1);