JSC test crash: stress/dont-strength-reduce-regexp-with-compile-error.js.default

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Michael Saboff]

After change set r242955: <https://trac.webkit.org/changeset/242955/webkit>, we are getting a crash in the test added as part of that change.  The crash is due to out-of-stack when compiling the RegExp in the YARR JIT:

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Subtype: KERN_PROTECTION_FAILURE at 0x000000016eb43fa8
VM Region Info: 0x16eb43fa8 is in 0x16eb40000-0x16eb44000;  bytes after start: 16296  bytes before end: 87
      REGION TYPE                      START - END             [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      WebKit Malloc          0000000104000000-0000000104100000 [ 1024K] rw-/rwx SM=PRV  
      GAP OF 0x6aa40000 BYTES
--->  STACK GUARD            000000016eb40000-000000016eb44000 [   16K] ---/rwx SM=NUL  ... for thread 0
      Stack                  000000016eb44000-000000016ec40000 [ 1008K] rw-/rwx SM=PRV  thread 0

Termination Signal: Segmentation fault: 11
Termination Reason: Namespace SIGNAL, Code 0xb
Terminating Process: exc handler [79242]
Triggered by Thread:  0

Thread 0 name:  Dispatch queue: com.apple.main-thread
Thread 0 Crashed:
0   JavaScriptCore                      0x00000001021d1450 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileParenthesesSubpattern(JSC::Yarr::PatternTerm*) + 40
1   JavaScriptCore                      0x00000001021d0f94 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileAlternative(JSC::Yarr::PatternAlternative*) + 128
2   JavaScriptCore                      0x00000001021d0f94 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileAlternative(JSC::Yarr::PatternAlternative*) + 128
3   JavaScriptCore                      0x00000001021d1724 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileParenthesesSubpattern(JSC::Yarr::PatternTerm*) + 764
4   JavaScriptCore                      0x00000001021d0f94 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileAlternative(JSC::Yarr::PatternAlternative*) + 128
5   JavaScriptCore                      0x00000001021d1724 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileParenthesesSubpattern(JSC::Yarr::PatternTerm*) + 764
6   JavaScriptCore                      0x00000001021d0f94 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileAlternative(JSC::Yarr::PatternAlternative*) + 128
7   JavaScriptCore                      0x00000001021d1724 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileParenthesesSubpattern(JSC::Yarr::PatternTerm*) + 764
8   JavaScriptCore                      0x00000001021d0f94 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileAlternative(JSC::Yarr::PatternAlternative*) + 128
9   JavaScriptCore                      0x00000001021d1724 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileParenthesesSubpattern(JSC::Yarr::PatternTerm*) + 764
10  JavaScriptCore                      0x00000001021d0f94 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileAlternative(JSC::Yarr::PatternAlternative*) + 128
11  JavaScriptCore                      0x00000001021d1724 JSC::Yarr::YarrGenerator<(JSC::Yarr::YarrJITCompileMode)1>::opCompileParenthesesSubpattern(JSC::Yarr::PatternTerm*) + 764
...

[Michael Saboff]

<rdar://problem/48929093>

[Michael Saboff]

Created attachment 365254 [details]
Patch

[Mark Lam]

Comment on attachment 365254 [details]
Patch

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

r=me.  Why not also do stack checks in opCompileAlternative(), opCompileBody(), and compile().  I think at minimum, it makes sense to do a check at the top level compile() function.  This check will probably cover many functions that are 1 level deeper than compile().  Anything that can recurse below that will need additional checks.

> Source/JavaScriptCore/ChangeLog:19
> +        This change is covered by the previously added test that is failing.

Would be nice to name the test here for reference.

[EWS Watchlist]

Comment on attachment 365254 [details]
Patch

Attachment 365254 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11573498

New failing tests:
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html

[EWS Watchlist]

Created attachment 365287 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Michael Saboff]

(In reply to Mark Lam from comment #3)
> Comment on attachment 365254 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365254&action=review
> 
> r=me.  Why not also do stack checks in opCompileAlternative(),
> opCompileBody(), and compile().  I think at minimum, it makes sense to do a
> check at the top level compile() function.  This check will probably cover
> many functions that are 1 level deeper than compile().  Anything that can
> recurse below that will need additional checks.

Added a check in opCompileBody().  The recursion chain is
opCompileParenthesesSubpattern() -> opCompileParenthesesSubpattern()
  -> opCompileAlternative() -> opCompileParenthesesSubpattern() ...
Where opCompileParenthesesSubpattern could also be opCompileParentheticalAssertion.

Given this, a stack check in opCompileParenthesesSubpattern and opCompileParentheticalAssertion is sufficient.


> > Source/JavaScriptCore/ChangeLog:19
> > +        This change is covered by the previously added test that is failing.
> 
> Would be nice to name the test here for reference.

Added.

[Michael Saboff]

Committed r243237: <https://trac.webkit.org/changeset/243237>