[PATCH v7 0/4] 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. -- v7: TMP: Use module info for tls in winedbg winedbg: Track loaded modules https://gitlab.winehq.org/wine/wine/-/merge_requests/1578
From: Evan Tang <etang(a)codeweavers.com> It doesn't on Windows and won't soon in wine --- programs/winedbg/symbol.c | 59 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/programs/winedbg/symbol.c b/programs/winedbg/symbol.c index 961dc95585a..126c36163c0 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; @@ -155,11 +203,20 @@ static BOOL fill_sym_lvalue(const SYMBOL_INFO* sym, ULONG_PTR base, head = &((PEB_LDR_DATA*)peb.LdrData)->InLoadOrderModuleList; do { + DWORD_PTR index_addr; if (!dbg_read_memory(CONTAINING_RECORD(current, LDR_DATA_TABLE_ENTRY, InLoadOrderLinks), &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; + index_addr = (DWORD_PTR)(is_dir64 ? tls.dir64.AddressOfIndex : tls.dir32.AddressOfIndex); + if (!dbg_read_memory((void*)index_addr, &tlsindex, sizeof(tlsindex))) + goto tls_error; break; } current = ldr_module.InLoadOrderLinks.Flink; -- GitLab 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 +++++++++++++++++++ 2 files changed, 40 insertions(+), 14 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(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1578
From: Evan Tang <etang(a)codeweavers.com> --- programs/winedbg/debugger.h | 11 ++++++++ programs/winedbg/tgt_active.c | 2 ++ programs/winedbg/winedbg.c | 48 +++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/programs/winedbg/debugger.h b/programs/winedbg/debugger.h index c4c0402ef5f..8e00770917c 100644 --- a/programs/winedbg/debugger.h +++ b/programs/winedbg/debugger.h @@ -246,6 +246,12 @@ struct dbg_thread BOOL suspended; }; +struct dbg_module +{ + struct list entry; + DWORD_PTR base; +}; + struct dbg_delayed_bp { BOOL is_symbol; @@ -271,6 +277,7 @@ struct dbg_process void* pio_data; const WCHAR* imageName; struct list threads; + struct list modules; struct backend_cpu* be_cpu; HANDLE event_on_first_exception; BOOL active_debuggee; @@ -522,6 +529,10 @@ extern void dbg_del_process(struct dbg_process* p); struct dbg_thread* dbg_add_thread(struct dbg_process* p, DWORD tid, HANDLE h, void* teb); extern struct dbg_thread* dbg_get_thread(struct dbg_process* p, DWORD tid); extern void dbg_del_thread(struct dbg_thread* t); +extern struct dbg_module* dbg_add_module(struct dbg_process* p, DWORD_PTR base); +extern struct dbg_module* dbg_get_module(struct dbg_process* p, DWORD_PTR base); +extern void dbg_del_module(struct dbg_module* mod); +extern BOOL dbg_del_module_by_base(struct dbg_process* p, DWORD_PTR base); extern BOOL dbg_init(HANDLE hProc, const WCHAR* in, BOOL invade); extern BOOL dbg_load_module(HANDLE hProc, HANDLE hFile, const WCHAR* name, DWORD_PTR base, DWORD size); extern void dbg_set_option(const char*, const char*); diff --git a/programs/winedbg/tgt_active.c b/programs/winedbg/tgt_active.c index 9e130038f22..bfaf9d0b654 100644 --- a/programs/winedbg/tgt_active.c +++ b/programs/winedbg/tgt_active.c @@ -485,6 +485,7 @@ static unsigned dbg_handle_debug_event(DEBUG_EVENT* de) de->u.LoadDll.nDebugInfoSize); dbg_load_module(dbg_curr_process->handle, de->u.LoadDll.hFile, u.buffer, (DWORD_PTR)de->u.LoadDll.lpBaseOfDll, 0); + dbg_add_module(dbg_curr_process, (DWORD_PTR)de->u.LoadDll.lpBaseOfDll); break_set_xpoints(FALSE); break_check_delayed_bp(); break_set_xpoints(TRUE); @@ -503,6 +504,7 @@ static unsigned dbg_handle_debug_event(DEBUG_EVENT* de) break_delete_xpoints_from_module((DWORD_PTR)de->u.UnloadDll.lpBaseOfDll); types_unload_module((DWORD_PTR)de->u.UnloadDll.lpBaseOfDll); SymUnloadModule64(dbg_curr_process->handle, (DWORD_PTR)de->u.UnloadDll.lpBaseOfDll); + dbg_del_module_by_base(dbg_curr_process, (DWORD_PTR)de->u.UnloadDll.lpBaseOfDll); break; case OUTPUT_DEBUG_STRING_EVENT: diff --git a/programs/winedbg/winedbg.c b/programs/winedbg/winedbg.c index 84ec7802396..ab2e94f5673 100644 --- a/programs/winedbg/winedbg.c +++ b/programs/winedbg/winedbg.c @@ -273,6 +273,7 @@ struct dbg_process* dbg_add_process(const struct be_process_io* pio, DWORD pid, p->pio_data = NULL; p->imageName = NULL; list_init(&p->threads); + list_init(&p->modules); p->event_on_first_exception = NULL; p->active_debuggee = FALSE; p->next_bp = 1; /* breakpoint 0 is reserved for step-over */ @@ -316,11 +317,16 @@ void dbg_del_process(struct dbg_process* p) { struct dbg_thread* t; struct dbg_thread* t2; + struct dbg_module* mod; + struct dbg_module* mod2; int i; LIST_FOR_EACH_ENTRY_SAFE(t, t2, &p->threads, struct dbg_thread, entry) dbg_del_thread(t); + LIST_FOR_EACH_ENTRY_SAFE(mod, mod2, &p->modules, struct dbg_module, entry) + dbg_del_module(mod); + for (i = 0; i < p->num_delayed_bp; i++) if (p->delayed_bp[i].is_symbol) free(p->delayed_bp[i].u.symbol.name); @@ -436,6 +442,48 @@ void dbg_del_thread(struct dbg_thread* t) free(t); } +struct dbg_module* dbg_add_module(struct dbg_process* p, DWORD_PTR base) +{ + struct dbg_module* mod = malloc(sizeof(struct dbg_module)); + + if (!mod) + return NULL; + + mod->base = base; + + list_add_head(&p->modules, &mod->entry); + + return mod; +} + +struct dbg_module* dbg_get_module(struct dbg_process* p, DWORD_PTR base) +{ + struct dbg_module* mod; + + if (!p) + return NULL; + LIST_FOR_EACH_ENTRY(mod, &p->modules, struct dbg_module, entry) + if (mod->base == base) + return mod; + return NULL; +} + +void dbg_del_module(struct dbg_module* mod) +{ + list_remove(&mod->entry); + free(mod); +} + +BOOL dbg_del_module_by_base(struct dbg_process* p, DWORD_PTR base) +{ + struct dbg_module* mod = dbg_get_module(p, base); + + if (mod) + dbg_del_module(mod); + + return !!mod; +} + void dbg_set_option(const char* option, const char* val) { if (!strcasecmp(option, "module_load_mismatched")) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1578
From: Evan Tang <etang(a)codeweavers.com> --- programs/winedbg/debugger.h | 19 ++++++-- programs/winedbg/symbol.c | 96 +++++-------------------------------- programs/winedbg/winedbg.c | 62 +++++++++++++++++++++++- 3 files changed, 88 insertions(+), 89 deletions(-) diff --git a/programs/winedbg/debugger.h b/programs/winedbg/debugger.h index 8e00770917c..7b2ed7da422 100644 --- a/programs/winedbg/debugger.h +++ b/programs/winedbg/debugger.h @@ -250,6 +250,7 @@ struct dbg_module { struct list entry; DWORD_PTR base; + DWORD_PTR tls_idx_addr_offset; }; struct dbg_delayed_bp @@ -542,16 +543,26 @@ extern void dbg_init_console(void); /* gdbproxy.c */ extern int gdb_main(int argc, char* argv[]); -static inline BOOL dbg_read_memory(const void* addr, void* buffer, size_t len) +static inline BOOL dbg_read_process_memory(const struct dbg_process* p, const void* addr, void* buffer, size_t len) { SIZE_T rlen; - return dbg_curr_process->process_io->read(dbg_curr_process->handle, addr, buffer, len, &rlen) && len == rlen; + return p->process_io->read(p->handle, addr, buffer, len, &rlen) && len == rlen; } -static inline BOOL dbg_write_memory(void* addr, const void* buffer, size_t len) +static inline BOOL dbg_read_memory(const void* addr, void* buffer, size_t len) +{ + return dbg_read_process_memory(dbg_curr_process, addr, buffer, len); +} + +static inline BOOL dbg_write_process_memory(const struct dbg_process* p, void* addr, const void* buffer, size_t len) { SIZE_T wlen; - return dbg_curr_process->process_io->write(dbg_curr_process->handle, addr, buffer, len, &wlen) && len == wlen; + return p->process_io->write(p->handle, addr, buffer, len, &wlen) && len == wlen; +} + +static inline BOOL dbg_write_memory(void* addr, const void* buffer, size_t len) +{ + return dbg_write_process_memory(dbg_curr_process, addr, buffer, len); } struct data_model diff --git a/programs/winedbg/symbol.c b/programs/winedbg/symbol.c index 126c36163c0..14e8fea8761 100644 --- a/programs/winedbg/symbol.c +++ b/programs/winedbg/symbol.c @@ -61,51 +61,6 @@ 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) { @@ -173,56 +128,29 @@ static BOOL fill_sym_lvalue(const SYMBOL_INFO* sym, ULONG_PTR base, } else if (sym->Flags & SYMFLAG_TLSREL) { - PROCESS_BASIC_INFORMATION pbi; + struct dbg_module* mod = dbg_get_module(dbg_curr_process, sym->ModBase); + union { ULONGLONG _64; DWORD _32; } tls_idx_addr_raw; + DWORD_PTR tls_idx_addr; 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; - PLIST_ENTRY head, current; - LDR_DATA_TABLE_ENTRY ldr_module; unsigned tlsindex = -1; - if (NtQueryInformationProcess(dbg_curr_process->handle, ProcessBasicInformation, - &pbi, sizeof(pbi), NULL) || - NtQueryInformationThread(dbg_curr_thread->handle, ThreadBasicInformation, - &tbi, sizeof(tbi), NULL)) + if (!mod || !mod->tls_idx_addr_offset || + FAILED(NtQueryInformationThread(dbg_curr_thread->handle, ThreadBasicInformation, &tbi, sizeof(tbi), NULL))) { tls_error: if (buffer) snprintf(buffer, sz, "Cannot read TLS address\n"); return FALSE; } - addr = (DWORD_PTR)&(((TEB*)tbi.TebBaseAddress)->ThreadLocalStoragePointer); - if (!dbg_read_memory((void*)addr, &addr, sizeof(addr)) || - !dbg_read_memory(pbi.PebBaseAddress, &peb, sizeof(peb)) || - !dbg_read_memory(peb.LdrData, &ldr_data, sizeof(ldr_data))) + if (!dbg_read_memory((void*)(mod->base + mod->tls_idx_addr_offset), &tls_idx_addr_raw, sizeof(tls_idx_addr_raw))) + goto tls_error; + if (!(tls_idx_addr = dbg_curr_process->be_cpu->pointer_size == 8 ? tls_idx_addr_raw._64 : tls_idx_addr_raw._32)) + goto tls_error; + if (!dbg_read_memory((void*)tls_idx_addr, &tlsindex, sizeof(tlsindex))) goto tls_error; - current = ldr_data.InLoadOrderModuleList.Flink; - head = &((PEB_LDR_DATA*)peb.LdrData)->InLoadOrderModuleList; - do - { - DWORD_PTR index_addr; - if (!dbg_read_memory(CONTAINING_RECORD(current, LDR_DATA_TABLE_ENTRY, InLoadOrderLinks), - &ldr_module, sizeof(ldr_module))) goto tls_error; - if ((DWORD_PTR)ldr_module.DllBase == sym->ModBase) - { - 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; - index_addr = (DWORD_PTR)(is_dir64 ? tls.dir64.AddressOfIndex : tls.dir32.AddressOfIndex); - if (!dbg_read_memory((void*)index_addr, &tlsindex, sizeof(tlsindex))) - goto tls_error; - break; - } - current = ldr_module.InLoadOrderLinks.Flink; - } while (current != head); - addr += tlsindex * sizeof(DWORD_PTR); + addr = (DWORD_PTR)&(((TEB*)tbi.TebBaseAddress)->ThreadLocalStoragePointer); + addr += tlsindex * dbg_curr_process->be_cpu->pointer_size; if (!dbg_read_memory((void*)addr, &addr, sizeof(addr))) goto tls_error; init_lvalue(lvalue, TRUE, (void*)(DWORD_PTR)(addr + sym->Address)); } diff --git a/programs/winedbg/winedbg.c b/programs/winedbg/winedbg.c index ab2e94f5673..7fa83ab5ddc 100644 --- a/programs/winedbg/winedbg.c +++ b/programs/winedbg/winedbg.c @@ -442,14 +442,74 @@ void dbg_del_thread(struct dbg_thread* t) free(t); } +static DWORD get_data_directory_addr(DWORD numberOfRvaAndSizes, const IMAGE_DATA_DIRECTORY* directory, DWORD index, DWORD requiredSize) +{ + if (index >= numberOfRvaAndSizes || directory[index].Size < requiredSize) + return 0; + return directory[index].VirtualAddress; +} + +#define GET_DATA_DIRECTORY_ADDR(optionalHeader, index, type) get_data_directory_addr((optionalHeader)->NumberOfRvaAndSizes, (optionalHeader)->DataDirectory, index, sizeof(type)) + +static BOOL try_read_pe_module(const struct dbg_process* p, struct dbg_module* mod) +{ + IMAGE_DOS_HEADER dos_header; + DWORD entry; + DWORD_PTR addr; + union { IMAGE_NT_HEADERS32 h32; IMAGE_NT_HEADERS64 h64; } nt_header; + + addr = mod->base; + if (!dbg_read_process_memory(p, (void*)addr, &dos_header, sizeof(dos_header))) + return FALSE; + if (dos_header.e_magic != IMAGE_DOS_SIGNATURE) + return FALSE; + + addr += dos_header.e_lfanew; + if (!dbg_read_process_memory(p, (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) + { + IMAGE_OPTIONAL_HEADER32* opt_header = &nt_header.h32.OptionalHeader; + if (p->be_cpu->pointer_size != 4) + { + WINE_ERR("32-bit module in a %lu-bit process???\n", p->be_cpu->pointer_size * 8); + return FALSE; + } + + if ((entry = GET_DATA_DIRECTORY_ADDR(opt_header, IMAGE_DIRECTORY_ENTRY_TLS, IMAGE_TLS_DIRECTORY32))) + mod->tls_idx_addr_offset = entry + offsetof(IMAGE_TLS_DIRECTORY32, AddressOfIndex); + } + else if (nt_header.h32.OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) + { + IMAGE_OPTIONAL_HEADER64* opt_header = &nt_header.h64.OptionalHeader; + if (p->be_cpu->pointer_size != 8) + { + WINE_ERR("64-bit module in a %lu-bit process???\n", p->be_cpu->pointer_size * 8); + return FALSE; + } + + if ((entry = GET_DATA_DIRECTORY_ADDR(opt_header, IMAGE_DIRECTORY_ENTRY_TLS, IMAGE_TLS_DIRECTORY64))) + mod->tls_idx_addr_offset = entry + offsetof(IMAGE_TLS_DIRECTORY64, AddressOfIndex); + } + else + { + return FALSE; + } + + return TRUE; +} + struct dbg_module* dbg_add_module(struct dbg_process* p, DWORD_PTR base) { - struct dbg_module* mod = malloc(sizeof(struct dbg_module)); + struct dbg_module* mod = calloc(1, sizeof(struct dbg_module)); if (!mod) return NULL; mod->base = base; + try_read_pe_module(p, mod); list_add_head(&p->modules, &mod->entry); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1578
On Wed Dec 14 07:41:26 2022 +0000, eric pouech wrote:
modules should be added/removed from 'current process' while processing load/unload events in dbg_handle_debug_event(). (handling for tgt_minidump will likely require more work) but that change will have to be done after 8.0 I'm afraid I added an implementation for that if you want to look it over (and test it with your tls exe)
(The TMP commit I plan to squash onto previous commits before merge, don't worry about the name) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1578#note_20202
On Thu Dec 29 21:46:28 2022 +0000, Evan Tang wrote:
I added an implementation for that if you want to look it over (and test it with your tls exe) (The TMP commit I plan to squash onto previous commits before merge, don't worry about the name) I had a look...
I changed a bit your first patch (about introducing dbg_module in winedbg): - IMO, we should keep the loaded module list in winedbg in sync with dbghelp... so don't have two separate functions... nothing critical, mostly stylish The last (TMP) didn't work with my local tests. Moreover: - current code (before your patches) didn't expect at all the WOW64 situation (32bit debuggee and 64bit debuggeer). - reduced a bunch of PE format reading by mapping the DLL image and using Rtl* functions that already implement it. - So I reworked it. You can find the two modified patches attached here [patch.tls](/uploads/d2230ff815ddfa90f674439dfbc52b7d/patch.tls). They've been successfully tested with and without the ntdll patch applied (and in the three cases of bitness: 32/32, 64/64 and 64/32). So these two patches should be first in serie, and the ntdll last. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1578#note_20249
participants (3)
-
eric pouech (＠epo) -
Evan Tang
-
Evan Tang (＠etang-cw)