[PATCH v4 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. -- v4: 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 | 35 +++++++++++++---------- dlls/ntdll/tests/rtl.c | 19 +++++++++++++ programs/winedbg/symbol.c | 58 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 97 insertions(+), 15 deletions(-) diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index 895a3f5b138..51e8519abc5 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -1222,7 +1222,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; @@ -1230,10 +1230,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++) { @@ -1255,7 +1255,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) @@ -1264,7 +1264,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 */ @@ -1296,7 +1296,7 @@ static SHORT alloc_tls_slot( LDR_DATA_TABLE_ENTRY *mod ) *(DWORD *)dir->AddressOfIndex = i; tls_dirs[i] = *dir; - return i; + return TRUE; } @@ -1308,9 +1308,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]) ); } @@ -1374,7 +1380,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 ))) @@ -1431,7 +1438,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; @@ -1783,7 +1790,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; @@ -3712,7 +3719,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 ); @@ -4216,7 +4223,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.ActivationContext) RtlDeactivateActivationContext( 0, cookie ); process_breakpoint(); } @@ -4225,7 +4232,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
Looks good, but could you please put the winedbg changes in a separate commit? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1578#note_19171
regarding the winedbg changes - dbghelp.ImageDirectoryEntryToDataEx does most of the painful job for you - (line 216 in winedbg/symbol: addr = (DWORD_PTR)(is_dir64 ? tls.dir64.AddressOfIndex : tls.dir32.AddressOfIndex);) doesn't seem to be relocated -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1578#note_19173
participants (4)
-
Alexandre Julliard (＠julliard) -
eric pouech (＠epo)
-
Evan Tang
-
Evan Tang (＠etang-cw)