Bug 121001 – [Win] Javascript crash with DFG JIT enabled.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

When I run with DFG JIT enabled on Windows, I'm frequently getting a NULL pointer access violation crash (writing).

The offending assembler instruction is:

07273082  movsd       mmword ptr ds:[0],xmm0  

I have traced this down to the code generation in the function osrExitGenerationThunkGenerator in DFGThunks.cpp.
The problem seems to be that using register GPRInfo::regT0 as parameter (e.g. JIT::storeDouble(..., GPRInfo::regT0)),
results in a call to JIT::storeDouble(FPRegisterID src, const void* address) on Windows, where the address parameter
gets the value of GPRInfo::regT0, which is 0 (eax on Windows). This causes the register to be written to address 0, hence the crash.
I assume the intention here is to write the register to the address in regT0. 


This is the stacktrace of the crash:

 	07273082()	
 	JavaScriptCore.dll!JSC::JITCode::execute(JSC::JSStack * stack, JSC::ExecState * callFrame, JSC::VM * vm)  Line 46 + 0x1e bytes	C++
 	JavaScriptCore.dll!JSC::Interpreter::execute(JSC::ProgramExecutable * program, JSC::ExecState * callFrame, JSC::JSObject * thisObj)  Line 844 + 0x36 bytes	C++
 	JavaScriptCore.dll!JSC::evaluate(JSC::ExecState * exec, const JSC::SourceCode & source, JSC::JSValue thisValue, JSC::JSValue * returnedException)  Line 85	C++
 	WebKit.dll!WebCore::JSMainThreadExecState::evaluate(JSC::ExecState * exec, const JSC::SourceCode & source, JSC::JSValue thisValue, JSC::JSValue * exception)  Line 61 + 0x20 bytes	C++
 	WebKit.dll!WebCore::ScriptController::evaluateInWorld(const WebCore::ScriptSourceCode & sourceCode, WebCore::DOMWrapperWorld * world)  Line 142 + 0x23 bytes	C++
 	WebKit.dll!WebCore::ScriptController::evaluate(const WebCore::ScriptSourceCode & sourceCode)  Line 158 + 0x16 bytes	C++
 	WebKit.dll!WebCore::ScriptElement::executeScript(const WebCore::ScriptSourceCode & sourceCode)  Line 315 + 0x17 bytes	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executePendingScriptAndDispatchEvent(WebCore::PendingScript & pendingScript)  Line 151	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executeParsingBlockingScript()  Line 123	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executeParsingBlockingScripts()  Line 201 + 0x8 bytes	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element> scriptElement, const WTF::TextPosition & scriptStartPosition)  Line 191	C++
 	WebKit.dll!WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder()  Line 273	C++
 	WebKit.dll!WebCore::HTMLDocumentParser::canTakeNextToken(WebCore::HTMLDocumentParser::SynchronousMode mode, WebCore::PumpSession & session)  Line 292	C++
 	WebKit.dll!WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode mode)  Line 536 + 0x10 bytes	C++
 	WebKit.dll!WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode mode)  Line 237	C++
 	WebKit.dll!WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution()  Line 899	C++
 	WebKit.dll!WebCore::HTMLDocumentParser::notifyFinished(WebCore::CachedResource * cachedResource)  Line 939	C++
 	WebKit.dll!WebCore::CachedResource::checkNotify()  Line 369 + 0x13 bytes	C++
 	WebKit.dll!WebCore::CachedResource::finishLoading(WebCore::ResourceBuffer * __formal)  Line 385 + 0xf bytes	C++
 	WebKit.dll!WebCore::CachedScript::finishLoading(WebCore::ResourceBuffer * data)  Line 90	C++
 	WebKit.dll!WebCore::SubresourceLoader::didFinishLoading(double finishTime)  Line 283 + 0x26 bytes	C++
 	WebKit.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal, double finishTime)  Line 489 + 0x18 bytes	C++
 	WebKit.dll!WebCore::ResourceHandleManager::downloadTimerCallback(WebCore::Timer<WebCore::ResourceHandleManager> * __formal)  Line 570 + 0x35 bytes	C++
 	WebKit.dll!WebCore::Timer<WebCore::ResourceLoadScheduler>::fired()  Line 114 + 0x23 bytes	C++
 	WebKit.dll!WebCore::ThreadTimers::sharedTimerFiredInternal()  Line 129 + 0xf bytes	C++
 	WebKit.dll!WebCore::ThreadTimers::sharedTimerFired()  Line 106	C++
 	WebKit.dll!WebCore::TimerWindowWndProc(HWND__ * hWnd, unsigned int message, unsigned int wParam, long lParam)  Line 110 + 0x8 bytes	C++

Created attachment 210972 [details]
Patch

[Filip Pizlo]

Comment on attachment 210972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210972&action=review

> Source/JavaScriptCore/dfg/DFGThunks.cpp:56
> +        jit.storeDouble(FPRInfo::toRegister(i), MacroAssembler::ImplicitAddress(GPRInfo::regT0));

Use Address not ImplicitAddres.

Created attachment 211012 [details]
Patch

[Karlen Simonyan]

(In reply to comment #3)
> Created an attachment (id=211012) [details]
> Patch

Hi! Provided patch resolves some issues, but there still crash behavior - using loops (with iteration > 100) JSC just crashes.

Created attachment 214651 [details]
Patch

Upgraded the patch with changes in trunk.
The same fix was also needed in dfg/DFGOSRExitCompiler32_64.cpp.
I also added an assert if we try to generate code which writes to a null pointer.

[Geoffrey Garen]

Comment on attachment 214651 [details]
Patch

r=me

Would be nice to make the GPRInfo::regT0 type incompatible with void*, so this became a compile error.

[WebKit Commit Bot]

Comment on attachment 214651 [details]
Patch

Clearing flags on attachment: 214651

Committed r158057: <http://trac.webkit.org/changeset/158057>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.