Changeset 259092 in webkit for trunk/Source/JavaScriptCore/yarr/YarrPattern.cpp

Timestamp:

Mar 26, 2020, 4:27:57 PM (5 years ago)

Author:

[email protected]

Message:

Refactor YARR Stack Overflow Checks
​https://bugs.webkit.org/show_bug.cgi?id=209435
rdar://problem/58988252

Reviewed by Mark Lam.

JSTests:

Added a new test and removed a now obsolete test.

• stress/regexp-compile-oom.js: Removed because the test is no longer valid.

Previously when therer where different stack check mechanisims we failed different.
This test was based on the different failure modes. With these changes, most of
the contain subtests no longer throw as this test expects.

• stress/regexp-huge-oom.js: Added.

(shouldBe):
(shouldThrow):

Source/JavaScriptCore:

Refactored stack checks in YARR code including adding a stack check to the YARR JIT'ed code.
The C++ code including the parser, byte code compiler and interpreter now all use StackCheck.
The JIT'ed code needs a stack limit passed via a parameter since the JIT'ed code can be
called from the compiler thread when compiling DFG / FTL code.

Instead of adding a new parameter, consolidated the two pattern context buffer values, buffer
pointer and size, with the new stack limit into a new MatchingContextHolder, an RAII object.
The MatchingContextHolder constructor uses either the VM stack limit or the current thread's
stack limit depending on how it is called.

• runtime/RegExp.cpp:

(JSC::RegExp::finishCreation):
(JSC::RegExp::byteCodeCompileIfNecessary):
(JSC::RegExp::compile):
(JSC::RegExp::matchConcurrently):
(JSC::RegExp::compileMatchOnly):

• runtime/RegExp.h:
• runtime/RegExpInlines.h:

(JSC::RegExp::matchInline):
(JSC::PatternContextBufferHolder::PatternContextBufferHolder): Deleted.
(JSC::PatternContextBufferHolder::~PatternContextBufferHolder): Deleted.
(JSC::PatternContextBufferHolder::buffer): Deleted.
(JSC::PatternContextBufferHolder::size): Deleted.
(): Deleted.

• yarr/Yarr.h:
• yarr/YarrInterpreter.cpp:

(JSC::Yarr::Interpreter::matchDisjunction):
(JSC::Yarr::Interpreter::isSafeToRecurse):

• yarr/YarrJIT.cpp:

(JSC::Yarr::MatchingContextHolder::MatchingContextHolder):
(JSC::Yarr::MatchingContextHolder::~MatchingContextHolder):
(JSC::Yarr::YarrGenerator::initParenContextFreeList):
(JSC::Yarr::YarrGenerator::alignCallFrameSizeInBytes):
(JSC::Yarr::YarrGenerator::compile):
(JSC::Yarr::YarrGenerator::initCallFrame): Deleted.

• yarr/YarrJIT.h:

(JSC::Yarr::MatchingContextHolder::offsetOfStackLimit):
(JSC::Yarr::MatchingContextHolder::offsetOfPatternContextBuffer):
(JSC::Yarr::MatchingContextHolder::offsetOfPatternContextBufferSize):
(JSC::Yarr::YarrCodeBlock::execute):

• yarr/YarrPattern.cpp:

(JSC::Yarr::YarrPatternConstructor::YarrPatternConstructor):
(JSC::Yarr::YarrPatternConstructor::isSafeToRecurse):
(JSC::Yarr::YarrPattern::compile):
(JSC::Yarr::YarrPattern::YarrPattern):
(JSC::Yarr::YarrPatternConstructor::isSafeToRecurse const): Deleted.

• yarr/YarrPattern.h:

LayoutTests:

Updated test for improved stack overflow checking.

• js/script-tests/stack-overflow-regexp.js:

(shouldThrow.recursiveCall):
(shouldThrow):
(recursiveCall):

• js/stack-overflow-regexp-expected.txt:

File:

• trunk/Source/JavaScriptCore/yarr/YarrPattern.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/yarr/YarrPattern.cpp

@@ -34 +34 @@
 #include <wtf/DataLog.h>
 #include <wtf/Optional.h>
-#include <wtf/StackPointer.h>
+#include <wtf/StackCheck.h>
 #include <wtf/Threading.h>
 #include <wtf/Vector.h>
@@ -437 +437 @@
 class YarrPatternConstructor {
 public:
-    YarrPatternConstructor(YarrPattern& pattern, void* stackLimit)
+    YarrPatternConstructor(YarrPattern& pattern)
         : m_pattern(pattern)
         , m_characterClassConstructor(pattern.ignoreCase(), pattern.unicode() ? CanonicalMode::Unicode : CanonicalMode::UCS2)
-        , m_stackLimit(stackLimit)
     {
         auto body = makeUnique<PatternDisjunction>();
@@ -1102 +1101 @@
 
 private:
-    bool isSafeToRecurse() const
-    {
-        if (!m_stackLimit)
-            return true;
-        int8_t* curr = reinterpret_cast<int8_t*>(currentStackPointer());
-        int8_t* limit = reinterpret_cast<int8_t*>(m_stackLimit);
-        return curr >= limit;
-    }
+    inline bool isSafeToRecurse() { return m_stackCheck.isSafeToRecurse(); }
 
     YarrPattern& m_pattern;
     PatternAlternative* m_alternative;
     CharacterClassConstructor m_characterClassConstructor;
-    void* m_stackLimit;
+    StackCheck m_stackCheck;
     ErrorCode m_error { ErrorCode::NoError };
     bool m_invertCharacterClass;
@@ -1120 +1112 @@
 };
 
-ErrorCode YarrPattern::compile(const String& patternString, void* stackLimit)
+ErrorCode YarrPattern::compile(const String& patternString)
 {
-    YarrPatternConstructor constructor(*this, stackLimit);
+    YarrPatternConstructor constructor(*this);
 
     {
@@ -1149 +1141 @@
 }
 
-YarrPattern::YarrPattern(const String& pattern, OptionSet<Flags> flags, ErrorCode& error, void* stackLimit)
+YarrPattern::YarrPattern(const String& pattern, OptionSet<Flags> flags, ErrorCode& error)
     : m_containsBackreferences(false)
     , m_containsBOL(false)
@@ -1158 +1150 @@
 {
     ASSERT(m_flags != Flags::DeletedValue);
-    error = compile(pattern, stackLimit);
+    error = compile(pattern);
 }