Skip to content
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

ASAN on arm64 windows builds but crashes because SANITIZER_WINDOWS64 is used to do x86_64 instruction parsing #64319

Closed
farzonl opened this issue Aug 1, 2023 · 2 comments

Comments

@farzonl
Copy link
Member

farzonl commented Aug 1, 2023

For context you can build asan for arm64 however it asserts in GetInstructionSize because it can't parse the arm instructions.

asan_build

SANITIZER_WINDOWS64 as defined should only be used to determine if we are working on a 32bit or 64bit operating system.

#if defined(_WIN64)
# define SANITIZER_WINDOWS64 1
#else
# define SANITIZER_WINDOWS64 0
#endif

However, in practice it is being used to parse x86_64 instructions.

static size_t GetInstructionSize(uptr address, size_t* rel_offset = nullptr) {
#if SANITIZER_WINDOWS64
if (memcmp((u8*)address, kPrologueWithShortJump1,
sizeof(kPrologueWithShortJump1)) == 0 ||
memcmp((u8*)address, kPrologueWithShortJump2,
sizeof(kPrologueWithShortJump2)) == 0) {
return 0;
}
#endif
switch (*(u64*)address) {
case 0x90909090909006EB: // stub: jmp over 6 x nop.
return 8;
}
switch (*(u8*)address) {
case 0x90: // 90 : nop
return 1;
case 0x50: // push eax / rax
case 0x51: // push ecx / rcx
case 0x52: // push edx / rdx
case 0x53: // push ebx / rbx
case 0x54: // push esp / rsp
case 0x55: // push ebp / rbp
case 0x56: // push esi / rsi
case 0x57: // push edi / rdi
case 0x5D: // pop ebp / rbp

I put up a patch: here https://reviews.llvm.org/D156839
based on:
farzonl@223ee30

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 1, 2023

@llvm/issue-subscribers-backend-aarch64

plotfi pushed a commit that referenced this issue Sep 13, 2023
Updates GetInstructionSize to account for arm64 instruction sizes.

ARM64 instruction are always 4 bytes long but GetInstructionSize in
interception_win.cpp assumes x86_64 which has mixed sizes.

Fix is for: #64319

Before the changeclang_rt.asan_dynamic-aarch64.dll would crash at:
OverrideFunction -> OverrideFunctionWithHotPatch -> GetInstructionSize:825

After the change:
dllthunkintercept -> dllthunkgetrealaddressordie -> InternalGetProcAddress
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this issue Sep 19, 2023
Updates GetInstructionSize to account for arm64 instruction sizes.

ARM64 instruction are always 4 bytes long but GetInstructionSize in
interception_win.cpp assumes x86_64 which has mixed sizes.

Fix is for: llvm#64319

Before the changeclang_rt.asan_dynamic-aarch64.dll would crash at:
OverrideFunction -> OverrideFunctionWithHotPatch -> GetInstructionSize:825

After the change:
dllthunkintercept -> dllthunkgetrealaddressordie -> InternalGetProcAddress
@farzonl
Copy link
Member Author

farzonl commented Feb 19, 2024

fixed by 5a48a82

@farzonl farzonl closed this as completed Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants