Fix regexp match pair end-index == -1 assumption. (r=dmandelin, a=blocker b=605754)
authorChris Leary <cdleary@mozilla.com>
Wed, 01 Dec 2010 16:33:49 -0800
changeset 58509 03ff7e80545f6713625571dc0442027fe9e193bb
parent 58508 5cacd338d84fa2ac583e4a1725a5d705d6f22391
child 58510 a93d62654d2d43b5176d431768dde5212772db46
push id17329
push userdmandelin@mozilla.com
push dateThu, 02 Dec 2010 19:57:29 +0000
treeherdermozilla-central@be1f9b4e522e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdmandelin, blocker
bugs605754
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Fix regexp match pair end-index == -1 assumption. (r=dmandelin, a=blocker b=605754)
js/src/jit-test/tests/basic/bug605754-regexp.js
js/src/jsregexp.h
js/src/jsregexpinlines.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug605754-regexp.js
@@ -0,0 +1,2 @@
+var result = "foobarbaz".replace(/foo(bar)?bar/, "$1");
+assertEq(result, "baz");
--- a/js/src/jsregexp.h
+++ b/js/src/jsregexp.h
@@ -119,29 +119,44 @@ class RegExpStatics
             JS_ASSERT(!matchPairsInput);
             return;
         }
 
         /* Pair count is non-zero, so there must be match pairs input. */
         JS_ASSERT(matchPairsInput);
         size_t mpiLen = matchPairsInput->length();
 
+        /* Both members of the first pair must be non-negative. */
         JS_ASSERT(pairIsPresent(0));
+        JS_ASSERT(get(0, 1) >= 0);
 
         /* Present pairs must be valid. */
         for (size_t i = 0; i < pairCount(); ++i) {
             if (!pairIsPresent(i))
                 continue;
             int start = get(i, 0);
             int limit = get(i, 1);
             JS_ASSERT(mpiLen >= size_t(limit) && limit >= start && start >= 0);
         }
 #endif
     }
 
+    bool pairIsPresent(size_t pairNum) const {
+        return getCrash(pairNum, 0) >= 0;
+    }
+
+    /* Precondition: paren is present. */
+    size_t getParenLength(size_t parenNum) const {
+        size_t pairNum = parenNum + 1;
+        if (pairCountCrash() <= pairNum)
+            return 0;
+        JS_CRASH_UNLESS(pairIsPresent(pairNum));
+        return getCrash(pairNum, 1) - getCrash(pairNum, 0);
+    }
+
     int get(size_t pairNum, bool which) const {
         JS_ASSERT(pairNum < pairCount());
         return matchPairs[2 * pairNum + which];
     }
 
     int getCrash(size_t pairNum, bool which) const {
          JS_CRASH_UNLESS(pairNum < pairCountCrash());
          return get(pairNum, which);
@@ -163,20 +178,16 @@ class RegExpStatics
 
   public:
     RegExpStatics() : bufferLink(NULL), copied(false) { clear(); }
 
     static RegExpStatics *extractFrom(JSObject *global);
 
     /* Mutators. */
 
-    /* 
-     * The inputOffset parameter is added to the present (i.e. non-negative) match items to emulate
-     * sticky mode.
-     */
     bool updateFromMatch(JSContext *cx, JSString *input, int *buf, size_t matchItemCount) {
         aboutToWrite();
         pendingInput = input;
 
         if (!matchPairs.resizeUninitialized(matchItemCount)) {
             js_ReportOutOfMemory(cx);
             return false;
         }
@@ -199,18 +210,16 @@ class RegExpStatics
     void clear() {
         aboutToWrite();
         flags = 0;
         pendingInput = NULL;
         matchPairsInput = NULL;
         matchPairs.clear();
     }
 
-    bool pairIsPresent(size_t pairNum) { return get(0, 0) != -1; }
-
     /* Corresponds to JSAPI functionality to set the pending RegExp input. */
     void reset(JSString *newInput, bool newMultiline) {
         aboutToWrite();
         clear();
         pendingInput = newInput;
         setMultiline(newMultiline);
         checkInvariants();
     }
@@ -233,53 +242,51 @@ class RegExpStatics
     }
 
     size_t matchLimit() const {
         int limit = get(0, 1);
         JS_ASSERT(size_t(limit) >= matchStart() && limit >= 0);
         return size_t(limit);
     }
 
+    /* Returns whether results for a non-empty match are present. */
     bool matched() const {
         JS_ASSERT(pairCount() > 0);
+        JS_ASSERT_IF(get(0, 1) == -1, get(1, 1) == -1);
         return get(0, 1) - get(0, 0) > 0;
     }
 
     size_t getParenCount() const {
+        /* The first pair is the whole match. */
         JS_ASSERT(pairCount() > 0);
         return pairCount() - 1;
     }
 
     void mark(JSTracer *trc) const {
         if (pendingInput)
             JS_CALL_STRING_TRACER(trc, pendingInput, "res->pendingInput");
         if (matchPairsInput)
             JS_CALL_STRING_TRACER(trc, matchPairsInput, "res->matchPairsInput");
     }
 
-    size_t getParenLength(size_t parenNum) const {
-        if (pairCountCrash() <= parenNum + 1)
-            return 0;
-        return getCrash(parenNum + 1, 1) - getCrash(parenNum + 1, 0);
-    }
-
     /* Value creators. */
 
     bool createPendingInput(JSContext *cx, Value *out) const;
     bool createLastMatch(JSContext *cx, Value *out) const { return makeMatch(cx, 0, 0, out); }
     bool createLastParen(JSContext *cx, Value *out) const;
     bool createLeftContext(JSContext *cx, Value *out) const;
     bool createRightContext(JSContext *cx, Value *out) const;
 
     bool createParen(JSContext *cx, size_t parenNum, Value *out) const {
         return makeMatch(cx, (parenNum + 1) * 2, parenNum + 1, out);
     }
 
     /* Substring creators. */
 
+    /* @param num   Zero-indexed paren number (i.e. $1 is 0). */
     void getParen(size_t num, JSSubString *out) const;
     void getLastMatch(JSSubString *out) const;
     void getLastParen(JSSubString *out) const;
     void getLeftContext(JSSubString *out) const;
     void getRightContext(JSSubString *out) const;
 };
 
 class PreserveRegExpStatics
--- a/js/src/jsregexpinlines.h
+++ b/js/src/jsregexpinlines.h
@@ -260,17 +260,16 @@ RegExp::createResult(JSContext *cx, JSSt
             JS_ASSERT(start <= end);
             JS_ASSERT(unsigned(end) <= input->length());
             captured = js_NewDependentString(cx, input, start, end - start);
             if (!(captured && builder.append(i / 2, captured)))
                 return NULL;
         } else {
             /* Missing parenthesized match. */
             JS_ASSERT(i != 0); /* Since we had a match, first pair must be present. */
-            JS_ASSERT(start == end && end == -1);
             if (!builder.append(INT_TO_JSID(i / 2), UndefinedValue()))
                 return NULL;
         }
     }
 
     if (!builder.appendIndex(buf[0]) ||
         !builder.appendInput(input))
         return NULL;
@@ -577,21 +576,21 @@ RegExpStatics::createLastParen(JSContext
     if (pairCount() <= 1) {
         out->setString(cx->runtime->emptyString);
         return true;
     }
     size_t num = pairCount() - 1;
     int start = get(num, 0);
     int end = get(num, 1);
     if (start == -1) {
-        JS_ASSERT(end == -1);
         out->setString(cx->runtime->emptyString);
         return true;
     }
     JS_ASSERT(start >= 0 && end >= 0);
+    JS_ASSERT(end >= start);
     return createDependent(cx, start, end, out);
 }
 
 inline bool
 RegExpStatics::createLeftContext(JSContext *cx, Value *out) const
 {
     if (!pairCount()) {
         out->setString(cx->runtime->emptyString);
@@ -616,17 +615,22 @@ RegExpStatics::createRightContext(JSCont
         return true;
     }
     return createDependent(cx, matchPairs[1], matchPairsInput->length(), out);
 }
 
 inline void
 RegExpStatics::getParen(size_t num, JSSubString *out) const
 {
-    out->chars = matchPairsInput->chars() + getCrash(num + 1, 0);
+    size_t pairNum = num + 1;
+    if (!pairIsPresent(pairNum)) {
+        *out = js_EmptySubString;
+        return;
+    }
+    out->chars = matchPairsInput->chars() + getCrash(pairNum, 0);
     out->length = getParenLength(num);
 }
 
 inline void
 RegExpStatics::getLastMatch(JSSubString *out) const
 {
     if (!pairCountCrash()) {
         *out = js_EmptySubString;
@@ -636,24 +640,22 @@ RegExpStatics::getLastMatch(JSSubString 
     out->chars = matchPairsInput->chars() + getCrash(0, 0);
     JS_CRASH_UNLESS(getCrash(0, 1) >= getCrash(0, 0));
     out->length = get(0, 1) - get(0, 0);
 }
 
 inline void
 RegExpStatics::getLastParen(JSSubString *out) const
 {
-    if (!pairCountCrash()) {
+    size_t parenCount = getParenCount();
+    if (!parenCount) {
         *out = js_EmptySubString;
         return;
     }
-    size_t num = pairCount() - 1;
-    out->chars = matchPairsInput->chars() + getCrash(num, 0);
-    JS_CRASH_UNLESS(getCrash(num, 1) >= get(num, 0));
-    out->length = get(num, 1) - get(num, 0);
+    getParen(parenCount - 1, out);
 }
 
 inline void
 RegExpStatics::getLeftContext(JSSubString *out) const
 {
     if (!pairCountCrash()) {
         *out = js_EmptySubString;
         return;