[PATCH v2 0/2] MR4518: ntdll: Integrate with GDB using r_debug.
Effectively implementing the debugger interface as described in https://sourceware.org/git/?p=glibc.git;a=blob;f=elf/rtld-debugger-interface... This is an actual attempt at having this upstream, split from https://gitlab.winehq.org/wine/wine/-/merge_requests/1074 which people have been finding useful for debugging Wine directly in GDB. This supports everything you would expect, from automatic symbol loading as well as cross-syscall backtraces with the new Python unwinder. -- v2: ntdll: Maintain a PE module link map and expose it to GDB. loader: Expose the standard debugging symbols for GDB. https://gitlab.winehq.org/wine/wine/-/merge_requests/4518
From: Rémi Bernon <rbernon(a)codeweavers.com> --- loader/main.c | 3 ++- loader/preloader.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/loader/main.c b/loader/main.c index c258e025183..5a208cc048c 100644 --- a/loader/main.c +++ b/loader/main.c @@ -38,7 +38,8 @@ extern char **environ; -/* the preloader will set this variable */ +/* the preloader will set these variables */ +__attribute((visibility("default"))) struct r_debug *wine_r_debug = NULL; const __attribute((visibility("default"))) struct wine_preload_info *wine_main_preload_info = NULL; /* canonicalize path and return its directory name */ diff --git a/loader/preloader.c b/loader/preloader.c index d0551bae63a..016f1def249 100644 --- a/loader/preloader.c +++ b/loader/preloader.c @@ -1387,6 +1387,32 @@ static void set_process_name( int argc, char *argv[] ) for (i = 1; i < argc; i++) argv[i] -= off; } +/* GDB integration, _dl_debug_state is *required* for GDB to hook the preloader */ +__attribute((visibility("default"))) void _dl_debug_state(void) {} +static struct r_debug_extended r_debug = {.base = {.r_version = 2}}; + +/* sets the preloader r_debug address into DT_DEBUG */ +static void init_r_debug( struct wld_auxv *av ) +{ + ElfW(Phdr) *phdr, *ph; + ElfW(Dyn) *dyn = NULL; + char *l_addr; + int phnum; + + if (!(phnum = get_auxiliary( av, AT_PHNUM, 0 ))) return; + if (!(phdr = (void *)get_auxiliary( av, AT_PHDR, 0 ))) return; + l_addr = (char *)phdr - sizeof(ElfW(Ehdr)); + + for (ph = phdr; ph < &phdr[phnum]; ++ph) if (ph->p_type == PT_DYNAMIC) break; + if (ph >= &phdr[phnum]) return; + + dyn = (void *)(ph->p_vaddr + l_addr); + while (dyn->d_tag != DT_DEBUG) dyn++; + + r_debug.base.r_ldbase = (ElfW(Addr))l_addr; + r_debug.base.r_brk = (ElfW(Addr))_dl_debug_state; + if (dyn->d_tag == DT_DEBUG) dyn->d_un.d_ptr = (uintptr_t)&r_debug; +} /* * wld_start @@ -1403,6 +1429,7 @@ void* wld_start( void **stack ) struct wld_auxv new_av[8], delete_av[3], *av; struct wld_link_map main_binary_map, ld_so_map; struct wine_preload_info **wine_main_preload_info; + struct r_debug *ld_so_r_debug, **wine_r_debug; pargc = *stack; argv = (char **)pargc + 1; @@ -1432,6 +1459,8 @@ void* wld_start( void **stack ) dump_auxiliary( av ); #endif + init_r_debug( av ); + /* reserve memory that Wine needs */ if (reserve) preload_reserve( reserve ); for (i = 0; preload_info[i].size; i++) @@ -1470,6 +1499,15 @@ void* wld_start( void **stack ) interp = (char *)main_binary_map.l_addr + main_binary_map.l_interp; map_so_lib( interp, &ld_so_map ); + /* expose ld.so _r_debug as a separate namespace in r_next */ + ld_so_r_debug = find_symbol( &ld_so_map, "_r_debug", STT_OBJECT ); + if (ld_so_r_debug) r_debug.r_next = (struct r_debug_extended *)ld_so_r_debug; + else wld_printf( "_r_debug not found in ld.so\n" ); + + wine_r_debug = find_symbol( &main_binary_map, "wine_r_debug", STT_OBJECT ); + if (wine_r_debug) *wine_r_debug = &r_debug.base; + else wld_printf( "wine_r_debug not found\n" ); + /* store pointer to the preload info into the appropriate main binary variable */ wine_main_preload_info = find_symbol( &main_binary_map, "wine_main_preload_info", STT_OBJECT ); if (wine_main_preload_info) *wine_main_preload_info = preload_info; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4518
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 124 +++++++++++++++++++++++++++++++++++++- 1 file changed, 123 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 75e6319c007..44045e042c6 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -51,6 +51,12 @@ #ifdef HAVE_SYS_USER_H # include <sys/user.h> #endif +#ifdef HAVE_LINK_H +# include <link.h> +#endif +#ifdef HAVE_SYS_LINK_H +# include <sys/link.h> +#endif #ifdef HAVE_LIBPROCSTAT_H # include <libprocstat.h> #endif @@ -79,6 +85,114 @@ WINE_DEFAULT_DEBUG_CHANNEL(virtual); WINE_DECLARE_DEBUG_CHANNEL(module); WINE_DECLARE_DEBUG_CHANNEL(virtual_ranges); +/* Gdb integration, in loader/main.c */ +static struct r_debug *wine_r_debug; + +#if defined(HAVE_LINK_H) || defined(HAVE_SYS_LINK_H) + +struct link_map_entry +{ + struct link_map map; + const void *module; +}; +static struct link_map link_map = {0}; + +static void r_debug_set_state( int state ) +{ + wine_r_debug->r_map = &link_map; + wine_r_debug->r_state = state; + ((void (*)(void))wine_r_debug->r_brk)(); +} + +static char *r_debug_realpath( const char *path ) +{ + char *real; + if (!path) return NULL; + if (!(real = realpath( path, NULL ))) return strdup( path ); + return real; +} + +static void r_debug_add_module( void *module, const WCHAR *filename, INT_PTR offset ) +{ + UNICODE_STRING redir, nt_name; + OBJECT_ATTRIBUTES attr; + char *unix_path = NULL; + + if (!wine_r_debug) return; + + RtlInitUnicodeString( &nt_name, filename ); + InitializeObjectAttributes( &attr, &nt_name, OBJ_CASE_INSENSITIVE, 0, 0 ); + get_redirect( &attr, &redir ); + + if (!nt_to_unix_file_name( &attr, &unix_path, FILE_OPEN )) + { + struct link_map *ptr = link_map.l_next, *next; + struct link_map_entry *entry; + + while (ptr) + { + entry = LIST_ENTRY(ptr, struct link_map_entry, map); + if (entry->module == module) break; + ptr = ptr->l_next; + } + + r_debug_set_state( RT_ADD ); + + if (ptr) entry->map.l_addr = offset; + else if ((entry = calloc( 1, sizeof(*entry) ))) + { + entry->module = module; + entry->map.l_addr = offset; + entry->map.l_name = r_debug_realpath( unix_path ); + + entry->map.l_next = link_map.l_next; + if ((next = entry->map.l_next)) next->l_prev = &entry->map; + entry->map.l_prev = &link_map; + link_map.l_next = &entry->map; + } + + r_debug_set_state( RT_CONSISTENT ); + } + + free( redir.Buffer ); + free( unix_path ); +} + +static void r_debug_remove_module( void *module ) +{ + struct link_map *ptr = link_map.l_next, *next; + struct link_map_entry *entry; + + if (!wine_r_debug) return; + + while (ptr) + { + entry = LIST_ENTRY(ptr, struct link_map_entry, map); + if (entry->module == module) break; + ptr = ptr->l_next; + } + if (!ptr) return; + + r_debug_set_state( RT_DELETE ); + + entry->map.l_prev->l_next = entry->map.l_next; + if ((next = entry->map.l_next)) next->l_prev = entry->map.l_prev; + + r_debug_set_state( RT_CONSISTENT ); + + free( entry->map.l_name ); + free( entry ); +} + +#else /* defined(HAVE_LINK_H) || defined(HAVE_SYS_LINK_H) */ + +#define RT_CONSISTENT 0 +static void r_debug_set_state( int state ) {} +static void r_debug_add_module( void *module, const WCHAR *filename, INT_PTR offset ) {} +static void r_debug_remove_module( void *module ) {} + +#endif /* defined(HAVE_LINK_H) || defined(HAVE_SYS_LINK_H) */ + struct preload_info { void *addr; @@ -2900,6 +3014,7 @@ static NTSTATUS map_image_into_view( struct file_view *view, const WCHAR *filena #ifdef VALGRIND_LOAD_PDB_DEBUGINFO VALGRIND_LOAD_PDB_DEBUGINFO(fd, ptr, total_size, ptr - (char *)wine_server_get_ptr( image_info->base )); #endif + r_debug_add_module( ptr, filename, ptr - (char *)wine_server_get_ptr( image_info->base ) ); return STATUS_SUCCESS; } @@ -3251,11 +3366,14 @@ static void *alloc_virtual_heap( SIZE_T size ) void virtual_init(void) { const struct preload_info **preload_info = dlsym( RTLD_DEFAULT, "wine_main_preload_info" ); + struct r_debug **r_debug = dlsym( RTLD_DEFAULT, "wine_r_debug" ); const char *preload = getenv( "WINEPRELOADRESERVE" ); size_t size; int i; pthread_mutexattr_t attr; + if (r_debug && (wine_r_debug = *r_debug)) r_debug_set_state( RT_CONSISTENT ); + pthread_mutexattr_init( &attr ); pthread_mutexattr_settype( &attr, PTHREAD_MUTEX_RECURSIVE ); pthread_mutex_init( &virtual_mutex, &attr ); @@ -5598,7 +5716,11 @@ static NTSTATUS unmap_view_of_section( HANDLE process, PVOID addr, ULONG flags ) SERVER_END_REQ; if (!status) { - if (view->protect & SEC_IMAGE) release_builtin_module( view->base ); + if (view->protect & SEC_IMAGE) + { + r_debug_remove_module( view->base ); + release_builtin_module( view->base ); + } if (flags & MEM_PRESERVE_PLACEHOLDER) free_pages_preserve_placeholder( view, view->base, view->size ); else delete_view( view ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4518
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=143085 Your paranoid android. === debian11 (build log) === ../wine/loader/preloader.c:1392:15: error: variable ���r_debug��� has initializer but incomplete type ../wine/loader/preloader.c:1392:44: error: ���struct r_debug_extended��� has no member named ���base��� ../wine/loader/preloader.c:1392:51: error: extra brace group at end of initializer ../wine/loader/preloader.c:1412:12: error: invalid use of undefined type ���struct r_debug_extended��� ../wine/loader/preloader.c:1413:12: error: invalid use of undefined type ���struct r_debug_extended��� ../wine/loader/preloader.c:1504:31: error: invalid use of undefined type ���struct r_debug_extended��� ../wine/loader/preloader.c:1508:47: error: invalid use of undefined type ���struct r_debug_extended��� ../wine/loader/preloader.c:1392:32: error: storage size of ���r_debug��� isn���t known Task: The win32 Wine build failed === debian11b (build log) === ../wine/loader/preloader.c:1392:15: error: variable ���r_debug��� has initializer but incomplete type ../wine/loader/preloader.c:1392:44: error: ���struct r_debug_extended��� has no member named ���base��� ../wine/loader/preloader.c:1392:51: error: extra brace group at end of initializer ../wine/loader/preloader.c:1412:12: error: invalid use of undefined type ���struct r_debug_extended��� ../wine/loader/preloader.c:1413:12: error: invalid use of undefined type ���struct r_debug_extended��� ../wine/loader/preloader.c:1504:31: error: invalid use of undefined type ���struct r_debug_extended��� ../wine/loader/preloader.c:1508:47: error: invalid use of undefined type ���struct r_debug_extended��� ../wine/loader/preloader.c:1392:32: error: storage size of ���r_debug��� isn���t known Task: The wow64 Wine build failed
Actually found out that it could be even simpler, and as long as the preloader exposes `_dl_debug_state` and fills the DT_DEBUG entry with a `struct r_debug` Gdb will be happy. In addition, with r_debug version >= 2, it's possible to have multiple r_debug chained together and we can simply lookup ld.so's own r_debug and chain it with ours without having to copy it at all. Without the systemtap probes, Gdb will hook each `r_debug.r_brk` with breakpoints and we can call our own PE-only `r_debug.r_brk`, while ld.so will call theirs for unix libraries and every library load / unload will then be dynamically updated in Gdb. Awesome. Well at least we get symbols and backtraces. For now the backtraces are only on either the unix side (with syscall frames unwinding over and skipping PE frames), or on the PE side only (with backtrace stopping on KiUserCallbackDispatcher). In order to cross the boundaries we need a custom unwinder, and because of some Gdb internal heuristics (refusing to unwind to inner frames) it also probably needs some small Gdb patches. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4518#note_61152
On Thu Dec 21 12:37:55 2023 +0000, Rémi Bernon wrote:
I think this is a LLD specific failure, looks like the same happens on x86_64. Funnily LLD doesn't even generate a PT_DYNAMIC program header, and so doesn't have DT_DEBUG entry where to put _r_debug anyway. Should be fixed now, I used `(char *)phdr - sizeof(ElfW(Ehdr))` as load address instead of `executable_start`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4518#note_61154
On Wed Feb 14 10:57:33 2024 +0000, Rémi Bernon wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/4518/diffs?diff_id=99518&start_sha=ea32d0a56e0034ddbd9e8b06007c84d3ef13ed9e#584f4313ed133393a8f1903c21dcaf4967ba7ab9_2928_3017) I don't know, I don't think this MR does anything special here, and it simply exposes whichever PE file is mapped to Gdb.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4518#note_61155
participants (2)
-
Marvin -
Rémi Bernon