122472 – Win64 JIT broken

RESOLVED FIXED Bug 122472

Win64 JIT broken

https://bugs.webkit.org/show_bug.cgi?id=122472

Summary Win64 JIT broken

Alex Christensen

Reported 2013-10-07 16:09:35 PDT

The Win64 JIT has broken in the last few days. It worked in r156671. It's broken in r157054. I think the culprit is r156896, but I'm not sure.

Attachments
Patch (5.78 KB, patch) 2013-10-29 20:18 PDT, Alex Christensen	ggaren: review+ commit-queue: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2013-10-08 14:15:35 PDT

<rdar://problem/15179750>

Alex Christensen

Comment 2 2013-10-08 14:27:45 PDT

I confirmed that r156896 broke the Win64 JIT, and it is because edx is now used for returnValue2Register and secondArgumentRegister, but only on Windows. My attempts at blindly switching registers are not successful, though. Michael, do you know what registers should be used instead?

Alex Christensen

Comment 3 2013-10-08 15:00:04 PDT

This is from r156896 because it works in r156895 but not in r156896, but it's not because of edx because returnValue2Register isn't even used in Win64. The call stack says it's crashing inside of operationNewRegexp, which was added in the same revision. exec is 0, regexpPtr is 0x00000000097f14cc.

Geoffrey Garen

Comment 4 2013-10-08 15:01:46 PDT

Perhaps it's secondArgumentRegister that's wrong?

Alex Christensen

Comment 5 2013-10-08 15:32:19 PDT

The first, second, and third argument registers worked with ecx, edx, and r8. I've tried changing them with no success. For those of you who like reading disassembled JIT code, here's what the JIT is generating. I put a break point in the first line of operationNewRegexp to get this. I hope this helps: 00000000088400B5 add byte ptr [rbp-77h],cl 00000000088400B8 pop rbp 00000000088400B9 mov eax,0ABB49h 00000000088400BE add byte ptr [rax],al 00000000088400C0 add byte ptr [rax],al 00000000088400C2 add byte ptr [rax],al 00000000088400C4 mov qword ptr [r13-50h],r11 00000000088400C8 mov r11,0 00000000088400D2 mov qword ptr [r13],r11 00000000088400D6 cmp qword ptr [r13],0 00000000088400DB jne 0000000008840112 00000000088400E1 mov rcx,rsp 00000000088400E4 mov qword ptr [rsp+0A0h],r13 00000000088400EC mov dword ptr [r13+34h],4 00000000088400F4 mov r11,8A72C80h 00000000088400FE mov qword ptr [r11],r13 0000000008840101 mov r11,5C0BA10h 000000000884010B call r11 000000000884010E mov qword ptr [r13],rax 0000000008840112 mov rax,87EF970h 000000000884011C mov qword ptr [r13-58h],rax 0000000008840120 mov rax,qword ptr [0000000008B4C010h] 000000000884012A mov qword ptr [r13-60h],rax 000000000884012E test r15,rax 0000000008840131 jne 00000000088407EB 0000000008840137 mov r11,0D1E7BEEFh 0000000008840141 cmp qword ptr [rax],r11 0000000008840144 jne 00000000088407EB 000000000884014A mov rax,qword ptr [rax+8] 000000000884014E mov rax,qword ptr [rax] 0000000008840152 mov qword ptr [r13-58h],rax 0000000008840156 test r15,rax 0000000008840159 jne 0000000008840835 000000000884015F mov r11,0D1E7BEEFh 0000000008840169 cmp qword ptr [rax],r11 000000000884016C jne 0000000008840835 0000000008840172 mov rax,qword ptr [rax+8] 0000000008840176 mov rax,qword ptr [rax] 000000000884017A mov qword ptr [r13-58h],rax 000000000884017E test r15,rax 0000000008840181 jne 000000000884087F 0000000008840187 mov r11,0D1E7BEEFh 0000000008840191 cmp qword ptr [rax],r11 0000000008840194 jne 000000000884087F 000000000884019A mov rax,qword ptr [rax+8] 000000000884019E mov rax,qword ptr [rax] 00000000088401A2 mov qword ptr [r13-8],rax 00000000088401A6 mov rax,87EF970h 00000000088401B0 mov qword ptr [r13-58h],rax 00000000088401B4 mov rax,qword ptr [0000000008B4C010h] 00000000088401BE mov qword ptr [r13-60h],rax 00000000088401C2 test r15,rax 00000000088401C5 jne 00000000088408C9 00000000088401CB mov r11,0D1E7BEEFh 00000000088401D5 cmp qword ptr [rax],r11 00000000088401D8 jne 00000000088408C9 00000000088401DE mov rax,qword ptr [rax+8] 00000000088401E2 mov rax,qword ptr [rax] 00000000088401E6 mov qword ptr [r13-10h],rax 00000000088401EA mov rax,qword ptr [r13-10h] 00000000088401EE test r15,rax 00000000088401F1 jne 0000000008840913 00000000088401F7 mov r11,0D1E7BEEFh 0000000008840201 cmp qword ptr [rax],r11 0000000008840204 jne 0000000008840913 000000000884020A mov rax,qword ptr [rax+8] 000000000884020E mov rax,qword ptr [rax] 0000000008840212 mov qword ptr [r13-18h],rax 0000000008840216 mov rax,qword ptr [r13-18h] 000000000884021A mov qword ptr [r13-70h],rax 000000000884021E test r15,rax 0000000008840221 jne 000000000884095D 0000000008840227 mov r11,0D1E7BEEFh 0000000008840231 cmp qword ptr [rax],r11 0000000008840234 jne 000000000884095D 000000000884023A mov rax,qword ptr [rax+8] 000000000884023E mov rax,qword ptr [rax] 0000000008840242 mov qword ptr [r13-58h],rax 0000000008840246 mov r11,7D0FB30h 0000000008840250 mov qword ptr [r13-68h],r11 0000000008840254 mov r11,0FFFF000000000000h 000000000884025E mov qword ptr [r13-60h],r11 0000000008840262 lea rdx,[r13-0A8h] 0000000008840269 mov dword ptr [rdx+30h],3 0000000008840270 mov dword ptr [r13+34h],5Eh 0000000008840278 mov qword ptr [rdx+28h],r13 000000000884027C mov qword ptr [rdx+20h],rax 0000000008840280 mov r13,rdx 0000000008840283 mov r11,0 000000000884028D cmp rax,r11 0000000008840290 jne 00000000088409A7 0000000008840296 mov rdx,qword ptr [rax+20h] 000000000884029A mov qword ptr [r13+18h],rdx 000000000884029E call 00000000088402A3 00000000088402A3 mov qword ptr [r13-20h],rax 00000000088402A7 mov rsi,8E0FE70h 00000000088402B1 mov rdi,r13 00000000088402B4 mov dword ptr [r13+34h],67h 00000000088402BC mov r11,8A72C80h 00000000088402C6 mov qword ptr [r11],r13 00000000088402C9 mov r11,5BF4100h 00000000088402D3 call r11 00000000088402D6 mov r11,8A74778h <- VISUAL STUDIO SAYS THIS IS THE CURRENT STATEMENT AND 0 IS IN r11 00000000088402E0 mov r11,qword ptr [r11] 00000000088402E3 test r11,r11 00000000088402E6 jne 0000000008841039 00000000088402EC mov qword ptr [r13-28h],rax 00000000088402F0 mov r11,94E6F70h 00000000088402FA mov qword ptr [rsp+28h],r11 00000000088402FF mov rcx,rsp 0000000008840302 mov qword ptr [rsp+0A0h],r13 000000000884030A mov dword ptr [r13+34h],6Ah 0000000008840312 mov r11,8A72C80h 000000000884031C mov qword ptr [r11],r13 000000000884031F mov r11,5C0E410h 0000000008840329 call r11 000000000884032C mov qword ptr [r13-30h],rax 0000000008840330 mov rax,qword ptr [r13-10h] 0000000008840334 test r15,rax 0000000008840337 jne 00000000088409B5 000000000884033D mov r11,0D1E7BEEFh 0000000008840347 cmp qword ptr [rax],r11 000000000884034A jne 00000000088409B5 0000000008840350 mov rax,qword ptr [rax+8] 0000000008840354 mov rax,qword ptr [rax] 0000000008840358 mov qword ptr [r13-38h],rax 000000000884035C mov r11,94E6E70h 0000000008840366 mov qword ptr [rsp+28h],r11 000000000884036B mov rcx,rsp 000000000884036E mov qword ptr [rsp+0A0h],r13 0000000008840376 mov dword ptr [r13+34h],76h 000000000884037E mov r11,8A72C80h 0000000008840388 mov qword ptr [r11],r13 000000000884038B mov r11,5C0E410h 0000000008840395 call r11

Mark Lam

Comment 6 2013-10-08 15:45:46 PDT

According to http://msdn.microsoft.com/en-us/library/9z1stfyw.aspx, the argument registers are all correct. Now, looking at the rest of the diff.

Alex Christensen

Comment 7 2013-10-09 12:53:04 PDT

I built 64-bit WinLauncher with the JIT disabled from r157159 and it runs most JavaScript fine, but it crashes doing the SunSpider 1.0.1 recursive control flow test. There might be another problem, or r156896 may have introduced a non-JIT bug as well, but SunSpider runs successfully at r156895.

Mark Lam

Comment 8 2013-10-11 11:05:56 PDT

(In reply to comment #7) > I built 64-bit WinLauncher with the JIT disabled from r157159 and it runs most JavaScript fine, but it crashes doing the SunSpider 1.0.1 recursive control flow test. There might be another problem, or r156896 may have introduced a non-JIT bug as well, but SunSpider runs successfully at r156895. Alex, did you get the LLINT building on Windows already? Regarding the crash you're seeing, were you using a release or debug build? I'm seeing what appears to be a different crash in a debug build.

Alex Christensen

Comment 9 2013-10-11 13:22:58 PDT

(In reply to comment #8) > (In reply to comment #7) > > I built 64-bit WinLauncher with the JIT disabled from r157159 and it runs most JavaScript fine, but it crashes doing the SunSpider 1.0.1 recursive control flow test. There might be another problem, or r156896 may have introduced a non-JIT bug as well, but SunSpider runs successfully at r156895. > > Alex, did you get the LLINT building on Windows already? Yes. I think it's been building in Win64 for a while. I just disabled the JIT in Platform.h and it should remove any JIT-related bugs. > > Regarding the crash you're seeing, were you using a release or debug build? I'm seeing what appears to be a different crash in a debug build. I'm pretty sure it was in a release build of WinCairo.

Mark Lam

Comment 10 2013-10-11 13:26:26 PDT

(In reply to comment #9) > (In reply to comment #8) > > Alex, did you get the LLINT building on Windows already? > Yes. I think it's been building in Win64 for a while. I just disabled the JIT in Platform.h and it should remove any JIT-related bugs. FYI, disabling JIT in Platform.h will also disable the ASM LLINT. Instead, it will force the use of the C loop LLINT. To disable the JIT but still use the ASM LLINT, add useJIT() = false; in Options::initialize().

Alex Christensen

Comment 11 2013-10-17 14:38:38 PDT

As of r157589, all tests run successfully on Win64 with ENABLE_JIT defined to be 0. They all fail with the JIT, which means that the only problem now is in the JIT. Should any of the assembly in JitStubsMSVC64.asm be changed?

Mark Lam

Comment 12 2013-10-17 14:43:16 PDT

(In reply to comment #11) > As of r157589, all tests run successfully on Win64 with ENABLE_JIT defined to be 0. They all fail with the JIT, which means that the only problem now is in the JIT. Should any of the assembly in JitStubsMSVC64.asm be changed? With ENABLE_JIT defined to 0, you're testing the C Loop LLINT. The next step is to ENABLE_JIT 1 and disable the JIT at runtime so that you can run the ASM LLINT. To disable the JIT at runtime, search in Source/JavaScriptCore/runtime/Options.cpp for "useJIT() = false;". That statement disables the JIT but is #ifdef'ed out. You can simply copy and paste it below the #endif. The useJIT() flag can be set more than once. The last value it is set to will be the one that takes effect. To disable the JIT, you'll want to set it to false.

Michael Saboff

Comment 13 2013-10-17 14:45:40 PDT

(In reply to comment #11) > As of r157589, all tests run successfully on Win64 with ENABLE_JIT defined to be 0. They all fail with the JIT, which means that the only problem now is in the JIT. Should any of the assembly in JitStubsMSVC64.asm be changed? I broke things in r157581. I'm currently working on a fix (https://bugs.webkit.org/show_bug.cgi?id=122980)

Alex Christensen

Comment 14 2013-10-25 13:34:50 PDT

The Win64 JIT is still broken. I assume nobody tested http://trac.webkit.org/changeset/157650 because it crashes immediately after the call rcx in ctiTrampoline. I'm not sure where rcx is coming from. Is there something I could read about how the JIT works? I'm not even sure what a trampoline is.

Geoffrey Garen

Comment 15 2013-10-25 14:33:27 PDT

ctiTrampoline is the function that is called before entering JIT code. Its job is to set up the C stack frame that JIT code will use, when calling from C to JIT code, and then to tear it down when returning to C from JIT code.

Alex Christensen

Comment 16 2013-10-29 20:18:33 PDT

Created attachment 215465 [details] Patch

Alex Christensen

Comment 17 2013-10-29 20:23:40 PDT

This is way outside of my expertise to fix. The argumentGPR registers need to be changed in GPRInfo.h and something is still wrong with the Visual Studio calling conventions. Let's disable it until somebody who knows the JIT fixes it.

Geoffrey Garen

Comment 18 2013-10-29 23:45:11 PDT

Comment on attachment 215465 [details] Patch r=me

WebKit Commit Bot

Comment 19 2013-10-30 08:32:02 PDT

Comment on attachment 215465 [details] Patch Rejecting attachment 215465 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 215465, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: t.org/repository/webkit/trunk ... Authentication realm: <http://svn.webkit.org:80> Mac OS Forge Password for 'commit-queue@webkit.org': Authentication realm: <http://svn.webkit.org:80> Mac OS Forge Username: Use of uninitialized value $username in chomp at /usr/share/git-core/perl/Git/SVN/Prompt.pm line 114. error: git-svn died of signal 11 Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 139 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource Current branch master is up to date. Full output: http://webkit-queues.appspot.com/results/17098194

Alex Christensen

Comment 20 2013-10-30 09:04:33 PDT

r158272

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P3

Severity Major

Classification Unclassified

Version 528+ (Nightly build)

Hardware PC

OS Windows 7

Product WebKit

Component JavaScriptCore

Assignee

Nobody

Reported

2013-10-07 16:09 PDT

Modified

2013-10-30 09:04 PDT History

CC List

8 users Show

URL

Keywords InRadar

Depends on

122787

Blocks

Dependencies

tree graph