Bug 171266 - [Win][x86-64] Some callee saved registers aren't preserved
Summary: [Win][x86-64] Some callee saved registers aren't preserved
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-25 02:52 PDT by Fujii Hironori
Modified: 2017-06-06 21:19 PDT (History)
9 users (show)

See Also:


Attachments
WIP patch (1.82 KB, patch)
2017-05-26 02:34 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (597 bytes, patch)
2017-06-06 00:52 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (1.37 KB, patch)
2017-06-06 20:25 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2017-04-25 02:52:47 PDT
[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
Comment 1 Fujii Hironori 2017-05-26 01:57:46 PDT
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.
Comment 2 Fujii Hironori 2017-05-26 02:34:21 PDT
Created attachment 311345 [details]
WIP patch

* I don't understand CalleeSaveSpaceAsVirtualRegisters
* r12-r15 are also callee saved registers
Comment 3 Fujii Hironori 2017-05-28 22:40:19 PDT
This is not a right fix.
copyCalleeSavesToVMEntryFrameCalleeSavesBuffer in LowLevelInterpreter.asm saves all callee saved registers.
Comment 4 Fujii Hironori 2017-06-06 00:52:46 PDT
Created attachment 312062 [details]
WIP patch
Comment 5 Fujii Hironori 2017-06-06 20:19:28 PDT
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.
Comment 6 Fujii Hironori 2017-06-06 20:25:55 PDT
Created attachment 312158 [details]
Patch
Comment 7 WebKit Commit Bot 2017-06-06 21:19:56 PDT
Comment on attachment 312158 [details]
Patch

Clearing flags on attachment: 312158

Committed r217874: <http://trac.webkit.org/changeset/217874>
Comment 8 WebKit Commit Bot 2017-06-06 21:19:57 PDT
All reviewed patches have been landed.  Closing bug.