Bug 163337

Summary: REGRESSION (r207179): ASSERTION FAILED: node.cell != previousCell
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149432    
Attachments:
Description Flags
the patch mark.lam: review+

Description Ryan Haddad 2016-10-12 09:53:36 PDT
ASSERTION FAILED: node.cell != previousCell
/Volumes/Data/slave/elcapitan-debug/build/Source/JavaScriptCore/heap/HeapSnapshot.cpp(125) : void JSC::HeapSnapshot::finalize()
1   0x104083180 WTFCrash
2   0x103ec60b4 JSC::HeapSnapshot::finalize()
3   0x103a872cc JSC::HeapSnapshotBuilder::buildSnapshot()
4   0x10398fc06 Inspector::InspectorHeapAgent::snapshot(WTF::String&, double*, WTF::String*)
5   0x1039623c2 Inspector::InspectorConsoleAgent::takeHeapSnapshot(WTF::String const&)
6   0x10a22d25b WebCore::InspectorInstrumentation::takeHeapSnapshotImpl(WebCore::InstrumentingAgents&, WTF::String const&)
7   0x10afb3b15 WebCore::InspectorInstrumentation::takeHeapSnapshot(WebCore::Frame&, WTF::String const&)
8   0x10afb351d WebCore::PageConsoleClient::takeHeapSnapshot(JSC::ExecState*, WTF::String const&)
9   0x1031217ce JSC::consoleProtoFuncTakeHeapSnapshot(JSC::ExecState*)
10  0x4c83c4e01028
11  0x103c5a054 llint_entry
12  0x103c5a054 llint_entry
13  0x103c52b6e vmEntryToJavaScript
14  0x103a3419c JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
15  0x1039b04ae JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*)
16  0x1032cc455 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
17  0x1032cc69e JSC::evaluateWithScopeExtension(JSC::ExecState*, JSC::SourceCode const&, JSC::JSObject*, WTF::NakedPtr<JSC::Exception>&)
18  0x103b2126b Inspector::JSInjectedScriptHost::evaluateWithScopeExtension(JSC::ExecState*)
19  0x103b2837a Inspector::jsInjectedScriptHostPrototypeFunctionEvaluateWithScopeExtension(JSC::ExecState*)
20  0x4c83c4e01028
21  0x103c59fda llint_entry
22  0x103c59fda llint_entry
23  0x103c59fda llint_entry
24  0x103c52b6e vmEntryToJavaScript
25  0x103a3419c JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
26  0x1039b0aaf JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
27  0x1031fc8de JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
28  0x1031fc9b9 JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
29  0x10a3cc62b WebCore::JSMainThreadExecState::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
30  0x10a7dbd4d WebCore::functionCallHandlerFromAnyThread(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
31  0x103e88288 Deprecated::ScriptFunctionCall::call(bool&)
Comment 1 Ryan Haddad 2016-10-12 09:53:54 PDT
Started with https://trac.webkit.org/changeset/207179
Comment 2 Ryan Haddad 2016-10-12 09:55:01 PDT
Seen with these tests on all mac debug builds:

inspector/console/heapSnapshot.html
inspector/timeline/setInstruments-programmatic-capture.html

https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK2%20(Tests)/r207192%20(299)/results.html
Comment 3 Filip Pizlo 2016-10-12 11:11:25 PDT
I have a fix!
Comment 4 Filip Pizlo 2016-10-12 11:20:39 PDT
Created attachment 291371 [details]
the patch
Comment 5 Mark Lam 2016-10-12 11:26:04 PDT
Comment on attachment 291371 [details]
the patch

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

r=me

> Source/JavaScriptCore/heap/HeapSnapshot.cpp:126
>              ASSERT(node.cell != previousCell);

Should this be a RELEASE_ASSERT?
Comment 6 Filip Pizlo 2016-10-12 11:27:05 PDT
Comment on attachment 291371 [details]
the patch

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

>> Source/JavaScriptCore/heap/HeapSnapshot.cpp:126
>>              ASSERT(node.cell != previousCell);
> 
> Should this be a RELEASE_ASSERT?

It could be.  But we're in a #ifndef NDEBUG block, so it wouldn't do any good.
Comment 7 Mark Lam 2016-10-12 20:11:47 PDT
Patch was landed in r207230: <https://trac.webkit.org/changeset/207230>.