Bug 1691184: Recompile if necessary before retrying interrupted regexp r=mgaudet
☠☠ backed out by 2f2fe1dadc7e ☠ ☠
authorIain Ireland <iireland@mozilla.com>
Wed, 10 Feb 2021 16:45:51 +0000
changeset 566852 e3a59a1dc7ca7f173667e390ef78433abcc8a95d
parent 566851 0f27a332d47fb58f2bfe334e136f71a608e2816a
child 566853 8ef970eae3d15bd78af6458148060b3a56485407
push id38191
push userbtara@mozilla.com
push dateThu, 11 Feb 2021 05:02:45 +0000
treeherdermozilla-central@5cbcb80f72bd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1691184
milestone87.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 1691184: Recompile if necessary before retrying interrupted regexp r=mgaudet If an interrupt occurs during regexp execution, we return up the stack to RegExpShared::execute to handle it, then try again. Normally it's safe (if slow) to GC and discard jitcode at this point, because we can fall back to interpreted bytecode (which is not discarded). However, if the input string is long enough, then we [jump straight to compilation without producing bytecode](https://searchfox.org/mozilla-central/rev/7067bbd8194f4346ec59d77c33cd88f06763e090/js/src/vm/RegExpObject.cpp#590-596). In that case, when we resume, we will have neither bytecode nor jitcode, and end up dereferencing a null pointer. The fix is to recompile after handling the interrupt. In addition to fixing the crash, forcing compilation here should improve our chance of eventual success (compared to resuming in the regexp interpreter). Differential Revision: https://phabricator.services.mozilla.com/D104479
js/src/irregexp/RegExpAPI.cpp
js/src/jit-test/tests/regexp/bug1691184.js
js/src/vm/RegExpObject.cpp
--- a/js/src/irregexp/RegExpAPI.cpp
+++ b/js/src/irregexp/RegExpAPI.cpp
@@ -677,16 +677,18 @@ RegExpRunStatus ExecuteRaw(jit::JitCode*
     JS::AutoSuppressGCAnalysis nogc;
     return (RegExpRunStatus)CALL_GENERATED_1(function, &data);
   }
 }
 
 RegExpRunStatus Interpret(JSContext* cx, MutableHandleRegExpShared re,
                           HandleLinearString input, size_t startIndex,
                           VectorMatchPairs* matches) {
+  MOZ_ASSERT(re->getByteCode(input->hasLatin1Chars()));
+
   HandleScope handleScope(cx->isolate);
   V8HandleRegExp wrappedRegExp(v8::internal::JSRegExp(re), cx->isolate);
   V8HandleString wrappedInput(v8::internal::String(input), cx->isolate);
 
   static_assert(RegExpRunStatus_Error ==
                 v8::internal::RegExp::kInternalRegExpException);
   static_assert(RegExpRunStatus_Success ==
                 v8::internal::RegExp::kInternalRegExpSuccess);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/regexp/bug1691184.js
@@ -0,0 +1,15 @@
+var r = /^b/;
+
+// Create a long enough input string to time out.
+var s = "a";
+try {
+    while (true) {
+	s += s;
+    }
+} catch {}
+
+startgc(7,'shrinking');
+
+// Time out during slow match.
+timeout(1);
+assertEq(s.match(r), null);
--- a/js/src/vm/RegExpObject.cpp
+++ b/js/src/vm/RegExpObject.cpp
@@ -663,16 +663,24 @@ RegExpRunStatus RegExpShared::execute(JS
        * third case, we want to handle the interrupt and try again.
        * We cap the number of times we will retry.
        */
       if (cx->hasAnyPendingInterrupt()) {
         if (!CheckForInterrupt(cx)) {
           return RegExpRunStatus_Error;
         }
         if (interruptRetries++ < maxInterruptRetries) {
+          // The initial execution may have been interpreted, or the
+          // interrupt may have triggered a GC that discarded jitcode.
+          // To maximize the chance of succeeding before being
+          // interrupted again, we want to ensure we are compiled.
+          if (!compileIfNecessary(cx, re, input,
+                                  RegExpShared::CodeKind::Jitcode)) {
+            return RegExpRunStatus_Error;
+          }
           continue;
         }
       }
       // If we have run out of retries, this regexp takes too long to execute.
       ReportOverRecursed(cx);
       return RegExpRunStatus_Error;
     }