Bug 1640479: Don't set kind if initializeNamedCaptures fails r=mgaudet
authorIain Ireland <iireland@mozilla.com>
Wed, 27 May 2020 20:33:39 +0000
changeset 596590 13469bdc290b5dbee625f6e9de5650a980af6bc9
parent 596589 6522d475c2c9dd80be03cf8656e3cd2cfb6d1049
child 596591 b11a4969f535171c1e416fa79ef2626f766fd542
push id13186
push userffxbld-merge
push dateMon, 01 Jun 2020 09:52:46 +0000
treeherdermozilla-beta@3e7c70a1e4a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1640479
milestone78.0a1
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
Bug 1640479: Don't set kind if initializeNamedCaptures fails r=mgaudet If we throw an OOM in initializeNamedCaptures for a RegExpShared, we will set kind to RegExp, but not initialize the named captures data. If we recover from the OOM and then execute the same regexp, the cached RegExpShared will not be reparsed, and we won't create named captures for it. The fix is to reorder CompilePattern so that we only change the state of the RegExpShared after all of the initialization has succeeded. initializeNamedCaptures already avoids this problem by saving the updates until the end. Differential Revision: https://phabricator.services.mozilla.com/D76957
js/src/jit-test/tests/regexp/bug1640479.js
js/src/new-regexp/RegExpAPI.cpp
js/src/vm/RegExpObject.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/regexp/bug1640479.js
@@ -0,0 +1,15 @@
+// |jit-test| skip-if: !('oomTest' in this)
+
+var failures = 0;
+var i = 0;
+
+function foo() {
+    var e;
+    var r = RegExp("(?<_" + (i++) + "a>)");
+    try { e = r.exec("a"); } catch {}
+    e = r.exec("a");
+    if (e.groups === undefined) failures++;
+}
+
+oomTest(foo);
+assertEq(failures, 0);
--- a/js/src/new-regexp/RegExpAPI.cpp
+++ b/js/src/new-regexp/RegExpAPI.cpp
@@ -475,26 +475,26 @@ bool CompilePattern(JSContext* cx, Mutab
         }
       }
       JS::AutoAssertNoGC nogc(cx);
       if (searchAtom && !UseBoyerMoore(searchAtom, nogc)) {
         re->useAtomMatch(searchAtom);
         return true;
       }
     }
-    // Add one to account for the whole-match capture
-    uint32_t pairCount = data.capture_count + 1;
-    re->useRegExpMatch(pairCount);
-
     if (!data.capture_name_map.is_null()) {
       RootedNativeObject namedCaptures(cx, data.capture_name_map->inner());
       if (!RegExpShared::initializeNamedCaptures(cx, re, namedCaptures)) {
         return false;
       }
     }
+    // All fallible initialization has succeeded, so we can change state.
+    // Add one to capture_count to account for the whole-match capture.
+    uint32_t pairCount = data.capture_count + 1;
+    re->useRegExpMatch(pairCount);
   }
 
   MOZ_ASSERT(re->kind() == RegExpShared::Kind::RegExp);
 
   HandleScope handleScope(cx->isolate);
   RegExpCompiler compiler(cx->isolate, &zone, data.capture_count,
                           input->hasLatin1Chars());
 
--- a/js/src/vm/RegExpObject.cpp
+++ b/js/src/vm/RegExpObject.cpp
@@ -1132,17 +1132,16 @@ void RegExpShared::useRegExpMatch(size_t
   kind_ = RegExpShared::Kind::RegExp;
   pairCount_ = pairCount;
   ticks_ = jit::JitOptions.regexpWarmUpThreshold;
 }
 
 /* static */
 bool RegExpShared::initializeNamedCaptures(JSContext* cx, HandleRegExpShared re,
                                            HandleNativeObject namedCaptures) {
-  MOZ_ASSERT(re->kind() == RegExpShared::Kind::RegExp);
   MOZ_ASSERT(!re->groupsTemplate_);
   MOZ_ASSERT(!re->namedCaptureIndices_);
 
   // The irregexp parser returns named capture information in the form
   // of an ArrayObject, where even elements store the capture name and
   // odd elements store the corresponding capture index. We create a
   // template object with a property for each capture name, and store
   // the capture indices as a heap-allocated array.