Bug 1438310 - Remove ScopedMatchPairs and devirtualize MatchPairs to avoid triggering undefined behavior. r=jwalden
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 01 Mar 2018 21:32:45 +0100
changeset 762214 41857846ad591e464cb690413edeab76cceac89e
parent 762213 35e4f2c811209ec8d5a27757e0cc3aedb26a1ec1
child 762215 e3d9710edbd780ce29c8635db6d8bd63f22fff3c
push id101102
push userbmo:jkt@mozilla.com
push dateThu, 01 Mar 2018 22:11:20 +0000
reviewersjwalden
bugs1438310
milestone60.0a1
Bug 1438310 - Remove ScopedMatchPairs and devirtualize MatchPairs to avoid triggering undefined behavior. r=jwalden
js/src/builtin/RegExp.cpp
js/src/vm/MatchPairs.h
js/src/vm/RegExpObject.cpp
js/src/vm/RegExpShared.h
js/src/vm/RegExpStatics.h
--- 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);