[PATCH 0/1] MR6791: 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)?
From: Bernhard Übelacker bernhardu@mailbox.org
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(). --- dlls/msvcr100/tests/msvcr100.c | 15 +++++++++++++++ dlls/msvcr110/tests/msvcr110.c | 15 +++++++++++++++ dlls/msvcrt/heap.c | 6 ++++++ dlls/msvcrt/tests/heap.c | 17 +++++++++++++++++ dlls/ucrtbase/tests/misc.c | 17 +++++++++++++++++ 5 files changed, 70 insertions(+)
diff --git a/dlls/msvcr100/tests/msvcr100.c b/dlls/msvcr100/tests/msvcr100.c index 7bde5c7935d..c8ecc53b16a 100644 --- a/dlls/msvcr100/tests/msvcr100.c +++ b/dlls/msvcr100/tests/msvcr100.c @@ -239,6 +239,8 @@ static size_t (__cdecl *p___strncnt)(const char*, size_t);
static int (__cdecl *p_strcmp)(const char *, const char *); static int (__cdecl *p_strncmp)(const char *, const char *, size_t); +static void* (__cdecl *p_calloc)(size_t,size_t); +static void (__cdecl *p_free)(void*);
/* make sure we use the correct errno */ #undef errno @@ -282,6 +284,8 @@ static BOOL init(void)
SET(p_strcmp, "strcmp"); SET(p_strncmp, "strncmp"); + SET(p_calloc, "calloc"); + SET(p_free, "free");
if(sizeof(void*) == 8) { /* 64-bit initialization */ SET(pSpinWait_ctor_yield, "??0?$_SpinWait@$00@details@Concurrency@@QEAA@P6AXXZ@Z"); @@ -1152,6 +1156,16 @@ static void test_strcmp(void) ok( ret == 0, "wrong ret %d\n", ret ); }
+static void test_HeapValidate(void) +{ + void *ptr; + + ptr = p_calloc(1, 1); + ok(ptr != NULL, "got %p\n", ptr); + ok(!HeapValidate(GetProcessHeap(), 0, ptr), "HeapValidate unexpectedly succeeded\n"); + p_free(ptr); +} + START_TEST(msvcr100) { if (!init()) @@ -1173,4 +1187,5 @@ START_TEST(msvcr100) test_setlocale(); test___strncnt(); test_strcmp(); + test_HeapValidate(); } diff --git a/dlls/msvcr110/tests/msvcr110.c b/dlls/msvcr110/tests/msvcr110.c index 35ba370bb49..2848d67bd8e 100644 --- a/dlls/msvcr110/tests/msvcr110.c +++ b/dlls/msvcr110/tests/msvcr110.c @@ -54,6 +54,8 @@ static _Context* (__cdecl *p__Context__CurrentContext)(_Context*);
static int (__cdecl *p_strcmp)(const char *, const char *); static int (__cdecl *p_strncmp)(const char *, const char *, size_t); +static void* (__cdecl *p_calloc)(size_t,size_t); +static void (__cdecl *p_free)(void*);
#define SETNOFAIL(x,y) x = (void*)GetProcAddress(module,y) #define SET(x,y) do { SETNOFAIL(x,y); ok(x != NULL, "Export '%s' not found\n", y); } while(0) @@ -89,6 +91,8 @@ static BOOL init(void)
SET(p_strcmp, "strcmp"); SET(p_strncmp, "strncmp"); + SET(p_calloc, "calloc"); + SET(p_free, "free");
return TRUE; } @@ -307,6 +311,16 @@ static void test_strcmp(void) ok( ret == 0, "wrong ret %d\n", ret ); }
+static void test_HeapValidate(void) +{ + void *ptr; + + ptr = p_calloc(1, 1); + ok(ptr != NULL, "got %p\n", ptr); + ok(HeapValidate(GetProcessHeap(), 0, ptr), "HeapValidate unexpectedly failed\n"); + p_free(ptr); +} + START_TEST(msvcr110) { if (!init()) return; @@ -315,4 +329,5 @@ START_TEST(msvcr110) test___strncnt(); test_CurrentContext(); test_strcmp(); + test_HeapValidate(); } diff --git a/dlls/msvcrt/heap.c b/dlls/msvcrt/heap.c index bf06c37e2c5..a82f308165b 100644 --- a/dlls/msvcrt/heap.c +++ b/dlls/msvcrt/heap.c @@ -829,13 +829,19 @@ int CDECL wmemcpy_s(wchar_t *dest, size_t numberOfElements,
BOOL msvcrt_init_heap(void) { +#if _MSVCR_VER >= 110 + heap = GetProcessHeap(); +#else heap = HeapCreate(0, 0, 0); +#endif return heap != NULL; }
void msvcrt_destroy_heap(void) { +#if _MSVCR_VER >= 110 HeapDestroy(heap); +#endif if(sb_heap) HeapDestroy(sb_heap); } diff --git a/dlls/msvcrt/tests/heap.c b/dlls/msvcrt/tests/heap.c index e6de728f2b7..ea6021f717a 100644 --- a/dlls/msvcrt/tests/heap.c +++ b/dlls/msvcrt/tests/heap.c @@ -523,10 +523,27 @@ static void test_calloc(void) free(ptr); }
+static void test_HeapValidate(void) +{ + /* use function pointers to bypass gcc builtins */ + void* (__cdecl *p_calloc)(size_t,size_t); + void (__cdecl *p_free)(void*); + void *ptr; + + p_calloc = (void *)GetProcAddress( GetModuleHandleA("msvcrt.dll"), "calloc"); + p_free = (void *)GetProcAddress( GetModuleHandleA("msvcrt.dll"), "free"); + + ptr = p_calloc(1, 1); + ok(ptr != NULL, "got %p\n", ptr); + ok(!HeapValidate(GetProcessHeap(), 0, ptr), "HeapValidate unexpectedly succeeded\n"); + p_free(ptr); +} + START_TEST(heap) { test_aligned(); test_sbheap(); test_malloc(); test_calloc(); + test_HeapValidate(); } diff --git a/dlls/ucrtbase/tests/misc.c b/dlls/ucrtbase/tests/misc.c index ff1ed20f2f8..72afcdc07fe 100644 --- a/dlls/ucrtbase/tests/misc.c +++ b/dlls/ucrtbase/tests/misc.c @@ -1706,6 +1706,22 @@ static void test_gmtime64(void) tm.tm_year, tm.tm_hour, tm.tm_min, tm.tm_sec); }
+static void test_HeapValidate(void) +{ + /* use function pointers to bypass gcc builtins */ + void* (__cdecl *p_calloc)(size_t,size_t); + void (__cdecl *p_free)(void*); + void *ptr; + + p_calloc = (void *)GetProcAddress( GetModuleHandleA("ucrtbase.dll"), "calloc"); + p_free = (void *)GetProcAddress( GetModuleHandleA("ucrtbase.dll"), "free"); + + ptr = p_calloc(1, 1); + ok(ptr != NULL, "got %p\n", ptr); + ok(HeapValidate(GetProcessHeap(), 0, ptr), "HeapValidate unexpectedly failed\n"); + p_free(ptr); +} + START_TEST(misc) { int arg_c; @@ -1752,4 +1768,5 @@ START_TEST(misc) test_rewind_i386_abi(); #endif test_gmtime64(); + test_HeapValidate(); }
How did you use ASan in Wine? Did you recompile PE parts of Wine with llvm-mingw and the `-fsanitize=address` option? I've heard that recompiling the Unix parts with ASan enabled causes some preloader issues
Piotr Caban (@piotr) commented about dlls/msvcr100/tests/msvcr100.c:
ok( ret == 0, "wrong ret %d\n", ret );}
+static void test_HeapValidate(void) +{
- void *ptr;
- ptr = p_calloc(1, 1);
- ok(ptr != NULL, "got %p\n", ptr);
- ok(!HeapValidate(GetProcessHeap(), 0, ptr), "HeapValidate unexpectedly succeeded\n");
- p_free(ptr);
+}
Please use `_get_heap_handle()` to check what heap is used.
Piotr Caban (@piotr) commented about dlls/msvcrt/heap.c:
BOOL msvcrt_init_heap(void) { +#if _MSVCR_VER >= 110
- heap = GetProcessHeap();
+#else heap = HeapCreate(0, 0, 0); +#endif return heap != NULL; }
void msvcrt_destroy_heap(void) { +#if _MSVCR_VER >= 110 HeapDestroy(heap); +#endif
Please fix #if condition.
On Fri Nov 8 15:35:49 2024 +0000, Aida Jonikienė wrote:
How did you use ASan in Wine? Did you recompile PE parts of Wine with llvm-mingw and the `-fsanitize=address` option? I've heard that recompiling the Unix parts with ASan enabled causes some preloader issues
Yes, this is a with a local llvm-mingw with this [llvm-project-patch](https://github.com/llvm/llvm-project/pull/113085), to allow ASan to hotpatch more instructions.
And a wine tree with at least [wine-patch-1](https://gitlab.winehq.org/bernhardu/wine/-/commit/31295ffc453bc17011b58c28e0...) and [wine-patch-2](https://gitlab.winehq.org/bernhardu/wine/-/commit/8ec62b18c5d35eb1f5775784fa...).
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` and `libunwind.dll` into PATH or copying them into a new directory.
A calling these ASan-enabled executables like this, either renamed the executable: ``` wine64 C:\x86_64\notepad-asan.exe ``` or with this additional [wine-patch-3](https://gitlab.winehq.org/bernhardu/wine/-/commit/d533f3f24493155805ef3093b4...) ``` 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?
-
Aida Jonikienė (＠DodoGTA) -
Bernhard Übelacker
-
Bernhard Übelacker (＠bernhardu)
-
Piotr Caban (＠piotr)