[5/4] libbacktrace: improve getting debug information for loaded dlls

  Am 03.01.2024 um 00:12 schrieb Björn Schäpers:
> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor:
>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote:
>>>
>>> From: Björn Schäpers <bjoern@hazardy.de>
>>>
>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except
>>> that libraries loaded after the backtrace_initialize are not handled.
>>> But as far as I can see that's the same for elf.
>>
>> Thanks, but I don't want a patch that loops using goto statements.
>> Please rewrite to avoid that.  It may be simpler to call a function.
>>
>> Also starting with a module count of 1000 seems like a lot.  Do
>> typical Windows programs load that many modules?
>>
>> Ian
>>
>>
> 
> Rewritten using a function.
> 
> If that is commited, could you attribute that commit to me (--author="Björn 
> Schäpers <bjoern@hazardy.de>")?
> 
> Thanks and kind regards,
> Björn.

I noticed that under 64 bit libraries loaded with LoadLibrary were missing. 
EnumProcessModules stated the correct number of modules, but did not fill the 
the HMODULEs, but set them to 0. While trying to investigate I noticed if I do 
the very same thing from main (in C++) I even got fewer module HMODULEs.

So I went a different way. This detects all libraries correctly, in 32 and 64 
bit. The question is, if it should be a patch on top of the previous, or should 
they be merged, or even only this solution and drop the EnumProcessModules variant?

Kind regards,
Björn.
From 784e01f1baf92c23c819aeb9e77010412023700f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de>
Date: Thu, 4 Jan 2024 22:02:03 +0100
Subject: [PATCH 2/2] libbacktrace: improve getting debug information for
 loaded dlls

EnumProcessModules does not always result in all modules loaded,
especially those that are loaded with LoadLibrary.

libbacktrace/Changelog:

	* configure.ac: Checked for tlhelp32.h
	* configure: Regenerate.
	* config.h.in: Regenerate.
	* pecoff.c: Include <tlhelp32.h> if available.
	 (backtrace_initialize): Use tlhelp32 api for a snapshot to
	                         detect loaded modules.
---
 libbacktrace/config.h.in  |   3 +
 libbacktrace/configure    | 185 ++++++++++++++++++++------------------
 libbacktrace/configure.ac |   4 +
 libbacktrace/pecoff.c     |  62 ++++++++++++-
 4 files changed, 164 insertions(+), 90 deletions(-)
  

Am 04.01.2024 um 23:33 schrieb Björn Schäpers:
> Am 03.01.2024 um 00:12 schrieb Björn Schäpers:
>> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor:
>>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote:
>>>>
>>>> From: Björn Schäpers <bjoern@hazardy.de>
>>>>
>>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except
>>>> that libraries loaded after the backtrace_initialize are not handled.
>>>> But as far as I can see that's the same for elf.
>>>
>>> Thanks, but I don't want a patch that loops using goto statements.
>>> Please rewrite to avoid that.  It may be simpler to call a function.
>>>
>>> Also starting with a module count of 1000 seems like a lot.  Do
>>> typical Windows programs load that many modules?
>>>
>>> Ian
>>>
>>>
>>
>> Rewritten using a function.
>>
>> If that is commited, could you attribute that commit to me (--author="Björn 
>> Schäpers <bjoern@hazardy.de>")?
>>
>> Thanks and kind regards,
>> Björn.
> 
> I noticed that under 64 bit libraries loaded with LoadLibrary were missing. 
> EnumProcessModules stated the correct number of modules, but did not fill the 
> the HMODULEs, but set them to 0. While trying to investigate I noticed if I do 
> the very same thing from main (in C++) I even got fewer module HMODULEs.
> 
> So I went a different way. This detects all libraries correctly, in 32 and 64 
> bit. The question is, if it should be a patch on top of the previous, or should 
> they be merged, or even only this solution and drop the EnumProcessModules variant?
> 
> Kind regards,
> Björn.

This patch adds libraries which are loaded after backtrace_initialize, like 
plugins or similar.

I don't know what style is preferred for the Win32 typedefs, should the code use 
PVOID or void*? And I'm not sure I wrapped every long line correctly.

Kind regards,
Björn.
From 02e76e727b95dc21dc07d1fe8424b68e37e91a83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de>
Date: Sat, 6 Jan 2024 22:53:54 +0100
Subject: [PATCH 3/3] libbacktrace: Add loaded dlls after initialize

libbacktrace/Changelog:

	* pecoff.c [HAVE_WINDOWS_H]:
	  (dll_notification_data): Added
	  (dll_notification_context): Added
	  (dll_notification): Added
	  (backtrace_initialize): Use LdrRegisterDllNotification to load
	                          debug information of later loaded
	                          dlls.
---
 libbacktrace/pecoff.c | 102 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 1 deletion(-)

diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index 647baa39640..bfe12cc2a0a 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -62,6 +62,32 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #undef Module32Next
 #endif
 #endif
+
+#if defined(_ARM_)
+#define NTAPI
+#else
+#define NTAPI __stdcall
+#endif
+
+/* This is a simplified (but binary compatible) version of what Microsoft
+   defines in their documentation. */
+struct dll_notifcation_data
+{
+  ULONG reserved;
+  /* The name as UNICODE_STRING struct. */
+  PVOID full_dll_name;
+  PVOID base_dll_name;
+  PVOID dll_base;
+  ULONG size_of_image;
+};
+
+typedef LONG NTSTATUS;
+typedef VOID CALLBACK (*LDR_DLL_NOTIFICATION)(ULONG,
+					      struct dll_notifcation_data*,
+					      PVOID);
+typedef NTSTATUS NTAPI (*LDR_REGISTER_FUNCTION)(ULONG,
+						LDR_DLL_NOTIFICATION, PVOID,
+						PVOID*);
 #endif
 
 /* Coff file header.  */
@@ -912,7 +938,8 @@ coff_add (struct backtrace_state *state, int descriptor,
   return 0;
 }
 
-#if defined(HAVE_WINDOWS_H) && !defined(HAVE_TLHELP32_H)
+#ifdef HAVE_WINDOWS_H
+#ifndef HAVE_TLHELP32_H
 static void
 free_modules (struct backtrace_state *state,
 	      backtrace_error_callback error_callback, void *data,
@@ -958,6 +985,51 @@ get_all_modules (struct backtrace_state *state,
     }
 }
 #endif
+struct dll_notification_context
+{
+  struct backtrace_state *state;
+  backtrace_error_callback error_callback;
+  void *data;
+};
+
+VOID CALLBACK
+dll_notification (ULONG reason,
+struct dll_notifcation_data *notification_data,
+PVOID context)
+{
+  char module_name[MAX_PATH];
+  int descriptor;
+  struct dll_notification_context* dll_context =
+    (struct dll_notification_context*) context;
+  struct backtrace_state *state = dll_context->state;
+  void *data = dll_context->data;
+  backtrace_error_callback error_callback = dll_context->data;
+  fileline fileline;
+  int found_sym;
+  int found_dwarf;
+  HMODULE module_handle;
+
+  if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1)
+    return;
+
+  if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
+			  | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
+			  (TCHAR*) notification_data->dll_base,
+			  &module_handle))
+    return;
+
+  if (!GetModuleFileNameA ((HMODULE) module_handle, module_name, MAX_PATH - 1))
+    return;
+
+  descriptor = backtrace_open (module_name, error_callback, data, NULL);
+
+  if (descriptor < 0)
+    return;
+
+  coff_add (state, descriptor, error_callback, data, &fileline, &found_sym,
+	    &found_dwarf, (uintptr_t) module_handle);
+}
+#endif
 
 /* Initialize the backtrace data we need from an ELF executable.  At
    the ELF level, all we need to do is find the debug info
@@ -978,6 +1050,7 @@ backtrace_initialize (struct backtrace_state *state,
 #ifdef HAVE_WINDOWS_H
   fileline module_fileline_fn;
   int module_found_sym;
+  HMODULE nt_dll_handle;
 
 #ifdef HAVE_TLHELP32_H
   HANDLE snapshot;
@@ -1065,6 +1138,33 @@ backtrace_initialize (struct backtrace_state *state,
   if (modules)
     free_modules (state, error_callback, data, &modules, bytes_allocated_for_modules);
 #endif
+
+  nt_dll_handle = GetModuleHandle (TEXT ("ntdll.dll"));
+  if (nt_dll_handle)
+    {
+      LDR_REGISTER_FUNCTION register_func;
+      const char register_name[] = "LdrRegisterDllNotification";
+      register_func = (LDR_REGISTER_FUNCTION) GetProcAddress (nt_dll_handle,
+							      register_name);
+
+      if (register_func)
+	{
+	  PVOID cookie;
+	  struct dll_notification_context *context
+	    = backtrace_alloc (state,
+			       sizeof(struct dll_notification_context),
+			       error_callback, data);
+
+	  if (context)
+	    {
+	      context->state = state;
+	      context->data = data;
+	      context->error_callback = error_callback;
+
+	      register_func (0, &dll_notification, context, &cookie);
+	    }
+	}
+    }
 #endif
 
   if (!state->threaded)
  

> Date: Sat, 6 Jan 2024 23:15:24 +0100
> From: Björn Schäpers <gcc@hazardy.de>
> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> 
> This patch adds libraries which are loaded after backtrace_initialize, like 
> plugins or similar.
> 
> I don't know what style is preferred for the Win32 typedefs, should the code use 
> PVOID or void*?

It doesn't matter, at least not if the source file includes the
Windows header files (where PVOID is defined).

> +  if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1)

IMO, it would be better to supply a #define if undefined:

#ifndef LDR_DLL_NOTIFICATION_REASON_LOADED
# define LDR_DLL_NOTIFICATION_REASON_LOADED 1
#endif

> +  if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
> +			  | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
> +			  (TCHAR*) notification_data->dll_base,

Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies
on compile-time definition of UNICODE?  (I'm not familiar with the
internals of libbacktrace, so apologies if this is a silly question.)

Thanks.
  

[I re-added the other addressees, as I don' think you meant to make
this discussion private between the two of us.]

> Date: Sun, 7 Jan 2024 12:58:29 +0100
> From: Björn Schäpers <gcc@hazardy.de>
> 
> Am 07.01.2024 um 07:50 schrieb Eli Zaretskii:
> >> Date: Sat, 6 Jan 2024 23:15:24 +0100
> >> From: Björn Schäpers <gcc@hazardy.de>
> >> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> >>
> >> This patch adds libraries which are loaded after backtrace_initialize, like
> >> plugins or similar.
> >>
> >> I don't know what style is preferred for the Win32 typedefs, should the code use
> >> PVOID or void*?
> > 
> > It doesn't matter, at least not if the source file includes the
> > Windows header files (where PVOID is defined).
> > 
> >> +  if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1)
> > 
> > IMO, it would be better to supply a #define if undefined:
> > 
> > #ifndef LDR_DLL_NOTIFICATION_REASON_LOADED
> > # define LDR_DLL_NOTIFICATION_REASON_LOADED 1
> > #endif
> > 
> 
> I surely can define it. But the ifndef is not needed, since there are no headers 
> containing the function signatures, structures or the defines:
> https://learn.microsoft.com/en-us/windows/win32/devnotes/ldrregisterdllnotification

OK, I wasn't sure about that.

> >> +  if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
> >> +			  | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
> >> +			  (TCHAR*) notification_data->dll_base,
> > 
> > Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies
> > on compile-time definition of UNICODE?  (I'm not familiar with the
> > internals of libbacktrace, so apologies if this is a silly question.)
> > 
> > Thanks.
> 
> As far as I can see it's the first time for TCHAR, I would've gone for 
> GetModuleHandleExW, but https://gcc.gnu.org/pipermail/gcc/2023-January/240534.html

That was about GetModuleHandle, not about GetModuleHandleEx.  For the
latter, all Windows versions that support it also support "wide" APIs.
So my suggestion is to use GetModuleHandleExW here.  However, you will
need to make sure that notification_data->dll_base is declared as
'wchar_t *', not 'char *'.  If dll_base is declared as 'char *', then
only GetModuleHandleExA will work, and you will lose the ability to
support file names with non-ASCII characters outside of the current
system codepage.

> But I didn't want to force GetModuleHandleExA, so I went for TCHAR and 
> GetModuleHandleEx so it automatically chooses which to use. Same for 
> GetModuleHandle of ntdll.dll.

The considerations for GetModuleHandle and for GetModuleHandleEx are
different: the former is also available on old versions of Windows
that doesn't support "wide" APIs.
  

Am 07.01.2024 um 15:46 schrieb Eli Zaretskii:
> [I re-added the other addressees, as I don' think you meant to make
> this discussion private between the two of us.]
> 

Yeah, that was a mistake.

>> Date: Sun, 7 Jan 2024 12:58:29 +0100
>> From: Björn Schäpers <gcc@hazardy.de>
>>
>> Am 07.01.2024 um 07:50 schrieb Eli Zaretskii:
>>>> Date: Sat, 6 Jan 2024 23:15:24 +0100
>>>> From: Björn Schäpers <gcc@hazardy.de>
>>>> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>>>>
>>>> This patch adds libraries which are loaded after backtrace_initialize, like
>>>> plugins or similar.
>>>>
>>>> I don't know what style is preferred for the Win32 typedefs, should the code use
>>>> PVOID or void*?
>>>
>>> It doesn't matter, at least not if the source file includes the
>>> Windows header files (where PVOID is defined).
>>>
>>>> +  if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1)
>>>
>>> IMO, it would be better to supply a #define if undefined:
>>>
>>> #ifndef LDR_DLL_NOTIFICATION_REASON_LOADED
>>> # define LDR_DLL_NOTIFICATION_REASON_LOADED 1
>>> #endif
>>>
>>
>> I surely can define it. But the ifndef is not needed, since there are no headers
>> containing the function signatures, structures or the defines:
>> https://learn.microsoft.com/en-us/windows/win32/devnotes/ldrregisterdllnotification
> 
> OK, I wasn't sure about that.
> 
>>>> +  if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
>>>> +			  | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
>>>> +			  (TCHAR*) notification_data->dll_base,
>>>
>>> Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies
>>> on compile-time definition of UNICODE?  (I'm not familiar with the
>>> internals of libbacktrace, so apologies if this is a silly question.)
>>>
>>> Thanks.
>>
>> As far as I can see it's the first time for TCHAR, I would've gone for
>> GetModuleHandleExW, but https://gcc.gnu.org/pipermail/gcc/2023-January/240534.html
> 
> That was about GetModuleHandle, not about GetModuleHandleEx.  For the
> latter, all Windows versions that support it also support "wide" APIs.
> So my suggestion is to use GetModuleHandleExW here.  However, you will
> need to make sure that notification_data->dll_base is declared as
> 'wchar_t *', not 'char *'.  If dll_base is declared as 'char *', then
> only GetModuleHandleExA will work, and you will lose the ability to
> support file names with non-ASCII characters outside of the current
> system codepage.

The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag 
GetModuleHandleEx does not look for a name, but uses an adress in the module to 
get the HMODULE, so you cast it to char* or wchar_t* depending on which function 
you call. Actually one could just cast the dll_base to HMODULE, at least in 
win32 on x86 the HMODULE of a dll is always its base adress. But to make it 
safer and future proof I went the way through GetModuleHandeEx.

> 
>> But I didn't want to force GetModuleHandleExA, so I went for TCHAR and
>> GetModuleHandleEx so it automatically chooses which to use. Same for
>> GetModuleHandle of ntdll.dll.
> 
> The considerations for GetModuleHandle and for GetModuleHandleEx are
> different: the former is also available on old versions of Windows
> that doesn't support "wide" APIs.
  

> Date: Sun, 7 Jan 2024 17:07:06 +0100
> Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: Björn Schäpers <gcc@hazardy.de>
> 
> > That was about GetModuleHandle, not about GetModuleHandleEx.  For the
> > latter, all Windows versions that support it also support "wide" APIs.
> > So my suggestion is to use GetModuleHandleExW here.  However, you will
> > need to make sure that notification_data->dll_base is declared as
> > 'wchar_t *', not 'char *'.  If dll_base is declared as 'char *', then
> > only GetModuleHandleExA will work, and you will lose the ability to
> > support file names with non-ASCII characters outside of the current
> > system codepage.
> 
> The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag 
> GetModuleHandleEx does not look for a name, but uses an adress in the module to 
> get the HMODULE, so you cast it to char* or wchar_t* depending on which function 
> you call. Actually one could just cast the dll_base to HMODULE, at least in 
> win32 on x86 the HMODULE of a dll is always its base adress. But to make it 
> safer and future proof I went the way through GetModuleHandeEx.

In that case, you an call either GetModuleHandeExA or
GetModuleHandeExW, the difference is minor.
  

Am 07.01.2024 um 18:03 schrieb Eli Zaretskii:
>> Date: Sun, 7 Jan 2024 17:07:06 +0100
>> Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>> From: Björn Schäpers <gcc@hazardy.de>
>>
>>> That was about GetModuleHandle, not about GetModuleHandleEx.  For the
>>> latter, all Windows versions that support it also support "wide" APIs.
>>> So my suggestion is to use GetModuleHandleExW here.  However, you will
>>> need to make sure that notification_data->dll_base is declared as
>>> 'wchar_t *', not 'char *'.  If dll_base is declared as 'char *', then
>>> only GetModuleHandleExA will work, and you will lose the ability to
>>> support file names with non-ASCII characters outside of the current
>>> system codepage.
>>
>> The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag
>> GetModuleHandleEx does not look for a name, but uses an adress in the module to
>> get the HMODULE, so you cast it to char* or wchar_t* depending on which function
>> you call. Actually one could just cast the dll_base to HMODULE, at least in
>> win32 on x86 the HMODULE of a dll is always its base adress. But to make it
>> safer and future proof I went the way through GetModuleHandeEx.
> 
> In that case, you an call either GetModuleHandeExA or
> GetModuleHandeExW, the difference is minor.

Here an updated version without relying on TEXT or TCHAR, directly calling 
GetModuleHandleExW.

Kind regards,
Björn.
From a8e1e64ccb56158ec8a7e5de0d5228f3f6f7e5c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de>
Date: Sat, 6 Jan 2024 22:53:54 +0100
Subject: [PATCH 3/3] libbacktrace: Add loaded dlls after initialize

libbacktrace/Changelog:

	* pecoff.c [HAVE_WINDOWS_H]:
	  (dll_notification_data): Added
	  (dll_notification_context): Added
	  (dll_notification): Added
	  (backtrace_initialize): Use LdrRegisterDllNotification to load
	                          debug information of later loaded
	                          dlls.
---
 libbacktrace/pecoff.c | 104 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 1 deletion(-)

diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index 647baa39640..d973a26f05a 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -62,6 +62,34 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #undef Module32Next
 #endif
 #endif
+
+#if defined(_ARM_)
+#define NTAPI
+#else
+#define NTAPI __stdcall
+#endif
+
+/* This is a simplified (but binary compatible) version of what Microsoft
+   defines in their documentation. */
+struct dll_notifcation_data
+{
+  ULONG reserved;
+  /* The name as UNICODE_STRING struct. */
+  PVOID full_dll_name;
+  PVOID base_dll_name;
+  PVOID dll_base;
+  ULONG size_of_image;
+};
+
+#define LDR_DLL_NOTIFICATION_REASON_LOADED 1
+
+typedef LONG NTSTATUS;
+typedef VOID CALLBACK (*LDR_DLL_NOTIFICATION)(ULONG,
+					      struct dll_notifcation_data*,
+					      PVOID);
+typedef NTSTATUS NTAPI (*LDR_REGISTER_FUNCTION)(ULONG,
+						LDR_DLL_NOTIFICATION, PVOID,
+						PVOID*);
 #endif
 
 /* Coff file header.  */
@@ -912,7 +940,8 @@ coff_add (struct backtrace_state *state, int descriptor,
   return 0;
 }
 
-#if defined(HAVE_WINDOWS_H) && !defined(HAVE_TLHELP32_H)
+#ifdef HAVE_WINDOWS_H
+#ifndef HAVE_TLHELP32_H
 static void
 free_modules (struct backtrace_state *state,
 	      backtrace_error_callback error_callback, void *data,
@@ -958,6 +987,51 @@ get_all_modules (struct backtrace_state *state,
     }
 }
 #endif
+struct dll_notification_context
+{
+  struct backtrace_state *state;
+  backtrace_error_callback error_callback;
+  void *data;
+};
+
+VOID CALLBACK
+dll_notification (ULONG reason,
+struct dll_notifcation_data *notification_data,
+PVOID context)
+{
+  char module_name[MAX_PATH];
+  int descriptor;
+  struct dll_notification_context* dll_context =
+    (struct dll_notification_context*) context;
+  struct backtrace_state *state = dll_context->state;
+  void *data = dll_context->data;
+  backtrace_error_callback error_callback = dll_context->data;
+  fileline fileline;
+  int found_sym;
+  int found_dwarf;
+  HMODULE module_handle;
+
+  if (reason != LDR_DLL_NOTIFICATION_REASON_LOADED)
+    return;
+
+  if (!GetModuleHandleExW (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
+			   | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
+			   (wchar_t*) notification_data->dll_base,
+			   &module_handle))
+    return;
+
+  if (!GetModuleFileNameA ((HMODULE) module_handle, module_name, MAX_PATH - 1))
+    return;
+
+  descriptor = backtrace_open (module_name, error_callback, data, NULL);
+
+  if (descriptor < 0)
+    return;
+
+  coff_add (state, descriptor, error_callback, data, &fileline, &found_sym,
+	    &found_dwarf, (uintptr_t) module_handle);
+}
+#endif
 
 /* Initialize the backtrace data we need from an ELF executable.  At
    the ELF level, all we need to do is find the debug info
@@ -978,6 +1052,7 @@ backtrace_initialize (struct backtrace_state *state,
 #ifdef HAVE_WINDOWS_H
   fileline module_fileline_fn;
   int module_found_sym;
+  HMODULE nt_dll_handle;
 
 #ifdef HAVE_TLHELP32_H
   HANDLE snapshot;
@@ -1065,6 +1140,33 @@ backtrace_initialize (struct backtrace_state *state,
   if (modules)
     free_modules (state, error_callback, data, &modules, bytes_allocated_for_modules);
 #endif
+
+  nt_dll_handle = GetModuleHandleW (L"ntdll.dll");
+  if (nt_dll_handle)
+    {
+      LDR_REGISTER_FUNCTION register_func;
+      const char register_name[] = "LdrRegisterDllNotification";
+      register_func = (LDR_REGISTER_FUNCTION) GetProcAddress (nt_dll_handle,
+							      register_name);
+
+      if (register_func)
+	{
+	  PVOID cookie;
+	  struct dll_notification_context *context
+	    = backtrace_alloc (state,
+			       sizeof(struct dll_notification_context),
+			       error_callback, data);
+
+	  if (context)
+	    {
+	      context->state = state;
+	      context->data = data;
+	      context->error_callback = error_callback;
+
+	      register_func (0, &dll_notification, context, &cookie);
+	    }
+	}
+    }
 #endif
 
   if (!state->threaded)
  

> Date: Tue, 9 Jan 2024 21:02:44 +0100
> Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: Björn Schäpers <gcc@hazardy.de>
> 
> Am 07.01.2024 um 18:03 schrieb Eli Zaretskii:
> > In that case, you an call either GetModuleHandeExA or
> > GetModuleHandeExW, the difference is minor.
> 
> Here an updated version without relying on TEXT or TCHAR, directly calling 
> GetModuleHandleExW.

Thanks, this LGTM (but I couldn't test it, I just looked at the
sour ce code).
  

On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers <gcc@hazardy.de> wrote:
>
> Am 03.01.2024 um 00:12 schrieb Björn Schäpers:
> > Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor:
> >> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote:
> >>>
> >>> From: Björn Schäpers <bjoern@hazardy.de>
> >>>
> >>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except
> >>> that libraries loaded after the backtrace_initialize are not handled.
> >>> But as far as I can see that's the same for elf.
> >>
> >> Thanks, but I don't want a patch that loops using goto statements.
> >> Please rewrite to avoid that.  It may be simpler to call a function.
> >>
> >> Also starting with a module count of 1000 seems like a lot.  Do
> >> typical Windows programs load that many modules?
> >>
> >> Ian
> >>
> >>
> >
> > Rewritten using a function.
> >
> > If that is commited, could you attribute that commit to me (--author="Björn
> > Schäpers <bjoern@hazardy.de>")?
> >
> > Thanks and kind regards,
> > Björn.
>
> I noticed that under 64 bit libraries loaded with LoadLibrary were missing.
> EnumProcessModules stated the correct number of modules, but did not fill the
> the HMODULEs, but set them to 0. While trying to investigate I noticed if I do
> the very same thing from main (in C++) I even got fewer module HMODULEs.
>
> So I went a different way. This detects all libraries correctly, in 32 and 64
> bit. The question is, if it should be a patch on top of the previous, or should
> they be merged, or even only this solution and drop the EnumProcessModules variant?

Is there any reason to use both patches?  Seems simpler to just use
this one if it works.  Thanks.

Ian
  

Am 23.01.2024 um 23:37 schrieb Ian Lance Taylor:
> On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers <gcc@hazardy.de> wrote:
>>
>> Am 03.01.2024 um 00:12 schrieb Björn Schäpers:
>>> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor:
>>>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote:
>>>>>
>>>>> From: Björn Schäpers <bjoern@hazardy.de>
>>>>>
>>>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except
>>>>> that libraries loaded after the backtrace_initialize are not handled.
>>>>> But as far as I can see that's the same for elf.
>>>>
>>>> Thanks, but I don't want a patch that loops using goto statements.
>>>> Please rewrite to avoid that.  It may be simpler to call a function.
>>>>
>>>> Also starting with a module count of 1000 seems like a lot.  Do
>>>> typical Windows programs load that many modules?
>>>>
>>>> Ian
>>>>
>>>>
>>>
>>> Rewritten using a function.
>>>
>>> If that is commited, could you attribute that commit to me (--author="Björn
>>> Schäpers <bjoern@hazardy.de>")?
>>>
>>> Thanks and kind regards,
>>> Björn.
>>
>> I noticed that under 64 bit libraries loaded with LoadLibrary were missing.
>> EnumProcessModules stated the correct number of modules, but did not fill the
>> the HMODULEs, but set them to 0. While trying to investigate I noticed if I do
>> the very same thing from main (in C++) I even got fewer module HMODULEs.
>>
>> So I went a different way. This detects all libraries correctly, in 32 and 64
>> bit. The question is, if it should be a patch on top of the previous, or should
>> they be merged, or even only this solution and drop the EnumProcessModules variant?
> 
> Is there any reason to use both patches?  Seems simpler to just use
> this one if it works.  Thanks.
> 
> Ian

This one needs the tlhelp32 header and its functions, which are (accoridng to 
the microsoft documentation) are only available since Windows XP rsp. Windows 
Server 2003.

If that's no problem, and in my opinion it shouldn't be, then I can basically 
drop patch 4 and rebase this one.

Kind regards,
Björn.
  

On Thu, Jan 25, 2024 at 11:53 AM Björn Schäpers <gcc@hazardy.de> wrote:
>
> Am 23.01.2024 um 23:37 schrieb Ian Lance Taylor:
> > On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers <gcc@hazardy.de> wrote:
> >>
> >> Am 03.01.2024 um 00:12 schrieb Björn Schäpers:
> >>> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor:
> >>>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote:
> >>>>>
> >>>>> From: Björn Schäpers <bjoern@hazardy.de>
> >>>>>
> >>>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except
> >>>>> that libraries loaded after the backtrace_initialize are not handled.
> >>>>> But as far as I can see that's the same for elf.
> >>>>
> >>>> Thanks, but I don't want a patch that loops using goto statements.
> >>>> Please rewrite to avoid that.  It may be simpler to call a function.
> >>>>
> >>>> Also starting with a module count of 1000 seems like a lot.  Do
> >>>> typical Windows programs load that many modules?
> >>>>
> >>>> Ian
> >>>>
> >>>>
> >>>
> >>> Rewritten using a function.
> >>>
> >>> If that is commited, could you attribute that commit to me (--author="Björn
> >>> Schäpers <bjoern@hazardy.de>")?
> >>>
> >>> Thanks and kind regards,
> >>> Björn.
> >>
> >> I noticed that under 64 bit libraries loaded with LoadLibrary were missing.
> >> EnumProcessModules stated the correct number of modules, but did not fill the
> >> the HMODULEs, but set them to 0. While trying to investigate I noticed if I do
> >> the very same thing from main (in C++) I even got fewer module HMODULEs.
> >>
> >> So I went a different way. This detects all libraries correctly, in 32 and 64
> >> bit. The question is, if it should be a patch on top of the previous, or should
> >> they be merged, or even only this solution and drop the EnumProcessModules variant?
> >
> > Is there any reason to use both patches?  Seems simpler to just use
> > this one if it works.  Thanks.
> >
> > Ian
>
> This one needs the tlhelp32 header and its functions, which are (accoridng to
> the microsoft documentation) are only available since Windows XP rsp. Windows
> Server 2003.
>
> If that's no problem, and in my opinion it shouldn't be, then I can basically
> drop patch 4 and rebase this one.

I don't see that as a problem.  It seems like the patch will fall back
cleanly on ancient Windows and simply fail to pick up other loaded
DLLs.  I think that is fine.  I'll look for an updated patch.  Thanks.

Ian