Bug 130066

Summary:

REGRESSION(r165407): DoYouEvenBench crashes in DRT

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

JavaScriptCore

Assignee:

Mark Hahnenberg <mhahnenberg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, commit-queue, fpizlo, ggaren, kling, mhahnenberg

Priority:

Keywords:

InRadar, Regression

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

130040

Bug Blocks:

Attachments:

Description	Flags
Patch	none

Ryosuke Niwa

Reported 2014-03-10 20:51:23 PDT

After http://trac.webkit.org/changeset/165407, DYEBench has been crashing on the perf builders: http://build.webkit.org/builders/Apple%20MountainLion%20Release%20%28Perf%29/builds/8249 http://build.webkit.org/builders/Apple%20Mavericks%20Release%20%28Perf%29/builds/843 While the blame list contains other changes, I've built r165406 and 165407 locally and only r165407 reproduces the crash quite reliably. It appears, however, that this crash doesn't always occur. While Tools/Scripts/run-perf-tests PerformanceTests/DoYouEvenBench/Full.html --test-runner-count=4 --reset-results reproduces the crash quite reliably, Tools/Scripts/run-perf-tests PerformanceTests/DoYouEvenBench/Full.html --test-runner-count=1 --reset-results doesn't.

Attachments
Patch (3.70 KB, patch) 2014-03-10 22:22 PDT, Mark Hahnenberg	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Geoffrey Garen

Comment 1 2014-03-10 21:37:45 PDT

<rdar://problem/16285126>

Mark Hahnenberg

Comment 2 2014-03-10 21:39:07 PDT

After testing with a debug build, I get a reliable ASSERT during compilation in the DFG, but no crash during GC.

Ryosuke Niwa

Comment 3 2014-03-10 22:03:43 PDT

In the debug build, we're hitting the following assertion: ASSERTION FAILED: node->op() == Phi || node->op() == SetArgument /Volumes/Data/webkit/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp(262) : void JSC::DFG::DCEPhase::cleanVariables(VariablesVectorType &) [VariablesVectorType = JSC::Operands<JSC::DFG::Node *, JSC::DFG::NodePointerTraits>] 1 0x1067dc750 WTFCrash 2 0x1062728e1 void JSC::DFG::DCEPhase::cleanVariables<JSC::Operands<JSC::DFG::Node*, JSC::DFG::NodePointerTraits> >(JSC::Operands<JSC::DFG::Node*, JSC::DFG::NodePointerTraits>&) 3 0x1062722d0 JSC::DFG::DCEPhase::fixupBlock(JSC::DFG::BasicBlock*) 4 0x106271e2e JSC::DFG::DCEPhase::run() 5 0x106271005 bool JSC::DFG::runAndLog<JSC::DFG::DCEPhase>(JSC::DFG::DCEPhase&) 6 0x106270f8e bool JSC::DFG::runPhase<JSC::DFG::DCEPhase>(JSC::DFG::Graph&) 7 0x106270f48 JSC::DFG::performDCE(JSC::DFG::Graph&) 8 0x106326f40 JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&) 9 0x106326674 JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*) 10 0x1063c9ac0 JSC::DFG::Worklist::runThread(JSC::DFG::ThreadData*) 11 0x1063c8714 JSC::DFG::Worklist::threadFunction(void*) 12 0x10682bf70 WTF::threadEntryPoint(void*) 13 0x10682cbf8 WTF::wtfThreadEntryPoint(void*) 14 0x7fff8f8d4899 _pthread_body 15 0x7fff8f8d472a _pthread_struct_init 16 0x7fff8f8d8fc9 thread_start

Mark Hahnenberg

Comment 4 2014-03-10 22:06:38 PDT

(In reply to comment #3) > In the debug build, we're hitting the following assertion: > > ASSERTION FAILED: node->op() == Phi || node->op() == SetArgument > /Volumes/Data/webkit/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp(262) : void JSC::DFG::DCEPhase::cleanVariables(VariablesVectorType &) [VariablesVectorType = JSC::Operands<JSC::DFG::Node *, JSC::DFG::NodePointerTraits>] > 1 0x1067dc750 WTFCrash > 2 0x1062728e1 void JSC::DFG::DCEPhase::cleanVariables<JSC::Operands<JSC::DFG::Node*, JSC::DFG::NodePointerTraits> >(JSC::Operands<JSC::DFG::Node*, JSC::DFG::NodePointerTraits>&) > 3 0x1062722d0 JSC::DFG::DCEPhase::fixupBlock(JSC::DFG::BasicBlock*) > 4 0x106271e2e JSC::DFG::DCEPhase::run() > 5 0x106271005 bool JSC::DFG::runAndLog<JSC::DFG::DCEPhase>(JSC::DFG::DCEPhase&) > 6 0x106270f8e bool JSC::DFG::runPhase<JSC::DFG::DCEPhase>(JSC::DFG::Graph&) > 7 0x106270f48 JSC::DFG::performDCE(JSC::DFG::Graph&) > 8 0x106326f40 JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&) > 9 0x106326674 JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*) > 10 0x1063c9ac0 JSC::DFG::Worklist::runThread(JSC::DFG::ThreadData*) > 11 0x1063c8714 JSC::DFG::Worklist::threadFunction(void*) > 12 0x10682bf70 WTF::threadEntryPoint(void*) > 13 0x10682cbf8 WTF::wtfThreadEntryPoint(void*) > 14 0x7fff8f8d4899 _pthread_body > 15 0x7fff8f8d472a _pthread_struct_init > 16 0x7fff8f8d8fc9 thread_start Yep, that's what I'm seeing.

Mark Hahnenberg

Comment 5 2014-03-10 22:07:25 PDT

Hmm, seems like the DFG assert is just a red herring. Running the benchmark in a release build and disabling the DFG I hit the original issue.

Mark Hahnenberg

Comment 6 2014-03-10 22:07:49 PDT

And disabling the baseline JIT avoids the crash entirely.

Filip Pizlo

Comment 7 2014-03-10 22:09:36 PDT

I'd like to see the IR at the point of this crash. Any chance you could convert the assertion so that if it fails, it dumps the graph (m_graph.dump()) and the node (dataLog("node = ", node, "\n"))? (In reply to comment #4) > (In reply to comment #3) > > In the debug build, we're hitting the following assertion: > > > > ASSERTION FAILED: node->op() == Phi || node->op() == SetArgument > > /Volumes/Data/webkit/Source/JavaScriptCore/dfg/DFGDCEPhase.cpp(262) : void JSC::DFG::DCEPhase::cleanVariables(VariablesVectorType &) [VariablesVectorType = JSC::Operands<JSC::DFG::Node *, JSC::DFG::NodePointerTraits>] > > 1 0x1067dc750 WTFCrash > > 2 0x1062728e1 void JSC::DFG::DCEPhase::cleanVariables<JSC::Operands<JSC::DFG::Node*, JSC::DFG::NodePointerTraits> >(JSC::Operands<JSC::DFG::Node*, JSC::DFG::NodePointerTraits>&) > > 3 0x1062722d0 JSC::DFG::DCEPhase::fixupBlock(JSC::DFG::BasicBlock*) > > 4 0x106271e2e JSC::DFG::DCEPhase::run() > > 5 0x106271005 bool JSC::DFG::runAndLog<JSC::DFG::DCEPhase>(JSC::DFG::DCEPhase&) > > 6 0x106270f8e bool JSC::DFG::runPhase<JSC::DFG::DCEPhase>(JSC::DFG::Graph&) > > 7 0x106270f48 JSC::DFG::performDCE(JSC::DFG::Graph&) > > 8 0x106326f40 JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&) > > 9 0x106326674 JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*) > > 10 0x1063c9ac0 JSC::DFG::Worklist::runThread(JSC::DFG::ThreadData*) > > 11 0x1063c8714 JSC::DFG::Worklist::threadFunction(void*) > > 12 0x10682bf70 WTF::threadEntryPoint(void*) > > 13 0x10682cbf8 WTF::wtfThreadEntryPoint(void*) > > 14 0x7fff8f8d4899 _pthread_body > > 15 0x7fff8f8d472a _pthread_struct_init > > 16 0x7fff8f8d8fc9 thread_start > > Yep, that's what I'm seeing.

Mark Hahnenberg

Comment 8 2014-03-10 22:11:29 PDT

I think I know what the issue is for the baseline JIT. The baseline JIT does a conditional store barrier for the put_by_id, but we need an unconditional store barrier so that we cover the butterfly case as well in emitPutTransitionStub.

Mark Hahnenberg

Comment 9 2014-03-10 22:22:22 PDT

Created attachment 226387 [details] Patch

Ryosuke Niwa

Comment 10 2014-03-10 22:33:35 PDT

(In reply to comment #5) > Hmm, seems like the DFG assert is just a red herring. Running the benchmark in a release build and disabling the DFG I hit the original issue. Yeah, I hit the same assertion at r165407.

Ryosuke Niwa

Comment 11 2014-03-10 22:45:26 PDT

(In reply to comment #7) > I'd like to see the IR at the point of this crash. Any chance you could convert the assertion so that if it fails, it dumps the graph (m_graph.dump()) and the node (dataLog("node = ", node, "\n"))? Since this doesn't seem to be the crash culprit, I've filed https://bugs.webkit.org/show_bug.cgi?id=130069 to track the assertion failure. I've posted the data there.

Geoffrey Garen

Comment 12 2014-03-10 23:05:38 PDT

Comment on attachment 226387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226387&action=review r=me > Source/JavaScriptCore/ChangeLog:9 > + The baseline JIT does a conditional store barrier for the put_by_id, but we need > + an unconditional store barrier so that we cover the butterfly case as well in emitPutTransitionStub. I wonder if we should just remove conditional store barriers from the baseline JIT -- to avoid mismatches like this, and because it wasn't a regression in the optimizing JIT.

Mark Hahnenberg

Comment 13 2014-03-10 23:22:42 PDT

Just to be sure, I ran benchmarks on this patch, and it's performance neutral.

WebKit Commit Bot

Comment 14 2014-03-10 23:51:51 PDT

Comment on attachment 226387 [details] Patch Clearing flags on attachment: 226387 Committed r165435: <http://trac.webkit.org/changeset/165435>

WebKit Commit Bot

Comment 15 2014-03-10 23:51:55 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.