This should match the cleanup done to WebCore::Timer.
Created attachment 282149 [details] Proposed Fix
Attachment 282149 [details] did not pass style-queue: ERROR: Source/WTF/wtf/RunLoop.h:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/RunLoop.h:149: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 83 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 282152 [details] Proposed Fix
Attachment 282152 [details] did not pass style-queue: ERROR: Source/WTF/wtf/RunLoop.h:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/RunLoop.h:149: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 85 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 282154 [details] Proposed Fix
Attachment 282154 [details] did not pass style-queue: ERROR: Source/WTF/wtf/RunLoop.h:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/RunLoop.h:149: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 87 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 282155 [details] Proposed Fix (fixed changelog)
Attachment 282155 [details] did not pass style-queue: ERROR: Source/WTF/wtf/RunLoop.h:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/RunLoop.h:149: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 87 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 282160 [details] Proposed Fix (fix USE(SOUP))
Attachment 282160 [details] did not pass style-queue: ERROR: Source/WTF/wtf/RunLoop.h:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/RunLoop.h:149: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 87 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 282160 [details] Proposed Fix (fix USE(SOUP)) View in context: https://bugs.webkit.org/attachment.cgi?id=282160&action=review This feels nice. r=me, don't break gtk. > Source/WebCore/platform/network/ResourceHandleInternal.h:94 > - , m_timeoutSource(RunLoop::main(), loader, &ResourceHandle::timeoutFired) > + , m_timeoutSource(RunLoop::main(), *loader, &ResourceHandle::timeoutFired) loader should be a ResourceHandle&.
Created attachment 282261 [details] Fix GTK
Attachment 282261 [details] did not pass style-queue: ERROR: Source/WTF/wtf/RunLoop.h:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WTF/wtf/RunLoop.h:149: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 89 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r202580: <http://trac.webkit.org/changeset/202580>
Re-opened since this is blocked by bug 159245
<rdar://problem/27070856>
This was rolled out because it caused Production and Production GuardMalloc to fail. Investigating here.
Some relevant register state and disassembly from the point where it crashed under WKTR: (lldb) reg read General Purpose Registers: rax = 0x00000001011492c0 WebKit`vtable for WebKit::WebInspectorProxy + 64 rbx = 0x000000010367e758 rcx = 0x000000010116fb28 (void *)0x001d80010116fb51 rdx = 0x001d80010116fb29 rdi = 0x000000010367e78c rsi = 0x00000001001e25d0 JavaScriptCore`WTF::initializeMainThreadOnce() rbp = 0x00007fff5fbfe390 rsp = 0x00007fff5fbfe370 r8 = 0x00000000000000f8 r9 = 0x000000010367e740 r10 = 0x003a4601003a4680 r11 = 0x000000010106ab22 WebKit`-[WKObject _apiObject] r12 = 0x0000000104401301 r13 = 0x0000000000000000 r14 = 0x000000010367e768 r15 = 0x000000010a000018 rip = 0x0000000100dd4641 WebKit`WebKit::WebInspectorProxy::WebInspectorProxy(WebKit::WebPageProxy*) + 57 rflags = 0x0000000000010206 cs = 0x000000000000002b fs = 0x0000000000000000 gs = 0x0000000000000000 (lldb) di -f WebKit`WebKit::WebInspectorProxy::WebInspectorProxy: 0x100dd4608 <+0>: pushq %rbp 0x100dd4609 <+1>: movq %rsp, %rbp 0x100dd460c <+4>: pushq %r15 0x100dd460e <+6>: pushq %r14 0x100dd4610 <+8>: pushq %rbx 0x100dd4611 <+9>: pushq %rax 0x100dd4612 <+10>: movq %rsi, %r15 0x100dd4615 <+13>: movq %rdi, %rbx 0x100dd4618 <+16>: callq 0x100e4b874 ; API::Object::Object() 0x100dd461d <+21>: leaq 0x10(%rbx), %r14 0x100dd4621 <+25>: leaq 0x374c68(%rip), %rax ; vtable for WebKit::WebInspectorProxy + 16 0x100dd4628 <+32>: movq %rax, (%rbx) 0x100dd462b <+35>: leaq 0x374c8e(%rip), %rax ; vtable for WebKit::WebInspectorProxy + 64 0x100dd4632 <+42>: movq %rax, 0x10(%rbx) 0x100dd4636 <+46>: movq %r15, 0x18(%rbx) 0x100dd463a <+50>: leaq 0x34(%rbx), %rdi 0x100dd463e <+54>: xorps %xmm0, %xmm0 -> 0x100dd4641 <+57>: movaps %xmm0, 0x20(%rbx) 0x100dd4645 <+61>: movb $0x0, 0x30(%rbx) 0x100dd4649 <+65>: callq 0x100e57d04 ; IPC::Attachment::Attachment() 0x100dd464e <+70>: movl $0x0, 0x40(%rbx) 0x100dd4655 <+77>: movq %rbx, %r15 0x100dd4658 <+80>: subq $-0x80, %r15 0x100dd465c <+84>: movq $0x0, 0x70(%rbx) 0x100dd4664 <+92>: movq $0x0, 0x68(%rbx) 0x100dd466c <+100>: movq $0x0, 0x60(%rbx) 0x100dd4674 <+108>: movq $0x0, 0x58(%rbx) 0x100dd467c <+116>: movq $0x0, 0x50(%rbx) 0x100dd4684 <+124>: movq $0x0, 0x48(%rbx) 0x100dd468c <+132>: callq 0x10109f44c ; symbol stub for: WTF::RunLoop::main() 0x100dd4691 <+137>: movq %r15, %rdi 0x100dd4694 <+140>: movq %rax, %rsi 0x100dd4697 <+143>: callq 0x10109f476 ; symbol stub for: WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&) 0x100dd469c <+148>: leaq 0x39c8ad(%rip), %rax ; vtable for WTF::RunLoop::Timer + 16 0x100dd46a3 <+155>: movq %rax, 0x80(%rbx) 0x100dd46aa <+162>: leaq 0xa0(%rbx), %rax 0x100dd46b1 <+169>: movq %rax, 0xc0(%rbx) 0x100dd46b8 <+176>: leaq 0x39dee9(%rip), %rax ; vtable for std::__1::__function::__func<std::__1::__bind<void (WebKit::WebInspectorProxy::*&)(), WebKit::WebInspectorProxy*>, std::__1::allocator<std::__1::__bind<void (WebKit::WebInspectorProxy::*&)(), WebKit::WebInspectorProxy*> >, void ()> + 16 0x100dd46bf <+183>: movq %rax, 0xa0(%rbx) 0x100dd46c6 <+190>: movq 0x364d22(%rip), %xmm0 ; (void *)0x0000000100f917da: WebKit::WebInspectorProxy::closeTimerFired(), xmm0 = mem[0],zero 0x100dd46ce <+198>: movdqu %xmm0, 0xa8(%rbx) 0x100dd46d6 <+206>: movq %rbx, 0xb8(%rbx) 0x100dd46dd <+213>: movq $0x0, 0xd0(%rbx) 0x100dd46e8 <+224>: movq 0x18(%rbx), %rax 0x100dd46ec <+228>: movq 0xd0(%rax), %rdi 0x100dd46f3 <+235>: movq 0x5b0(%rax), %rcx 0x100dd46fa <+242>: leaq 0x33ae6c(%rip), %rsi ; "WebInspectorProxy" 0x100dd4701 <+249>: movl $0x11, %edx 0x100dd4706 <+254>: movq %r14, %r8 0x100dd4709 <+257>: addq $0x8, %rsp 0x100dd470d <+261>: popq %rbx 0x100dd470e <+262>: popq %r14 0x100dd4710 <+264>: popq %r15 0x100dd4712 <+266>: popq %rbp 0x100dd4713 <+267>: jmp 0x100e64cd0 ; WebKit::ChildProcessProxy::addMessageReceiver(IPC::StringReference, unsigned long long, IPC::MessageReceiver&)
Let’s dive into the assembly a bit and see what’s happening here: WebKit`WebKit::WebInspectorProxy::WebInspectorProxy: 0x100dd4608 <+0>: pushq %rbp // Save the caller frame pointer 0x100dd4609 <+1>: movq %rsp, %rbp // Setup the new frame pointer 0x100dd460c <+4>: pushq %r15 // Save a few registers r15, r14, rbx, rax. 0x100dd460e <+6>: pushq %r14 0x100dd4610 <+8>: pushq %rbx 0x100dd4611 <+9>: pushq %rax 0x100dd4612 <+10>: movq %rsi, %r15 // Copy arg1 (i.e. inspectedPage) to %r15. %r15 is a callee saved reg. 0x100dd4615 <+13>: movq %rdi, %rbx // Copy arg0 (i.e. this pointer) to $rbx. $rbx is a callee saved reg. 0x100dd4618 <+16>: callq 0x100e4b874 ; API::Object::Object() // Call the super’s super’s constructor i.e. WebKit2’s API::Object(). 0x100dd461d <+21>: leaq 0x10(%rbx), %r14 // Set %r14 (a callee saved reg) = value at offset 0x10 ie.. 16 in this. // Offset 0 is the vtbl pointer. // Offset 8 is m_refCount from the inheritted ThreadSafeRefCountedBase. // Offset 16 is NSObject* m_wrapper from API::Object. // Hence, %r14 is this.m_wrapper. // Note: API::Object does not appear to initialize m_wrapper though. So this looks funky. Why is it loaded? 0x100dd4621 <+25>: leaq 0x374c68(%rip), %rax ; vtable for WebKit::WebInspectorProxy + 16 // Set %rax = &vtbl 0x100dd4628 <+32>: movq %rax, (%rbx) // Set this.vtbl = vtbl i.e install the WebInspectorProxy vtbl for API::ObjectImpl ??? 0x100dd462b <+35>: leaq 0x374c8e(%rip), %rax ; vtable for WebKit::WebInspectorProxy + 64 // Neat … another vtbl because WebInspectorProxy multiply inherits from IPC::MessageReceiver as well // and IPC::MessageReceiver has virtual functions too. 0x100dd4632 <+42>: movq %rax, 0x10(%rbx) // What is this??? Why is this.m_wrapper being set to the 2nd vtbl? 0x100dd4636 <+46>: movq %r15, 0x18(%rbx). // Set this + offset 24 i.e. this.m_inspectedPage to %r15 i.e. inspectedPage. // We just initialized the m_inspectedPage field above. The next step (according to the code for WebInspectorProxy’s constructor) is to call RunLoop::main(). 0x100dd463a <+50>: leaq 0x34(%rbx), %rdi // Set %rdi i.e. arg0 = &this + offset 0x34 i.e. 52 // Strange!!! // offset 0x18: m_inspectedPage // offset 0x20: m_inspectorPage // offset 0x28: m_underTest // offset 0x29: m_isVisible // offset 0x2a: m_isAttached // offset 0x2b: m_canAttach // offset 0x2c: m_isProfilingPage // offset 0x2d: m_showMessageSent // offset 0x2e: m_ignoreFirstBringToFront // offset 0x2f: m_elementSelectionActive // offset 0x30: m_ignoreElementSelectionChange // And presumably: // offset 0x38: m_connectionIdentifier // What is at offset 0x34? 0x100dd463e <+54>: xorps %xmm0, %xmm0 // xor xmm0 with self i.e. set xmm0 = 0. -> 0x100dd4641 <+57>: movaps %xmm0, 0x20(%rbx) // mov 0 to address %rbx + 0x20 i.e. this.m_inspectorPage. // Another bit of strangeness: why is instruction <+57>: crashing? The only memory we access here is %rbx, and we’re just trying to set this.m_inspectorPage to nullptr. // Note that we had just access the written to the contents of this object just a few instructions up e.g. <+46>:, <+42>:, and <+32>:, and those did not result in a crash. I did my analysis above based on the source from ToT. Maybe some code has changed, and this disassembly doesn't match what's in ToT. Or maybe I made a mistake somewhere. Brian, can you please recheck my analysis with the proper source code?
Pull request: https://github.com/WebKit/WebKit/pull/7693
Committed 257984@main (d94e96e0fc4a): <https://commits.webkit.org/257984@main> Reviewed commits have been landed. Closing PR #7693 and removing active labels.