Bug 1640479: Don't set kind if initializeNamedCaptures fails r=mgaudet
☠☠ backed out by 9a6631a07b75 ☠ ☠
authorIain Ireland <iireland@mozilla.com>
Wed, 27 May 2020 19:20:26 +0000
changeset 532559 2a44daf61d7f452aaf55f47c2962f8c2ed625704
parent 532558 a453b998c4f2fced09f98a372dbe97aff1b50499
child 532560 e31d66f10e6d162f2c83960a41fd484f64e2638f
push id37456
push userrmaries@mozilla.com
push dateThu, 28 May 2020 03:25:13 +0000
treeherdermozilla-central@cfa4bd8e6f78 [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,13 @@
+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.