[win/asan] Support instructions in GetInstructionSize used by Wine.

[bernhardu]

This patch adds several instructions seen when trying to run a executable built with ASan with llvm-mingw,
with the tip of the main branch from llvm-project (x86 and x86_64). Also another missing instruction visible in the issue 96270.

Also includes instructions collected by
Roman Pišl and Eric Pouech in the Wine bug reports below.

Also changes "44 0f b6 1a" to return 4 instead of 5 (MR 111638). Got fixed in 8417f6a.

Fixes: #96270

Co-authored-by: Roman Pišl
https://bugs.winehq.org/show_bug.cgi?id=50993
https://bugs.winehq.org/attachment.cgi?id=70233

Co-authored-by: Eric Pouech
https://bugs.winehq.org/show_bug.cgi?id=52386
https://bugs.winehq.org/attachment.cgi?id=71626

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[bernhardu]

v2:

• Added another instruction "81 ec XX XX XX XX" which showed up using clang to compile the 32-bit PE side of Wine.

[bernhardu]

v3:

• Another few instructions, which showed up after getting the wine build working, with the 64-bit clang PE side.

[bernhardu]

v4:

• rebase over the recent fix 8417f6a
• add a few more instructions found when trying more applications.

@zmodem Thanks for the offer, and sorry for the delay.
I wonder if this has chances to go in, as it targets not a native windows platform?
And for "41 b8 XX XX XX XX : mov r8d, XX XX XX XX" I am not sure if it should
also consider rel_offset, but I don't know how to test this.

[bernhardu]

v5:

• Rebase to current tip.
• Improve commit comment (hopefully).

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (bernhardu)

Changes

This patch adds several instructions seen when trying to run a executable built with ASan with llvm-mingw,
with the tip of the main branch from llvm-project (x86 and x86_64). Also another missing instruction visible in the issue 96270.

Also includes instructions collected by
Roman Pišl and Eric Pouech in the Wine bug reports below.

Also changes "44 0f b6 1a" to return 4 instead of 5 (MR 111638). Got fixed in 8417f6a.

Fixes: #96270

Co-authored-by: Roman Pišl
https://bugs.winehq.org/show_bug.cgi?id=50993
https://bugs.winehq.org/attachment.cgi?id=70233

Co-authored-by: Eric Pouech
https://bugs.winehq.org/show_bug.cgi?id=52386
https://bugs.winehq.org/attachment.cgi?id=71626

---

Full diff: https://github.com/llvm/llvm-project/pull/113085.diff

1 Files Affected:

• (modified) compiler-rt/lib/interception/interception_win.cpp (+73)

diff --git a/compiler-rt/lib/interception/interception_win.cpp b/compiler-rt/lib/interception/interception_win.cpp
index 077a536dd2a3103..e0d15a49e171dcb 100644
--- a/compiler-rt/lib/interception/interception_win.cpp
+++ b/compiler-rt/lib/interception/interception_win.cpp
@@ -528,6 +528,7 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
 
     case 0xb8:  // b8 XX XX XX XX : mov eax, XX XX XX XX
     case 0xB9:  // b9 XX XX XX XX : mov ecx, XX XX XX XX
+    case 0xBA:  // ba XX XX XX XX : mov edx, XX XX XX XX
       return 5;
 
     // Cannot overwrite control-instruction. Return 0 to indicate failure.
@@ -558,16 +559,31 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     case 0xFF8B:  // 8B FF : mov edi, edi
     case 0xEC8B:  // 8B EC : mov ebp, esp
     case 0xc889:  // 89 C8 : mov eax, ecx
+    case 0xD189:  // 89 D1 : mov ecx, edx
     case 0xE589:  // 89 E5 : mov ebp, esp
     case 0xC18B:  // 8B C1 : mov eax, ecx
+    case 0xC031:  // 31 C0 : xor eax, eax
+    case 0xC931:  // 31 C9 : xor ecx, ecx
+    case 0xD231:  // 31 D2 : xor edx, edx
     case 0xC033:  // 33 C0 : xor eax, eax
     case 0xC933:  // 33 C9 : xor ecx, ecx
     case 0xD233:  // 33 D2 : xor edx, edx
     case 0xDB84:  // 84 DB : test bl,bl
+    case 0xC084:  // 84 C0 : test al,al
     case 0xC984:  // 84 C9 : test cl,cl
     case 0xD284:  // 84 D2 : test dl,dl
       return 2;
 
+    case 0x3980:  // 80 39 XX : cmp BYTE PTR [rcx], XX
+    case 0xE483:  // 83 E4 XX : and esp, XX
+    case 0x4D8B:  // 8B 4D XX : mov XX(%ebp), ecx
+    case 0x558B:  // 8B 55 XX : mov XX(%ebp), edx
+    case 0x758B:  // 8B 75 XX : mov XX(%ebp), esp
+      return 3;
+
+    case 0xec81:  // 81 ec XX XX XX XX : sub esp, XX XX XX XX
+      return 6;
+
     // Cannot overwrite control-instruction. Return 0 to indicate failure.
     case 0x25FF:  // FF 25 XX XX XX XX : jmp [XXXXXXXX]
       return 0;
@@ -577,6 +593,8 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     case 0xF8E483:  // 83 E4 F8 : and esp, 0xFFFFFFF8
     case 0x64EC83:  // 83 EC 64 : sub esp, 64h
       return 3;
+    case 0x244C8D:  // 8D 4C 24 XX : lea ecx, [esp + XX]
+      return 4;
     case 0x24A48D:  // 8D A4 24 XX XX XX XX : lea esp, [esp + XX XX XX XX]
       return 7;
   }
@@ -643,6 +661,8 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     case 0x7980:  // 80 79 YY XX  cmp BYTE ptr [rcx+YY], XX
       return 4;
 
+    case 0xb841:  // 41 b8 XX XX XX XX : mov r8d, XX XX XX XX
+      return 6;
     case 0x058B:  // 8B 05 XX XX XX XX : mov eax, dword ptr [XX XX XX XX]
       if (rel_offset)
         *rel_offset = 2;
@@ -673,6 +693,9 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     case 0xc1ff48:    // 48 ff c1 : inc rcx
     case 0xc1ff49:    // 49 ff c1 : inc r9
     case 0xc28b41:    // 41 8b c2 : mov eax, r10d
+    case 0x01b60f:    // 0f b6 01 : movzx eax, BYTE PTR [rcx]
+    case 0x09b60f:    // 0f b6 09 : movzx ecx, BYTE PTR [rcx]
+    case 0x11b60f:    // 0f b6 11 : movzx edx, BYTE PTR [rcx]
     case 0xc2b60f:    // 0f b6 c2 : movzx eax, dl
     case 0xc2ff48:    // 48 ff c2 : inc rdx
     case 0xc2ff49:    // 49 ff c2 : inc r10
@@ -691,6 +714,7 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     case 0xc98548:    // 48 85 c9 : test rcx, rcx
     case 0xc9854d:    // 4d 85 c9 : test r9, r9
     case 0xc98b4c:    // 4c 8b c9 : mov r9, rcx
+    case 0xd12948:    // 48 29 d1 : sub rcx, rdx
     case 0xca2b48:    // 48 2b ca : sub rcx, rdx
     case 0xca3b48:    // 48 3b ca : cmp rcx, rdx
     case 0xd12b48:    // 48 2b d1 : sub rdx, rcx
@@ -700,17 +724,34 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     case 0xd2854d:    // 4d 85 d2 : test r10, r10
     case 0xd28b4c:    // 4c 8b d2 : mov r10, rdx
     case 0xd2b60f:    // 0f b6 d2 : movzx edx, dl
+    case 0xd2be0f:    // 0f be d2 : movsx edx, dl
     case 0xd98b4c:    // 4c 8b d9 : mov r11, rcx
     case 0xd9f748:    // 48 f7 d9 : neg rcx
+    case 0xc03145:    // 45 31 c0 : xor r8d,r8d
+    case 0xc93145:    // 45 31 c9 : xor r9d,r9d
     case 0xdb3345:    // 45 33 db : xor r11d, r11d
+    case 0xc08445:    // 45 84 c0 : test r8b,r8b
+    case 0xd28445:    // 45 84 d2 : test r10b,r10b
     case 0xdb8548:    // 48 85 db : test rbx, rbx
     case 0xdb854d:    // 4d 85 db : test r11, r11
     case 0xdc8b4c:    // 4c 8b dc : mov r11, rsp
     case 0xe0e483:    // 83 e4 e0 : and esp, 0xFFFFFFE0
     case 0xe48548:    // 48 85 e4 : test rsp, rsp
     case 0xe4854d:    // 4d 85 e4 : test r12, r12
+    case 0xc88948:    // 48 89 c8 : mov rax,rcx
+    case 0xcb8948:    // 48 89 cb : mov rbx,rcx
+    case 0xd08948:    // 48 89 d0 : mov rax,rdx
+    case 0xd18948:    // 48 89 d1 : mov rcx,rdx
+    case 0xd38948:    // 48 89 d3 : mov rbx,rdx
     case 0xe58948:    // 48 89 e5 : mov rbp, rsp
     case 0xed8548:    // 48 85 ed : test rbp, rbp
+    case 0xc88949:    // 49 89 c8 : mov r8, rcx
+    case 0xc98949:    // 49 89 c9 : mov r9, rcx
+    case 0xca8949:    // 49 89 ca : mov r10,rcx
+    case 0xd08949:    // 49 89 d0 : mov r8, rdx
+    case 0xd18949:    // 49 89 d1 : mov r9, rdx
+    case 0xd28949:    // 49 89 d2 : mov r10, rdx
+    case 0xd38949:    // 49 89 d3 : mov r11, rdx
     case 0xed854d:    // 4d 85 ed : test r13, r13
     case 0xf6854d:    // 4d 85 f6 : test r14, r14
     case 0xff854d:    // 4d 85 ff : test r15, r15
@@ -721,6 +762,8 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     case 0x588948:    // 48 89 58 XX : mov QWORD PTR[rax + XX], rbx
     case 0xec8348:    // 48 83 ec XX : sub rsp, XX
     case 0xf88349:    // 49 83 f8 XX : cmp r8, XX
+    case 0x148d4e:    // 4e 8d 14 XX : lea r10, [rcx+r8*XX]
+    case 0x398366:    // 66 83 39 XX : cmp WORD PTR [rcx], XX
       return 4;
 
     case 0x246483:  // 83 64 24 XX YY :   and    DWORD PTR [rsp+XX], YY
@@ -735,6 +778,7 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
       return 6;
 
     case 0xec8148:    // 48 81 EC XX XX XX XX : sub rsp, XXXXXXXX
+    case 0xc0c748:    // 48 c7 c0 XX XX XX XX : mov rax, XX XX XX XX
       return 7;
 
     // clang-format off
@@ -768,7 +812,13 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
   }
 
   switch (*(u32*)(address)) {
+    case 0x01b60f44:  // 44 0f b6 01 : movzx r8d, BYTE PTR [rcx]
+    case 0x09b60f44:  // 44 0f b6 09 : movzx r9d, BYTE PTR [rcx]
+    case 0x0ab60f44:  // 44 0f b6 0a : movzx r8d, BYTE PTR [rdx]
+    case 0x11b60f44:  // 44 0f b6 11 : movzx r10d, BYTE PTR [rcx]
     case 0x1ab60f44:  // 44 0f b6 1a : movzx r11d, BYTE PTR [rdx]
+    case 0x11048d4c:  // 4c 8d 04 11 : lea r8,[rcx+rdx*1]
+    case 0xff488d49:  // 49 8d 48 ff : lea rcx,[r8-0x1]
       return 4;
     case 0x24448b48:  // 48 8b 44 24 XX : mov rax, QWORD ptr [rsp + XX]
     case 0x246c8948:  // 48 89 6C 24 XX : mov QWORD ptr [rsp + XX], rbp
@@ -785,6 +835,29 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
       return 5;
     case 0x24648348:  // 48 83 64 24 XX YY : and QWORD PTR [rsp + XX], YY
       return 6;
+    case 0x24A48D48:  // 48 8D A4 24 XX YY ZZ WW : lea rsp, [rsp + WWZZYYXX]
+      return 8;
+  }
+
+  switch (0xFFFFFFFFFFULL & *(u64*)(address)) {
+    case 0xC07E0F4866:  // 66 48 0F 7E C0 : movq rax,xmm0 (for wine fexp)
+    case 0x0000441F0F:  // 0F 1F 44 00 00 : nop DWORD PTR [rax+rax*1+0x0]
+      return 5;
+  }
+
+  switch (0xFFFFFFFFFFFFULL & *(u64*)(address)) {
+    case 0x841f0f2e6666:  // 66 66 2e 0f 1f 84 YY XX XX XX XX
+                          // data16 cs nop WORD PTR [rax+rax*1 + XX XX XX XX]
+      return 11;
+  }
+
+  switch (*(u64*)(address)) {
+    case 0x010101010101b848: // 48 b8 01 01 01 01 01 01 01 01
+                             // movabs rax,0x101010101010101
+      return 10;
+    case 0x841f0f2e66666666:  // 66 66 66 66 2e 0f 1f 84 YY XX XX XX XX
+                              // data16 data16 data16 cs nop WORD PTR [rax+rax*1 + XX XX XX XX]
+      return 13;
   }
 
 #else

[bernhardu]

Unfortunately just found and will probably change in next version, depending on feedback.

• 83 E4 F8 : and esp, 0xFFFFFFF8 and 83 e4 e0 : and esp, 0xFFFFFFE0
  should be already handled by 83 E4 XX : and esp, XX

• 83 EC 64 : sub esp, 64h should be handled by 83 EC XX : sub esp, XX

• Should't 8A 05 XX XX XX XX return 6, currently it does return 4.
  Is it ok to just change it in the same commit, separate commit, or separte PR?

• Still unsure how to handle (or test) rel_offset?
  I hope I understand now - rel_offset is just how many bytes to skip from the instruction to extract the offset.

• Make f6 c1 07 : test cl, 0x7 also a generic instruction like f6 c1 XX : test cl, XX?

• Should this patch be split up in a few more chunks, or some parts taken out into a different PR?

[bernhardu]

v6:

• split off the changes modifying existing lines in separate commits.
• Added a simple test for known instructions.

[bernhardu]

v7:

• Found with the test a duplicate instruction, removed the line which got never reached.
• Split the test addition into different commits.

Sorry for the ongoing changes, please drop me a note if some commits should go into a separate PR.

[rnk]

@zmodem, are you able to review this at some point?

[mstorsjo]

If this interacts with 8417f6a, can you rebase this change past that, to make things clearer with how it fits together with that?

[bernhardu]

If this interacts with 8417f6a, can you rebase this change past that, to make things clearer with how it fits together with that?

Since the comment "v4" it is already rebased behind 8417f6a, so this should make no problems.
But I see now there was another commit to this file in cdfd4cf. It seems to make no conflict, but should I still rebase over this one?

Thanks for having a look.
So should I mark this PR as draft again, and feed in separate PRs for easier review?

[mstorsjo]

If this interacts with 8417f6a, can you rebase this change past that, to make things clearer with how it fits together with that?

Since the comment "v4" it is already rebased behind 8417f6a, so this should make no problems. But I see now there was another commit to this file in cdfd4cf. It seems to make no conflict, but should I still rebase over this one?

Thanks for having a look. So should I mark this PR as draft again, and feed in separate PRs for easier review?

Ah, ok, I see.

As I'm not qualified enough to really review this, I would hold off of updating it until some of those who might be (@rnk, @zmodem, @vitalybuka or @barcharcraz) says which way they prefer to handle it.

As for adding the extra #ifs to silence warnings due to the unused code blocks - just file that in a separate PR which I can accept quite quickly.

[bernhardu]

... The policy within llvm is ... that PRs are merged with "squash and merge" ...

Oh, I was not aware of this, so this seems also to suggest to use the separate PR approach, Thanks.

[alvinhochun]

This looks wrong. The second operand is an immediate, not a relative offset, so it shouldn't be leading to a branch that sets rel_offset.

[zmodem]

Sorry for the delay, I'll try to prioritize this.

I did a first pass, but didn't spot check any of the actual encodings yet. If you do have some parts that could be separate PRs, that would be helpful.

[zmodem]

What happened to this one?

[bernhardu]

I found this when I created the test.
There is a similar line some lines above which returns 0 as failure for a jmp instruction.

Splitted of this PR: Remove duplicate instruction FF 25 ....
#116894

[zmodem]

Maybe add a comment somewhere explain that 0 means "cannot override"

[bernhardu]

Done in the splitted PR.

[zmodem]

Did you use any tooling to generate these tests? If so, maybe add a comment about it.

[bernhardu]

Unfortunately not, I was basically just copying GetInstructionSize
and editing with notepad++`s multi-line-edit and macro feature.

[zmodem]

There could be more than 3 bytes though. Maybe call a utility function that returns a hex string instead? (same below)

[bernhardu]

Done in the splitted PR.