Bug 195735

Summary: ASSERTION FAILED: regexp->isValid() or ASSERTION FAILED: !isCompilationThread()
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mark.lam: review+

Description Michael Saboff 2019-03-13 22:03:16 PDT
Summary:
with --jitPolicyScale=0 the following asserts on a Debug build:

function foo(a) {
  try {
    eval('bar(/' + a[0].source + '/)');
  } catch(e) {
  }
}

function bar(r) {
  foo([r]);
  foo([r]);
  r.exec('x');
}

bar(/((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((x))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))/);


Steps To Reproduce:
jsc --jitPolicyScale=0 repro.js

it's not 100% reproducible, but it shouldn't take more than three attempts.

Results:
ASSERTION FAILED: regexp->isValid()
/Users/user/dev/CleanWebKit/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp(9750) : void JSC::DFG::SpeculativeJIT::compileNewRegexp(JSC::DFG::Node *)
1   0x107f44359 WTFCrash
2   0x107f4731b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x1080df5d9 JSC::DFG::SpeculativeJIT::compileNewRegexp(JSC::DFG::Node*)
4   0x1082c13d2 JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*)
5   0x10809633b JSC::DFG::SpeculativeJIT::compileCurrentBlock()
6   0x108097db5 JSC::DFG::SpeculativeJIT::compile()

or

ASSERTION FAILED: !isCompilationThread()
/Users/user/dev/CleanWebKit/Source/JavaScriptCore/runtime/LazyClassStructure.h(80) : JSC::Structure *JSC::LazyClassStructure::get(const JSC::JSGlobalObject *) const
1   0x10b0a9359 WTFCrash
2   0x10b0ac31b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x10b6734c0 JSC::LazyClassStructure::get(JSC::JSGlobalObject const*) const
4   0x10b666e19 JSC::JSGlobalObject::errorStructure()
5   0x10c56a053 JSC::createError(JSC::ExecState*, WTF::String const&, WTF::String (*)(WTF::String const&, WTF::String const&, JSC::RuntimeType, JSC::ErrorInstance::SourceTextWhereErrorOccurred))
6   0x10c56c27d JSC::createOutOfMemoryError(JSC::ExecState*, WTF::String const&)
7   0x10c986364 JSC::Yarr::errorToThrow(JSC::ExecState*, JSC::Yarr::ErrorCode)
8   0x10bec3261 JSC::RegExp::errorToThrow(JSC::ExecState*)
9   0x10c76dec5 int JSC::RegExp::matchInline<WTF::Vector<int, 0ul, WTF::CrashOnOverflow, 16ul> >(JSC::VM&, WTF::String const&, unsigned int, WTF::Vector<int, 0ul, WTF::CrashOnOverflow, 16ul>&)
10  0x10c76dda3 JSC::RegExp::match(JSC::VM&, WTF::String const&, unsigned int, WTF::Vector<int, 0ul, WTF::CrashOnOverflow, 16ul>&)
11  0x10c76e41a JSC::RegExp::matchConcurrently(JSC::VM&, WTF::String const&, unsigned int, int&, WTF::Vector<int, 0ul, WTF::CrashOnOverflow, 16ul>&)
12  0x10bfc6d41 JSC::DFG::StrengthReductionPhase::handleNode()::'lambda'()::operator()() const
Comment 1 Michael Saboff 2019-03-13 22:03:35 PDT
<rdar://problem/48801596>
Comment 2 Michael Saboff 2019-03-13 22:17:28 PDT
The two crashes are slightly different.

The first one happens due to a race condition when we are compiling on a separate thread.  The main thread in this recursive test can run out of memory compiling the regular expression.  When that happens, the RegExp becomes invalid due to the error.  We throw an exception, but we then reset the RegExp as it might compile successfully the next time we try to execute it on a shallower stack.  The main thread will see the regular expression as valid when it executes the JIT'ed code and makes calls out to slow path code.  Therefore this ASSERT can be eliminated.

The second case happens to to incorrect logic when we go to run the regexp in the Strength Reduction phase.  The current check for "do we have code to run the RegExp?" only checks that the RegExp's state is != NotCompiled.  We also can't run the RegExp if there the state is ParseError.  Changing hasCode() to take this into account fixes the second issue.
Comment 3 Michael Saboff 2019-03-13 22:47:43 PDT
Created attachment 364634 [details]
Patch
Comment 4 Mark Lam 2019-03-13 22:55:42 PDT
Comment on attachment 364634 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364634&action=review

r=me if EWS bots are happy.

> Source/JavaScriptCore/ChangeLog:18
> +        The second but is due to incorrect logic when we go to run the regexp in the Strength Reduction phase.

/but/bug/.
Comment 5 Michael Saboff 2019-03-14 12:31:54 PDT
Committed r242955: <https://trac.webkit.org/changeset/242955>
Comment 6 Saam Barati 2019-03-14 17:48:12 PDT
Comment on attachment 364634 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364634&action=review

> Source/JavaScriptCore/runtime/RegExp.h:111
> +        return m_state == JITCode || m_state == ByteCode;

Do we need any kind of StoreStoreFence on the main thread to synchronize this?