Re: [PATCH v2] configure: Avoid clobbering x18 on arm64 when building with clang
Martin Storsjo <martin(a)martin.st> writes:
On aarch64/arm64, clang/LLVM feels free to use x18 for normal code generation on linux (while the register is reserved on iOS/darwin). For windows/arm64, this register must not be clobbered.
This would have to be made the default system-wide, fixing it on the Wine side isn't good enough since we call system libraries. -- Alexandre Julliard julliard(a)winehq.org
On Thu, 22 Jun 2017, Alexandre Julliard wrote:
Martin Storsjo <martin(a)martin.st> writes:
On aarch64/arm64, clang/LLVM feels free to use x18 for normal code generation on linux (while the register is reserved on iOS/darwin). For windows/arm64, this register must not be clobbered.
This would have to be made the default system-wide, fixing it on the Wine side isn't good enough since we call system libraries.
Fair point. However, it seems like gcc never uses this register (which probably is why the arm64 build has seemed to work more or less so far). So this would only be an issue with clang-built system libraries. At least it has seemed to not have been an issue in debian jessie/arm64 (presumably built with gcc), with a gcc-built wine. FWIW, I tested out wine for arm64 with some of the few arm64 exes present in the win10 sdk (afaik the only public source of windows arm64 binaries?), and it generally seems to work, except for ms va_list handling - something like __builtin_ms_va_list (like on x86_64) is needed from the compiler. I tried implementing this in clang (since I'm much more familiar with that codebase already, than with gcc), see https://reviews.llvm.org/D34475. However, in order for my clang-built wine not to fail right away on things my gcc-built wine handled fine, I had to set this flag for the full wine build. I agree this would be an issue with clang built system libraries on linux though. Is it still worthwhile to add to configure, for cases where system libraries have been built with gcc? If not, I'll just keep on adding it manually to my cflags when configuring, while experimenting with this. // Martin
Martin Storsjö <martin(a)martin.st> writes:
I agree this would be an issue with clang built system libraries on linux though. Is it still worthwhile to add to configure, for cases where system libraries have been built with gcc? If not, I'll just keep on adding it manually to my cflags when configuring, while experimenting with this.
I don't think it's worthwhile adding it, particularly if the compiler doesn't work anyway because of va_list. Hopefully you can fix both issues while you're at it ;-) -- Alexandre Julliard julliard(a)winehq.org
On Fri, 23 Jun 2017, Alexandre Julliard wrote:
Martin Storsjö <martin(a)martin.st> writes:
I agree this would be an issue with clang built system libraries on linux though. Is it still worthwhile to add to configure, for cases where system libraries have been built with gcc? If not, I'll just keep on adding it manually to my cflags when configuring, while experimenting with this.
I don't think it's worthwhile adding it, particularly if the compiler doesn't work anyway because of va_list. Hopefully you can fix both issues while you're at it ;-)
GCC also doesn't work due to va_list - the __builtin_ms_va_list is only implemented on x86_64 at the moment, not on aarch64. Well, I'm not sure how much the x18 register issue really can be fixed - the AAPCS calling convention says that it's a temporary register that the compiler can use freely, unless the platform has reserved it (like on darwin, and apparently also on windows). So I don't think clang/llvm would be interested in a patch to disable use of this on linux in general. Apparently André talked to GCC about the issue a few years ago: https://gcc.gnu.org/ml/gcc/2015-07/msg00106.html I'm not sure if that discussion resulted in something else somewhere - if I'm just lucky that it has happened to work for me so far (but could break randomly if gcc would happen to use x18 anywhere in the called system libraries, and just clang seems to be using it more often so I only ran into it with clang?), or if something really was changed elsewhere with respect to this. André, can you fill us in on this matter? In any case, thanks for the attention so far, I'll return once __builtin_ms_va_list is implemented in either of the compilers, and I'll keep using these flags locally for the time being. // Martin
Am 23.06.2017 um 11:38 schrieb Martin Storsjö:
On Fri, 23 Jun 2017, Alexandre Julliard wrote:
Martin Storsjö <martin(a)martin.st> writes:
I agree this would be an issue with clang built system libraries on linux though. Is it still worthwhile to add to configure, for cases where system libraries have been built with gcc? If not, I'll just keep on adding it manually to my cflags when configuring, while experimenting with this.
I don't think it's worthwhile adding it, particularly if the compiler doesn't work anyway because of va_list. Hopefully you can fix both issues while you're at it ;-)
GCC also doesn't work due to va_list - the __builtin_ms_va_list is only implemented on x86_64 at the moment, not on aarch64.
Well, I'm not sure how much the x18 register issue really can be fixed - the AAPCS calling convention says that it's a temporary register that the compiler can use freely, unless the platform has reserved it (like on darwin, and apparently also on windows). So I don't think clang/llvm would be interested in a patch to disable use of this on linux in general.
Apparently André talked to GCC about the issue a few years ago: https://gcc.gnu.org/ml/gcc/2015-07/msg00106.html
Hi, yes, this went nowhere... One idea is to set x18 to the TEB after a function call. That could be done by some new mechanism based on our relay functionality (and AJ already said no to it), or by using the thunks Wine generates for its dlls, which would be done in import.c from winebuild... The problem here is how to get the TEB from assembler (and possibly performance...)? There are some steps involved until someone can call NtCurrentTeb I guess. Maybe can try to preserve it in the thunks, then we need to assume that it's correct on entry... And then there are callbacks...
I'm not sure if that discussion resulted in something else somewhere - if I'm just lucky that it has happened to work for me so far (but could break randomly if gcc would happen to use x18 anywhere in the called system libraries, and just clang seems to be using it more often so I only ran into it with clang?), or if something really was changed elsewhere with respect to this. André, can you fill us in on this matter?
So your Wine build crashes even when running Wines cmd/winecfg/...? Is that fixed by removing the assembler line in dlls/ntdll/signal_arm64.c:signal_init_thread ? As you already mentioned, if x18 is not treated special it should be a normal scratch register and one should be able to write to it nearly always without producing a crash...
In any case, thanks for the attention so far, I'll return once __builtin_ms_va_list is implemented in either of the compilers, and I'll keep using these flags locally for the time being.
Thanks for your work on it! I'm also interested in doing the ARM64 work in mingw-w64 again (as soon as clang is ready for that) See also: https://bugs.winehq.org/show_bug.cgi?id=38780 https://bugs.winehq.org/show_bug.cgi?id=38886
On Fri, 23 Jun 2017, André Hentschel wrote:
Am 23.06.2017 um 11:38 schrieb Martin Storsjö:
On Fri, 23 Jun 2017, Alexandre Julliard wrote:
Martin Storsjö <martin(a)martin.st> writes:
I'm not sure if that discussion resulted in something else somewhere - if I'm just lucky that it has happened to work for me so far (but could break randomly if gcc would happen to use x18 anywhere in the called system libraries, and just clang seems to be using it more often so I only ran into it with clang?), or if something really was changed elsewhere with respect to this. André, can you fill us in on this matter?
So your Wine build crashes even when running Wines cmd/winecfg/...? Is that fixed by removing the assembler line in dlls/ntdll/signal_arm64.c:signal_init_thread ? As you already mentioned, if x18 is not treated special it should be a normal scratch register and one should be able to write to it nearly always without producing a crash...
No, plain "wine64 cmd" runs fine (iirc), with wine built with both gcc and clang - with a debian jessie userland. (I've built wine without most extra libraries, so no X output or anything such, so I didn't try winecfg or anything like that.) Now with a gcc-built wine, if I try "wine64 uuidgen.exe", it runs fine. If I instead try "wine64 midl.exe", I get similar crashes as you have in https://bugs.winehq.org/show_bug.cgi?id=38886. If I instead switch to clang-built wine, "wine64 cmd" runs fine, but uuidgen.exe crashes before printing anything, with a crash point indicating that x18 had the wrong value. Now if I rebuild wine with clang, with "-Xclang -target-feature -Xclang +reserve-x18", uuidgen.exe works just as with gcc. And later with https://reviews.llvm.org/D34474 and https://reviews.llvm.org/D34475 and some trivial hookups of __builtin_ms_va_list and __attribute__((ms_abi)), midl.exe also gives working printouts. That's about as far as I've gone.
Thanks for your work on it! I'm also interested in doing the ARM64 work in mingw-w64 again (as soon as clang is ready for that)
I guess clang support for actually targeting win/arm64 is much further away (especially since there's no public msvc tools yet to compare to); the only thing so far is my RFC for handling their va_list.
See also: https://bugs.winehq.org/show_bug.cgi?id=38780 https://bugs.winehq.org/show_bug.cgi?id=38886
Thanks, these are good pointers to follow up from! // Martin
Am 23.06.2017 um 20:22 schrieb Martin Storsjö:
On Fri, 23 Jun 2017, André Hentschel wrote:
Am 23.06.2017 um 11:38 schrieb Martin Storsjö:
On Fri, 23 Jun 2017, Alexandre Julliard wrote:
Martin Storsjö <martin(a)martin.st> writes:
I'm not sure if that discussion resulted in something else somewhere - if I'm just lucky that it has happened to work for me so far (but could break randomly if gcc would happen to use x18 anywhere in the called system libraries, and just clang seems to be using it more often so I only ran into it with clang?), or if something really was changed elsewhere with respect to this. André, can you fill us in on this matter?
So your Wine build crashes even when running Wines cmd/winecfg/...? Is that fixed by removing the assembler line in dlls/ntdll/signal_arm64.c:signal_init_thread ?
While I signed off your patch, could you please try what happens without reserve-x18 and without the assemblerline i referenced?
On Sat, 24 Jun 2017, André Hentschel wrote:
Am 23.06.2017 um 20:22 schrieb Martin Storsjö:
On Fri, 23 Jun 2017, André Hentschel wrote:
Am 23.06.2017 um 11:38 schrieb Martin Storsjö:
On Fri, 23 Jun 2017, Alexandre Julliard wrote:
Martin Storsjö <martin(a)martin.st> writes:
I'm not sure if that discussion resulted in something else somewhere - if I'm just lucky that it has happened to work for me so far (but could break randomly if gcc would happen to use x18 anywhere in the called system libraries, and just clang seems to be using it more often so I only ran into it with clang?), or if something really was changed elsewhere with respect to this. André, can you fill us in on this matter?
So your Wine build crashes even when running Wines cmd/winecfg/...? Is that fixed by removing the assembler line in dlls/ntdll/signal_arm64.c:signal_init_thread ?
While I signed off your patch, could you please try what happens without reserve-x18 and without the assemblerline i referenced?
Ah, sorry, I missed to test that. It still crashes - the crash is in a code segment like this: 0x0000000140011570: add x8, x18, #0x0 => 0x0000000140011574: ldr x19, [x8,#8] I haven't really identified where this piece of code comes from though. // Martin
participants (3)
-
Alexandre Julliard -
André Hentschel -
Martin Storsjö