Diff
Modified: trunk/JSTests/ChangeLog (237752 => 237753)
--- trunk/JSTests/ChangeLog 2018-11-02 21:43:42 UTC (rev 237752)
+++ trunk/JSTests/ChangeLog 2018-11-02 22:05:51 UTC (rev 237753)
@@ -1,3 +1,15 @@
+2018-11-02 Michael Saboff <msab...@apple.com>
+
+ Running out of stack space not properly handled in RegExp::compile() and its callers
+ https://bugs.webkit.org/show_bug.cgi?id=191206
+
+ Reviewed by Filip Pizlo.
+
+ New regression test.
+
+ * stress/regexp-compile-oom.js: Added.
+ (recurseAndTest):
+
2018-11-01 Guillaume Emont <guijem...@igalia.com>
Skip tests on arm/mips that time out now we're running on CLoop
Added: trunk/JSTests/stress/regexp-compile-oom.js (0 => 237753)
--- trunk/JSTests/stress/regexp-compile-oom.js (rev 0)
+++ trunk/JSTests/stress/regexp-compile-oom.js 2018-11-02 22:05:51 UTC (rev 237753)
@@ -0,0 +1,64 @@
+// Test that throw an OOM exception when compiling a pathological, but valid nested RegExp.
+
+function recurseAndTest(depth, f, expectedException)
+{
+ // Probe stack depth
+ try {
+ let result = recurseAndTest(depth + 1, f, expectedException);
+ if (result == 0) {
+ try {
+ // Call the test function with a nearly full stack.
+ f();
+ } catch (e) {
+ return e.toString();
+ }
+
+ return 1;
+ } else if (result < 0)
+ return result + 1;
+ else
+ return result;
+ } catch (e) {
+ // Go up a several frames and then call the test function
+ return -10;
+ }
+
+ return 1;
+}
+
+let deepRE = /((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((x))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))/;
+let matchLen = 381; // The number of parens plus 1 for the whole match.
+
+let regExpOOMError = "Error: Out of memory: Invalid regular _expression_: too many nested disjunctions";
+
+// Test that both exec (captured compilation) and test (match only compilation) handles OOM.
+let result = recurseAndTest(1, () => { deepRE.exec(); });
+if (result != regExpOOMError)
+ throw "Expected: \"" + regExpOOMError + "\" but got \"" + result + "\"";
+
+result = recurseAndTest(1, () => { deepRE.test(); });
+if (result != regExpOOMError)
+ throw "Expected: \"" + regExpOOMError + "\" but got \"" + result + "\"";
+
+// Test that the RegExp works correctly with RegExp.exec() and RegExp.test() when there is sufficient stack space to compile it.
+let m = deepRE.exec("x");
+let matched = true;
+if (m.length != matchLen)
+ matched = false
+else {
+ for (i = 0; i < matchLen; i++) {
+ if (m[i] != "x")
+ matched = false;
+ }
+}
+
+if (!matched) {
+ let expectedMatch = [];
+ for (i = 0; i < matchLen; i++)
+ expectedMatch[i] = "x";
+
+ throw "Expected RegExp.exec(...) to be [" + expectedMatch + "] but got [" + m + "]";
+}
+
+if (!deepRE.test("x"))
+ throw "Expected RegExp.test(...) to be true, but was false";
Modified: trunk/Source/_javascript_Core/ChangeLog (237752 => 237753)
--- trunk/Source/_javascript_Core/ChangeLog 2018-11-02 21:43:42 UTC (rev 237752)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-11-02 22:05:51 UTC (rev 237753)
@@ -1,3 +1,27 @@
+2018-11-02 Michael Saboff <msab...@apple.com>
+
+ Running out of stack space not properly handled in RegExp::compile() and its callers
+ https://bugs.webkit.org/show_bug.cgi?id=191206
+
+ Reviewed by Filip Pizlo.
+
+ Eliminated two RELEASE_ASSERT_NOT_REACHED() for errors returned by Yarr parsing code. Bubbled those errors
+ up to where they are turned into the appropriate exceptions in matchInline(). If the errors are not due
+ to syntax, we reset the RegExp state in case the parsing is tried with a smaller stack.
+
+ * runtime/RegExp.cpp:
+ (JSC::RegExp::compile):
+ (JSC::RegExp::compileMatchOnly):
+ * runtime/RegExp.h:
+ * runtime/RegExpInlines.h:
+ (JSC::RegExp::compileIfNecessary):
+ (JSC::RegExp::matchInline):
+ (JSC::RegExp::compileIfNecessaryMatchOnly):
+ * runtime/RegExpObjectInlines.h:
+ (JSC::RegExpObject::execInline):
+ * yarr/YarrErrorCode.h:
+ (JSC::Yarr::hasHardError):
+
2018-11-02 Keith Miller <keith_mil...@apple.com>
API should use wrapper object if address is 32-bit
Modified: trunk/Source/_javascript_Core/runtime/RegExp.cpp (237752 => 237753)
--- trunk/Source/_javascript_Core/runtime/RegExp.cpp 2018-11-02 21:43:42 UTC (rev 237752)
+++ trunk/Source/_javascript_Core/runtime/RegExp.cpp 2018-11-02 22:05:51 UTC (rev 237753)
@@ -23,6 +23,7 @@
#include "config.h"
#include "RegExp.h"
+#include "ExceptionHelpers.h"
#include "Lexer.h"
#include "JSCInlines.h"
#include "RegExpCache.h"
@@ -290,11 +291,8 @@
Yarr::YarrPattern pattern(m_patternString, m_flags, m_constructionErrorCode, vm->stackLimit());
if (hasError(m_constructionErrorCode)) {
- RELEASE_ASSERT_NOT_REACHED();
-#if COMPILER_QUIRK(CONSIDERS_UNREACHABLE_CODE)
m_state = ParseError;
return;
-#endif
}
ASSERT(m_numSubpatterns == pattern.m_numSubpatterns);
@@ -350,11 +348,8 @@
Yarr::YarrPattern pattern(m_patternString, m_flags, m_constructionErrorCode, vm->stackLimit());
if (hasError(m_constructionErrorCode)) {
- RELEASE_ASSERT_NOT_REACHED();
-#if COMPILER_QUIRK(CONSIDERS_UNREACHABLE_CODE)
m_state = ParseError;
return;
-#endif
}
ASSERT(m_numSubpatterns == pattern.m_numSubpatterns);
Modified: trunk/Source/_javascript_Core/runtime/RegExp.h (237752 => 237753)
--- trunk/Source/_javascript_Core/runtime/RegExp.h 2018-11-02 21:43:42 UTC (rev 237752)
+++ trunk/Source/_javascript_Core/runtime/RegExp.h 2018-11-02 22:05:51 UTC (rev 237753)
@@ -64,6 +64,11 @@
bool isValid() const { return !Yarr::hasError(m_constructionErrorCode) && m_flags != InvalidFlags; }
const char* errorMessage() const { return Yarr::errorMessage(m_constructionErrorCode); }
JSObject* errorToThrow(ExecState* exec) { return Yarr::errorToThrow(exec, m_constructionErrorCode); }
+ void reset()
+ {
+ m_state = NotCompiled;
+ m_constructionErrorCode = Yarr::ErrorCode::NoError;
+ }
JS_EXPORT_PRIVATE int match(VM&, const String&, unsigned startOffset, Vector<int>& ovector);
Modified: trunk/Source/_javascript_Core/runtime/RegExpInlines.h (237752 => 237753)
--- trunk/Source/_javascript_Core/runtime/RegExpInlines.h 2018-11-02 21:43:42 UTC (rev 237752)
+++ trunk/Source/_javascript_Core/runtime/RegExpInlines.h 2018-11-02 22:05:51 UTC (rev 237753)
@@ -123,6 +123,9 @@
if (hasCodeFor(charSize))
return;
+ if (m_state == ParseError)
+ return;
+
compile(&vm, charSize);
}
@@ -129,14 +132,22 @@
template<typename VectorType>
ALWAYS_INLINE int RegExp::matchInline(VM& vm, const String& s, unsigned startOffset, VectorType& ovector)
{
+ auto throwScope = DECLARE_THROW_SCOPE(vm);
#if ENABLE(REGEXP_TRACING)
m_rtMatchCallCount++;
m_rtMatchTotalSubjectStringLen += (double)(s.length() - startOffset);
#endif
- ASSERT(m_state != ParseError);
compileIfNecessary(vm, s.is8Bit() ? Yarr::Char8 : Yarr::Char16);
+ if (m_state == ParseError) {
+ ExecState* exec = vm.topCallFrame;
+ throwScope.throwException(exec, errorToThrow(exec));
+ if (!hasHardError(m_constructionErrorCode))
+ reset();
+ return -1;
+ }
+
int offsetVectorSize = (m_numSubpatterns + 1) * 2;
ovector.resize(offsetVectorSize);
int* offsetVector = ovector.data();
@@ -237,19 +248,30 @@
if (hasMatchOnlyCodeFor(charSize))
return;
+ if (m_state == ParseError)
+ return;
+
compileMatchOnly(&vm, charSize);
}
ALWAYS_INLINE MatchResult RegExp::matchInline(VM& vm, const String& s, unsigned startOffset)
{
+ auto throwScope = DECLARE_THROW_SCOPE(vm);
#if ENABLE(REGEXP_TRACING)
m_rtMatchOnlyCallCount++;
m_rtMatchOnlyTotalSubjectStringLen += (double)(s.length() - startOffset);
#endif
- ASSERT(m_state != ParseError);
compileIfNecessaryMatchOnly(vm, s.is8Bit() ? Yarr::Char8 : Yarr::Char16);
+ if (m_state == ParseError) {
+ ExecState* exec = vm.topCallFrame;
+ throwScope.throwException(exec, errorToThrow(exec));
+ if (!hasHardError(m_constructionErrorCode))
+ reset();
+ return MatchResult::failed();
+ }
+
#if ENABLE(YARR_JIT)
MatchResult result;
Modified: trunk/Source/_javascript_Core/runtime/RegExpObjectInlines.h (237752 => 237753)
--- trunk/Source/_javascript_Core/runtime/RegExpObjectInlines.h 2018-11-02 21:43:42 UTC (rev 237752)
+++ trunk/Source/_javascript_Core/runtime/RegExpObjectInlines.h 2018-11-02 22:05:51 UTC (rev 237753)
@@ -85,6 +85,7 @@
JSArray* array =
createRegExpMatchesArray(vm, globalObject, string, input, regExp, lastIndex, result);
if (!array) {
+ RETURN_IF_EXCEPTION(scope, { });
scope.release();
if (globalOrSticky)
setLastIndex(exec, 0);
Modified: trunk/Source/_javascript_Core/yarr/YarrErrorCode.h (237752 => 237753)
--- trunk/Source/_javascript_Core/yarr/YarrErrorCode.h 2018-11-02 21:43:42 UTC (rev 237752)
+++ trunk/Source/_javascript_Core/yarr/YarrErrorCode.h 2018-11-02 22:05:51 UTC (rev 237753)
@@ -60,6 +60,13 @@
{
return errorCode != ErrorCode::NoError;
}
+
+inline bool hasHardError(ErrorCode errorCode)
+{
+ // TooManyDisjunctions means that we ran out stack compiling.
+ // All other errors are due to problems in the _expression_.
+ return hasError(errorCode) && errorCode != ErrorCode::TooManyDisjunctions;
+}
JS_EXPORT_PRIVATE JSObject* errorToThrow(ExecState*, ErrorCode);
} } // namespace JSC::Yarr
