Bug 171266

Summary: [Win][x86-64] Some callee saved registers aren't preserved
Product: WebKit Reporter: Fujii Hironori <fujii.hironori>
Component: JavaScriptCoreAssignee: Fujii Hironori <fujii.hironori>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, buildbot, commit-queue, keith_miller, mark.lam, msaboff, pvollan, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=148666
Attachments:
Description Flags
WIP patch
none
WIP patch
none
Patch none

Fujii Hironori
Reported 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
Attachments
WIP patch (1.82 KB, patch)
2017-05-26 02:34 PDT, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
WIP patch (597 bytes, patch)
2017-06-06 00:52 PDT, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Patch (1.37 KB, patch)
2017-06-06 20:25 PDT, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Fujii Hironori
Comment 1 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.
Fujii Hironori
Comment 2 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
Fujii Hironori
Comment 3 2017-05-28 22:40:19 PDT
This is not a right fix. copyCalleeSavesToVMEntryFrameCalleeSavesBuffer in LowLevelInterpreter.asm saves all callee saved registers.
Fujii Hironori
Comment 4 2017-06-06 00:52:46 PDT
Created attachment 312062 [details] WIP patch
Fujii Hironori
Comment 5 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.
Fujii Hironori
Comment 6 2017-06-06 20:25:55 PDT
Created attachment 312158 [details] Patch
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2017-06-06 21:19:57 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.