msvcrt: Do not create in newer msvcrt versions a separate heap. (ASan)
A llvm-mingw built winver.exe with ASan enabled shows below error when exiting the application.
This seems to be caused by libc++ using register_onexit_function before ASan has all function hooks in place, therefore allocates memory with plain calloc function.
On process exit ASan uses in asan_allocator.cpp/Deallocate the function HeapValidate, which fails if msvcrt does not use the heap returned by GetProcessHeap().
wine64 winver_asan.exe
=================================================================
==292==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x7ffffea05a10 in thread T0
0130:fixme:file:server_get_file_info Unsupported info class e
#0 0x6ffffa809fe1 in free .../llvm-mingw/llvm-project/compiler-rt\lib/asan/asan_malloc_win.cpp:71:3
#1 0x6ffffeb048c5 in execute_onexit_table /home/bernhard/wine/dlls/msvcrt\exit.c:141:5
#2 0x6ffffbfb109e in _CRT_INIT .../llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64\../crt\crtdll.c:130:11
#3 0x6ffffbfb1316 in __DllMainCRTStartup .../llvm-mingw/mingw-w64/mingw-w64-crt/build-x86_64\../crt\crtdll.c:196:6
#4 0x6fffffc6d223 in call_dll_entry_point (C:\windows\system32\ntdll.dll+0x17002d223)
#5 0x6fffffc71fdd in MODULE_InitDLL /home/bernhard/wine/dlls/ntdll\loader.c:1720:16
#6 0x6fffffc72601 in process_detach /home/bernhard/wine/dlls/ntdll\loader.c:1866:13
#7 0x6fffffc726f5 in RtlExitUserProcess /home/bernhard/wine/dlls/ntdll\loader.c:3893:5
#8 0x6fffffa8b899 in ExitProcess /home/bernhard/wine/dlls/kernel32\process.c:207:5
#9 0x6ffffeb054a7 in exit /home/bernhard/wine/dlls/msvcrt\exit.c:383:3
#10 0x000140001338 in mainCRTStartup /home/bernhard/wine/dlls/msvcrt\crt_main.c:60:5
#11 0x6fffffa98d58 in BaseThreadInitThunk /home/bernhard/wine/dlls/kernel32\thread.c:61:5
#12 0x6fffffc95afa in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x170055afa)
Address 0x7ffffea05a10 is a wild pointer inside of access range of size 0x000000000001.
SUMMARY: AddressSanitizer: bad-free /home/bernhard/wine/dlls/msvcrt\exit.c:141:5 in execute_onexit_table
==292==ABORTING
Please provide some guidance if this needs to be separated commits per msvcrt version, and if they are all needed (or more)?
Merge request reports
Activity
requested review from @piotr
Yes, this is a with a local llvm-mingw with this llvm-project-patch, to allow ASan to hotpatch more instructions.
And a wine tree with at least wine-patch-1 and wine-patch-2.
Configured with this options:
PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig \ CFLAGS='-g -O2' \ x86_64_CC='ccache /home/bernhard/llvm-mingw_inst/bin/x86_64-w64-mingw32-gcc' \ CROSSCFLAGS='-g -O2 -fsanitize=address -Wno-pragma-pack' \ x86_64_LDFLAGS='-lasan' \ /home/bernhard/wine/configure \ --prefix=/usr \ --with-mingw \ --without-oss \ --enable-win64
Then putting
libc++.dll
,libclang_rt.asan_dynamic-x86_64.dll
andlibunwind.dll
into PATH or copying them into a new directory.And calling this ASan-enabled executables like this, either with a renamed executable:
wine64 C:\\x86_64\\notepad-asan.exe
or with this additional wine-patch-3
WINEDLLOVERRIDES="notepad.exe=n" wine64 C:\\x86_64\\notepad.exe
I guess the wine-patch-1 is also something which should be handled in llvm-mingw, as I assume
x86_64-w64-mingw32-gcc
should behave the same as the mingw-w64 counterpart, which does not need the extra-c
.And wine-patch-2 might be achieved by some other configure argument?
Edited by Bernhard ÜbelackerForgot to mention there is also a llvm-symbolizer.exe needed to get nice stack traces.
I had installed following file from llvm-project installed:
LLVM-19.1.3-win64.exe
Maybe this would be a better configure line, at least with this ccache is used later:
export PATH=/home/bernhard/llvm-mingw_inst/bin:$PATH PKG_CONFIG_PATH=/usr/lib/x86_64-linux-gnu/pkgconfig \ /home/bernhard/data/entwicklung/2024/wine/wine/configure \ CC='ccache gcc' \ CFLAGS='-g -O2' \ CROSSCFLAGS='-g -O2 -fsanitize=address -Wno-pragma-pack -gdwarf-4' \ x86_64_CC='ccache x86_64-w64-mingw32-gcc' \ --prefix=/usr \ --with-mingw \ --without-oss \ --enable-win64 \ --disable-tests \
Yet another update after this conversation in llvm-mingw.
Latest ASan related wine patches collected in this branch.
Was more or less still just able to use renamed ASan instrumented exe files inside a regular built Wineprefix.
Edited by Bernhard Übelacker
- Resolved by Bernhard Übelacker
- Resolved by Bernhard Übelacker
added 1 commit
- 97cbdb71 - msvcrt: Do not create in newer msvcrt versions a separate heap.
v2:
- Fixed #if condition in msvcrt_destroy_heap.
- Use
_get_heap_handle
instead of the indirect detection viaHeapValidate(GetProcessHeap(), ...)
For convenience this are the links into llvm-project, where this
HeapValidate(GetProcessHeap(), ...)
is called: IsSystemHeapAddress and Deallocate.1706 1706 tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); 1707 1707 } 1708 1708 1709 static void test__get_heap_handle(void) 1710 { 1711 void* (__cdecl *p__get_heap_handle)(void); 1712 p__get_heap_handle = (void *)GetProcAddress( GetModuleHandleA("ucrtbase.dll"), "_get_heap_handle"); 1713 ok(p__get_heap_handle() == GetProcessHeap(), "Expected _get_heap_handle() to return GetProcessHeap()\n"); No reason, except to have the four tests look kind of equal. Should I change these two tests to direct linking like this?
diff --git a/dlls/msvcrt/tests/heap.c b/dlls/msvcrt/tests/heap.c index e6de728f2b7..e93f909be8d 100644 --- a/dlls/msvcrt/tests/heap.c +++ b/dlls/msvcrt/tests/heap.c @@ -524,4 +524,11 @@ static void test_calloc(void) } +void* __cdecl _get_heap_handle(void); + +static void test__get_heap_handle(void) +{ + ok(_get_heap_handle() != GetProcessHeap(), "Expected _get_heap_handle() not to return GetProcessHeap()\n"); +} + START_TEST(heap) { @@ -530,3 +537,4 @@ START_TEST(heap) test_malloc(); test_calloc(); + test__get_heap_handle(); } diff --git a/dlls/ucrtbase/tests/misc.c b/dlls/ucrtbase/tests/misc.c index ff1ed20f2f8..33fb326316a 100644 --- a/dlls/ucrtbase/tests/misc.c +++ b/dlls/ucrtbase/tests/misc.c @@ -1707,4 +1707,11 @@ static void test_gmtime64(void) } +void* __cdecl _get_heap_handle(void); + +static void test__get_heap_handle(void) +{ + ok(_get_heap_handle() == GetProcessHeap(), "Expected _get_heap_handle() to return GetProcessHeap()\n"); +} + START_TEST(misc) { @@ -1753,3 +1760,4 @@ START_TEST(misc) #endif test_gmtime64(); + test__get_heap_handle(); }
changed this line in version 3 of the diff
Thanks, I have pushed v3.
Edited by Bernhard Übelacker
added 1 commit
- 4f6718d5 - msvcrt: Do not create in newer msvcrt versions a separate heap.
added 1 commit
- 0c703994 - msvcrt: Do not create in newer msvcrt versions a separate heap.
added 15 commits
-
0c703994...c9a8333c - 14 commits from branch
wine:master
- d78cbe84 - msvcrt: Do not create a separate heap in newer msvcrt versions.
-
0c703994...c9a8333c - 14 commits from branch
mentioned in merge request !7041 (merged)