Skip to content

[win/asan] GetInstructionSize: Fix 83 E4 XX to return 3. #119644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

bernhardu
Copy link
Contributor

This consolidates the two different lines for x86 and x86_64 into a single line for both architectures.
And adds a test line.

CC: @zmodem

@llvmbot
Copy link
Member

llvmbot commented Dec 12, 2024

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

Author: None (bernhardu)

Changes

This consolidates the two different lines for x86 and x86_64 into a single line for both architectures.
And adds a test line.

CC: @zmodem


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

2 Files Affected:

  • (modified) compiler-rt/lib/interception/interception_win.cpp (+2-3)
  • (modified) compiler-rt/lib/interception/tests/interception_win_test.cpp (+1)
diff --git a/compiler-rt/lib/interception/interception_win.cpp b/compiler-rt/lib/interception/interception_win.cpp
index 4afc74933a33bc..a5897274521e92 100644
--- a/compiler-rt/lib/interception/interception_win.cpp
+++ b/compiler-rt/lib/interception/interception_win.cpp
@@ -634,6 +634,7 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     case 0xD284:  // 84 D2 : test dl,dl
       return 2;
 
+    case 0xE483:  // 83 E4 XX : and esp, XX
     case 0xEC83:  // 83 EC XX : sub esp, XX
     case 0xC1F6:  // F6 C1 XX : test cl, XX
       return 3;
@@ -643,8 +644,7 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
       return 0;
   }
 
-  switch (0x00FFFFFF & *(u32*)address) {
-    case 0xF8E483:  // 83 E4 F8 : and esp, 0xFFFFFFF8
+  switch (0x00FFFFFF & *(u32 *)address) {
     case 0x24A48D:  // 8D A4 24 XX XX XX XX : lea esp, [esp + XX XX XX XX]
       return 7;
   }
@@ -773,7 +773,6 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
     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 0xe58948:    // 48 89 e5 : mov rbp, rsp
diff --git a/compiler-rt/lib/interception/tests/interception_win_test.cpp b/compiler-rt/lib/interception/tests/interception_win_test.cpp
index 6e01209ac3a7e4..04d9a6766f65ad 100644
--- a/compiler-rt/lib/interception/tests/interception_win_test.cpp
+++ b/compiler-rt/lib/interception/tests/interception_win_test.cpp
@@ -852,6 +852,7 @@ const struct InstructionSizeData {
     { 2, {0x8B, 0xC1}, 0, "8B C1 : mov eax, ecx"},
     { 2, {0x8B, 0xEC}, 0, "8B EC : mov ebp, esp"},
     { 2, {0x8B, 0xFF}, 0, "8B FF : mov edi, edi"},
+    { 3, {0x83, 0xE4, 0x72}, 0, "83 E4 XX : and esp, XX"},
     { 3, {0x83, 0xEC, 0x72}, 0, "83 EC XX : sub esp, XX"},
     { 3, {0xc2, 0x71, 0x72}, 0, "C2 XX XX : ret XX (needed for registering weak functions)"},
     { 5, {0x68, 0x71, 0x72, 0x73, 0x74}, 0, "68 XX XX XX XX : push imm32"},

Copy link
Collaborator

@zmodem zmodem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@@ -643,8 +644,7 @@ static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
return 0;
}

switch (0x00FFFFFF & *(u32*)address) {
case 0xF8E483: // 83 E4 F8 : and esp, 0xFFFFFFF8
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we returned 7 for this before, which seems like a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching, seems I got lost track of this information during perparing this instruction for submission.
I have pushed a new version, just changing the commit message.

Also moves the x86 and x86-64 lines together into a single generic line.

```
$ echo -n -e "0x83, 0xE4, 0x72, 0x90" | ./llvm/build/bin/llvm-mc --disassemble --show-encoding --arch=x86
        .text
        andl    $114, %esp                      # encoding: [0x83,0xe4,0x72]
        nop                                     # encoding: [0x90]
$ echo -n -e "0x83, 0xE4, 0x72, 0x90" | ./llvm/build/bin/llvm-mc --disassemble --show-encoding --arch=x86-64
        .text
        andl    $114, %esp                      # encoding: [0x83,0xe4,0x72]
        nop                                     # encoding: [0x90]
```
@bernhardu bernhardu force-pushed the mr-interception_win-make-83-E4-generic branch from e2a2947 to fbf9cbe Compare December 12, 2024 10:33
@bernhardu
Copy link
Contributor Author

v2:

  • patch is unchanged, just changed commit message to reflect this being a fix to return for 83 E4 F8 a 3 instead of 7.

@bernhardu bernhardu changed the title [win/asan] GetInstructionSize: Make 83 E4 XX a generic entry. [win/asan] GetInstructionSize: Fix 83 E4 XX to return 3. Dec 12, 2024
Copy link
Collaborator

@zmodem zmodem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@zmodem zmodem merged commit f85579f into llvm:main Dec 12, 2024
7 checks passed
@bernhardu bernhardu deleted the mr-interception_win-make-83-E4-generic branch December 13, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants