[PATCH v6 0/27] MR8026: Draft: RFC: Running wine with AddressSanitizer
Please see: https://marc.info/?l=wine-devel&m=174715050805731 as well as the commit messages for more information. -- v6: ntdll: Report more info about heap problems detected by ASan. ntdll: Show (partial) stack trace in ASan reports. ntdll: Add heap quarantine for ASan. ntdll: Add asan poisoning and redzoning to heap allocator ntdll: Implement ASan fake stack. ntdll: Don't use address of local variables as the frame address. ntdll: During unwind, also check if frame is on fake stack. ntdll: Implement __asan_{un,}poison_memory_region. ntdll: Make sure to not write into poisoned memory in KeUserModeCallback. ntdll: Implement __asan_set_shadow_*. ntdll: Implement reporting of ASan errors. ntdll: Implement __asan_{memory,region}_is_poisoned. ntdll: Implement __asan_{un,}poison_stack_memory. ntdll: Unpoison stack in __asan_handle_no_return. ntdll: Add API for checking whether address is in fake stack frame. kernel32: Check for poison in LocalLock if ASan is enabled. kernel32: Fix ASan reports in IsBad* ntdll: Call __asan_handle_no_return in RtlRestoreContext. makedep: Support sanitizer flags. asan_dynamic_thunk: Add ASan dynamic thunk for DLLs. ntdll: Add stub ASan runtime. This merge request has too many patches to be relayed via email. Please visit the URL below to see the contents of the merge request. https://gitlab.winehq.org/wine/wine/-/merge_requests/8026
On Thu May 15 19:44:03 2025 +0000, Yuxuan Shui wrote:
updated based on suggestions from @iamahuman - now no new symbols are exported from ntdll and i was able to get rid of the pesky DLL initialization hook as well (see https://gitlab.winehq.org/wine/wine/-/commit/6616df42bcaf754ddeb40100f89bddb...). thanks! updated the dynamic thunk to add loader stubs (placeholder wrappers that loads the real function pointers) for every ASan API, not just `__asan_stack_malloc_*`
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103637
On Fri May 16 15:28:23 2025 +0000, Jinoh Kang wrote:
Some ASAN annotations can be shared with Valgrind, so maybe we can keep them; just make them nonspecific to ASAN. For the rest, I'd generally vote for -fno-sanitize for 90% of the code. As we see more bug cases in support of ASAN, we can gradually and incrementally adopt the instrumentation. I sympathise with the worry about long term maintainability. I will say I am willing to take on this task and resolve any ASan related problems that might come up in the future. I don't know how much weight you will put on my words, but there's that.
we should be careful not to affect compatibility (maybe achieavable)
If I did everything correctly, my changes should compile to nothing if ASan isn't enabled. * * * Then I want to defend the changes a little bit. Some of the changes are arguably not really ASan specific:
RtlRestoreContext (that is probably not exhaustive, we have more no return places like that?)
I inserted a call to `__asan_handle_no_return`, but the same can be achieved by correctly annotating no return functions with `DECLSPEC_NORETURN`.
KeUserModeCallback (while maybe `__builtin_frame_address` is not portable)
using the address of a local variable as the value of the stack pointer works, but I would argue using compiler intrinsic is the more proper way. clang/gcc both support `__builtin_frame_address`, on the MSVC side there's `_AddressOfReturnAddress`.
unwinding
i am not very knowledgeable on this, but wouldn't it be better to always use native unwinding mechanism rather than wine's hack? i don't know if this is doable, but if we can make `__TRY/__EXCEPT` expand to SEH directives for PE and dwarf directives for ELF, then get rid of `__WINE_FRAME`, then we don't need ASan specific workarounds. * * * but some workarounds are ASan specific:
kernelbase (probably similar should be in virtual_check_buffer_for_write and friends in ntdll unless it is already there), kernel32
yes, but these changes pretty clean and shouldn't be too difficult to maintain. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103639
I sympathise with the worry about long term maintainability. I will say I am willing to take on this task and resolve any ASan related problems that might come up in the future. I don't know how much weight you will put on my words, but there's that.
Sure, it's great that you are willing to do that. But note that it's actually an argument for keeping it as a separate fork that you have the responsibility to maintain, as opposed to merging it upstream where it becomes everybody's responsibility when making changes to affected areas. Considering that you have to rebuild the whole tree anyway, it doesn't seem too onerous to tell people "to do an ASAN test run, apply this patch and rebuild".
i am not very knowledgeable on this, but wouldn't it be better to always use native unwinding mechanism rather than wine's hack? i don't know if this is doable, but if we can make __TRY/__EXCEPT expand to SEH directives for PE and dwarf directives for ELF, then get rid of __WINE_FRAME, then we don't need ASan specific workarounds.
Sadly that's not feasible without compiler support. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103666
But note that it's actually an argument for keeping it as a separate fork that you have the responsibility to maintain.
That doesn't sound too bad actually. This is intended for developers anyway, so as long as devs are made aware and have easy access to this, it can be useful. maybe things will change in the future and this eventually get merged, but for now keeping it as a fork doesn't sound too bad.
Sadly that's not feasible without compiler support.
can you expand a bit on this? i am curious and do want to learn more. i know mingw has their crappy `__try1/__except1` macros, that doesn't quite work like the real `__try/__except`. is there something that prevents us from improving upon them? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103686
On Fri May 16 20:26:42 2025 +0000, Yuxuan Shui wrote:
But note that it's actually an argument for keeping it as a separate fork that you have the responsibility to maintain. That doesn't sound too bad actually. This is intended for developers anyway, so as long as devs are made aware and have easy access to this, it can be useful. maybe things will change in the future and this eventually get merged, but for now keeping it as a fork doesn't sound too bad. Sadly that's not feasible without compiler support. can you expand a bit on this? i am curious and do want to learn more. i know mingw has their crappy `__try1/__except1` macros, that doesn't quite work like the real `__try/__except`. is there something that prevents us from improving upon them? also did a quick & crude survey on this, based on bernhardu's work, out of ~3.6M total lines of code, almost exactly 3M can be covered by PE ASan, so about 83.5%. out of those lines that can't be covered, 35.2% are unixlibs, the rest are DLLs that have to be exempted, presumably because they caused problems. (i excluded `libs`, and `tools`. comments and empty lines are not counted. obviously SLOC doesn't mean much, so this is more or less just for fun.).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103687
i know mingw has their crappy `__try1/__except1` macros, that doesn't quite work like the real `__try/__except`. is there something that prevents us from improving upon them?
Doing proper exception handling implies generating correct code range tables, and correct handler and unwinding code sequences, all of which depend on the code generated by the compiler. There's just no way to do that by injecting random asm statements inside the C code. The only way to do SEH without compiler support is by writing entire functions in assembly, which we've done in a few places for simple things, but obviously that doesn't scale. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103688
On Fri May 16 20:43:23 2025 +0000, Alexandre Julliard wrote:
i know mingw has their crappy `__try1/__except1` macros, that doesn't quite work like the real `__try/__except`. is there something that prevents us from improving upon them? Doing proper exception handling implies generating correct code range tables, and correct handler and unwinding code sequences, all of which depend on the code generated by the compiler. There's just no way to do that by injecting random asm statements inside the C code. The only way to do SEH without compiler support is by writing entire functions in assembly, which we've done in a few places for simple things, but obviously that doesn't scale. I'm sure others know more, but my understanding is that GCC/mingw doesn't support Windows SEH, hence the need for the mingw macros and Wine's own support. Clang does support SEH, although it doesn't look like Wine uses it (`__TRY` always uses `WINE_FRAME` except on MSVC). If Wine's macros used real SEH on Clang, an ASan build could just require Clang be used for PE cross-compiling and you could ignore `WINE_FRAME`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103689
On Fri May 16 20:53:16 2025 +0000, Brendan Shanks wrote:
I'm sure others know more, but my understanding is that GCC/mingw doesn't support Windows SEH, hence the need for the mingw macros and Wine's own support. Clang does support SEH, although it doesn't look like Wine uses it (`__TRY` always uses `WINE_FRAME` except on MSVC). If Wine's macros used real SEH on Clang, an ASan build could just require Clang be used for PE cross-compiling and you could ignore `WINE_FRAME`. I think GCC does support SEH? i.e. it will happily use it for C++ exceptions. (I contemplated suggesting using C++ but only for exception handling). It also generates enough SEH directives for functions so they can be unwound (urgh, i don't know the terminologies for these). That's why mingw's `__try1/__except1` work (to an extent).
And yes, clang does support `__try/__except`, but for windows targets only. wine also uses them on the unix side. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103690
On Fri May 16 21:05:07 2025 +0000, Yuxuan Shui wrote:
I think GCC does support SEH? i.e. it will happily use it for C++ exceptions. (I contemplated suggesting using C++ but only for exception handling). It also generates enough SEH directives for functions so they can be unwound (urgh, i don't know the terminologies for these). That's why mingw's `__try1/__except1` work (to an extent). And yes, clang does support `__try/__except`, but for windows targets only. wine also uses them on the unix side. Yes, gcc only supports them in C++ mode, and clang only in MSVC mode. That's not good enough in general, but it may be usable for your purposes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103691
On Fri May 16 21:30:59 2025 +0000, Alexandre Julliard wrote:
Yes, gcc only supports them in C++ mode, and clang only in MSVC mode. That's not good enough in general, but it may be usable for your purposes. I created !8067, which enables building Wine with compiler exception support. As mentioned in the MR, it's not quite ready as a full replacement yet.
BTW, it’s possible to use this with Clang in mingw mode by passing `-fms-extensions`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_103720
:thumbsup: for this patch, and getting a rebased version would be great. I tried, export CC=clang export CXX=clang++ export CFLAGS=" -O0 -g -fno-omit-frame-pointer -fsanitize=address -fno-sanitize=null -fsanitize=undefined -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize-recover=all -fsanitize-recover=address -fsanitize-address-use-after-scope -fno-sanitize=alignment" export CXXFLAGS=" -O0 -g -fno-omit-frame-pointer -fsanitize=address -fno-sanitize=null -fsanitize=undefined -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize-recover=all -fsanitize-recover=address -fsanitize-address-use-after-scope -fno-sanitize=alignment" export LDFLAGS="-fno-omit-frame-pointer -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize=null -fno-sanitize=alignment" ./configure --prefix=/usr/local/packages/wine-master --enable-win64 and 'configure' didn't work, so getting this functionality as part of the core build would be great. Trying to figure out https://bugs.winehq.org/show_bug.cgi?id=58322 so all core infrastructure would help greatly. Thanks for working on this ! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_105382
On Thu Jun 5 15:16:12 2025 +0000, Jesper Pedersen wrote:
:thumbsup: for this patch, and getting a rebased version would be great. I tried, export CC=clang export CXX=clang++ export CFLAGS=" -O0 -g -fno-omit-frame-pointer -fsanitize=address -fno-sanitize=null -fsanitize=undefined -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize-recover=all -fsanitize-recover=address -fsanitize-address-use-after-scope -fno-sanitize=alignment" export CXXFLAGS=" -O0 -g -fno-omit-frame-pointer -fsanitize=address -fno-sanitize=null -fsanitize=undefined -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize-recover=all -fsanitize-recover=address -fsanitize-address-use-after-scope -fno-sanitize=alignment" export LDFLAGS="-fno-omit-frame-pointer -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow -fno-sanitize=null -fno-sanitize=alignment" ./configure --prefix=/usr/local/packages/wine-master --enable-win64 and 'configure' didn't work, so getting this functionality as part of the core build would be great. Trying to figure out https://bugs.winehq.org/show_bug.cgi?id=58322 so all core infrastructure would help greatly. Thanks for working on this ! hi, you don't need to manually set sanitizer CFLAGS, and many of the flags you set is not yet supported by this MR. and although non-WoW64 should work, what i've been extensively testing with is the new WoW64 mode.
please check out this branch: [`wip/address-sanitizer`](https://gitlab.winehq.org/yshui/wine/-/tree/wip/address-sanitizer?ref_type=h...), and run configure with `CC=clang ./configure --enable-archs=x86_64,i386 --enable-sanitize=address` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_105532
On Thu Jun 5 15:16:12 2025 +0000, Yuxuan Shui wrote:
hi, you don't need to manually set sanitizer CFLAGS, and many of the flags you set is not yet supported by this MR. and although non-WoW64 should work, what i've been extensively testing with is the new WoW64 mode. please check out this branch: [`wip/address-sanitizer`](https://gitlab.winehq.org/yshui/wine/-/tree/wip/address-sanitizer?ref_type=h...), and run configure with `CC=clang ./configure --enable-archs=x86_64,i386 --enable-sanitize=address` Hi, Thanks for your reply ! How do I get a .patch of your branch because I need to apply it to 'master' since previous versions of Wine didn't work at all for my bug. Or, can you rebase your patch ? Thanks again for working on this.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_105534
On Thu Jun 5 15:33:28 2025 +0000, Jesper Pedersen wrote:
Hi, Thanks for your reply ! How do I get a .patch of your branch because I need to apply it to 'master' since previous versions of Wine didn't work at all for my bug. Or, can you rebase your patch ? Thanks again for working on this. Hi Yuxuan, Thanks for your update ! But, it blows up with
<pre> ./configure --prefix=/usr/local/packages/wine-master --enable-win64 --enable-silent-rules --disable-tests --enable-archs=x86_64 --enable-sanitize=address </pre> build -- using autoconf 2.72. How do you build it ? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8026#note_105545
participants (5)
-
Alexandre Julliard (＠julliard) -
Brendan Shanks (＠bshanks)
-
Jacek Caban (＠jacek)
-
Jesper Pedersen (＠jesper.pedersen)
-
Yuxuan Shui (＠yshui)