Bug 124675 – [Win] JavaScript JIT crash (with DFG enabled).

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

I'm currently getting a reproducible crash in JIT code with DFG enabled.
The crash is an access reading violation. 

The offending instruction is shown in the disassembly below:

058F204B  jmp         05501EA3  
058F2050  mov         esi,eax  
058F2052  mov         eax,0FFFFFFFBh      < The value assigned to eax causing the access violation is set here, I think.
058F2057  mov         ebx,5501EA3h  
058F205C  push        ebx  
058F205D  jmp         058F2080  
058F2062  add         byte ptr [eax],al  
058F2064  add         byte ptr [eax],al  
058F2066  add         byte ptr [eax],al  
058F2068  add         byte ptr [eax],al  
058F206A  add         byte ptr [eax],al  
058F206C  add         byte ptr [eax],al  
058F206E  add         byte ptr [eax],al  
058F2070  add         byte ptr [eax],al  
058F2072  add         byte ptr [eax],al  
058F2074  add         byte ptr [eax],al  
058F2076  add         byte ptr [eax],al  
058F2078  add         byte ptr [eax],al  
058F207A  add         byte ptr [eax],al  
058F207C  add         byte ptr [eax],al  
058F207E  add         byte ptr [eax],al  
058F2080  cmp         edx,0FFFFFFFBh  
058F2083  jne         058F20B7  
058F2089  mov         ebx,dword ptr [eax]  < Crash, access reading violation 0FFFFFFFBh

Register values:

EAX = FFFFFFFB EBX = 03B62732 ECX = 007F046E EDX = FFFFFFFB ESI = 07EDF658 EDI = 00DB8F48 EIP = 058F2089 ESP = 002BEEA8 EBP = 0624FBD0 EFL = 00210246

Created attachment 217474 [details]
Patch

I'm not sure that this is the right way to fix it, but it does fix the crash I've been having.

[Geoffrey Garen]

Comment on attachment 217474 [details]
Patch

What this means is that there's some Windows-specific DFG code that uses "nonArgGPR0" when it should have used "regT0". Your patch hides the bug by making nonArgGPR0 equal to regT0. That's not a good long-term solution because the two registers can't be equal on all platforms.

[Michael Saboff]

*** Bug 124683 has been marked as a duplicate of this bug. ***

[Michael Saboff]

(In reply to comment #0)
> I'm currently getting a reproducible crash in JIT code with DFG enabled.
> The crash is an access reading violation. 
> 
> The offending instruction is shown in the disassembly below:
> 
> 058F204B  jmp         05501EA3  
> 058F2050  mov         esi,eax  
> 058F2052  mov         eax,0FFFFFFFBh      < The value assigned to eax causing the access violation is set here, I think.
> 058F2057  mov         ebx,5501EA3h  
> 058F205C  push        ebx  
> 058F205D  jmp         058F2080  
The code above is generated by linkClosureCall():
…
    AssemblyHelpers::Jump done = stubJit.jump();
    
    slowPath.link(&stubJit);
    stubJit.move(calleeGPR, GPRInfo::nonArgGPR0);
#if USE(JSVALUE32_64)
    stubJit.move(CCallHelpers::TrustedImm32(JSValue::CellTag), GPRInfo::nonArgGPR1);
#endif
    stubJit.move(CCallHelpers::TrustedImmPtr(callLinkInfo.callReturnLocation.executableAddress()), GPRInfo::nonArgGPR2);
    stubJit.restoreReturnAddressBeforeReturn(GPRInfo::nonArgGPR2);
    AssemblyHelpers::Jump slow = stubJit.jump();
..



> 058F2062  add         byte ptr [eax],al  
> 058F2064  add         byte ptr [eax],al  
> 058F2066  add         byte ptr [eax],al  
> 058F2068  add         byte ptr [eax],al  
> 058F206A  add         byte ptr [eax],al  
> 058F206C  add         byte ptr [eax],al  
> 058F206E  add         byte ptr [eax],al  
> 058F2070  add         byte ptr [eax],al  
> 058F2072  add         byte ptr [eax],al  
> 058F2074  add         byte ptr [eax],al  
> 058F2076  add         byte ptr [eax],al  
> 058F2078  add         byte ptr [eax],al  
> 058F207A  add         byte ptr [eax],al  
> 058F207C  add         byte ptr [eax],al  
> 058F207E  add         byte ptr [eax],al  
> 058F2080  cmp         edx,0FFFFFFFBh  
> 058F2083  jne         058F20B7  
> 058F2089  mov         ebx,dword ptr [eax]  < Crash, access reading violation 0FFFFFFFBh
This code is generated by virtualForThunkGenerator()
#if USE(JSVALUE64)
    slowCase.append(
        jit.branchTest64(
            CCallHelpers::NonZero, GPRInfo::regT0, GPRInfo::tagMaskRegister));
#else
    slowCase.append(
        jit.branch32(
            CCallHelpers::NotEqual, GPRInfo::regT1,
            CCallHelpers::TrustedImm32(JSValue::CellTag)));
#endif
    jit.loadPtr(CCallHelpers::Address(GPRInfo::regT0, JSCell::structureOffset()), GPRInfo::nonArgGPR2);
    slowCase.append(
        jit.branchPtr(
            CCallHelpers::NotEqual,
            CCallHelpers::Address(GPRInfo::nonArgGPR2, Structure::classInfoOffset()),
            CCallHelpers::TrustedImmPtr(JSFunction::info())));

The fix is to change nonArgGPR0 and nonArgGPR1 to be regT0 and regT1 respectively in linkClosureCall().

Patch in progress.

[Michael Saboff]

Created attachment 217493 [details]
Patch

[Michael Saboff]

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

Tested this on Mac OS X 32 & 64 and ARMv7.

[Geoffrey Garen]

Comment on attachment 217493 [details]
Patch

r=me

[Michael Saboff]

Committed r159593: <http://trac.webkit.org/changeset/159593>

[Babak Shafiei]

<rdar://problem/15516424>

[Darin Adler]

Why does the title of this bug say [Win] when it’s not a Windows-specific issue?

(In reply to comment #11)
> Why does the title of this bug say [Win] when it’s not a Windows-specific issue?

When filed, I assumed it was Win-only. I'm not aware that it crashed on other platforms. The fix was cross-platform, though.

[Darin Adler]

OK. Next time, folks, lets retitle once we discover it’s not platform specific.