[PATCH 0/1] MR6179: include: Ensure that x86_64 syscall thunks have a consistent length when built with Clang.
GCC always assembles `jmp 1f` to `eb 01`, as does Clang when using -O1 or higher optimization. But with -O0, Clang outputs `e9 01 00 00 00`. The `subq $0xb,0x70(%rcx)` line in __wine_syscall_dispatcher relies on `jmp 1f` being 2 bytes. Since Wine defaults to `-g -O2` for `CFLAGS`, this wouldn't show up with a default Clang build. But the FreeBSD `wine-devel` port must override `CFLAGS`, it uses Clang for PE and was crashing on launch. See <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=280000>. Here are compiler explorer links to see the difference: [PE](https://godbolt.org/z/35TxW8dK9) and [ELF](https://godbolt.org/z/1h3j4c4ja). Adding -O1 to the clang side will make them equivalent. I may also report this as an LLVM bug, it doesn't seem like there's any reason why Clang would prefer the long form at `-O0`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6179
From: Brendan Shanks <bshanks(a)codeweavers.com> GCC always assembles 'jmp 1f' to 'eb 01', as does Clang when using -O1 or higher optimization. But with -O0, Clang outputs 'e9 01 00 00 00'. The 'subq $0xb,0x70(%rcx)' line in __wine_syscall_dispatcher relies on 'jmp 1f' being 2 bytes. Based on patch by Alex S. --- include/wine/asm.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/wine/asm.h b/include/wine/asm.h index 7a6c03b5df7..37760a59dcd 100644 --- a/include/wine/asm.h +++ b/include/wine/asm.h @@ -245,9 +245,9 @@ ".byte 0x75,0x03\n\t" /* jne 1f */ \ ".byte 0x0f,0x05\n\t" /* syscall */ \ ".byte 0xc3\n\t" /* ret */ \ - "jmp 1f\n\t" \ - ".byte 0xc3\n" /* ret */ \ - "1:\t.byte 0xff,0x14,0x25\n\t" /* 1: callq *(0x7ffe1000) */ \ + ".byte 0xeb,0x01\n\t" /* jmp 1f */ \ + ".byte 0xc3\n\t" /* ret */ \ + ".byte 0xff,0x14,0x25\n\t" /* 1: callq *(0x7ffe1000) */ \ ".long 0x7ffe1000\n\t" \ "ret" ) # else @@ -260,10 +260,10 @@ ".byte 0x75,0x03\n\t" /* jne 1f */ \ ".byte 0x0f,0x05\n\t" /* syscall */ \ ".byte 0xc3\n\t" /* ret */ \ - "jmp 1f\n\t" \ + ".byte 0xeb,0x02\n\t" /* jmp 1f */ \ ".byte 0xc3\n" /* ret */ \ - "nop\n" \ - "1:\tcallq *" __ASM_NAME("__wine_syscall_dispatcher") "(%rip)\n\t" \ + "nop\n\t" \ + "callq *" __ASM_NAME("__wine_syscall_dispatcher") "(%rip)\n\t" /* 1: callq __wine_syscall_dispatcher */ \ "ret" ) # endif #elif defined __arm__ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6179
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147453 Your paranoid android. === debian11 (build log) === error: patch failed: include/wine/asm.h:245 Task: Patch failed to apply
After more investigation, Clang uses the large form of `jmp` when `-mrelax-all` is passed. Currently this is used at `-O0` (with the intention of speeding compile times), but not at higher optimization levels. This is changing in Clang 19, which [will not](https://maskray.me/blog/2024-04-27-clang-o0-output-branch-displacement-and-s...) use `-mrelax-all` at `-O0`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6179#note_77472
participants (3)
-
Brendan Shanks -
Brendan Shanks (＠bshanks)
-
Marvin