Changeset 186920 in webkit

Timestamp:

Jul 16, 2015, 7:27:22 PM (10 years ago)

Author:

mark.lam@apple.com

Message:

RegExp::match() should set m_state to ByteCode if compilation fails.
​https://bugs.webkit.org/show_bug.cgi?id=147023

Reviewed by Michael Saboff.

A RegExp has a YarrCodeBlock that has 4 MacroAssemblerCodeRefs for compiled code.
If one of these compilations succeeds, RegExp::m_state will be set to JITCode.
Subsequently, if RegExp tries to compile another one of these but fails, m_state
will be left untouched i.e. it still says JITCode. As a result, when
RegExp::match() later tries to execute the non-existant compiled code, it will
crash.

The fix is to downgrade m_state to ByteCode if RegExp ever fails to compile.
This failure should be rare. We'll do the minimal work here to fix the issue and
keep an eye on the perf bots. If perf regresses, we can do some optimization work then.

This issue is difficult to test for since it either requires a low memory condition
to trigger a failed RegExp compilation at the right moment, or for the RegExp to
succeed compilation in the MatchedOnly mode but fail in IncludeSubpatterns mode.
Instead, I manually tested it by instrumenting RegExp::compile() to fail once in every
10 compilation attempts.

• runtime/RegExp.cpp:

(JSC::RegExp::compile):
(JSC::RegExp::compileMatchOnly):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/RegExp.cpp (modified) (4 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-07-16  Mark Lam  <mark.lam@apple.com>
+
+        RegExp::match() should set m_state to ByteCode if compilation fails.
+        https://bugs.webkit.org/show_bug.cgi?id=147023
+
+        Reviewed by Michael Saboff.
+
+        A RegExp has a YarrCodeBlock that has 4 MacroAssemblerCodeRefs for compiled code.
+        If one of these compilations succeeds, RegExp::m_state will be set to JITCode.
+        Subsequently, if RegExp tries to compile another one of these but fails, m_state
+        will be left untouched i.e. it still says JITCode.  As a result, when
+        RegExp::match() later tries to execute the non-existant compiled code, it will
+        crash.
+
+        The fix is to downgrade m_state to ByteCode if RegExp ever fails to compile.
+        This failure should be rare.  We'll do the minimal work here to fix the issue and
+        keep an eye on the perf bots.  If perf regresses, we can do some optimization work then.
+
+        This issue is difficult to test for since it either requires a low memory condition
+        to trigger a failed RegExp compilation at the right moment, or for the RegExp to
+        succeed compilation in the MatchedOnly mode but fail in IncludeSubpatterns mode.
+        Instead, I manually tested it by instrumenting RegExp::compile() to fail once in every
+        10 compilation attempts.
+
+        * runtime/RegExp.cpp:
+        (JSC::RegExp::compile):
+        (JSC::RegExp::compileMatchOnly):
+
 2015-07-15  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/JavaScriptCore/runtime/RegExp.cpp

@@ -290 +290 @@
     if (!pattern.m_containsBackreferences && !pattern.containsUnsignedLengthPattern() && vm->canUseRegExpJIT()) {
         Yarr::jitCompile(pattern, charSize, vm, m_regExpJITCode);
-#if ENABLE(YARR_JIT_DEBUG)
-        if (!m_regExpJITCode.isFallBack())
-            m_state = JITCode;
-        else
-            m_state = ByteCode;
-#else
         if (!m_regExpJITCode.isFallBack()) {
             m_state = JITCode;
             return;
         }
-#endif
     }
 #else
@@ -306 +299 @@
 #endif
 
+    m_state = ByteCode;
     m_regExpBytecode = Yarr::byteCompile(pattern, &vm->m_regExpAllocator);
 }
@@ -415 +409 @@
     if (!pattern.m_containsBackreferences && !pattern.containsUnsignedLengthPattern() && vm->canUseRegExpJIT()) {
         Yarr::jitCompile(pattern, charSize, vm, m_regExpJITCode, Yarr::MatchOnly);
-#if ENABLE(YARR_JIT_DEBUG)
-        if (!m_regExpJITCode.isFallBack())
-            m_state = JITCode;
-        else
-            m_state = ByteCode;
-#else
         if (!m_regExpJITCode.isFallBack()) {
             m_state = JITCode;
             return;
         }
-#endif
     }
 #else
@@ -431 +418 @@
 #endif
 
+    m_state = ByteCode;
     m_regExpBytecode = Yarr::byteCompile(pattern, &vm->m_regExpAllocator);
 }