[PATCH v3 0/1] MR1578: ntdll: TlsIndex should not actually contain tls indices
(The TlsIndex field in the LDR_DATA_TABLE_ENTRY structure appears to be unused except as a flag that the module has TLS (being always set to -1), at least as far back as Windows XP. It is worth mentioning that the WINE implementation of implicit TLS incorrectly uses TlsIndex as the real module TLS index, so it may be unreliable to assume that it is always -1 if you care about working on WINE.)
and the "links to that article but still doesn't work in wine" award goes to... [the D runtime](https://github.com/dlang/dmd/blob/6bf60ea0eb174631ede0074a77d3898d943e0b30/d... (Admittedly, there aren't too many ways to do what they're trying to do.) With this, the D runtime will now work in Wine, even if in a dll loaded into an exe with no tls (which gets it the tls index 0) The changes to the debugger are a bit icky, a possible alternative is to find some other easily-debugger-accessible place to stuff the tls index. -- v3: ntdll: TlsIndex should not actually contain tls indices https://gitlab.winehq.org/wine/wine/-/merge_requests/1578
From: Evan Tang <etang(a)codeweavers.com> It actually contains a -1 if the module has a tls slot and a 0 if it doesn't. Putting tls indices in it breaks initialization of the D runtime if a D dll is loaded into a tls-free exe and gets assigned tls slot 0, as it makes the D runtime think the OS hasn't initialized a tls slot: https://github.com/dlang/dmd/blob/6bf60ea0eb174631ede0074a77d3898d943e0b30/d... --- dlls/ntdll/loader.c | 37 +++++++++++++++---------- dlls/ntdll/tests/rtl.c | 19 +++++++++++++ programs/winedbg/symbol.c | 58 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 98 insertions(+), 16 deletions(-) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 682fda9ed15..09439957a66 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -1221,7 +1221,7 @@ static BOOL is_dll_native_subsystem( LDR_DATA_TABLE_ENTRY *mod, const IMAGE_NT_H * Allocate a TLS slot for a newly-loaded module. * The loader_section must be locked while calling this function. */ -static SHORT alloc_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) +static BOOL alloc_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) { const IMAGE_TLS_DIRECTORY *dir; ULONG i, size; @@ -1229,10 +1229,10 @@ static SHORT alloc_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) LIST_ENTRY *entry; if (!(dir = RtlImageDirectoryEntryToData( mod->DllBase, TRUE, IMAGE_DIRECTORY_ENTRY_TLS, &size ))) - return -1; + return FALSE; size = dir->EndAddressOfRawData - dir->StartAddressOfRawData; - if (!size && !dir->SizeOfZeroFill && !dir->AddressOfCallBacks) return -1; + if (!size && !dir->SizeOfZeroFill && !dir->AddressOfCallBacks) return FALSE; for (i = 0; i < tls_module_count; i++) { @@ -1254,7 +1254,7 @@ static SHORT alloc_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) else new_ptr = RtlReAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY, tls_dirs, new_count * sizeof(*tls_dirs) ); - if (!new_ptr) return -1; + if (!new_ptr) return FALSE; /* resize the pointer block in all running threads */ for (entry = tls_links.Flink; entry != &tls_links; entry = entry->Flink) @@ -1263,7 +1263,7 @@ static SHORT alloc_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) void **old = teb->ThreadLocalStoragePointer; void **new = RtlAllocateHeap( GetProcessHeap(), HEAP_ZERO_MEMORY, new_count * sizeof(*new)); - if (!new) return -1; + if (!new) return FALSE; if (old) memcpy( new, old, tls_module_count * sizeof(*new) ); teb->ThreadLocalStoragePointer = new; #ifdef __x86_64__ /* macOS-specific hack */ @@ -1295,7 +1295,7 @@ static SHORT alloc_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) *(DWORD *)dir->AddressOfIndex = i; tls_dirs[i] = *dir; - return i; + return TRUE; } @@ -1307,9 +1307,15 @@ static SHORT alloc_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) */ static void free_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) { - ULONG i = (USHORT)mod->TlsIndex; + const IMAGE_TLS_DIRECTORY *dir; + ULONG i, size; + + if (mod->TlsIndex != -1) + return; + if (!(dir = RtlImageDirectoryEntryToData( mod->DllBase, TRUE, IMAGE_DIRECTORY_ENTRY_TLS, &size ))) + return; - if (mod->TlsIndex == -1) return; + i = *(ULONG*)dir->AddressOfIndex; assert( i < tls_module_count ); memset( &tls_dirs[i], 0, sizeof(tls_dirs[i]) ); } @@ -1373,7 +1379,8 @@ static NTSTATUS fixup_imports( WINE_MODREF *wm, LPCWSTR load_path ) if (!(wm->ldr.Flags & LDR_DONT_RESOLVE_REFS)) return STATUS_SUCCESS; /* already done */ wm->ldr.Flags &= ~LDR_DONT_RESOLVE_REFS; - wm->ldr.TlsIndex = alloc_tls_slot( &wm->ldr ); + if (alloc_tls_slot( &wm->ldr )) + wm->ldr.TlsIndex = -1; if (!(imports = RtlImageDirectoryEntryToData( wm->ldr.DllBase, TRUE, IMAGE_DIRECTORY_ENTRY_IMPORT, &size ))) @@ -1430,7 +1437,7 @@ static WINE_MODREF *alloc_module( HMODULE hModule, const UNICODE_STRING *nt_name wm->ldr.DllBase = hModule; wm->ldr.SizeOfImage = nt->OptionalHeader.SizeOfImage; wm->ldr.Flags = LDR_DONT_RESOLVE_REFS | (builtin ? LDR_WINE_INTERNAL : 0); - wm->ldr.TlsIndex = -1; + wm->ldr.TlsIndex = 0; wm->ldr.LoadCount = 1; wm->CheckSum = nt->OptionalHeader.CheckSum; wm->ldr.TimeDateStamp = nt->FileHeader.TimeDateStamp; @@ -1571,7 +1578,7 @@ static NTSTATUS MODULE_InitDLL( WINE_MODREF *wm, UINT reason, LPVOID lpReserved /* Skip calls for modules loaded with special load flags */ if (wm->ldr.Flags & LDR_DONT_RESOLVE_REFS) return STATUS_SUCCESS; - if (wm->ldr.TlsIndex != -1) call_tls_callbacks( wm->ldr.DllBase, reason ); + if (wm->ldr.TlsIndex == -1) call_tls_callbacks( wm->ldr.DllBase, reason ); if (wm->ldr.Flags & LDR_WINE_INTERNAL && reason == DLL_PROCESS_ATTACH) NTDLL_UNIX_CALL( init_builtin_dll, wm->ldr.DllBase ); if (!entry) return STATUS_SUCCESS; @@ -1784,7 +1791,7 @@ NTSTATUS WINAPI LdrDisableThreadCalloutsForDll(HMODULE hModule) RtlEnterCriticalSection( &loader_section ); wm = get_modref( hModule ); - if (!wm || wm->ldr.TlsIndex != -1) + if (!wm || wm->ldr.TlsIndex == -1) ret = STATUS_DLL_NOT_FOUND; else wm->ldr.Flags |= LDR_NO_DLL_CALLS; @@ -3708,7 +3715,7 @@ void WINAPI LdrShutdownThread(void) DLL_THREAD_DETACH, NULL ); } - if (wm->ldr.TlsIndex != -1) call_tls_callbacks( wm->ldr.DllBase, DLL_THREAD_DETACH ); + if (wm->ldr.TlsIndex == -1) call_tls_callbacks( wm->ldr.DllBase, DLL_THREAD_DETACH ); RtlAcquirePebLock(); if (NtCurrentTeb()->TlsLinks.Flink) RemoveEntryList( &NtCurrentTeb()->TlsLinks ); @@ -4206,7 +4213,7 @@ void WINAPI LdrInitializeThunk( CONTEXT *context, ULONG_PTR unknown2, ULONG_PTR NtTerminateProcess( GetCurrentProcess(), status ); } release_address_space(); - if (wm->ldr.TlsIndex != -1) call_tls_callbacks( wm->ldr.DllBase, DLL_PROCESS_ATTACH ); + if (wm->ldr.TlsIndex == -1) call_tls_callbacks( wm->ldr.DllBase, DLL_PROCESS_ATTACH ); if (wm->ldr.Flags & LDR_WINE_INTERNAL) NTDLL_UNIX_CALL( init_builtin_dll, wm->ldr.DllBase ); if (wm->ldr.ActivationContext) RtlDeactivateActivationContext( 0, cookie ); process_breakpoint(); @@ -4216,7 +4223,7 @@ void WINAPI LdrInitializeThunk( CONTEXT *context, ULONG_PTR unknown2, ULONG_PTR if ((status = alloc_thread_tls()) != STATUS_SUCCESS) NtTerminateThread( GetCurrentThread(), status ); thread_attach(); - if (wm->ldr.TlsIndex != -1) call_tls_callbacks( wm->ldr.DllBase, DLL_THREAD_ATTACH ); + if (wm->ldr.TlsIndex == -1) call_tls_callbacks( wm->ldr.DllBase, DLL_THREAD_ATTACH ); } RtlLeaveCriticalSection( &loader_section ); diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c index 2e0bfb650e4..7ffdc9b9382 100644 --- a/dlls/ntdll/tests/rtl.c +++ b/dlls/ntdll/tests/rtl.c @@ -3730,6 +3730,24 @@ static void test_RtlFirstFreeAce(void) HeapFree(GetProcessHeap(), 0, acl); } +static void test_TlsIndex(void) +{ + LIST_ENTRY *root = &NtCurrentTeb()->Peb->LdrData->InLoadOrderModuleList; + for (LIST_ENTRY *entry = root->Flink; entry != root; entry = entry->Flink) { + LDR_DATA_TABLE_ENTRY *mod = CONTAINING_RECORD(entry, LDR_DATA_TABLE_ENTRY, InLoadOrderLinks); + if (lstrcmpiW(L"ntdll.dll", mod->BaseDllName.Buffer) == 0) { + /* Pick ntdll as a dll that definitely won't have TLS */ + ok(mod->TlsIndex == 0, "ntdll.dll TlsIndex: %d instead of 0\n", mod->TlsIndex); + } else if (mod->DllBase == GetModuleHandleA(NULL)) { + /* mingw gcc doesn't support MSVC-style TLS */ + /* If we do get a way to add tls to this exe, uncomment the following test: */ + /* ok(mod->TlsIndex == -1, "Test exe TlsIndex: %d instead of -1\n", mod->TlsIndex); */ + } else { + ok(mod->TlsIndex == 0 || mod->TlsIndex == -1, "%s TlsIndex: %d\n", debugstr_w(mod->BaseDllName.Buffer), mod->TlsIndex); + } + } +} + START_TEST(rtl) { InitFunctionPtrs(); @@ -3774,4 +3792,5 @@ START_TEST(rtl) test_DbgPrint(); test_RtlDestroyHeap(); test_RtlFirstFreeAce(); + test_TlsIndex(); } diff --git a/programs/winedbg/symbol.c b/programs/winedbg/symbol.c index 961dc95585a..5c6bebf47a6 100644 --- a/programs/winedbg/symbol.c +++ b/programs/winedbg/symbol.c @@ -61,6 +61,51 @@ static BOOL symbol_get_debug_start(const struct dbg_type* func, ULONG64* start) return FALSE; } +static BOOL read_image_directory_data(ULONG64 modbase, WORD dir, void* buffer, size_t* size, BOOL* is_64) +{ + IMAGE_DOS_HEADER dos_header; + IMAGE_DATA_DIRECTORY entry; + DWORD_PTR addr; + union { IMAGE_NT_HEADERS32 h32; IMAGE_NT_HEADERS64 h64; } nt_header; + + addr = (DWORD_PTR)modbase; + if (!dbg_read_memory((void*)addr, &dos_header, sizeof(dos_header))) + return FALSE; + if (dos_header.e_magic != IMAGE_DOS_SIGNATURE) + return FALSE; + addr = (DWORD_PTR)(modbase + dos_header.e_lfanew); + if (!dbg_read_memory((void*)addr, &nt_header, sizeof(nt_header))) + return FALSE; + if (nt_header.h32.Signature != IMAGE_NT_SIGNATURE) + return FALSE; + if (nt_header.h32.OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) + { + if (dir >= nt_header.h32.OptionalHeader.NumberOfRvaAndSizes) + return FALSE; + if (!(entry = nt_header.h32.OptionalHeader.DataDirectory[dir]).VirtualAddress) + return FALSE; + *is_64 = FALSE; + } + else if (nt_header.h64.OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) + { + if (dir >= nt_header.h64.OptionalHeader.NumberOfRvaAndSizes) + return FALSE; + if (!(entry = nt_header.h64.OptionalHeader.DataDirectory[dir]).VirtualAddress) + return FALSE; + *is_64 = TRUE; + } + else + { + return FALSE; + } + if (entry.Size < *size) + *size = entry.Size; + addr = (DWORD_PTR)(modbase + entry.VirtualAddress); + if (!dbg_read_memory((void*)addr, buffer, *size)) + return FALSE; + return TRUE; +} + static BOOL fill_sym_lvalue(const SYMBOL_INFO* sym, ULONG_PTR base, struct dbg_lvalue* lvalue, char* buffer, size_t sz) { @@ -130,6 +175,9 @@ static BOOL fill_sym_lvalue(const SYMBOL_INFO* sym, ULONG_PTR base, { PROCESS_BASIC_INFORMATION pbi; THREAD_BASIC_INFORMATION tbi; + union { IMAGE_TLS_DIRECTORY32 dir32; IMAGE_TLS_DIRECTORY64 dir64; } tls; + size_t read_size = sizeof(tls); + BOOL is_dir64; DWORD_PTR addr; PEB peb; PEB_LDR_DATA ldr_data; @@ -159,7 +207,15 @@ static BOOL fill_sym_lvalue(const SYMBOL_INFO* sym, ULONG_PTR base, &ldr_module, sizeof(ldr_module))) goto tls_error; if ((DWORD_PTR)ldr_module.DllBase == sym->ModBase) { - tlsindex = ldr_module.TlsIndex; + if (ldr_module.TlsIndex != -1) + goto tls_error; + if (!read_image_directory_data(sym->ModBase, IMAGE_DIRECTORY_ENTRY_TLS, &tls, &read_size, &is_dir64)) + goto tls_error; + if (read_size < (is_dir64 ? sizeof(IMAGE_TLS_DIRECTORY64) : sizeof(IMAGE_TLS_DIRECTORY32))) + goto tls_error; + addr = (DWORD_PTR)(is_dir64 ? tls.dir64.AddressOfIndex : tls.dir32.AddressOfIndex); + if (!dbg_read_memory((void*)addr, &tlsindex, sizeof(tlsindex))) + goto tls_error; break; } current = ldr_module.InLoadOrderLinks.Flink; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1578
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=126888 Your paranoid android. === debian11 (32 bit report) === msctf: inputprocessor.c:2437: Test failed: Expected DocumentMgr not focused Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x0040ae26).
participants (3)
-
Evan Tang -
Evan Tang (＠etang-cw)
-
Marvin