Bug 137955 - Crashes in WinCairo 64-bit
Summary: Crashes in WinCairo 64-bit
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 8
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 139229
Blocks:
  Show dependency treegraph
 
Reported: 2014-10-22 05:31 PDT by Alexei Bykov
Modified: 2020-10-12 14:58 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexei Bykov 2014-10-22 05:31:24 PDT
I've been using WinCairo port about 2,5 years. Few months ago I started to build 64-bit configurations. Since then I've got a bunch of crashes in JIT code. In all cases callstacks were useless. I think this is caused by manual stack manipulating in JIT which may be incompatible with win64 calling convention. In the end I found an almost stable way to get WebKit crashed. All you need to do is open a complex HTML-page (for example, GWT-based web app), then open Web Inspector and randomly click on different nodes. Once you get a crash in this piece of code:

00000000209610C0  test        r15,rcx  
00000000209610C3  jne         0000000020961842  
00000000209610C9  cmp         dword ptr [rcx],4EA6h  
00000000209610CF  jne         0000000020961858  
00000000209610D5  mov         rdx,1415E210h  
00000000209610DF  mov         rdx,qword ptr [rdx+8]  
00000000209610E3  mov         dword ptr [rsp+18h],1  
00000000209610EB  mov         qword ptr [rsp+10h],rdx  
00000000209610F0  mov         qword ptr [rsp+20h],rcx  
00000000209610F5  mov         qword ptr [rbp-28h],rax  
00000000209610F9  mov         dword ptr [rbp+2Ch],80000002h  
0000000020961100  mov         r11,0  
000000002096110A  cmp         rdx,r11  
000000002096110D  jne         0000000020961126  
0000000020961113  mov         rax,qword ptr [rdx+18h]  ; <- RDX is always zero here because of 3 previous commands
0000000020961117  mov         qword ptr [rsp+8],rax  
000000002096111C  call        0000000020961121  
0000000020961121  jmp         0000000020961138  
0000000020961126  mov         rax,rdx  
0000000020961129  mov         rcx,1FE630C0h  
0000000020961133  call        000000000AFA0900  
0000000020961138  mov         r11,0Eh  
0000000020961142  cmp         rax,r11  
0000000020961145  ja          0000000020961884  
000000002096114B  mov         rcx,qword ptr [rbp-28h]  
000000002096114F  cmp         rax,rcx  
0000000020961152  sete        dl  
0000000020961155  movzx       edx,dl  
0000000020961158  or          edx,6  
000000002096115B  test        dl,1  
000000002096115E  je          0000000020961169  
0000000020961164  jmp         00000000209613DB  

Code above is always the same. Callstack always is broken and contain only 1 entry. I'm not sure if this code related to JIT or not. It might be a bug of Web Inspector.
Recently, I've turned off JIT and enabled C-loop. Today I got WebKit crashed again. But now I have a normal callstack.

>	JavaScriptCore.dll!JSC::JSObject::canSetIndexQuicklyForPutDirect(unsigned int i) Line 309	C++
 	JavaScriptCore.dll!JSC::JSObject::putDirectIndex(JSC::ExecState * exec, unsigned int propertyName, JSC::JSValue value, unsigned int attributes, JSC::PutDirectIndexMode mode) Line 164	C++
 	JavaScriptCore.dll!JSC::JSObject::putDirectIndexBeyondVectorLength(JSC::ExecState * exec, unsigned int i, JSC::JSValue value, unsigned int attributes, JSC::PutDirectIndexMode mode) Line 2249	C++
 	JavaScriptCore.dll!JSC::JSObject::putDirectIndex(JSC::ExecState * exec, unsigned int propertyName, JSC::JSValue value) Line 172	C++
 	JavaScriptCore.dll!JSC::arrayProtoFuncSlice(JSC::ExecState * exec) Line 618	C++
 	JavaScriptCore.dll!JSC::LLInt::CLoop::execute(JSC::OpcodeID entryOpcodeID, void * executableAddress, JSC::VM * vm, JSC::ProtoCallFrame * protoCallFrame, bool isInitializationPass) Line 6780	C++
 	JavaScriptCore.dll!vmEntryToJavaScript(void * executableAddress, JSC::VM * vm, JSC::ProtoCallFrame * protoCallFrame) Line 100	C++
 	JavaScriptCore.dll!JSC::JITCode::execute(JSC::VM * vm, JSC::ProtoCallFrame * protoCallFrame) Line 57	C++
 	JavaScriptCore.dll!JSC::Interpreter::executeCall(JSC::ExecState * callFrame, JSC::JSObject * function, JSC::CallType callType, const JSC::CallData & callData, JSC::JSValue thisValue, const JSC::ArgList & args) Line 975	C++
 	JavaScriptCore.dll!JSC::call(JSC::ExecState * exec, JSC::JSValue functionObject, JSC::CallType callType, const JSC::CallData & callData, JSC::JSValue thisValue, const JSC::ArgList & args, JSC::JSValue * exception) Line 45	C++
 	WebKit.dll!WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext * scriptExecutionContext, WebCore::Event * event) Line 128	C++
 	WebKit.dll!WebCore::EventTarget::fireEventListeners(WebCore::Event * event, WebCore::EventTargetData * d, WTF::Vector<WebCore::RegisteredEventListener,1,WTF::CrashOnOverflow> & entry) Line 247	C++
 	WebKit.dll!WebCore::EventTarget::fireEventListeners(WebCore::Event * event) Line 197	C++
 	WebKit.dll!WebCore::EventTarget::dispatchEvent(WTF::PassRefPtr<WebCore::Event> event) Line 161	C++
 	WebKit.dll!WebCore::XMLHttpRequestProgressEventThrottle::dispatchEvent(WTF::PassRefPtr<WebCore::Event> event) Line 104	C++
 	WebKit.dll!WebCore::XMLHttpRequestProgressEventThrottle::dispatchReadyStateChangeEvent(WTF::PassRefPtr<WebCore::Event> event, WebCore::ProgressEventAction progressEventAction) Line 91	C++
 	WebKit.dll!WebCore::XMLHttpRequest::callReadyStateChangeListener() Line 367	C++
 	WebKit.dll!WebCore::XMLHttpRequest::didFinishLoading(unsigned long identifier, double __formal) Line 1106	C++
 	WebKit.dll!WebCore::CachedResource::checkNotify() Line 346	C++
 	WebKit.dll!WebCore::CachedRawResource::finishLoading(WebCore::ResourceBuffer * data) Line 101	C++
 	WebKit.dll!WebCore::SubresourceLoader::didFinishLoading(double finishTime) Line 311	C++
 	WebKit.dll!WebCore::ResourceHandleManager::downloadTimerCallback(WebCore::Timer<WebCore::ResourceHandleManager> * __formal) Line 726	C++
 	WebKit.dll!WebCore::ThreadTimers::sharedTimerFiredInternal() Line 135	C++
 	WebKit.dll!WebCore::TimerWindowWndProc(HWND__ * hWnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 108	C++

I'm not sure if it's reproducible. I will try to reproduce it.
Comment 1 Michael Saboff 2014-12-02 11:33:40 PST
The disassembly that you listed does look like JIT'ed code, due to comparing with r15, the use of r11 as a temp register, the move 0 into r11, comparing r11 to rdx.  The sequence
    0000000020961100  mov         r11,0  
    000000002096110A  cmp         rdx,r11  
    000000002096110D  jne         0000000020961126
is probably emitted by a call to branchPtrWithPatch().

Looking at the whole sequence, it looks like it was emitted by SpeculativeJIT::emitCall().  What is curious is that the value moved into r11 at 
    0000000020961100  mov         r11,0
and the call target at
    000000002096111C  call        0000000020961121
have not been "linked" to real call target values.  I think that is the bug here, we are calling a function but haven't been linked to that function.
Comment 2 peavo 2014-12-03 10:41:54 PST
I just ran the stress tests without any crashes (some failures, though), so it doesn't seem this crash is caught by the tests.
Comment 3 peavo 2014-12-03 11:04:18 PST
This is a long shot, but I've noticed that there is missing a stack allocation for parameters in CallEdgeProfile.cpp:


Index: bytecode/CallEdgeProfile.cpp
===================================================================
--- bytecode/CallEdgeProfile.cpp        (revisjon 176518)
+++ bytecode/CallEdgeProfile.cpp        (arbeidskopi)
@@ -310,9 +310,15 @@
     jit.subPtr(CCallHelpers::TrustedImm32(stackAlignmentBytes()), CCallHelpers::stackPointerRegister);

     jit.storeValue(calleeRegs, CCallHelpers::Address(CCallHelpers::stackPointerRegister, sizeof(JSValue)));
+#if OS(WINDOWS) && CPU(X86_64)
+    jit.sub64(CCallHelpers::TrustedImm32(4 * sizeof(int64_t)), X86Registers::esp);
+#endif
     jit.setupArguments(CCallHelpers::TrustedImmPtr(this));
     jit.move(CCallHelpers::TrustedImmPtr(bitwise_cast<void*>(operationProcessCallEdgeLog)), GPRInfo::nonArgGPR0);
     jit.call(GPRInfo::nonArgGPR0);
+#if OS(WINDOWS) && CPU(X86_64)
+    jit.add64(CCallHelpers::TrustedImm32(4 * sizeof(int64_t)), X86Registers::esp);
+#endif
     jit.loadValue(CCallHelpers::Address(CCallHelpers::stackPointerRegister, sizeof(JSValue)), calleeRegs);

     jit.addPtr(CCallHelpers::TrustedImm32(stackAlignmentBytes()), CCallHelpers::stackPointerRegister);
Comment 4 Michael Saboff 2014-12-03 11:17:51 PST
(In reply to comment #3)
> This is a long shot, but I've noticed that there is missing a stack
> allocation for parameters in CallEdgeProfile.cpp:
> 
> 
> Index: bytecode/CallEdgeProfile.cpp
> ===================================================================
> --- bytecode/CallEdgeProfile.cpp        (revisjon 176518)
> +++ bytecode/CallEdgeProfile.cpp        (arbeidskopi)
> @@ -310,9 +310,15 @@
>      jit.subPtr(CCallHelpers::TrustedImm32(stackAlignmentBytes()),
> CCallHelpers::stackPointerRegister);
> 
>      jit.storeValue(calleeRegs,
> CCallHelpers::Address(CCallHelpers::stackPointerRegister, sizeof(JSValue)));
> +#if OS(WINDOWS) && CPU(X86_64)
> +    jit.sub64(CCallHelpers::TrustedImm32(4 * sizeof(int64_t)),
> X86Registers::esp);
> +#endif
>      jit.setupArguments(CCallHelpers::TrustedImmPtr(this));
>     
> jit.move(CCallHelpers::
> TrustedImmPtr(bitwise_cast<void*>(operationProcessCallEdgeLog)),
> GPRInfo::nonArgGPR0);
>      jit.call(GPRInfo::nonArgGPR0);
> +#if OS(WINDOWS) && CPU(X86_64)
> +    jit.add64(CCallHelpers::TrustedImm32(4 * sizeof(int64_t)),
> X86Registers::esp);
> +#endif
>      jit.loadValue(CCallHelpers::Address(CCallHelpers::stackPointerRegister,
> sizeof(JSValue)), calleeRegs);
> 
>      jit.addPtr(CCallHelpers::TrustedImm32(stackAlignmentBytes()),
> CCallHelpers::stackPointerRegister);

That could be a problem, but likely not the problem.  We are crashing before we get to the call because what we intend to call is null.  I happen to be working on another issue that could be this issue.  Basically we try calling a custom getter, but there isn't one, it is null.

I just filed https://bugs.webkit.org/show_bug.cgi?id=139229 to track that issue.
Comment 5 Fujii Hironori 2020-10-12 14:58:42 PDT
This ticket seems obsolete. Closed as WORKSFORME.