Bug 209824

Summary: REGRESSION: ASSERTION FAILED: regExpObjectNode in JSC::DFG::StrengthReductionPhase::handleNode
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ews-watchlist, keith_miller, mark.lam, msaboff, ross.kirsling, saam, tzagallo, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=209375
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Ryan Haddad 2020-03-31 11:49:03 PDT 
36 tests are asserting on the Debug JSC queue:

 ASSERTION FAILED: regExpObjectNode
 ./dfg/DFGStrengthReductionPhase.cpp(542) : void JSC::DFG::StrengthReductionPhase::handleNode()
 1   0x10a4c7c29 WTFCrash
 2   0x10ac2601b WTFCrashWithInfo(int, char const*, char const*, int)
 3   0x10b514e36 JSC::DFG::StrengthReductionPhase::handleNode()
 4   0x10b512942 JSC::DFG::StrengthReductionPhase::run()
 5   0x10b512704 bool JSC::DFG::runAndLog<JSC::DFG::StrengthReductionPhase>(JSC::DFG::StrengthReductionPhase&)
 6   0x10b504a2b bool JSC::DFG::runPhase<JSC::DFG::StrengthReductionPhase>(JSC::DFG::Graph&)
 7   0x10b5049f5 JSC::DFG::performStrengthReduction(JSC::DFG::Graph&)
 8   0x10b413440 JSC::DFG::Plan::compileInThreadImpl()
 9   0x10b40ff48 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*)
 10  0x10b56b1fe JSC::DFG::Worklist::ThreadBody::work()
 11  0x10a4de593 WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const
 12  0x10a4de17e WTF::Detail::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0, void>::call()
 13  0x10a4f2252 WTF::Function<void ()>::operator()() const
 14  0x10a5a6b78 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*)
 15  0x10a5b3fa8 WTF::wtfThreadEntryPoint(void*)
 16  0x7fff70703109 _pthread_start
 17  0x7fff706feb8b thread_start

** The following JSC stress test failures have been introduced:
	microbenchmarks/emscripten-cube2hash.js.ftl-eager
	microbenchmarks/emscripten-cube2hash.js.ftl-eager-no-cjit
	microbenchmarks/emscripten-cube2hash.js.ftl-eager-no-cjit-b3o1
	stress/ftl-regexp-exec.js.bytecode-cache
	stress/ftl-regexp-exec.js.default
	stress/ftl-regexp-exec.js.ftl-eager
	stress/ftl-regexp-exec.js.ftl-eager-no-cjit
	stress/ftl-regexp-exec.js.ftl-eager-no-cjit-b3o1
	stress/ftl-regexp-exec.js.ftl-no-cjit-b3o0
	stress/ftl-regexp-exec.js.ftl-no-cjit-no-inline-validate
	stress/ftl-regexp-exec.js.ftl-no-cjit-no-put-stack-validate
	stress/ftl-regexp-exec.js.ftl-no-cjit-small-pool
	stress/ftl-regexp-exec.js.ftl-no-cjit-validate-sampling-profiler
	stress/phantom-regexp-regexp-exec.js.bytecode-cache
	stress/phantom-regexp-regexp-exec.js.default
	stress/phantom-regexp-regexp-exec.js.ftl-eager
	stress/phantom-regexp-regexp-exec.js.ftl-eager-no-cjit
	stress/phantom-regexp-regexp-exec.js.ftl-eager-no-cjit-b3o1
	stress/phantom-regexp-regexp-exec.js.ftl-no-cjit-b3o0
	stress/phantom-regexp-regexp-exec.js.ftl-no-cjit-no-inline-validate
	stress/phantom-regexp-regexp-exec.js.ftl-no-cjit-no-put-stack-validate
	stress/phantom-regexp-regexp-exec.js.ftl-no-cjit-small-pool
	stress/phantom-regexp-regexp-exec.js.ftl-no-cjit-validate-sampling-profiler
	stress/regexp-exec-effect-after-exception.js.ftl-eager
	stress/regexp-exec-effect-after-exception.js.ftl-eager-no-cjit
	stress/regexp-exec-effect-after-exception.js.ftl-eager-no-cjit-b3o1
	stress/regexp-exec-effect-after-exception.js.ftl-no-cjit-b3o0
	stress/regexp-exec-effect-after-exception.js.ftl-no-cjit-no-inline-validate
	stress/regexp-exec-effect-after-exception.js.ftl-no-cjit-no-put-stack-validate
	stress/regexp-exec-effect-after-exception.js.ftl-no-cjit-validate-sampling-profiler
	stress/v8-regexp-strict.js.ftl-eager
	stress/v8-regexp-strict.js.ftl-eager-no-cjit
	stress/v8-regexp-strict.js.ftl-eager-no-cjit-b3o1
	v8-v6/v8-regexp.js.ftl-eager
	v8-v6/v8-regexp.js.ftl-eager-no-cjit
	v8-v6/v8-regexp.js.ftl-eager-no-cjit-b3o1

https://build.webkit.org/builders/Apple-Catalina-Debug-JSC-Tests/builds/596/steps/jscore-test/logs/stdio
Comment 1 Ryan Haddad 2020-03-31 11:49:48 PDT 
This assert was added with https://trac.webkit.org/changeset/259246/webkit
Comment 2 Mark Lam 2020-03-31 11:51:46 PDT 
<rdar://problem/61092415>
Comment 3 Ross Kirsling 2020-03-31 12:05:05 PDT 
Argh, looks like this must be when we've already successfully done convertToStatic and are now reconsidering foldToConstant.
Comment 4 Ross Kirsling 2020-03-31 12:52:45 PDT 
Created attachment 395082 [details]
Patch
Comment 5 Ross Kirsling 2020-03-31 12:53:20 PDT 
Sorry about that!
Comment 6 Darin Adler 2020-03-31 12:55:35 PDT 
Comment on attachment 395082 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:556
> +                        && otherNode->child2()->isInt32Constant()
> +                        && otherNode->child2()->asInt32() >= 0) {
> +                        lastIndex = static_cast<unsigned>(otherNode->child2()->asInt32());

Moved code, not new. This seems a lot like isUInt32/asUInt32 and a little strange that we’re using static_cast instead.
Comment 7 Mark Lam 2020-03-31 13:08:30 PDT 
Comment on attachment 395082 [details]
Patch

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

r=me

>> Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:556
>> +                        lastIndex = static_cast<unsigned>(otherNode->child2()->asInt32());
> 
> Moved code, not new. This seems a lot like isUInt32/asUInt32 and a little strange that we’re using static_cast instead.

Yes, let's just use asUint32() and remove the need for the cast.
Comment 8 Ross Kirsling 2020-03-31 13:27:42 PDT 
Created attachment 395086 [details]
Patch for landing
Comment 9 EWS 2020-03-31 13:57:15 PDT 
Committed r259310: <https://trac.webkit.org/changeset/259310>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395086 [details].