Skip to content

Commit

Permalink
Merge r228481 - REGRESSION(225695) : com.apple.WebKit.WebContent at c…
Browse files Browse the repository at this point in the history 
…om.apple.JavaScriptCore: JSC::RegExp::match + 630 :: stack overflow

https://bugs.webkit.org/show_bug.cgi?id=182705

Reviewed by Mark Lam.

Source/JavaScriptCore:

Moved the pattern context buffer used by YARR JIT'ed code from a stack local to a lazily allocated
buffer on the VM.  Exposed when the buffer is needed to reduce likelihood that we'd allocated it.
Guarded use of the buffer with a lock since the DFG compiler may call into YARR JIT'ed code on a
compilation thread.

* runtime/RegExpInlines.h:
(JSC::RegExp::matchInline):
* runtime/VM.cpp:
(JSC::VM::~VM):
(JSC::VM::acquireRegExpPatternContexBuffer):
(JSC::VM::releaseRegExpPatternContexBuffer):
* runtime/VM.h:
* yarr/YarrJIT.cpp:
(JSC::Yarr::YarrGenerator::generate):
(JSC::Yarr::YarrGenerator::backtrack):
(JSC::Yarr::YarrGenerator::opCompileParenthesesSubpattern):
(JSC::Yarr::YarrGenerator::generateEnter):
(JSC::Yarr::YarrGenerator::generateReturn):
(JSC::Yarr::YarrGenerator::YarrGenerator):
(JSC::Yarr::YarrGenerator::compile):
* yarr/YarrJIT.h:
(JSC::Yarr::YarrCodeBlock::usesPatternContextBuffer):
(JSC::Yarr::YarrCodeBlock::setUsesPaternContextBuffer):

Source/WTF:

Moved the setting of ENABLE_YARR_JIT_ALL_PARENS_EXPRESSIONS to Platform.h since more than just the YARR
code needs to know if that feature is enabled.

* wtf/Platform.h:
  • Loading branch information
@msaboff @carlosgcampos
msaboff authored and carlosgcampos committed Feb 20, 2018
1 parent ad85e07 commit f632575
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 37 deletions.
31 changes: 31 additions & 0 deletions Source/JavaScriptCore/ChangeLog
View file
@@ -1,3 +1,34 @@
2018-02-14 Michael Saboff <msaboff@apple.com>

REGRESSION(225695) : com.apple.WebKit.WebContent at com.apple.JavaScriptCore: JSC::RegExp::match + 630 :: stack overflow
https://bugs.webkit.org/show_bug.cgi?id=182705

Reviewed by Mark Lam.

Moved the pattern context buffer used by YARR JIT'ed code from a stack local to a lazily allocated
buffer on the VM. Exposed when the buffer is needed to reduce likelihood that we'd allocated it.
Guarded use of the buffer with a lock since the DFG compiler may call into YARR JIT'ed code on a
compilation thread.

* runtime/RegExpInlines.h:
(JSC::RegExp::matchInline):
* runtime/VM.cpp:
(JSC::VM::~VM):
(JSC::VM::acquireRegExpPatternContexBuffer):
(JSC::VM::releaseRegExpPatternContexBuffer):
* runtime/VM.h:
* yarr/YarrJIT.cpp:
(JSC::Yarr::YarrGenerator::generate):
(JSC::Yarr::YarrGenerator::backtrack):
(JSC::Yarr::YarrGenerator::opCompileParenthesesSubpattern):
(JSC::Yarr::YarrGenerator::generateEnter):
(JSC::Yarr::YarrGenerator::generateReturn):
(JSC::Yarr::YarrGenerator::YarrGenerator):
(JSC::Yarr::YarrGenerator::compile):
* yarr/YarrJIT.h:
(JSC::Yarr::YarrCodeBlock::usesPatternContextBuffer):
(JSC::Yarr::YarrCodeBlock::setUsesPaternContextBuffer):

2018-02-13 Saam Barati <sbarati@apple.com>

putDirectIndexSlowOrBeyondVectorLength needs to convert to dictionary indexing mode always if attributes are present
Expand Down
77 changes: 60 additions & 17 deletions Source/JavaScriptCore/runtime/RegExpInlines.h
View file
Expand Up @@ -85,6 +85,39 @@ ALWAYS_INLINE bool RegExp::hasCodeFor(Yarr::YarrCharSize charSize)
return false;
}

#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
class PatternContextBufferHolder {
public:
PatternContextBufferHolder(VM& vm, bool needBuffer)
: m_vm(vm)
, m_needBuffer(needBuffer)
{
if (m_needBuffer) {
m_buffer = m_vm.acquireRegExpPatternContexBuffer();
m_size = VM::patternContextBufferSize;
} else {
m_buffer = nullptr;
m_size = 0;
}
}

~PatternContextBufferHolder()
{
if (m_needBuffer)
m_vm.releaseRegExpPatternContexBuffer();
}

void* buffer() { return m_buffer; }
unsigned size() { return m_size; }

private:
VM& m_vm;
bool m_needBuffer;
void* m_buffer;
unsigned m_size;
};
#endif

ALWAYS_INLINE void RegExp::compileIfNecessary(VM& vm, Yarr::YarrCharSize charSize)
{
if (hasCodeFor(charSize))
Expand All @@ -110,18 +143,23 @@ ALWAYS_INLINE int RegExp::matchInline(VM& vm, const String& s, unsigned startOff

int result;
#if ENABLE(YARR_JIT)
#ifdef JIT_ALL_PARENS_EXPRESSIONS
char patternContextBuffer[patternContextBufferSize];
#define EXTRA_JIT_PARAMS , patternContextBuffer, patternContextBufferSize
if (m_state == JITCode) {
{
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
PatternContextBufferHolder patternContextBufferHolder(vm, m_regExpJITCode.usesPatternContextBuffer());

#define EXTRA_JIT_PARAMS , patternContextBufferHolder.buffer(), patternContextBufferHolder.size()
#else
#define EXTRA_JIT_PARAMS
#endif

if (m_state == JITCode) {
if (s.is8Bit())
result = m_regExpJITCode.execute(s.characters8(), startOffset, s.length(), offsetVector EXTRA_JIT_PARAMS).start;
else
result = m_regExpJITCode.execute(s.characters16(), startOffset, s.length(), offsetVector EXTRA_JIT_PARAMS).start;
if (s.is8Bit())
result = m_regExpJITCode.execute(s.characters8(), startOffset, s.length(), offsetVector EXTRA_JIT_PARAMS).start;
else
result = m_regExpJITCode.execute(s.characters16(), startOffset, s.length(), offsetVector EXTRA_JIT_PARAMS).start;

#undef EXTRA_JIT_PARAMS
}

if (result == Yarr::JSRegExpJITCodeFailure) {
// JIT'ed code couldn't handle expression, so punt back to the interpreter.
Expand Down Expand Up @@ -213,20 +251,25 @@ ALWAYS_INLINE MatchResult RegExp::matchInline(VM& vm, const String& s, unsigned
compileIfNecessaryMatchOnly(vm, s.is8Bit() ? Yarr::Char8 : Yarr::Char16);

#if ENABLE(YARR_JIT)
#ifdef JIT_ALL_PARENS_EXPRESSIONS
char patternContextBuffer[patternContextBufferSize];
#define EXTRA_JIT_PARAMS , patternContextBuffer, patternContextBufferSize
MatchResult result;

if (m_state == JITCode) {
{
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
PatternContextBufferHolder patternContextBufferHolder(vm, m_regExpJITCode.usesPatternContextBuffer());

#define EXTRA_JIT_PARAMS , patternContextBufferHolder.buffer(), patternContextBufferHolder.size()
#else
#define EXTRA_JIT_PARAMS
#endif

MatchResult result;
if (s.is8Bit())
result = m_regExpJITCode.execute(s.characters8(), startOffset, s.length() EXTRA_JIT_PARAMS);
else
result = m_regExpJITCode.execute(s.characters16(), startOffset, s.length() EXTRA_JIT_PARAMS);

if (m_state == JITCode) {
if (s.is8Bit())
result = m_regExpJITCode.execute(s.characters8(), startOffset, s.length() EXTRA_JIT_PARAMS);
else
result = m_regExpJITCode.execute(s.characters16(), startOffset, s.length() EXTRA_JIT_PARAMS);
#undef EXTRA_JIT_PARAMS
}

#if ENABLE(REGEXP_TRACING)
if (!result)
Expand Down
25 changes: 25 additions & 0 deletions Source/JavaScriptCore/runtime/VM.cpp
View file
Expand Up @@ -532,6 +532,12 @@ VM::~VM()

delete clientData;
delete m_regExpCache;

#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
if (m_regExpPatternContexBuffer)
delete[] m_regExpPatternContexBuffer;
#endif

#if ENABLE(REGEXP_TRACING)
delete m_rtTraceList;
#endif
Expand Down Expand Up @@ -896,6 +902,25 @@ void logSanitizeStack(VM* vm)
}
}

#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
char* VM::acquireRegExpPatternContexBuffer()
{
ASSERT(!m_regExpPatternContextLock.isLocked());

m_regExpPatternContextLock.lock();
if (!m_regExpPatternContexBuffer)
m_regExpPatternContexBuffer = new char[VM::patternContextBufferSize];
return m_regExpPatternContexBuffer;
}

void VM::releaseRegExpPatternContexBuffer()
{
ASSERT(m_regExpPatternContextLock.isLocked());

m_regExpPatternContextLock.unlock();
}
#endif

#if ENABLE(REGEXP_TRACING)
void VM::addRegExpToTrace(RegExp* regExp)
{
Expand Down
9 changes: 9 additions & 0 deletions Source/JavaScriptCore/runtime/VM.h
View file
Expand Up @@ -681,6 +681,15 @@ class VM : public ThreadSafeRefCounted<VM>, public DoublyLinkedListNode<VM> {
BumpPointerAllocator m_regExpAllocator;
ConcurrentJSLock m_regExpAllocatorLock;

#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
static constexpr size_t patternContextBufferSize = 8192; // Space allocated to save nested parenthesis context
char* m_regExpPatternContexBuffer { nullptr };
Lock m_regExpPatternContextLock;
char* acquireRegExpPatternContexBuffer();
void releaseRegExpPatternContexBuffer();
#endif


std::unique_ptr<HasOwnPropertyCache> m_hasOwnPropertyCache;
ALWAYS_INLINE HasOwnPropertyCache* hasOwnPropertyCache() { return m_hasOwnPropertyCache.get(); }
HasOwnPropertyCache* ensureHasOwnPropertyCache();
Expand Down
37 changes: 21 additions & 16 deletions Source/JavaScriptCore/yarr/YarrJIT.cpp
View file
Expand Up @@ -156,7 +156,7 @@ class YarrGenerator : private MacroAssembler {
#define JIT_UNICODE_EXPRESSIONS
#endif

#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
struct ParenContextSizes {
size_t m_numSubpatterns;
size_t m_frameSlots;
Expand Down Expand Up @@ -2173,7 +2173,7 @@ class YarrGenerator : private MacroAssembler {
//
// These nodes support generic subpatterns.
case OpParenthesesSubpatternBegin: {
#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
PatternTerm* term = op.m_term;
unsigned parenthesesFrameLocation = term->frameLocation;

Expand Down Expand Up @@ -2230,13 +2230,13 @@ class YarrGenerator : private MacroAssembler {
} else
setSubpatternStart(index, term->parentheses.subpatternId);
}
#else // !JIT_ALL_PARENS_EXPRESSIONS
#else // !YARR_JIT_ALL_PARENS_EXPRESSIONS
RELEASE_ASSERT_NOT_REACHED();
#endif
break;
}
case OpParenthesesSubpatternEnd: {
#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
PatternTerm* term = op.m_term;
unsigned parenthesesFrameLocation = term->frameLocation;

Expand Down Expand Up @@ -2288,7 +2288,7 @@ class YarrGenerator : private MacroAssembler {
YarrOp& beginOp = m_ops[op.m_previousOp];
beginOp.m_jumps.link(this);
}
#else // !JIT_ALL_PARENS_EXPRESSIONS
#else // !YARR_JIT_ALL_PARENS_EXPRESSIONS
RELEASE_ASSERT_NOT_REACHED();
#endif
break;
Expand Down Expand Up @@ -2837,7 +2837,7 @@ class YarrGenerator : private MacroAssembler {
// out of the start of the parentheses, or jump back to the forwards
// matching start, depending of whether the match is Greedy or NonGreedy.
case OpParenthesesSubpatternBegin: {
#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
PatternTerm* term = op.m_term;
unsigned parenthesesFrameLocation = term->frameLocation;

Expand Down Expand Up @@ -2880,13 +2880,13 @@ class YarrGenerator : private MacroAssembler {

m_backtrackingState.fallthrough();
}
#else // !JIT_ALL_PARENS_EXPRESSIONS
#else // !YARR_JIT_ALL_PARENS_EXPRESSIONS
RELEASE_ASSERT_NOT_REACHED();
#endif
break;
}
case OpParenthesesSubpatternEnd: {
#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
PatternTerm* term = op.m_term;

if (term->quantityType != QuantifierFixedCount) {
Expand Down Expand Up @@ -2917,7 +2917,7 @@ class YarrGenerator : private MacroAssembler {
}

m_backtrackingState.append(op.m_jumps);
#else // !JIT_ALL_PARENS_EXPRESSIONS
#else // !YARR_JIT_ALL_PARENS_EXPRESSIONS
RELEASE_ASSERT_NOT_REACHED();
#endif
break;
Expand Down Expand Up @@ -3022,7 +3022,7 @@ class YarrGenerator : private MacroAssembler {
parenthesesBeginOpCode = OpParenthesesSubpatternTerminalBegin;
parenthesesEndOpCode = OpParenthesesSubpatternTerminalEnd;
} else {
#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
// We only handle generic parenthesis with greedy counts.
if (term->quantityType != QuantifierGreedy) {
// This subpattern is not supported by the JIT.
Expand Down Expand Up @@ -3273,7 +3273,7 @@ class YarrGenerator : private MacroAssembler {
if (m_pattern.m_saveInitialStartValue)
push(X86Registers::ebx);

#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
if (m_containsNestedSubpatterns) {
#if OS(WINDOWS)
push(X86Registers::edi);
Expand Down Expand Up @@ -3360,7 +3360,7 @@ class YarrGenerator : private MacroAssembler {
pop(X86Registers::r13);
}

#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
if (m_containsNestedSubpatterns) {
pop(X86Registers::r12);
#if OS(WINDOWS)
Expand Down Expand Up @@ -3400,7 +3400,7 @@ class YarrGenerator : private MacroAssembler {
, m_decodeSurrogatePairs(m_charSize == Char16 && m_pattern.unicode())
, m_unicodeIgnoreCase(m_pattern.unicode() && m_pattern.ignoreCase())
, m_canonicalMode(m_pattern.unicode() ? CanonicalMode::Unicode : CanonicalMode::UCS2)
#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
, m_containsNestedSubpatterns(false)
, m_parenContextSizes(compileMode == IncludeSubpatterns ? m_pattern.m_numSubpatterns : 0, m_pattern.m_body->m_callFrameSize)
#endif
Expand All @@ -3416,6 +3416,11 @@ class YarrGenerator : private MacroAssembler {
}
#endif

#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
if (m_containsNestedSubpatterns)
jitObject.setUsesPaternContextBuffer();
#endif

// We need to compile before generating code since we set flags based on compilation that
// are used during generation.
opCompileBody(m_pattern.m_body);
Expand All @@ -3431,7 +3436,7 @@ class YarrGenerator : private MacroAssembler {
generateFailReturn();
hasInput.link(this);

#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
if (m_containsNestedSubpatterns)
move(TrustedImm32(matchLimit), remainingMatchCount);
#endif
Expand All @@ -3446,7 +3451,7 @@ class YarrGenerator : private MacroAssembler {

initCallFrame();

#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
if (m_containsNestedSubpatterns)
initParenContextFreeList();
#endif
Expand Down Expand Up @@ -3510,7 +3515,7 @@ class YarrGenerator : private MacroAssembler {
bool m_decodeSurrogatePairs;
bool m_unicodeIgnoreCase;
CanonicalMode m_canonicalMode;
#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
bool m_containsNestedSubpatterns;
ParenContextSizes m_parenContextSizes;
#endif
Expand Down
13 changes: 9 additions & 4 deletions Source/JavaScriptCore/yarr/YarrJIT.h
View file
Expand Up @@ -38,8 +38,7 @@
#define YARR_CALL
#endif

#if CPU(ARM64) || (CPU(X86_64) && !OS(WINDOWS))
#define JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
constexpr size_t patternContextBufferSize = 8192; // Space caller allocates to save nested parenthesis context
#endif

Expand All @@ -61,7 +60,7 @@ enum class JITFailureReason : uint8_t {

class YarrCodeBlock {
#if CPU(X86_64) || CPU(ARM64)
#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
typedef MatchResult (*YarrJITCode8)(const LChar* input, unsigned start, unsigned length, int* output, void* freeParenContext, unsigned parenContextSize) YARR_CALL;
typedef MatchResult (*YarrJITCode16)(const UChar* input, unsigned start, unsigned length, int* output, void* freeParenContext, unsigned parenContextSize) YARR_CALL;
typedef MatchResult (*YarrJITCodeMatchOnly8)(const LChar* input, unsigned start, unsigned length, void*, void* freeParenContext, unsigned parenContextSize) YARR_CALL;
Expand Down Expand Up @@ -95,7 +94,10 @@ class YarrCodeBlock {
void set8BitCodeMatchOnly(MacroAssemblerCodeRef matchOnly) { m_matchOnly8 = matchOnly; }
void set16BitCodeMatchOnly(MacroAssemblerCodeRef matchOnly) { m_matchOnly16 = matchOnly; }

#ifdef JIT_ALL_PARENS_EXPRESSIONS
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
bool usesPatternContextBuffer() { return m_usesPatternContextBuffer; }
void setUsesPaternContextBuffer() { m_usesPatternContextBuffer = true; }

MatchResult execute(const LChar* input, unsigned start, unsigned length, int* output, void* freeParenContext, unsigned parenContextSize)
{
ASSERT(has8BitCode());
Expand Down Expand Up @@ -198,6 +200,9 @@ class YarrCodeBlock {
MacroAssemblerCodeRef m_ref16;
MacroAssemblerCodeRef m_matchOnly8;
MacroAssemblerCodeRef m_matchOnly16;
#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
bool m_usesPatternContextBuffer;
#endif
std::optional<JITFailureReason> m_failureReason;
};

Expand Down

0 comments on commit f632575

Please sign in to comment.