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.