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.
-- v6: ntdll: Maintain a PE module link map and expose it to GDB. loader: Expose the standard debugging symbols for GDB.
From: Rémi Bernon rbernon@codeweavers.com
--- loader/main.c | 3 ++- loader/preloader.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 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..f27d6df8c17 100644 --- a/loader/preloader.c +++ b/loader/preloader.c @@ -79,12 +79,16 @@ #ifdef HAVE_ELF_H # include <elf.h> #endif + +/* define _r_debug so we can re-define it as r_debug_extended */ +#define _r_debug no_r_debug; #ifdef HAVE_LINK_H # include <link.h> #endif #ifdef HAVE_SYS_LINK_H # include <sys/link.h> #endif +#undef _r_debug
#include "wine/asm.h" #include "main.h" @@ -1387,6 +1391,39 @@ static void set_process_name( int argc, char *argv[] ) for (i = 1; i < argc; i++) argv[i] -= off; }
+struct wld_r_debug_extended +{ + struct r_debug base; + struct wld_r_debug_extended *r_next; +}; + +/* GDB integration, _r_debug_state is *required* for GDB to hook the preloader */ +__attribute((visibility("default"))) void _r_debug_state(void) {} +__attribute((visibility("default"))) struct wld_r_debug_extended _r_debug = {{0}}; + +/* 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; + + _r_debug.base.r_version = 2; + _r_debug.base.r_brk = (ElfW(Addr))_r_debug_state; + + 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)); + _r_debug.base.r_ldbase = (ElfW(Addr))l_addr; + + 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->d_tag != DT_NULL) dyn++; + if (dyn->d_tag == DT_DEBUG) dyn->d_un.d_ptr = (uintptr_t)&_r_debug; +}
/* * wld_start @@ -1403,6 +1440,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 +1470,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 +1510,17 @@ 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 wld_r_debug_extended *)ld_so_r_debug; + else wld_printf( "_r_debug not found in ld.so\n" ); + + _r_debug_state(); /* notify GDB that _r_debug is ready */ + + 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;
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/ntdll/unix/virtual.c | 130 +++++++++++++++++++++++++++++++++++++- 1 file changed, 129 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 02c0bebe879..13bc0b4aa03 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,120 @@ 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 = {.l_name = (char *)""}; + +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 *get_path_from_fd( int fd, int sz ) +{ +#ifdef linux + char *ret = malloc( 32 + sz ); + + if (ret) sprintf( ret, "/proc/self/fd/%u", fd ); + return ret; +#elif defined(F_GETPATH) + char *ret = malloc( PATH_MAX + sz ); + + if (!ret) return NULL; + if (!fcntl( fd, F_GETPATH, ret )) return ret; + free( ret ); + return NULL; +#else + return NULL; +#endif +} + +static char *r_debug_path_from_fd( int fd ) +{ + char *real, *path; + if (!(path = get_path_from_fd( fd, 0 ))) return NULL; + if (!(real = realpath( path, NULL ))) return path; + free( path ); + return real; +} + +static void r_debug_add_module( void *module, int fd, INT_PTR offset ) +{ + 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; + } + + 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_path_from_fd( fd ); + + 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 ); +} + +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, int fd, 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; @@ -2924,6 +3044,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, fd, ptr - (char *)wine_server_get_ptr( image_info->base ) ); return STATUS_SUCCESS; }
@@ -3279,11 +3400,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 ); @@ -5641,7 +5765,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 ); }
Updated with an additional call of the rendez-vous function when everything is setup.
Renamed from `_dl_debug_state` to `_r_debug_state` which is equivalent for GDB but looks more consistent with `_r_debug`.
Also added a custom `wld_r_debug_extended` definition as the `r_debug_extended` structure seems to be missing in some older distributions.
On Wed Feb 28 19:15:39 2024 +0000, Rémi Bernon wrote:
changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/4518/diffs?diff_id=102108&start_sha=3c4cb10af12f107e94a2808829d6a2878dafe716#8de1f39ed579b059e12d845afcdf25dd7603ab8a_1395_1394)
Fwiw I have implemented 3) in https://gitlab.winehq.org/rbernon/binutils-gdb/-/commit/f9f0ebde1e53d4553e50..., though I don't intend to send it upstream until I hear back from my previous submission.
On Wed Feb 28 19:18:36 2024 +0000, Rémi Bernon wrote:
Fwiw I have implemented 3) in https://gitlab.winehq.org/rbernon/binutils-gdb/-/commit/f9f0ebde1e53d4553e50..., though I don't intend to send it upstream until I hear back from my previous submission.
Thanks, that looks promising. I think that changing gdb like that is the cleanest solution.
On Mon Feb 26 06:48:52 2024 +0000, Rémi Bernon wrote:
The interface for JIT code is meant for block of code without any corresponding file on disk. It requires to load the debug info in memory, and attach it to the debugger notifications. It doesn't even support passing the module file name. This doesn't seem like a good fit to me, as we would need to find and load the debug info ourselves. On the other hand, using `_r_debug` would let us benefit from all the standard mechanisms. For instance, using `_r_debug` Gdb can find the debug link and load split debug info directly. Or even read the build-id, and lookup the PE file debug info in the systems debug info database.
I don't see a reason why debugger wouldn't be able to load symbols, symbol links or build ids from the file when passed a PE file. My understanding is that the separated symbol blobs are meant to simplify things for JIT engines, so they can just generate a blobs without full ELF, but debugger could just fallback to its normal treatment when its got a proper image.
For file module file names, the debugger could do the same thing as perf does and try to get it from the mapping. Moving the overhead of getting file names from Wine (where it's paid even when no debugger is involved) to the debugger does not seem like a bad thing. And even without that, it shouldn't be needed to get symbols, unless some relative links are involved.
That said, I generally like the `_r_debug` idea. If we can make it work, that's fine with me.
It's just preloader, loader, ntdll interaction that is not great. I don't have a good idea how to improve that part, the best I could think of is to try harder to not do it at all and handle PEs separately in ntdll. With JIT interface, the preloader part and debugger patch would still be needed to make gdb intercept ld.so (although perhaps we could then just set `DT_DEBUG` to ld.so `_r_debug` and use `_r_debug_state` to trigger gdb with your patch to hook it).
On Thu Feb 29 21:58:43 2024 +0000, Jacek Caban wrote:
I don't see a reason why debugger wouldn't be able to load symbols, symbol links or build ids from the file when passed a PE file. My understanding is that the separated symbol blobs are meant to simplify things for JIT engines, so they can just generate a blobs without full ELF, but debugger could just fallback to its normal treatment when its got a proper image. For file module file names, the debugger could do the same thing as perf does and try to get it from the mapping. Moving the overhead of getting file names from Wine (where it's paid even when no debugger is involved) to the debugger does not seem like a bad thing. And even without that, it shouldn't be needed to get symbols, unless some relative links are involved. That said, I generally like the `_r_debug` idea. If we can make it work, that's fine with me. It's just preloader, loader, ntdll interaction that is not great. I don't have a good idea how to improve that part, the best I could think of is to try harder to not do it at all and handle PEs separately in ntdll. With JIT interface, the preloader part and debugger patch would still be needed to make gdb intercept ld.so (although perhaps we could then just set `DT_DEBUG` to ld.so `_r_debug` and use `_r_debug_state` to trigger gdb with your patch to hook it).
Quoting the GDB manual https://sourceware.org/gdb/current/onlinedocs/gdb.html/Registering-Code.html... [emphasis mine]
To register code with GDB, the JIT should follow this protocol:
Generate an object file **in memory** with symbols and other desired debug information. The file must include the **virtual addresses** of the sections.
This sounds twice as complicated as r_debug approach. Also, holding mostly-unused debug info will just waste memory.
On Tue May 21 00:59:37 2024 +0000, Fabian Maurer wrote:
I found another issue: When changing a DLL and adding a few newlines, then GDB still uses the old line numbers for the stacktrace / breakpoints. `wineboot -u` after changing the DLL helps.
Just noting: This was solved long ago, was the same issue with the wrong filepaths being used.
There's been loader changes resulting in merge conflicts
On Mon Sep 16 17:22:29 2024 +0000, Fabian Maurer wrote:
There's been loader changes resulting in merge conflicts
There's also another merge conflict in `virtual_init()`
Any updates on this? Would be really nice to have gdb integration upstreamed.
-
Aida Jonikienė (＠DodoGTA) -
Fabian Maurer (＠DarkShadow44)
-
Jacek Caban (＠jacek)
-
Jinoh Kang (＠iamahuman)
-
Rémi Bernon
-
Rémi Bernon (＠rbernon)