Subtle problems with "info sharedlibrary" on MS-Windows
Eli Zaretskii
eliz@gnu.org
Sat Apr 10 08:46:01 GMT 2021
> Cc: ssbssa@yahoo.de, gdb-patches@sourceware.org
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Thu, 8 Apr 2021 09:57:27 -0400
>
> >> According to our style guideline, we would use
> >>
> >> dll_name != nullptr
> >
> > Does this also mean the GDB style prefers, e.g.,
> >
> > if (load_addr == nullptr)
> >
> > to
> >
> > if (!load_addr)
> >
> > ? Because you didn't comment on those lines, only on those where the
> > value is tested for NOT being null.
>
> Exactly, sorry for not being clear. We always want explicit comparison
> with nullptr for pointers, whether that is == or !=.
>
> Same for integers that are not used as booleans, we want
>
> if (item_count == 0)
>
> and not
>
> if (!item_count)
>
> This is described here:
>
> https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_nullptr_And_Zero
>
> >>> + if (!(load_addr && mi.lpBaseOfDll != load_addr))
> >>
> >> Perhaps matter of personal preference, but I would understand it better
> >> (less mental steps) as
> >>
> >> if (!load_addr || mi.lpBaseOfDll == load_addr)
> >
> > I feel the other way around, but maybe I'm the odd one out here.
> > Pedro, Joel: what say you?
> >
> > In any case, I guess I could add a comment there explaining the logic
> > in plain English, so everyone would understand the intent.
>
> That's alright, leave it the way it is. But a comment explaining the
> intent is always welcome, in my book.
OK, thanks. Below is what I actually pushed:
commit b3885679dd71bed069e7e6fc2ae8b9eb05f90d62
Author: Eli Zaretskii <eliz@gnu.org>
AuthorDate: Sat Apr 10 11:33:08 2021 +0300
Commit: Eli Zaretskii <eliz@gnu.org>
CommitDate: Sat Apr 10 11:42:54 2021 +0300
Fix handling DLL loads at run time
This patch makes handling a DLL load at run time (using LoadLibrary)
much more reliable when its file name cannot be obtained using the
lpImageName pointer provided by the DLL load debug event. The
solution is to enumerate all the DLLs loaded by the inferior, looking
for the DLL that's loaded at base address provided by the lpBaseOfDll
pointer of the debug event. Correctly resolving the DLL file name is
important, because without that GDB doesn't record the DLL in the list
of solibs, and then later is unable to show functions in that DLL in
the backtraces, which produces corrupted and truncated backtraces.
See this thread for the problems that causes:
https://sourceware.org/pipermail/gdb-patches/2021-March/177022.html
gdb/ChangeLog:
2021-04-10 Eli Zaretskii <eliz@gnu.org>
* windows-nat.c (windows_nat::handle_load_dll): Call
windows_add_dll if get_image_name failed to glean the name of the
DLL by using the lpImageName pointer.
(windows_add_all_dlls): Now a thin wrapper around windows_add_dll.
(windows_add_dll): Now does what windows_add_all_dlls did before,
but also accepts an argument LOAD_ADDR, which, if non-NULL,
specifies the address where the DLL was loaded into the inferior,
and looks for the single DLL loaded at that address.
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c5ed593..5c38a4e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2021-04-10 Eli Zaretskii <eliz@gnu.org>
+
+ * windows-nat.c (windows_nat::handle_load_dll): Call
+ windows_add_dll if get_image_name failed to glean the name of the
+ DLL by using the lpImageName pointer.
+ (windows_add_all_dlls): Now a thin wrapper around windows_add_dll.
+ (windows_add_dll): Now does what windows_add_all_dlls did before,
+ but also accepts an argument LOAD_ADDR, which, if non-NULL,
+ specifies the address where the DLL was loaded into the inferior,
+ and looks for the single DLL loaded at that address.
+
2021-04-09 Luis Machado <luis.machado@linaro.org>
* nat/aarch64-mte-linux-ptrace.c: Update include file order.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 00f8353..c0f64f8 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -869,6 +869,8 @@ windows_make_so (const char *name, LPVOID load_addr)
return so;
}
+static bool windows_add_dll (LPVOID);
+
/* See nat/windows-nat.h. */
void
@@ -884,11 +886,21 @@ windows_nat::handle_load_dll ()
(source: MSDN LOAD_DLL_DEBUG_INFO structure). */
dll_name = get_image_name (current_process_handle,
event->lpImageName, event->fUnicode);
- if (!dll_name)
- return;
+ /* If the DLL name could not be gleaned via lpImageName, try harder
+ by enumerating all the DLLs loaded into the inferior, looking for
+ one that is loaded at base address = lpBaseOfDll. */
+ if (dll_name != nullptr)
+ {
- solib_end->next = windows_make_so (dll_name, event->lpBaseOfDll);
- solib_end = solib_end->next;
+ solib_end->next = windows_make_so (dll_name, event->lpBaseOfDll);
+ solib_end = solib_end->next;
+ }
+ else if (event->lpBaseOfDll != nullptr
+ && windows_add_dll (event->lpBaseOfDll))
+ dll_name = solib_end->so_name;
+
+ if (dll_name == nullptr)
+ return;
lm_info_windows *li = (lm_info_windows *) solib_end->lm_info;
@@ -1899,6 +1911,19 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
static void
windows_add_all_dlls (void)
{
+ windows_add_dll (NULL);
+}
+
+/* Iterate over all DLLs currently mapped by our inferior, looking for
+ a DLL which is loaded at LOAD_ADDR. If found, add the DLL to our
+ list of solibs and return 'true'; otherwise do nothing and return
+ 'false'. LOAD_ADDR NULL means add all DLLs to the list of solibs;
+ this is used when the inferior finishes its initialization, and all
+ the DLLs it statically depends on are presumed loaded. */
+
+static bool
+windows_add_dll (LPVOID load_addr)
+{
HMODULE dummy_hmodule;
DWORD cb_needed;
HMODULE *hmodules;
@@ -1910,18 +1935,18 @@ windows_add_all_dlls (void)
if (EnumProcessModulesEx (current_process_handle, &dummy_hmodule,
sizeof (HMODULE), &cb_needed,
LIST_MODULES_32BIT) == 0)
- return;
+ return false;
}
else
#endif
{
if (EnumProcessModules (current_process_handle, &dummy_hmodule,
sizeof (HMODULE), &cb_needed) == 0)
- return;
+ return false;
}
if (cb_needed < 1)
- return;
+ return false;
hmodules = (HMODULE *) alloca (cb_needed);
#ifdef __x86_64__
@@ -1930,14 +1955,14 @@ windows_add_all_dlls (void)
if (EnumProcessModulesEx (current_process_handle, hmodules,
cb_needed, &cb_needed,
LIST_MODULES_32BIT) == 0)
- return;
+ return false;
}
else
#endif
{
if (EnumProcessModules (current_process_handle, hmodules,
cb_needed, &cb_needed) == 0)
- return;
+ return false;
}
char system_dir[__PMAX];
@@ -1983,6 +2008,7 @@ windows_add_all_dlls (void)
if (GetModuleInformation (current_process_handle, hmodules[i],
&mi, sizeof (mi)) == 0)
continue;
+
if (GetModuleFileNameEx (current_process_handle, hmodules[i],
dll_name, sizeof (dll_name)) == 0)
continue;
@@ -2005,9 +2031,17 @@ windows_add_all_dlls (void)
name = syswow_dll_path.c_str();
}
- solib_end->next = windows_make_so (name, mi.lpBaseOfDll);
- solib_end = solib_end->next;
+ /* Record the DLL if either LOAD_ADDR is NULL or the address
+ at which the DLL was loaded is equal to LOAD_ADDR. */
+ if (!(load_addr != nullptr && mi.lpBaseOfDll != load_addr))
+ {
+ solib_end->next = windows_make_so (name, mi.lpBaseOfDll);
+ solib_end = solib_end->next;
+ if (load_addr != nullptr)
+ return true;
+ }
}
+ return load_addr == nullptr ? true : false;
}
void
More information about the Gdb-patches
mailing list