Bug 171266 – [Win][x86-64] Some callee saved registers aren't preserved

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Fujii Hironori]

[Win] Crashes in JSC::JITCode::execute while using WebInspector

WinCairo port, Release build, trunk@215722

While I'm using WebInspector of MiniBrowser, I often see the JSC crash.
I don't see the same crash in Debug build.

Callstack:

> JavaScriptCore.dll!JSC::JITCode::execute(JSC::VM * vm, JSC::ProtoCallFrame * protoCallFrame) Line 82	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 954	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) Line 39	C++
> JavaScriptCore.dll!JSC::callGetter(JSC::ExecState * exec, JSC::JSValue base, JSC::JSValue getterSetter) Line 87	C++
> JavaScriptCore.dll!JSC::PropertySlot::functionGetter(JSC::ExecState * exec) Line 33	C++
> JavaScriptCore.dll!JSC::operationGetByIdOptimize::__l2::<lambda>(bool found, JSC::PropertySlot & slot) Line 264	C++
> JavaScriptCore.dll!operationGetByIdOptimize(JSC::ExecState * exec, JSC::StructureStubInfo * stubInfo, __int64 base, WTF::UniquedStringImpl * uid) Line 261	C++
> [External Code]	

Disassembly:

> --- c:\webkit\ga\source\javascriptcore\jit\jitcode.cpp -------------------------
> 
> JSValue JITCode::execute(VM* vm, ProtoCallFrame* protoCallFrame)
> {
> 00007FFF08523E50  push        rbx  
> 00007FFF08523E52  push        rsi  
> 00007FFF08523E53  push        rdi  
> 00007FFF08523E54  sub         rsp,30h  
> 00007FFF08523E58  mov         rbx,r9  
> 00007FFF08523E5B  mov         rdi,r8  
> 00007FFF08523E5E  mov         r9,rcx  
> 00007FFF08523E61  mov         rsi,rdx  
>     auto scope = DECLARE_THROW_SCOPE(*vm);
>     void* entryAddress;
>     JSFunction* function = jsDynamicCast<JSFunction*>(*vm, protoCallFrame->callee());
> 00007FFF08523E64  mov         rax,qword ptr [rbx+8]  
> 00007FFF08523E68  mov         ecx,dword ptr [rax]  
> 00007FFF08523E6A  mov         rax,qword ptr [r8+0C8h]  
> 00007FFF08523E71  btr         ecx,1Fh  
> 00007FFF08523E75  mov         rax,qword ptr [rax+rcx*8]  
> 00007FFF08523E79  mov         rax,qword ptr [rax+40h]  
> 00007FFF08523E7D  test        rax,rax  
> 00007FFF08523E80  je          JSC::JITCode::execute+4Eh (07FFF08523E9Eh)  
> 00007FFF08523E82  lea         rcx,[JSC::JSFunction::s_info (07FFF08910340h)]  
> 00007FFF08523E89  nop         dword ptr [rax]  
> 00007FFF08523E90  cmp         rax,rcx  
> 00007FFF08523E93  je          JSC::JITCode::execute+86h (07FFF08523ED6h)  
> 00007FFF08523E95  mov         rax,qword ptr [rax+8]  
> 00007FFF08523E99  test        rax,rax  
> 00007FFF08523E9C  jne         JSC::JITCode::execute+40h (07FFF08523E90h)  
>         ASSERT(!protoCallFrame->needArityCheck());
>         entryAddress = executableAddress();
> 00007FFF08523E9E  mov         rax,qword ptr [r9]  
> 00007FFF08523EA1  xor         edx,edx  
> 00007FFF08523EA3  mov         rcx,r9  
> 00007FFF08523EA6  call        qword ptr [rax+10h]  
>     JSValue result = JSValue::decode(vmEntryToJavaScript(entryAddress, vm, protoCallFrame));
> 00007FFF08523EA9  mov         r8,rbx  
> 00007FFF08523EAC  mov         rdx,rdi  
> 00007FFF08523EAF  mov         rcx,rax  
> 00007FFF08523EB2  call        vmEntryToJavaScript (07FFF08853CC0h)  
>     return scope.exception() ? jsNull() : result;
> 00007FFF08523EB7  cmp         qword ptr [rdi+81A8h],0  
> 00007FFF08523EBF  mov         ecx,2  
> 00007FFF08523EC4  cmovne      rax,rcx  
> 00007FFF08523EC8  mov         qword ptr [rsi],rax  
> 00007FFF08523ECB  mov         rax,rsi  
> }
> 00007FFF08523ECE  add         rsp,30h  
> 00007FFF08523ED2  pop         rdi  
> 00007FFF08523ED3  pop         rsi  
> 00007FFF08523ED4  pop         rbx  
> 00007FFF08523ED5  ret  
> 
>     if (!function || !protoCallFrame->needArityCheck()) {
> 00007FFF08523ED6  cmp         byte ptr [rbx+24h],0  
> 00007FFF08523EDA  je          JSC::JITCode::execute+4Eh (07FFF08523E9Eh)  
>     } else
>         entryAddress = addressForCall(MustCheckArity).executableAddress();
> 00007FFF08523EDC  mov         rax,qword ptr [r9]  
> 00007FFF08523EDF  lea         rdx,[rsp+20h]  
> 00007FFF08523EE4  mov         r8d,1  
> 00007FFF08523EEA  mov         rcx,r9  
> 00007FFF08523EED  call        qword ptr [rax+8]  
> 00007FFF08523EF0  mov         rax,qword ptr [rax]  
> 00007FFF08523EF3  jmp         JSC::JITCode::execute+59h (07FFF08523EA9h)  
> --- No source file -------------------------------------------------------------


Registers:

> RAX = 0000000000000002 RBX = 0000003A6D6FAF68 RCX = 0000000000000002 RDX = 0000021C04823720
> RSI = 0000000700000001 RDI = 0000021C83EF42E0 R8  = 0000003A6D6FBA80 R9  = 40252A53C8000000
> R10 = 0000003A6D6FAE40 R11 = 0000000000000000 R12 = 0000003A6D6FB080 R13 = 0000000000000001
> R14 = 0000021C04823720 R15 = 0000000000000002 RIP = 00007FFF08523EC8 RSP = 0000003A6D6FAEA0
> RBP = 0000003A6D6FAF99 EFL = 00010200

[Fujii Hironori]

current RIP was 00007FFF08523EC8.

> 00007FFF08523EC8  mov         qword ptr [rsi],rax

rsi pointed to a invalid address.
rsi should point to a address to where the return value (JSValue) will be stored.

rsi is a callee saved registers.
It seems that one of the functions called from JITCode::execute destructed rsi.

[Fujii Hironori]

Created attachment 311345 [details]
WIP patch

* I don't understand CalleeSaveSpaceAsVirtualRegisters
* r12-r15 are also callee saved registers

[Fujii Hironori]

This is not a right fix.
copyCalleeSavesToVMEntryFrameCalleeSavesBuffer in LowLevelInterpreter.asm saves all callee saved registers.

[Fujii Hironori]

Created attachment 312062 [details]
WIP patch

[Fujii Hironori]

It takes about 1-15 minutes to reproduce this crash by using Web Inspector of WinCairo port.
I don't know any other method or web sites to reproduce this crash.
I have no idea how to create a regression test for this.
And, the change of Bug 148666 didn't have a layout test.

[Fujii Hironori]

Created attachment 312158 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 312158 [details]
Patch

Clearing flags on attachment: 312158

Committed r217874: <http://trac.webkit.org/changeset/217874>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.