[PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t

[PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t

[Björn Schäpers]

From: Björn Schäpers <bjoern@hazardy.de>

It's the right thing to do, since the PC shouldn't go out of the
uintptr_t domain, and in backtrace_pcinfo the pc is uintptr_t.
This is a preparation for a following patch.

Tested on x86_64-linux and i686-w64-mingw32.

-- >8 --

	* dwarf.c: changed variables holding pc values from uint64_t to
	uintptr_t.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libbacktrace/dwarf.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 45cc9e77e40..0707ccddd3e 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -274,8 +274,8 @@ struct function
 struct function_addrs
 {
   /* Range is LOW <= PC < HIGH.  */
-  uint64_t low;
-  uint64_t high;
+  uintptr_t low;
+  uintptr_t high;
   /* Function for this address range.  */
   struct function *function;
 };
@@ -356,8 +356,8 @@ struct unit
 struct unit_addrs
 {
   /* Range is LOW <= PC < HIGH.  */
-  uint64_t low;
-  uint64_t high;
+  uintptr_t low;
+  uintptr_t high;
   /* Compilation unit for this address range.  */
   struct unit *u;
 };
@@ -1094,7 +1094,7 @@ resolve_addr_index (const struct dwarf_sections *dwarf_sections,
 		    uint64_t addr_base, int addrsize, int is_bigendian,
 		    uint64_t addr_index,
 		    backtrace_error_callback error_callback, void *data,
-		    uint64_t *address)
+		    uintptr_t *address)
 {
   uint64_t offset;
   struct dwarf_buf addr_buf;
@@ -1194,7 +1194,7 @@ function_addrs_search (const void *vkey, const void *ventry)
 
 static int
 add_unit_addr (struct backtrace_state *state, void *rdata,
-	       uint64_t lowpc, uint64_t highpc,
+	       uintptr_t lowpc, uintptr_t highpc,
 	       backtrace_error_callback error_callback, void *data,
 	       void *pvec)
 {
@@ -1530,10 +1530,10 @@ lookup_abbrev (struct abbrevs *abbrevs, uint64_t code,
    lowpc/highpc is set or ranges is set.  */
 
 struct pcrange {
-  uint64_t lowpc;		/* The low PC value.  */
+  uintptr_t lowpc;		/* The low PC value.  */
   int have_lowpc;		/* Whether a low PC value was found.  */
   int lowpc_is_addr_index;	/* Whether lowpc is in .debug_addr.  */
-  uint64_t highpc;		/* The high PC value.  */
+  uintptr_t highpc;		/* The high PC value.  */
   int have_highpc;		/* Whether a high PC value was found.  */
   int highpc_is_relative;	/* Whether highpc is relative to lowpc.  */
   int highpc_is_addr_index;	/* Whether highpc is in .debug_addr.  */
@@ -1613,16 +1613,16 @@ add_low_high_range (struct backtrace_state *state,
 		    uintptr_t base_address, int is_bigendian,
 		    struct unit *u, const struct pcrange *pcrange,
 		    int (*add_range) (struct backtrace_state *state,
-				      void *rdata, uint64_t lowpc,
-				      uint64_t highpc,
+				      void *rdata, uintptr_t lowpc,
+				      uintptr_t highpc,
 				      backtrace_error_callback error_callback,
 				      void *data, void *vec),
 		    void *rdata,
 		    backtrace_error_callback error_callback, void *data,
 		    void *vec)
 {
-  uint64_t lowpc;
-  uint64_t highpc;
+  uintptr_t lowpc;
+  uintptr_t highpc;
 
   lowpc = pcrange->lowpc;
   if (pcrange->lowpc_is_addr_index)
@@ -1663,7 +1663,7 @@ add_ranges_from_ranges (
     struct unit *u, uint64_t base,
     const struct pcrange *pcrange,
     int (*add_range) (struct backtrace_state *state, void *rdata,
-		      uint64_t lowpc, uint64_t highpc,
+		      uintptr_t lowpc, uintptr_t highpc,
 		      backtrace_error_callback error_callback, void *data,
 		      void *vec),
     void *rdata,
@@ -1727,10 +1727,10 @@ add_ranges_from_rnglists (
     struct backtrace_state *state,
     const struct dwarf_sections *dwarf_sections,
     uintptr_t base_address, int is_bigendian,
-    struct unit *u, uint64_t base,
+    struct unit *u, uintptr_t base,
     const struct pcrange *pcrange,
     int (*add_range) (struct backtrace_state *state, void *rdata,
-		      uint64_t lowpc, uint64_t highpc,
+		      uintptr_t lowpc, uintptr_t highpc,
 		      backtrace_error_callback error_callback, void *data,
 		      void *vec),
     void *rdata,
@@ -1796,8 +1796,8 @@ add_ranges_from_rnglists (
 	case DW_RLE_startx_endx:
 	  {
 	    uint64_t index;
-	    uint64_t low;
-	    uint64_t high;
+	    uintptr_t low;
+	    uintptr_t high;
 
 	    index = read_uleb128 (&rnglists_buf);
 	    if (!resolve_addr_index (dwarf_sections, u->addr_base,
@@ -1819,8 +1819,8 @@ add_ranges_from_rnglists (
 	case DW_RLE_startx_length:
 	  {
 	    uint64_t index;
-	    uint64_t low;
-	    uint64_t length;
+	    uintptr_t low;
+	    uintptr_t length;
 
 	    index = read_uleb128 (&rnglists_buf);
 	    if (!resolve_addr_index (dwarf_sections, u->addr_base,
@@ -1905,7 +1905,7 @@ add_ranges (struct backtrace_state *state,
 	    uintptr_t base_address, int is_bigendian,
 	    struct unit *u, uint64_t base, const struct pcrange *pcrange,
 	    int (*add_range) (struct backtrace_state *state, void *rdata, 
-			      uint64_t lowpc, uint64_t highpc,
+			      uintptr_t lowpc, uintptr_t highpc,
 			      backtrace_error_callback error_callback,
 			      void *data, void *vec),
 	    void *rdata,
@@ -3183,7 +3183,7 @@ read_referenced_name (struct dwarf_data *ddata, struct unit *u,
 
 static int
 add_function_range (struct backtrace_state *state, void *rdata,
-		    uint64_t lowpc, uint64_t highpc,
+		    uintptr_t lowpc, uintptr_t highpc,
 		    backtrace_error_callback error_callback, void *data,
 		    void *pvec)
 {
@@ -3223,7 +3223,7 @@ add_function_range (struct backtrace_state *state, void *rdata,
 
 static int
 read_function_entry (struct backtrace_state *state, struct dwarf_data *ddata,
-		     struct unit *u, uint64_t base, struct dwarf_buf *unit_buf,
+		     struct unit *u, uintptr_t base, struct dwarf_buf *unit_buf,
 		     const struct line_header *lhdr,
 		     backtrace_error_callback error_callback, void *data,
 		     struct function_vector *vec_function,
-- 
2.38.1

[PATCH 2/4] libbacktrace: detect executable path on windows

[Björn Schäpers]

From: Björn Schäpers <bjoern@hazardy.de>

This is actually needed so that libstdc++'s <stacktrace> implementation
to be able to work on windows.

Tested on x86_64-linux and i686-w64-mingw32.

-- >8 --

	* configure.ac: Add a check for windows.h.
	* configure, config.h.in: Regenerate.
	* fileline.c: Add windows_get_executable_path.
	* fileline.c (fileline_initialiez): Add a pass using
	windows_get_executable_path.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libbacktrace/config.h.in  |  3 +++
 libbacktrace/configure    | 13 ++++++++++++
 libbacktrace/configure.ac |  2 ++
 libbacktrace/fileline.c   | 43 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/libbacktrace/config.h.in b/libbacktrace/config.h.in
index a21e2eaf525..355e820741b 100644
--- a/libbacktrace/config.h.in
+++ b/libbacktrace/config.h.in
@@ -100,6 +100,9 @@
 /* Define to 1 if you have the <unistd.h> header file. */
 #undef HAVE_UNISTD_H
 
+/* Define to 1 if you have the <windows.h> header file. */
+#undef HAVE_WINDOWS_H
+
 /* Define if -lz is available. */
 #undef HAVE_ZLIB
 
diff --git a/libbacktrace/configure b/libbacktrace/configure
index a5bd133f4e4..ef677423733 100755
--- a/libbacktrace/configure
+++ b/libbacktrace/configure
@@ -13403,6 +13403,19 @@ $as_echo "#define HAVE_LOADQUERY 1" >>confdefs.h
 
 fi
 
+for ac_header in windows.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "windows.h" "ac_cv_header_windows_h" "$ac_includes_default"
+if test "x$ac_cv_header_windows_h" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_WINDOWS_H 1
+_ACEOF
+
+fi
+
+done
+
+
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
    case "${host}" in
diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac
index 1daaa2f62d2..b5feb29bcdc 100644
--- a/libbacktrace/configure.ac
+++ b/libbacktrace/configure.ac
@@ -377,6 +377,8 @@ if test "$have_loadquery" = "yes"; then
   AC_DEFINE(HAVE_LOADQUERY, 1, [Define if AIX loadquery is available.])
 fi
 
+AC_CHECK_HEADERS(windows.h)
+
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
    case "${host}" in
diff --git a/libbacktrace/fileline.c b/libbacktrace/fileline.c
index a40cd498114..73c2c8e8bc9 100644
--- a/libbacktrace/fileline.c
+++ b/libbacktrace/fileline.c
@@ -47,6 +47,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #include <mach-o/dyld.h>
 #endif
 
+#ifdef HAVE_WINDOWS_H
+#ifndef WIN32_MEAN_AND_LEAN
+#define WIN32_MEAN_AND_LEAN
+#endif
+
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+
+#include <windows.h>
+#endif
+
 #include "backtrace.h"
 #include "internal.h"
 
@@ -155,6 +167,28 @@ macho_get_executable_path (struct backtrace_state *state,
 
 #endif /* !defined (HAVE_MACH_O_DYLD_H) */
 
+#ifdef HAVE_WINDOWS_H
+
+static char *
+windows_get_executable_path (char *buf, backtrace_error_callback error_callback,
+			     void *data)
+{
+  if (GetModuleFileNameA (NULL, buf, MAX_PATH - 1) == 0)
+    {
+      error_callback (data,
+		      "could not get the filename of the current executable",
+		      (int) GetLastError ());
+      return NULL;
+    }
+  return buf;
+}
+
+#else /* !defined (HAVE_WINDOWS_H) */
+
+#define windows_get_executable_path(buf, error_callback, data) NULL
+
+#endif /* !defined (HAVE_WINDOWS_H) */
+
 /* Initialize the fileline information from the executable.  Returns 1
    on success, 0 on failure.  */
 
@@ -168,7 +202,11 @@ fileline_initialize (struct backtrace_state *state,
   int called_error_callback;
   int descriptor;
   const char *filename;
+#ifdef HAVE_WINDOWS_H
+  char buf[MAX_PATH];
+#else
   char buf[64];
+#endif
 
   if (!state->threaded)
     failed = state->fileline_initialization_failed;
@@ -192,7 +230,7 @@ fileline_initialize (struct backtrace_state *state,
 
   descriptor = -1;
   called_error_callback = 0;
-  for (pass = 0; pass < 8; ++pass)
+  for (pass = 0; pass < 9; ++pass)
     {
       int does_not_exist;
 
@@ -224,6 +262,9 @@ fileline_initialize (struct backtrace_state *state,
 	case 7:
 	  filename = macho_get_executable_path (state, error_callback, data);
 	  break;
+	case 8:
+	  filename = windows_get_executable_path (buf, error_callback, data);
+	  break;
 	default:
 	  abort ();
 	}
-- 
2.38.1

[PATCH 3/4] libbacktrace: work with aslr on windows

[Björn Schäpers]

From: Björn Schäpers <bjoern@hazardy.de>

Any underflow which might happen, will be countered by an overflow in
dwarf.c.

Tested on x86_64-linux and i686-w64-mingw32.

-- >8 --

Fixes https://github.com/ianlancetaylor/libbacktrace/issues/89 and
https://github.com/ianlancetaylor/libbacktrace/issues/82.

	* pecoff.c (coff_add): Set the base_address of the module, to
	find the debug information on moved applications.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libbacktrace/pecoff.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index 87b3c0cc647..296f1357b5f 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -39,6 +39,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #include "backtrace.h"
 #include "internal.h"
 
+#ifdef HAVE_WINDOWS_H
+#ifndef WIN32_MEAN_AND_LEAN
+#define WIN32_MEAN_AND_LEAN
+#endif
+
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+
+#include <windows.h>
+#endif
+
 /* Coff file header.  */
 
 typedef struct {
@@ -610,6 +622,8 @@ coff_add (struct backtrace_state *state, int descriptor,
   int debug_view_valid;
   int is_64;
   uintptr_t image_base;
+  uintptr_t base_address = 0;
+  uintptr_t module_handle;
   struct dwarf_sections dwarf_sections;
 
   *found_sym = 0;
@@ -856,7 +870,12 @@ coff_add (struct backtrace_state *state, int descriptor,
 				  + (sections[i].offset - min_offset));
     }
 
-  if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections,
+#ifdef HAVE_WINDOWS_H
+    module_handle = (uintptr_t) GetModuleHandleW (NULL);
+    base_address = module_handle - image_base;
+#endif
+
+  if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
 			    0, /* FIXME: is_bigendian */
 			    NULL, /* altlink */
 			    error_callback, data, fileline_fn,
-- 
2.38.1

[PATCH 4/4] libbacktrace: get debug information for loaded dlls

[Björn Schäpers]

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.

Tested on x86_64-linux and i686-w64-mingw32.

-- >8 --

	* pecoff.c (coff_add): New argument for the module handle of the
	file, to get the base address.
	* pecoff.c (backtrace_initialize): Iterate over loaded libraries
	and call coff_add.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libbacktrace/pecoff.c | 76 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 4 deletions(-)

diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index 296f1357b5f..40395109e51 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -49,6 +49,7 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #endif
 
 #include <windows.h>
+#include <psapi.h>
 #endif
 
 /* Coff file header.  */
@@ -592,7 +593,8 @@ coff_syminfo (struct backtrace_state *state, uintptr_t addr,
 static int
 coff_add (struct backtrace_state *state, int descriptor,
 	  backtrace_error_callback error_callback, void *data,
-	  fileline *fileline_fn, int *found_sym, int *found_dwarf)
+	  fileline *fileline_fn, int *found_sym, int *found_dwarf,
+	  uintptr_t module_handle ATTRIBUTE_UNUSED)
 {
   struct backtrace_view fhdr_view;
   off_t fhdr_off;
@@ -623,7 +625,6 @@ coff_add (struct backtrace_state *state, int descriptor,
   int is_64;
   uintptr_t image_base;
   uintptr_t base_address = 0;
-  uintptr_t module_handle;
   struct dwarf_sections dwarf_sections;
 
   *found_sym = 0;
@@ -871,7 +872,6 @@ coff_add (struct backtrace_state *state, int descriptor,
     }
 
 #ifdef HAVE_WINDOWS_H
-    module_handle = (uintptr_t) GetModuleHandleW (NULL);
     base_address = module_handle - image_base;
 #endif
 
@@ -914,12 +914,80 @@ backtrace_initialize (struct backtrace_state *state,
   int found_sym;
   int found_dwarf;
   fileline coff_fileline_fn;
+  uintptr_t module_handle = 0;
+
+#ifdef HAVE_WINDOWS_H
+  DWORD i;
+  DWORD module_count;
+  DWORD bytes_needed_for_modules;
+  HMODULE *modules;
+  char module_name[MAX_PATH];
+  int module_found_sym;
+  fileline module_fileline_fn;
+
+  module_handle = (uintptr_t) GetModuleHandleW (NULL);
+#endif
 
   ret = coff_add (state, descriptor, error_callback, data,
-		  &coff_fileline_fn, &found_sym, &found_dwarf);
+		  &coff_fileline_fn, &found_sym, &found_dwarf, module_handle);
   if (!ret)
     return 0;
 
+#ifdef HAVE_WINDOWS_H
+  module_count = 1000;
+ alloc_modules:
+  modules = backtrace_alloc (state, module_count * sizeof(HMODULE),
+			     error_callback, data);
+  if (modules == NULL)
+    goto skip_modules;
+  if (!EnumProcessModules (GetCurrentProcess (), modules, module_count,
+			   &bytes_needed_for_modules))
+    {
+      error_callback(data, "Could not enumerate process modules",
+		     (int) GetLastError ());
+      goto free_modules;
+    }
+  if (bytes_needed_for_modules > module_count * sizeof(HMODULE))
+    {
+      backtrace_free (state, modules, module_count * sizeof(HMODULE),
+		      error_callback, data);
+      // Add an extra of 2, if some module is loaded in another thread.
+      module_count = bytes_needed_for_modules / sizeof(HMODULE) + 2;
+      modules = NULL;
+      goto alloc_modules;
+    }
+
+  for (i = 0; i < bytes_needed_for_modules / sizeof(HMODULE); ++i)
+    {
+      if (GetModuleFileNameA (modules[i], module_name, MAX_PATH - 1))
+	{
+	  if (strcmp (filename, module_name) == 0)
+	    continue;
+
+	  module_handle = (uintptr_t) GetModuleHandleA (module_name);
+	  if (module_handle == 0)
+	    continue;
+
+	  descriptor = backtrace_open (module_name, error_callback, data, NULL);
+	  if (descriptor < 0)
+	    continue;
+
+	  coff_add (state, descriptor, error_callback, data,
+		    &module_fileline_fn, &module_found_sym, &found_dwarf,
+		    module_handle);
+	  if (module_found_sym)
+	    found_sym = 1;
+	}
+    }
+
+ free_modules:
+  if (modules)
+    backtrace_free(state, modules, module_count * sizeof(HMODULE),
+		   error_callback, data);
+  modules = NULL;
+ skip_modules:
+#endif
+
   if (!state->threaded)
     {
       if (found_sym)
-- 
2.38.1

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Eli Zaretskii]

> From: Björn Schäpers <gcc@hazardy.de>
> Date: Fri, 20 Jan 2023 11:54:08 +0100
> 
> @@ -856,7 +870,12 @@ coff_add (struct backtrace_state *state, int descriptor,
>  				  + (sections[i].offset - min_offset));
>      }
>  
> -  if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections,
> +#ifdef HAVE_WINDOWS_H
> +    module_handle = (uintptr_t) GetModuleHandleW (NULL);
> +    base_address = module_handle - image_base;
> +#endif
> +
> +  if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
>  			    0, /* FIXME: is_bigendian */
>  			    NULL, /* altlink */
>  			    error_callback, data, fileline_fn,

Why do you force using the "wide" APIs here?  Won't GetModuleHandle do
the job, whether it resolves to GetModuleHandleA or GetModuleHandleW?

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Gabriel Ravier]

On 1/20/23 14:39, Eli Zaretskii via Gcc wrote:
>> From: Björn Schäpers <gcc@hazardy.de>
>> Date: Fri, 20 Jan 2023 11:54:08 +0100
>>
>> @@ -856,7 +870,12 @@ coff_add (struct backtrace_state *state, int descriptor,
>>   				  + (sections[i].offset - min_offset));
>>       }
>>   
>> -  if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections,
>> +#ifdef HAVE_WINDOWS_H
>> +    module_handle = (uintptr_t) GetModuleHandleW (NULL);
>> +    base_address = module_handle - image_base;
>> +#endif
>> +
>> +  if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
>>   			    0, /* FIXME: is_bigendian */
>>   			    NULL, /* altlink */
>>   			    error_callback, data, fileline_fn,
> Why do you force using the "wide" APIs here?  Won't GetModuleHandle do
> the job, whether it resolves to GetModuleHandleA or GetModuleHandleW?

I would expect the reason to be either that:

- using wide APIs with Windows is generally considered to be a best 
practice, even when not strictly needed (and in this case I can't see 
any problem with doing so, unless maybe we want to code to work with 
Windows 95 or something like that...)

- using the narrow API somehow has an actual drawback, for example maybe 
it might not work if the name of the exe file the NULL will tell it to 
get a handle to contains wide characters

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Eli Zaretskii]

> Date: Fri, 20 Jan 2023 17:46:59 +0100
> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: Gabriel Ravier <gabravier@gmail.com>
> 
> On 1/20/23 14:39, Eli Zaretskii via Gcc wrote:
> >> From: Björn Schäpers <gcc@hazardy.de>
> >> Date: Fri, 20 Jan 2023 11:54:08 +0100
> >>
> >> @@ -856,7 +870,12 @@ coff_add (struct backtrace_state *state, int descriptor,
> >>   				  + (sections[i].offset - min_offset));
> >>       }
> >>   
> >> -  if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections,
> >> +#ifdef HAVE_WINDOWS_H
> >> +    module_handle = (uintptr_t) GetModuleHandleW (NULL);
> >> +    base_address = module_handle - image_base;
> >> +#endif
> >> +
> >> +  if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
> >>   			    0, /* FIXME: is_bigendian */
> >>   			    NULL, /* altlink */
> >>   			    error_callback, data, fileline_fn,
> > Why do you force using the "wide" APIs here?  Won't GetModuleHandle do
> > the job, whether it resolves to GetModuleHandleA or GetModuleHandleW?
> 
> I would expect the reason to be either that:
> 
> - using wide APIs with Windows is generally considered to be a best 
> practice, even when not strictly needed (and in this case I can't see 
> any problem with doing so, unless maybe we want to code to work with 
> Windows 95 or something like that...)

There's no reason to forcibly break GDB on platforms where wide APIs
are not available.

> - using the narrow API somehow has an actual drawback, for example maybe 
> it might not work if the name of the exe file the NULL will tell it to 
> get a handle to contains wide characters

Native Windows port of GDB doesn't support Unicode file names anyway,
which is why you used the *A APIs elsewhere in the patch, and
rightfully so.  So there's no reason to use "wide" APIs in this one
place, and every reason not to.

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Gabriel Ravier]

On 1/20/23 20:19, Eli Zaretskii wrote:
>> Date: Fri, 20 Jan 2023 17:46:59 +0100
>> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>> From: Gabriel Ravier <gabravier@gmail.com>
>>
>> On 1/20/23 14:39, Eli Zaretskii via Gcc wrote:
>>>> From: Björn Schäpers <gcc@hazardy.de>
>>>> Date: Fri, 20 Jan 2023 11:54:08 +0100
>>>>
>>>> @@ -856,7 +870,12 @@ coff_add (struct backtrace_state *state, int descriptor,
>>>>    				  + (sections[i].offset - min_offset));
>>>>        }
>>>>    
>>>> -  if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections,
>>>> +#ifdef HAVE_WINDOWS_H
>>>> +    module_handle = (uintptr_t) GetModuleHandleW (NULL);
>>>> +    base_address = module_handle - image_base;
>>>> +#endif
>>>> +
>>>> +  if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
>>>>    			    0, /* FIXME: is_bigendian */
>>>>    			    NULL, /* altlink */
>>>>    			    error_callback, data, fileline_fn,
>>> Why do you force using the "wide" APIs here?  Won't GetModuleHandle do
>>> the job, whether it resolves to GetModuleHandleA or GetModuleHandleW?
>> I would expect the reason to be either that:
>>
>> - using wide APIs with Windows is generally considered to be a best
>> practice, even when not strictly needed (and in this case I can't see
>> any problem with doing so, unless maybe we want to code to work with
>> Windows 95 or something like that...)
> There's no reason to forcibly break GDB on platforms where wide APIs
> are not available.
Are there even any platforms that have GetModuleHandleA but not 
GetModuleHandleW ? MSDN states that Windows XP and Windows Server 2003 
are the first versions to support both of the APIs, so if this is 
supposed to work on Windows 98, for instance, whether we're using 
GetModuleHandleA or GetModuleHandleW won't matter.
>
>> - using the narrow API somehow has an actual drawback, for example maybe
>> it might not work if the name of the exe file the NULL will tell it to
>> get a handle to contains wide characters
> Native Windows port of GDB doesn't support Unicode file names anyway,
> which is why you used the *A APIs elsewhere in the patch, and
> rightfully so.  So there's no reason to use "wide" APIs in this one
> place, and every reason not to.

(just as a clarification, I did not write this patch)

Re: [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]

On Fri, Jan 20, 2023 at 2:54 AM Björn Schäpers <gcc@hazardy.de> wrote:
>
> From: Björn Schäpers <bjoern@hazardy.de>
>
> It's the right thing to do, since the PC shouldn't go out of the
> uintptr_t domain, and in backtrace_pcinfo the pc is uintptr_t.
> This is a preparation for a following patch.
>
> Tested on x86_64-linux and i686-w64-mingw32.

Thanks.  Committed like so, with some additional tweaks.

For future reference, when pinging a patch, please reply to the
original patch to maintain the thread.  Or at least mention the
original patch.  It was still on my list, I just hadn't gotten to it.
Thanks.

Ian

Change variables holding PC values from uint64_t to uintptr_t.
Patch by Björn Schäpers.
* dwarf.c (struct function_addrs): Change low and high fields to
uintptr_t.
(struct unit_addrs): Likewise.
(resolve_addr_index): Change address parameter to uintptr_t*.
(add_unit_addr): Change lowpc and highpc parameters to uintptr_t.
(add_function_range): Likewise.
(struct pcrange): Change lowpc and highpc fields to uintptr_t.
(add_low_high_range): Change add_range lowpc and highpc parameters
to uintptr_t.
(add_ranges_from_ranges): Likewise.
(add_ranges_from_rnglists): Likewise.
(add_low_high_range): Chnage lowpc and highpc variables to
uintpr_t.
(add_ranges_from_rnglists): Change some local variables to
uintptr_t.
(add_ranges_from_ranges): Change base parameter to uintptr_t.
(add_ranges_from_rnglists): Likewise.
(read_function_entry): Likewise.
(resolve_addr_index): Add explicit casts to uintptr_t.
(update_pcrange): Likewise.
(add_ranges_from_ranges): Likewise.
(add_ranges_from_rnglists): Likewise.
(read_function_entry): Likewise.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 9646 bytes --]

0c193cabe1d8f209359f3ccb8e74cf87b38fc4bc
diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 2d41f3b0397..8ff1fb3ce3d 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -136,7 +136,7 @@ enum attr_val_encoding
   /* An address.  */
   ATTR_VAL_ADDRESS,
   /* An index into the .debug_addr section, whose value is relative to
-   * the DW_AT_addr_base attribute of the compilation unit.  */
+     the DW_AT_addr_base attribute of the compilation unit.  */
   ATTR_VAL_ADDRESS_INDEX,
   /* A unsigned integer.  */
   ATTR_VAL_UINT,
@@ -274,8 +274,8 @@ struct function
 struct function_addrs
 {
   /* Range is LOW <= PC < HIGH.  */
-  uint64_t low;
-  uint64_t high;
+  uintptr_t low;
+  uintptr_t high;
   /* Function for this address range.  */
   struct function *function;
 };
@@ -356,8 +356,8 @@ struct unit
 struct unit_addrs
 {
   /* Range is LOW <= PC < HIGH.  */
-  uint64_t low;
-  uint64_t high;
+  uintptr_t low;
+  uintptr_t high;
   /* Compilation unit for this address range.  */
   struct unit *u;
 };
@@ -1094,7 +1094,7 @@ resolve_addr_index (const struct dwarf_sections *dwarf_sections,
 		    uint64_t addr_base, int addrsize, int is_bigendian,
 		    uint64_t addr_index,
 		    backtrace_error_callback error_callback, void *data,
-		    uint64_t *address)
+		    uintptr_t *address)
 {
   uint64_t offset;
   struct dwarf_buf addr_buf;
@@ -1115,7 +1115,7 @@ resolve_addr_index (const struct dwarf_sections *dwarf_sections,
   addr_buf.data = data;
   addr_buf.reported_underflow = 0;
 
-  *address = read_address (&addr_buf, addrsize);
+  *address = (uintptr_t) read_address (&addr_buf, addrsize);
   return 1;
 }
 
@@ -1194,7 +1194,7 @@ function_addrs_search (const void *vkey, const void *ventry)
 
 static int
 add_unit_addr (struct backtrace_state *state, void *rdata,
-	       uint64_t lowpc, uint64_t highpc,
+	       uintptr_t lowpc, uintptr_t highpc,
 	       backtrace_error_callback error_callback, void *data,
 	       void *pvec)
 {
@@ -1530,10 +1530,10 @@ lookup_abbrev (struct abbrevs *abbrevs, uint64_t code,
    lowpc/highpc is set or ranges is set.  */
 
 struct pcrange {
-  uint64_t lowpc;		/* The low PC value.  */
+  uintptr_t lowpc;             /* The low PC value.  */
   int have_lowpc;		/* Whether a low PC value was found.  */
   int lowpc_is_addr_index;	/* Whether lowpc is in .debug_addr.  */
-  uint64_t highpc;		/* The high PC value.  */
+  uintptr_t highpc;            /* The high PC value.  */
   int have_highpc;		/* Whether a high PC value was found.  */
   int highpc_is_relative;	/* Whether highpc is relative to lowpc.  */
   int highpc_is_addr_index;	/* Whether highpc is in .debug_addr.  */
@@ -1553,12 +1553,12 @@ update_pcrange (const struct attr* attr, const struct attr_val* val,
     case DW_AT_low_pc:
       if (val->encoding == ATTR_VAL_ADDRESS)
 	{
-	  pcrange->lowpc = val->u.uint;
+	  pcrange->lowpc = (uintptr_t) val->u.uint;
 	  pcrange->have_lowpc = 1;
 	}
       else if (val->encoding == ATTR_VAL_ADDRESS_INDEX)
 	{
-	  pcrange->lowpc = val->u.uint;
+	  pcrange->lowpc = (uintptr_t) val->u.uint;
 	  pcrange->have_lowpc = 1;
 	  pcrange->lowpc_is_addr_index = 1;
 	}
@@ -1567,18 +1567,18 @@ update_pcrange (const struct attr* attr, const struct attr_val* val,
     case DW_AT_high_pc:
       if (val->encoding == ATTR_VAL_ADDRESS)
 	{
-	  pcrange->highpc = val->u.uint;
+	  pcrange->highpc = (uintptr_t) val->u.uint;
 	  pcrange->have_highpc = 1;
 	}
       else if (val->encoding == ATTR_VAL_UINT)
 	{
-	  pcrange->highpc = val->u.uint;
+	  pcrange->highpc = (uintptr_t) val->u.uint;
 	  pcrange->have_highpc = 1;
 	  pcrange->highpc_is_relative = 1;
 	}
       else if (val->encoding == ATTR_VAL_ADDRESS_INDEX)
 	{
-	  pcrange->highpc = val->u.uint;
+	  pcrange->highpc = (uintptr_t) val->u.uint;
 	  pcrange->have_highpc = 1;
 	  pcrange->highpc_is_addr_index = 1;
 	}
@@ -1613,16 +1613,16 @@ add_low_high_range (struct backtrace_state *state,
 		    uintptr_t base_address, int is_bigendian,
 		    struct unit *u, const struct pcrange *pcrange,
 		    int (*add_range) (struct backtrace_state *state,
-				      void *rdata, uint64_t lowpc,
-				      uint64_t highpc,
+				      void *rdata, uintptr_t lowpc,
+				      uintptr_t highpc,
 				      backtrace_error_callback error_callback,
 				      void *data, void *vec),
 		    void *rdata,
 		    backtrace_error_callback error_callback, void *data,
 		    void *vec)
 {
-  uint64_t lowpc;
-  uint64_t highpc;
+  uintptr_t lowpc;
+  uintptr_t highpc;
 
   lowpc = pcrange->lowpc;
   if (pcrange->lowpc_is_addr_index)
@@ -1660,10 +1660,10 @@ add_ranges_from_ranges (
     struct backtrace_state *state,
     const struct dwarf_sections *dwarf_sections,
     uintptr_t base_address, int is_bigendian,
-    struct unit *u, uint64_t base,
+    struct unit *u, uintptr_t base,
     const struct pcrange *pcrange,
     int (*add_range) (struct backtrace_state *state, void *rdata,
-		      uint64_t lowpc, uint64_t highpc,
+		      uintptr_t lowpc, uintptr_t highpc,
 		      backtrace_error_callback error_callback, void *data,
 		      void *vec),
     void *rdata,
@@ -1702,12 +1702,12 @@ add_ranges_from_ranges (
 	break;
 
       if (is_highest_address (low, u->addrsize))
-	base = high;
+	base = (uintptr_t) high;
       else
 	{
 	  if (!add_range (state, rdata, 
-			  low + base + base_address,
-			  high + base + base_address,
+			  (uintptr_t) low + base + base_address,
+			  (uintptr_t) high + base + base_address,
 			  error_callback, data, vec))
 	    return 0;
 	}
@@ -1727,10 +1727,10 @@ add_ranges_from_rnglists (
     struct backtrace_state *state,
     const struct dwarf_sections *dwarf_sections,
     uintptr_t base_address, int is_bigendian,
-    struct unit *u, uint64_t base,
+    struct unit *u, uintptr_t base,
     const struct pcrange *pcrange,
     int (*add_range) (struct backtrace_state *state, void *rdata,
-		      uint64_t lowpc, uint64_t highpc,
+		      uintptr_t lowpc, uintptr_t highpc,
 		      backtrace_error_callback error_callback, void *data,
 		      void *vec),
     void *rdata,
@@ -1796,8 +1796,8 @@ add_ranges_from_rnglists (
 	case DW_RLE_startx_endx:
 	  {
 	    uint64_t index;
-	    uint64_t low;
-	    uint64_t high;
+	    uintptr_t low;
+	    uintptr_t high;
 
 	    index = read_uleb128 (&rnglists_buf);
 	    if (!resolve_addr_index (dwarf_sections, u->addr_base,
@@ -1819,8 +1819,8 @@ add_ranges_from_rnglists (
 	case DW_RLE_startx_length:
 	  {
 	    uint64_t index;
-	    uint64_t low;
-	    uint64_t length;
+	    uintptr_t low;
+	    uintptr_t length;
 
 	    index = read_uleb128 (&rnglists_buf);
 	    if (!resolve_addr_index (dwarf_sections, u->addr_base,
@@ -1850,16 +1850,16 @@ add_ranges_from_rnglists (
 	  break;
 
 	case DW_RLE_base_address:
-	  base = read_address (&rnglists_buf, u->addrsize);
+	  base = (uintptr_t) read_address (&rnglists_buf, u->addrsize);
 	  break;
 
 	case DW_RLE_start_end:
 	  {
-	    uint64_t low;
-	    uint64_t high;
+	    uintptr_t low;
+	    uintptr_t high;
 
-	    low = read_address (&rnglists_buf, u->addrsize);
-	    high = read_address (&rnglists_buf, u->addrsize);
+	    low = (uintptr_t) read_address (&rnglists_buf, u->addrsize);
+	    high = (uintptr_t) read_address (&rnglists_buf, u->addrsize);
 	    if (!add_range (state, rdata, low + base_address,
 			    high + base_address, error_callback, data,
 			    vec))
@@ -1869,11 +1869,11 @@ add_ranges_from_rnglists (
 
 	case DW_RLE_start_length:
 	  {
-	    uint64_t low;
-	    uint64_t length;
+	    uintptr_t low;
+	    uintptr_t length;
 
-	    low = read_address (&rnglists_buf, u->addrsize);
-	    length = read_uleb128 (&rnglists_buf);
+	    low = (uintptr_t) read_address (&rnglists_buf, u->addrsize);
+	    length = (uintptr_t) read_uleb128 (&rnglists_buf);
 	    low += base_address;
 	    if (!add_range (state, rdata, low, low + length,
 			    error_callback, data, vec))
@@ -1903,9 +1903,9 @@ static int
 add_ranges (struct backtrace_state *state,
 	    const struct dwarf_sections *dwarf_sections,
 	    uintptr_t base_address, int is_bigendian,
-	    struct unit *u, uint64_t base, const struct pcrange *pcrange,
+	    struct unit *u, uintptr_t base, const struct pcrange *pcrange,
 	    int (*add_range) (struct backtrace_state *state, void *rdata, 
-			      uint64_t lowpc, uint64_t highpc,
+			      uintptr_t lowpc, uintptr_t highpc,
 			      backtrace_error_callback error_callback,
 			      void *data, void *vec),
 	    void *rdata,
@@ -3183,7 +3183,7 @@ read_referenced_name (struct dwarf_data *ddata, struct unit *u,
 
 static int
 add_function_range (struct backtrace_state *state, void *rdata,
-		    uint64_t lowpc, uint64_t highpc,
+		    uintptr_t lowpc, uintptr_t highpc,
 		    backtrace_error_callback error_callback, void *data,
 		    void *pvec)
 {
@@ -3223,7 +3223,7 @@ add_function_range (struct backtrace_state *state, void *rdata,
 
 static int
 read_function_entry (struct backtrace_state *state, struct dwarf_data *ddata,
-		     struct unit *u, uint64_t base, struct dwarf_buf *unit_buf,
+		     struct unit *u, uintptr_t base, struct dwarf_buf *unit_buf,
 		     const struct line_header *lhdr,
 		     backtrace_error_callback error_callback, void *data,
 		     struct function_vector *vec_function,
@@ -3287,7 +3287,7 @@ read_function_entry (struct backtrace_state *state, struct dwarf_data *ddata,
 	      && abbrev->attrs[i].name == DW_AT_low_pc)
 	    {
 	      if (val.encoding == ATTR_VAL_ADDRESS)
-		base = val.u.uint;
+		base = (uintptr_t) val.u.uint;
 	      else if (val.encoding == ATTR_VAL_ADDRESS_INDEX)
 		{
 		  if (!resolve_addr_index (&ddata->dwarf_sections,

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Eli Zaretskii]

> Date: Fri, 20 Jan 2023 21:39:56 +0100
> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: Gabriel Ravier <gabravier@gmail.com>
> 
> >> - using wide APIs with Windows is generally considered to be a best
> >> practice, even when not strictly needed (and in this case I can't see
> >> any problem with doing so, unless maybe we want to code to work with
> >> Windows 95 or something like that...)
> > There's no reason to forcibly break GDB on platforms where wide APIs
> > are not available.
> Are there even any platforms that have GetModuleHandleA but not 
> GetModuleHandleW ? MSDN states that Windows XP and Windows Server 2003 
> are the first versions to support both of the APIs, so if this is 
> supposed to work on Windows 98, for instance, whether we're using 
> GetModuleHandleA or GetModuleHandleW won't matter.

I'm not sure I follow the logic.  A program that calls
GetModuleHandleW will refuse to start on Windows that doesn't have
that API.  So any version before XP is automatically excluded the
moment you use code which calls that API directly (i.e. not through a
function pointer or somesuch).

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[LIU Hao]

[-- Attachment #1.1: Type: text/plain, Size: 1006 bytes --]

在 2023-01-21 12:05, Eli Zaretskii via Gcc 写道:
> I'm not sure I follow the logic.  A program that calls
> GetModuleHandleW will refuse to start on Windows that doesn't have
> that API.  So any version before XP is automatically excluded the
> moment you use code which calls that API directly (i.e. not through a
> function pointer or somesuch).

Are _you_ still willing to maintain backward compatibility with Windows 9x? Even mingw-w64 has been 
defaulting to Windows Server 2003 since 2007. Why would anyone build a modern compiler for such old 
operating systems?

With any Windows that is modern enough, wide APIs should always be preferred to ANSI ones, 
especially when the argument is constant. Almost all ANSI APIs (the only exception I know of is 
`OutputDebugStringA` which does the inverse) translate their ANSI string arguments to wide strings 
and delegate to wide ones, so by calling wide APIs explicitly, such overhead can be avoided.


-- 
Best regards,
LIU Hao


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Eli Zaretskii]

> Date: Sat, 21 Jan 2023 17:18:14 +0800
> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: LIU Hao <lh_mouse@126.com>
> 
> 在 2023-01-21 12:05, Eli Zaretskii via Gcc 写道:
> > I'm not sure I follow the logic.  A program that calls
> > GetModuleHandleW will refuse to start on Windows that doesn't have
> > that API.  So any version before XP is automatically excluded the
> > moment you use code which calls that API directly (i.e. not through a
> > function pointer or somesuch).
> 
> Are _you_ still willing to maintain backward compatibility with Windows 9x? Even mingw-w64 has been 
> defaulting to Windows Server 2003 since 2007. Why would anyone build a modern compiler for such old 
> operating systems?

I'm only saying that we should not deliberately break those old
platforms unless we have a good reason.  And I see no such good reason
in this case: GetModuleHandleA will do the job exactly like
GetModuleHandleW will.

> With any Windows that is modern enough, wide APIs should always be preferred to ANSI ones, 
> especially when the argument is constant. Almost all ANSI APIs (the only exception I know of is 
> `OutputDebugStringA` which does the inverse) translate their ANSI string arguments to wide strings 
> and delegate to wide ones, so by calling wide APIs explicitly, such overhead can be avoided.

The overhead is only relevant in code that is run in performance
critical places.  I don't think this is such a place.

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Gabriel Ravier]

On 1/21/23 05:05, Eli Zaretskii wrote:
>> Date: Fri, 20 Jan 2023 21:39:56 +0100
>> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>> From: Gabriel Ravier <gabravier@gmail.com>
>>
>>>> - using wide APIs with Windows is generally considered to be a best
>>>> practice, even when not strictly needed (and in this case I can't see
>>>> any problem with doing so, unless maybe we want to code to work with
>>>> Windows 95 or something like that...)
>>> There's no reason to forcibly break GDB on platforms where wide APIs
>>> are not available.
>> Are there even any platforms that have GetModuleHandleA but not
>> GetModuleHandleW ? MSDN states that Windows XP and Windows Server 2003
>> are the first versions to support both of the APIs, so if this is
>> supposed to work on Windows 98, for instance, whether we're using
>> GetModuleHandleA or GetModuleHandleW won't matter.
> I'm not sure I follow the logic.  A program that calls
> GetModuleHandleW will refuse to start on Windows that doesn't have
> that API.  So any version before XP is automatically excluded the
> moment you use code which calls that API directly (i.e. not through a
> function pointer or somesuch).
A program that calls GetModuleHandleA will also refuse to start on 
Windows if it doesn't have that API. The set of Windows versions that do 
not have GetModuleHandleA is, according to MSDN, the same as the set of 
Windows versions that do not have GetModuleHandleW.

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Eli Zaretskii]

> Date: Sat, 21 Jan 2023 11:47:42 +0100
> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: Gabriel Ravier <gabravier@gmail.com>
> 
> 
> On 1/21/23 05:05, Eli Zaretskii wrote:
> >> Date: Fri, 20 Jan 2023 21:39:56 +0100
> >> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> >> From: Gabriel Ravier <gabravier@gmail.com>
> >>
> >>>> - using wide APIs with Windows is generally considered to be a best
> >>>> practice, even when not strictly needed (and in this case I can't see
> >>>> any problem with doing so, unless maybe we want to code to work with
> >>>> Windows 95 or something like that...)
> >>> There's no reason to forcibly break GDB on platforms where wide APIs
> >>> are not available.
> >> Are there even any platforms that have GetModuleHandleA but not
> >> GetModuleHandleW ? MSDN states that Windows XP and Windows Server 2003
> >> are the first versions to support both of the APIs, so if this is
> >> supposed to work on Windows 98, for instance, whether we're using
> >> GetModuleHandleA or GetModuleHandleW won't matter.
> > I'm not sure I follow the logic.  A program that calls
> > GetModuleHandleW will refuse to start on Windows that doesn't have
> > that API.  So any version before XP is automatically excluded the
> > moment you use code which calls that API directly (i.e. not through a
> > function pointer or somesuch).
> A program that calls GetModuleHandleA will also refuse to start on 
> Windows if it doesn't have that API. The set of Windows versions that do 
> not have GetModuleHandleA is, according to MSDN, the same as the set of 
> Windows versions that do not have GetModuleHandleW.

MSDN lies (because it wants to pretend that older versions don't
exist).  Try this much more useful site:

  http://winapi.freetechsecrets.com/win32/WIN32GetModuleHandle.htm

Re: [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t

[Björn Schäpers]

Am 20.01.2023 um 23:25 schrieb Ian Lance Taylor:
> On Fri, Jan 20, 2023 at 2:54 AM Björn Schäpers <gcc@hazardy.de> wrote:
>>
>> From: Björn Schäpers <bjoern@hazardy.de>
>>
>> It's the right thing to do, since the PC shouldn't go out of the
>> uintptr_t domain, and in backtrace_pcinfo the pc is uintptr_t.
>> This is a preparation for a following patch.
>>
>> Tested on x86_64-linux and i686-w64-mingw32.
> 
> Thanks.  Committed like so, with some additional tweaks.
> 
> For future reference, when pinging a patch, please reply to the
> original patch to maintain the thread.  Or at least mention the
> original patch.  It was still on my list, I just hadn't gotten to it.
> Thanks.
> 
> Ian
> 

Thanks for the commit, and sorry for the repost. Because of a fault of my own I 
had no copy in my mailbox and thus couldn't reply to the first batch.

Regards,
Björn.

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Ian Lance Taylor]

On Fri, Jan 20, 2023 at 2:56 AM Björn Schäpers <gcc@hazardy.de> wrote:
>
> From: Björn Schäpers <bjoern@hazardy.de>
>
> This is actually needed so that libstdc++'s <stacktrace> implementation
> to be able to work on windows.
>
> Tested on x86_64-linux and i686-w64-mingw32.
>
> -- >8 --
>
>         * configure.ac: Add a check for windows.h.
>         * configure, config.h.in: Regenerate.
>         * fileline.c: Add windows_get_executable_path.
>         * fileline.c (fileline_initialiez): Add a pass using
>         windows_get_executable_path.
>
> +#ifdef HAVE_WINDOWS_H
> +
> +static char *
> +windows_get_executable_path (char *buf, backtrace_error_callback error_callback,
> +                            void *data)
> +{
> +  if (GetModuleFileNameA (NULL, buf, MAX_PATH - 1) == 0)
> +    {
> +      error_callback (data,
> +                     "could not get the filename of the current executable",
> +                     (int) GetLastError ());
> +      return NULL;
> +    }
> +  return buf;
> +}

Thanks, but this seems incomplete.  The docs for GetModuleFileNameA
say that if the pathname is too long to fit into the buffer it returns
the size of the buffer and sets the error to
ERROR_INSUFFICIENT_BUFFER.  It seems to me that in that case we should
allocate a larger buffer and try again.  And, in general, it will be
simpler if we always allocate the buffer, as macho_get_executable_path
does.  Unfortunately it appears that Windows does not provide a way to
ask for the required length.  On Windows it seems that MAX_PATH is not
a true limit, as an extended length path may be up to 32767 bytes.

So probably something like (untested)

static char *
windows_get_executable_path (struct backtrace_state *state,
     backtrace_error_callback error_callback,
     void *data)
{
  uint32_t len;
  char *buf;

  len = MAX_PATH;
  while (1)
    {
      uint32_t got;

      name = (char *) backtrace_alloc (state, len, error_callback, data);
      if (name == NULL)
return NULL;
      got = GetModuleFileNameA (NULL, name, len);
      if (got < len - 1) /* -1 because NULB is not counted */
return name;
      backtrace_free (state, name, len, error_callback, data);
      if (GetLastError () != ERROR_INSUFFICIENT_BUFFER)
return NULL;
      len *= 2;
    }
}

Ian

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Eli Zaretskii]

> Date: Mon, 23 Jan 2023 15:00:56 -0800
> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: Ian Lance Taylor via Gcc <gcc@gcc.gnu.org>
> 
> > +#ifdef HAVE_WINDOWS_H
> > +
> > +static char *
> > +windows_get_executable_path (char *buf, backtrace_error_callback error_callback,
> > +                            void *data)
> > +{
> > +  if (GetModuleFileNameA (NULL, buf, MAX_PATH - 1) == 0)
> > +    {
> > +      error_callback (data,
> > +                     "could not get the filename of the current executable",
> > +                     (int) GetLastError ());
> > +      return NULL;
> > +    }
> > +  return buf;
> > +}
> 
> Thanks, but this seems incomplete.  The docs for GetModuleFileNameA
> say that if the pathname is too long to fit into the buffer it returns
> the size of the buffer and sets the error to
> ERROR_INSUFFICIENT_BUFFER.  It seems to me that in that case we should
> allocate a larger buffer and try again.

This is correct in general, but not in this particular case.

> On Windows it seems that MAX_PATH is not
> a true limit, as an extended length path may be up to 32767 bytes.

The limit of 32767 characters (not bytes, AFAIK) is only applicable
when using the Unicode (a.k.a. "wide") versions of the Windows Win32
APIs, see

  https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation

Since the above code uses GetModuleFileNameA, which is an "ANSI"
single-byte API, it is still subject to the MAX_PATH limitation, and
MAX_PATH is defined as 260 on Windows headers.

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Ian Lance Taylor]

On Tue, Jan 24, 2023 at 5:11 AM Eli Zaretskii via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> > Date: Mon, 23 Jan 2023 15:00:56 -0800
> > Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> > From: Ian Lance Taylor via Gcc <gcc@gcc.gnu.org>
> >
> > > +#ifdef HAVE_WINDOWS_H
> > > +
> > > +static char *
> > > +windows_get_executable_path (char *buf, backtrace_error_callback error_callback,
> > > +                            void *data)
> > > +{
> > > +  if (GetModuleFileNameA (NULL, buf, MAX_PATH - 1) == 0)
> > > +    {
> > > +      error_callback (data,
> > > +                     "could not get the filename of the current executable",
> > > +                     (int) GetLastError ());
> > > +      return NULL;
> > > +    }
> > > +  return buf;
> > > +}
> >
> > Thanks, but this seems incomplete.  The docs for GetModuleFileNameA
> > say that if the pathname is too long to fit into the buffer it returns
> > the size of the buffer and sets the error to
> > ERROR_INSUFFICIENT_BUFFER.  It seems to me that in that case we should
> > allocate a larger buffer and try again.
>
> This is correct in general, but not in this particular case.
>
> > On Windows it seems that MAX_PATH is not
> > a true limit, as an extended length path may be up to 32767 bytes.
>
> The limit of 32767 characters (not bytes, AFAIK) is only applicable
> when using the Unicode (a.k.a. "wide") versions of the Windows Win32
> APIs, see
>
>   https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
>
> Since the above code uses GetModuleFileNameA, which is an "ANSI"
> single-byte API, it is still subject to the MAX_PATH limitation, and
> MAX_PATH is defined as 260 on Windows headers.

Thanks.  Should this code be using GetModuleFileNameW?  Or would that
mean that the later call to open will fail?

260 bytes does not seem like very much for a path name these days.

Ian

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Eli Zaretskii]

> From: Ian Lance Taylor <iant@golang.org>
> Date: Tue, 24 Jan 2023 06:35:21 -0800
> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> 
> > > On Windows it seems that MAX_PATH is not
> > > a true limit, as an extended length path may be up to 32767 bytes.
> >
> > The limit of 32767 characters (not bytes, AFAIK) is only applicable
> > when using the Unicode (a.k.a. "wide") versions of the Windows Win32
> > APIs, see
> >
> >   https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
> >
> > Since the above code uses GetModuleFileNameA, which is an "ANSI"
> > single-byte API, it is still subject to the MAX_PATH limitation, and
> > MAX_PATH is defined as 260 on Windows headers.
> 
> Thanks.  Should this code be using GetModuleFileNameW?  Or would that
> mean that the later call to open will fail?

We'd need to use _wopen or somesuch, and the file name will have to be
a wchar_t array, not a char array, yes.  So this is not very practical
when file names need to be passed between functions, unless they are
converted to UTF-8 (and back again before using them in Windows APIs).

And note that even then, the 260-byte limit could be lifted only if
the user has a new enough Windows version _and_ has opted in to the
long-name feature by turning it on in the Registry.  Otherwise, file
names used in "wide" APIs can only break the 260-byte limit if they
use the special format "\\?\D:\foo\bar", which means file names
specified by user outside of the program or file names that come from
other programs will need to be reformatted to this special format.

> 260 bytes does not seem like very much for a path name these days.

That's true.  But complications with using longer file names are still
a PITA on Windows, even though they are a step closer to practically
possible.

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Ian Lance Taylor]

On Tue, Jan 24, 2023 at 8:53 AM Eli Zaretskii via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> > From: Ian Lance Taylor <iant@golang.org>
> > Date: Tue, 24 Jan 2023 06:35:21 -0800
> > Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> >
> > > > On Windows it seems that MAX_PATH is not
> > > > a true limit, as an extended length path may be up to 32767 bytes.
> > >
> > > The limit of 32767 characters (not bytes, AFAIK) is only applicable
> > > when using the Unicode (a.k.a. "wide") versions of the Windows Win32
> > > APIs, see
> > >
> > >   https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
> > >
> > > Since the above code uses GetModuleFileNameA, which is an "ANSI"
> > > single-byte API, it is still subject to the MAX_PATH limitation, and
> > > MAX_PATH is defined as 260 on Windows headers.
> >
> > Thanks.  Should this code be using GetModuleFileNameW?  Or would that
> > mean that the later call to open will fail?
>
> We'd need to use _wopen or somesuch, and the file name will have to be
> a wchar_t array, not a char array, yes.  So this is not very practical
> when file names need to be passed between functions, unless they are
> converted to UTF-8 (and back again before using them in Windows APIs).
>
> And note that even then, the 260-byte limit could be lifted only if
> the user has a new enough Windows version _and_ has opted in to the
> long-name feature by turning it on in the Registry.  Otherwise, file
> names used in "wide" APIs can only break the 260-byte limit if they
> use the special format "\\?\D:\foo\bar", which means file names
> specified by user outside of the program or file names that come from
> other programs will need to be reformatted to this special format.
>
> > 260 bytes does not seem like very much for a path name these days.
>
> That's true.  But complications with using longer file names are still
> a PITA on Windows, even though they are a step closer to practically
> possible.


OK, thanks.

I'd rather that the patch look like the appended.  Can someone with a
Windows system test to see what that builds and passes the tests?
Thanks.

Ian

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Eli Zaretskii]

> From: Ian Lance Taylor <iant@golang.org>
> Date: Tue, 24 Jan 2023 09:58:10 -0800
> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> 
> I'd rather that the patch look like the appended.  Can someone with a
> Windows system test to see what that builds and passes the tests?

ENOPATCH

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

On Tue, Jan 24, 2023 at 10:12 AM Eli Zaretskii via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> > From: Ian Lance Taylor <iant@golang.org>
> > Date: Tue, 24 Jan 2023 09:58:10 -0800
> > Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> >
> > I'd rather that the patch look like the appended.  Can someone with a
> > Windows system test to see what that builds and passes the tests?
>
> ENOPATCH

Gah.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3480 bytes --]

diff --git a/libbacktrace/config.h.in b/libbacktrace/config.h.in
index 94621c2e385..29d1ad3911a 100644
--- a/libbacktrace/config.h.in
+++ b/libbacktrace/config.h.in
@@ -100,6 +100,9 @@
 /* Define to 1 if you have the <unistd.h> header file. */
 #undef HAVE_UNISTD_H
 
+/* Define to 1 if you have the <windows.h> header file. */
+#undef HAVE_WINDOWS_H
+
 /* Define if -lz is available. */
 #undef HAVE_ZLIB
 
diff --git a/libbacktrace/configure b/libbacktrace/configure
index 6af2c04c81a..0a27cfb7799 100755
--- a/libbacktrace/configure
+++ b/libbacktrace/configure
@@ -13409,6 +13409,19 @@ $as_echo "#define HAVE_LOADQUERY 1" >>confdefs.h
 
 fi
 
+for ac_header in windows.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "windows.h" "ac_cv_header_windows_h" "$ac_includes_default"
+if test "x$ac_cv_header_windows_h" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_WINDOWS_H 1
+_ACEOF
+
+fi
+
+done
+
+
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
    case "${host}" in
diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac
index 39e6bf41e35..e3e10abd7b5 100644
--- a/libbacktrace/configure.ac
+++ b/libbacktrace/configure.ac
@@ -377,6 +377,8 @@ if test "$have_loadquery" = "yes"; then
   AC_DEFINE(HAVE_LOADQUERY, 1, [Define if AIX loadquery is available.])
 fi
 
+AC_CHECK_HEADERS(windows.h)
+
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
    case "${host}" in
diff --git a/libbacktrace/fileline.c b/libbacktrace/fileline.c
index 674bf33cdcf..e110b54ee24 100644
--- a/libbacktrace/fileline.c
+++ b/libbacktrace/fileline.c
@@ -47,6 +47,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #include <mach-o/dyld.h>
 #endif
 
+#ifdef HAVE_WINDOWS_H
+#ifndef WIN32_MEAN_AND_LEAN
+#define WIN32_MEAN_AND_LEAN
+#endif
+
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+
+#include <windows.h>
+#endif
+
 #include "backtrace.h"
 #include "internal.h"
 
@@ -155,6 +167,27 @@ macho_get_executable_path (struct backtrace_state *state,
 
 #endif /* !defined (HAVE_MACH_O_DYLD_H) */
 
+#ifdef HAVE_WINDOWS_H
+
+static char *
+windows_get_executable_path (char *buf, backtrace_error_callback error_callback,
+			     void *data)
+{
+  size_t got;
+
+  got = GetModuleFileNameA (NULL, buf, MAX_PATH - 1);
+  if (got == 0
+      || (got == MAX_PATH - 1 && GetLastError () == ERROR_INSUFFICIENT_BUFFER))
+    return NULL;
+  return buf;
+}
+
+#else /* !defined (HAVE_WINDOWS_H) */
+
+#define windows_get_executable_path(buf, error_callback, data) NULL
+
+#endif /* !defined (HAVE_WINDOWS_H) */
+
 /* Initialize the fileline information from the executable.  Returns 1
    on success, 0 on failure.  */
 
@@ -168,7 +201,11 @@ fileline_initialize (struct backtrace_state *state,
   int called_error_callback;
   int descriptor;
   const char *filename;
+#ifdef HAVE_WINDOWS_H
+  char buf[MAX_PATH];
+#else
   char buf[64];
+#endif
 
   if (!state->threaded)
     failed = state->fileline_initialization_failed;
@@ -192,7 +229,7 @@ fileline_initialize (struct backtrace_state *state,
 
   descriptor = -1;
   called_error_callback = 0;
-  for (pass = 0; pass < 8; ++pass)
+  for (pass = 0; pass < 9; ++pass)
     {
       int does_not_exist;
 
@@ -224,6 +261,9 @@ fileline_initialize (struct backtrace_state *state,
 	case 7:
 	  filename = macho_get_executable_path (state, error_callback, data);
 	  break;
+	case 8:
+	  filename = windows_get_executable_path (buf, error_callback, data);
+	  break;
 	default:
 	  abort ();
 	}

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Björn Schäpers]

Am 24.01.2023 um 17:52 schrieb Eli Zaretskii:
>> From: Ian Lance Taylor <iant@golang.org>
>> Date: Tue, 24 Jan 2023 06:35:21 -0800
>> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>>
>>>> On Windows it seems that MAX_PATH is not
>>>> a true limit, as an extended length path may be up to 32767 bytes.
>>>
>>> The limit of 32767 characters (not bytes, AFAIK) is only applicable
>>> when using the Unicode (a.k.a. "wide") versions of the Windows Win32
>>> APIs, see
>>>
>>>    https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
>>>
>>> Since the above code uses GetModuleFileNameA, which is an "ANSI"
>>> single-byte API, it is still subject to the MAX_PATH limitation, and
>>> MAX_PATH is defined as 260 on Windows headers.
>>
>> Thanks.  Should this code be using GetModuleFileNameW?  Or would that
>> mean that the later call to open will fail?
> 
> We'd need to use _wopen or somesuch, and the file name will have to be
> a wchar_t array, not a char array, yes.  So this is not very practical
> when file names need to be passed between functions, unless they are
> converted to UTF-8 (and back again before using them in Windows APIs).
> 
> And note that even then, the 260-byte limit could be lifted only if
> the user has a new enough Windows version _and_ has opted in to the
> long-name feature by turning it on in the Registry.  Otherwise, file
> names used in "wide" APIs can only break the 260-byte limit if they
> use the special format "\\?\D:\foo\bar", which means file names
> specified by user outside of the program or file names that come from
> other programs will need to be reformatted to this special format.
> 
>> 260 bytes does not seem like very much for a path name these days.
> 
> That's true.  But complications with using longer file names are still
> a PITA on Windows, even though they are a step closer to practically
> possible.

That was basically also my reasoning for choosing the A variant instead of W.

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Björn Schäpers]

Am 24.01.2023 um 19:32 schrieb Ian Lance Taylor:
> On Tue, Jan 24, 2023 at 10:12 AM Eli Zaretskii via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>>> From: Ian Lance Taylor <iant@golang.org>
>>> Date: Tue, 24 Jan 2023 09:58:10 -0800
>>> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>>>
>>> I'd rather that the patch look like the appended.  Can someone with a
>>> Windows system test to see what that builds and passes the tests?
>>
>> ENOPATCH
> 
> Gah.
> 
> Ian
> 
That seems to be my original patch, right? That one I have tested (and 
am actually using) on x86 and x64 windows.

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Ian Lance Taylor]

On Sun, Feb 5, 2023 at 1:21 AM Björn Schäpers <gcc@hazardy.de> wrote:
>
> Am 24.01.2023 um 19:32 schrieb Ian Lance Taylor:
> > On Tue, Jan 24, 2023 at 10:12 AM Eli Zaretskii via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>> From: Ian Lance Taylor <iant@golang.org>
> >>> Date: Tue, 24 Jan 2023 09:58:10 -0800
> >>> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> >>>
> >>> I'd rather that the patch look like the appended.  Can someone with a
> >>> Windows system test to see what that builds and passes the tests?
> >>
> >> ENOPATCH
> >
> > Gah.
> >
> > Ian
> >
> That seems to be my original patch, right? That one I have tested (and
> am actually using) on x86 and x64 windows.

It's very similar but I changed the windows_get_executable_path function.

Ian

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Björn Schäpers]

[-- Attachment #1: Type: text/plain, Size: 982 bytes --]

Hi,

this is what I'm using with GCC 12 and 13 on my windows machines, rebased onto 
the current HEAD.

Kind regards,
Björn.

Am 06.02.2023 um 01:22 schrieb Ian Lance Taylor:
> On Sun, Feb 5, 2023 at 1:21 AM Björn Schäpers <gcc@hazardy.de> wrote:
>>
>> Am 24.01.2023 um 19:32 schrieb Ian Lance Taylor:
>>> On Tue, Jan 24, 2023 at 10:12 AM Eli Zaretskii via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>
>>>>> From: Ian Lance Taylor <iant@golang.org>
>>>>> Date: Tue, 24 Jan 2023 09:58:10 -0800
>>>>> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>>>>>
>>>>> I'd rather that the patch look like the appended.  Can someone with a
>>>>> Windows system test to see what that builds and passes the tests?
>>>>
>>>> ENOPATCH
>>>
>>> Gah.
>>>
>>> Ian
>>>
>> That seems to be my original patch, right? That one I have tested (and
>> am actually using) on x86 and x64 windows.
> 
> It's very similar but I changed the windows_get_executable_path function.
> 
> Ian

[-- Attachment #2: 0001-libbacktrace-detect-executable-path-on-windows.patch --]
[-- Type: text/plain, Size: 4635 bytes --]

From e0ee58b71f726606205aa1f0168a724859162c21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de>
Date: Sun, 30 Apr 2023 23:48:18 +0200
Subject: [PATCH 1/3] libbacktrace: detect executable path on windows
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Tested on x86_64-linux with GCC 12, i686-w64-mingw32 and
x86_64-w64-mingw32 with GCC 12 & 13. This patch is rebased onto the
current HEAD.

-- >8 --

	* configure.ac: Add a check for windows.h.
	* configure, config.h.in: Regenerate.
	* fileline.c: Add windows_get_executable_path.
	* fileline.c (fileline_initialize): Add a pass using
	windows_get_executable_path.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libbacktrace/config.h.in  |  3 +++
 libbacktrace/configure    | 13 +++++++++++
 libbacktrace/configure.ac |  2 ++
 libbacktrace/fileline.c   | 49 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/libbacktrace/config.h.in b/libbacktrace/config.h.in
index a4f5bddddf6..ee2616335c7 100644
--- a/libbacktrace/config.h.in
+++ b/libbacktrace/config.h.in
@@ -104,6 +104,9 @@
 /* Define to 1 if you have the <unistd.h> header file. */
 #undef HAVE_UNISTD_H
 
+/* Define to 1 if you have the <windows.h> header file. */
+#undef HAVE_WINDOWS_H
+
 /* Define if -lz is available. */
 #undef HAVE_ZLIB
 
diff --git a/libbacktrace/configure b/libbacktrace/configure
index 0ccc060901d..7ade966b54d 100755
--- a/libbacktrace/configure
+++ b/libbacktrace/configure
@@ -13509,6 +13509,19 @@ $as_echo "#define HAVE_LOADQUERY 1" >>confdefs.h
 
 fi
 
+for ac_header in windows.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "windows.h" "ac_cv_header_windows_h" "$ac_includes_default"
+if test "x$ac_cv_header_windows_h" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_WINDOWS_H 1
+_ACEOF
+
+fi
+
+done
+
+
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
    case "${host}" in
diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac
index 71cd50f8cdf..00acb42eb6d 100644
--- a/libbacktrace/configure.ac
+++ b/libbacktrace/configure.ac
@@ -379,6 +379,8 @@ if test "$have_loadquery" = "yes"; then
   AC_DEFINE(HAVE_LOADQUERY, 1, [Define if AIX loadquery is available.])
 fi
 
+AC_CHECK_HEADERS(windows.h)
+
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
    case "${host}" in
diff --git a/libbacktrace/fileline.c b/libbacktrace/fileline.c
index 0e560b44e7a..28d752e2625 100644
--- a/libbacktrace/fileline.c
+++ b/libbacktrace/fileline.c
@@ -47,6 +47,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #include <mach-o/dyld.h>
 #endif
 
+#ifdef HAVE_WINDOWS_H
+#ifndef WIN32_MEAN_AND_LEAN
+#define WIN32_MEAN_AND_LEAN
+#endif
+
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+
+#include <windows.h>
+#endif
+
 #include "backtrace.h"
 #include "internal.h"
 
@@ -165,6 +177,34 @@ macho_get_executable_path (struct backtrace_state *state,
 
 #endif /* !HAVE_DECL__PGMPTR */
 
+#ifdef HAVE_WINDOWS_H
+
+static char *
+windows_get_executable_path (char *buf, backtrace_error_callback error_callback,
+			     void *data)
+{
+  size_t got;
+  int error;
+
+  got = GetModuleFileNameA (NULL, buf, MAX_PATH - 1);
+  error = GetLastError ();
+  if (got == 0
+      || (got == MAX_PATH - 1 && error == ERROR_INSUFFICIENT_BUFFER))
+    {
+      error_callback (data,
+		      "could not get the filename of the current executable",
+		      error);
+      return NULL;
+    }
+  return buf;
+}
+
+#else /* !defined (HAVE_WINDOWS_H) */
+
+#define windows_get_executable_path(buf, error_callback, data) NULL
+
+#endif /* !defined (HAVE_WINDOWS_H) */
+
 /* Initialize the fileline information from the executable.  Returns 1
    on success, 0 on failure.  */
 
@@ -178,7 +218,11 @@ fileline_initialize (struct backtrace_state *state,
   int called_error_callback;
   int descriptor;
   const char *filename;
+#ifdef HAVE_WINDOWS_H
+  char buf[MAX_PATH];
+#else
   char buf[64];
+#endif
 
   if (!state->threaded)
     failed = state->fileline_initialization_failed;
@@ -202,7 +246,7 @@ fileline_initialize (struct backtrace_state *state,
 
   descriptor = -1;
   called_error_callback = 0;
-  for (pass = 0; pass < 9; ++pass)
+  for (pass = 0; pass < 10; ++pass)
     {
       int does_not_exist;
 
@@ -239,6 +283,9 @@ fileline_initialize (struct backtrace_state *state,
 	case 8:
 	  filename = macho_get_executable_path (state, error_callback, data);
 	  break;
+	case 9:
+	  filename = windows_get_executable_path (buf, error_callback, data);
+	  break;
 	default:
 	  abort ();
 	}
-- 
2.42.1

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Björn Schäpers]

[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]

An updated version, using neither A or W, but just the macro.

Am 21.01.2023 um 12:42 schrieb Eli Zaretskii:
>> Date: Sat, 21 Jan 2023 11:47:42 +0100
>> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>> From: Gabriel Ravier <gabravier@gmail.com>
>>
>>
>> On 1/21/23 05:05, Eli Zaretskii wrote:
>>>> Date: Fri, 20 Jan 2023 21:39:56 +0100
>>>> Cc: gcc@hazardy.de, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>>>> From: Gabriel Ravier <gabravier@gmail.com>
>>>>
>>>>>> - using wide APIs with Windows is generally considered to be a best
>>>>>> practice, even when not strictly needed (and in this case I can't see
>>>>>> any problem with doing so, unless maybe we want to code to work with
>>>>>> Windows 95 or something like that...)
>>>>> There's no reason to forcibly break GDB on platforms where wide APIs
>>>>> are not available.
>>>> Are there even any platforms that have GetModuleHandleA but not
>>>> GetModuleHandleW ? MSDN states that Windows XP and Windows Server 2003
>>>> are the first versions to support both of the APIs, so if this is
>>>> supposed to work on Windows 98, for instance, whether we're using
>>>> GetModuleHandleA or GetModuleHandleW won't matter.
>>> I'm not sure I follow the logic.  A program that calls
>>> GetModuleHandleW will refuse to start on Windows that doesn't have
>>> that API.  So any version before XP is automatically excluded the
>>> moment you use code which calls that API directly (i.e. not through a
>>> function pointer or somesuch).
>> A program that calls GetModuleHandleA will also refuse to start on
>> Windows if it doesn't have that API. The set of Windows versions that do
>> not have GetModuleHandleA is, according to MSDN, the same as the set of
>> Windows versions that do not have GetModuleHandleW.
> 
> MSDN lies (because it wants to pretend that older versions don't
> exist).  Try this much more useful site:
> 
>    http://winapi.freetechsecrets.com/win32/WIN32GetModuleHandle.htm

[-- Attachment #2: 0002-libbacktrace-work-with-aslr-on-windows.patch --]
[-- Type: text/plain, Size: 2121 bytes --]

From 52cbe06b1c165172191f66ff7e55a49adecf661d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de>
Date: Sun, 30 Apr 2023 23:52:37 +0200
Subject: [PATCH 2/3] libbacktrace: work with aslr on windows
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Any underflow which might happen, will be countered by an overflow in
dwarf.c.

Tested on x86_64-linux and i686-w64-mingw32.

-- >8 --

Fixes https://github.com/ianlancetaylor/libbacktrace/issues/89 and
https://github.com/ianlancetaylor/libbacktrace/issues/82.

	* pecoff.c (coff_add): Set the base_address of the module, to
	find the debug information on moved applications.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libbacktrace/pecoff.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index 56af4828e27..9cc13b47947 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -39,6 +39,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #include "backtrace.h"
 #include "internal.h"
 
+#ifdef HAVE_WINDOWS_H
+#ifndef WIN32_MEAN_AND_LEAN
+#define WIN32_MEAN_AND_LEAN
+#endif
+
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+
+#include <windows.h>
+#endif
+
 /* Coff file header.  */
 
 typedef struct {
@@ -610,6 +622,8 @@ coff_add (struct backtrace_state *state, int descriptor,
   int debug_view_valid;
   int is_64;
   uintptr_t image_base;
+  uintptr_t base_address = 0;
+  uintptr_t module_handle;
   struct dwarf_sections dwarf_sections;
 
   *found_sym = 0;
@@ -856,7 +870,12 @@ coff_add (struct backtrace_state *state, int descriptor,
 				  + (sections[i].offset - min_offset));
     }
 
-  if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections,
+#ifdef HAVE_WINDOWS_H
+    module_handle = (uintptr_t) GetModuleHandle (NULL);
+    base_address = module_handle - image_base;
+#endif
+
+  if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
 			    0, /* FIXME: is_bigendian */
 			    NULL, /* altlink */
 			    error_callback, data, fileline_fn,
-- 
2.42.1

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Eli Zaretskii]

> Date: Mon, 20 Nov 2023 20:57:38 +0100
> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: Björn Schäpers <gcc@hazardy.de>
> 
> +#ifndef NOMINMAX
> +#define NOMINMAX
> +#endif

Why is this part needed?

Otherwise, LGTM, thanks.  (But I'm don't have the approval rights, so
please wait for Ian to chime in.)

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Björn Schäpers]

I'll guess it is not needed here, but otherwise <windows.h> defines the macros 
max and min, they then conflict e.g. with C++'s std::max/std::min. So I consider 
it best practice to always define it, before including <windows.h>.

Am 20.11.2023 um 21:07 schrieb Eli Zaretskii:
>> Date: Mon, 20 Nov 2023 20:57:38 +0100
>> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
>> From: Björn Schäpers <gcc@hazardy.de>
>>
>> +#ifndef NOMINMAX
>> +#define NOMINMAX
>> +#endif
> 
> Why is this part needed?
> 
> Otherwise, LGTM, thanks.  (But I'm don't have the approval rights, so
> please wait for Ian to chime in.)

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[LIU Hao]

[-- Attachment #1.1: Type: text/plain, Size: 598 bytes --]

在 2023/11/22 03:35, Björn Schäpers 写道:
> I'll guess it is not needed here, but otherwise <windows.h> defines the macros max and min, they 
> then conflict e.g. with C++'s std::max/std::min. So I consider it best practice to always define it, 
> before including <windows.h>.

The mingw-w64 header does not define them for C++ [1] and GCC defines `NOMINMAX` in 'os_defines.h', 
so either way it is not necessary.


[1] 
https://github.com/mingw-w64/mingw-w64/blob/3ebb92804e3125d1be8f61bcd42f15a8db15ba1e/mingw-w64-headers/include/minmax.h#L9



-- 
Best regards,
LIU Hao


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 2/4] libbacktrace: detect executable path on windows

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On Mon, Nov 20, 2023 at 11:57 AM Björn Schäpers <gcc@hazardy.de> wrote:
>
> this is what I'm using with GCC 12 and 13 on my windows machines, rebased onto
> the current HEAD.

Thanks.  Committed as follows.

Ian

            * fileline.c: Include <windows.h> if available.
            (windows_get_executable_path): New static function.
            (fileline_initialize): Call windows_get_executable_path.
            * configure.ac: Checked for windows.h
            * configure: Regenerate.
            * config.h.in: Regenerate.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3728 bytes --]

0ee01dfacbcc9bc05d11433a69c0a0ac13afa42f
diff --git a/libbacktrace/config.h.in b/libbacktrace/config.h.in
index a4f5bddddf6..ee2616335c7 100644
--- a/libbacktrace/config.h.in
+++ b/libbacktrace/config.h.in
@@ -104,6 +104,9 @@
 /* Define to 1 if you have the <unistd.h> header file. */
 #undef HAVE_UNISTD_H
 
+/* Define to 1 if you have the <windows.h> header file. */
+#undef HAVE_WINDOWS_H
+
 /* Define if -lz is available. */
 #undef HAVE_ZLIB
 
diff --git a/libbacktrace/configure b/libbacktrace/configure
index 0ccc060901d..7ade966b54d 100755
--- a/libbacktrace/configure
+++ b/libbacktrace/configure
@@ -13509,6 +13509,19 @@ $as_echo "#define HAVE_LOADQUERY 1" >>confdefs.h
 
 fi
 
+for ac_header in windows.h
+do :
+  ac_fn_c_check_header_mongrel "$LINENO" "windows.h" "ac_cv_header_windows_h" "$ac_includes_default"
+if test "x$ac_cv_header_windows_h" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_WINDOWS_H 1
+_ACEOF
+
+fi
+
+done
+
+
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
    case "${host}" in
diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac
index 71cd50f8cdf..00acb42eb6d 100644
--- a/libbacktrace/configure.ac
+++ b/libbacktrace/configure.ac
@@ -379,6 +379,8 @@ if test "$have_loadquery" = "yes"; then
   AC_DEFINE(HAVE_LOADQUERY, 1, [Define if AIX loadquery is available.])
 fi
 
+AC_CHECK_HEADERS(windows.h)
+
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
    case "${host}" in
diff --git a/libbacktrace/fileline.c b/libbacktrace/fileline.c
index 0e560b44e7a..773f3a92969 100644
--- a/libbacktrace/fileline.c
+++ b/libbacktrace/fileline.c
@@ -47,6 +47,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #include <mach-o/dyld.h>
 #endif
 
+#ifdef HAVE_WINDOWS_H
+#ifndef WIN32_MEAN_AND_LEAN
+#define WIN32_MEAN_AND_LEAN
+#endif
+
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+
+#include <windows.h>
+#endif
+
 #include "backtrace.h"
 #include "internal.h"
 
@@ -165,6 +177,37 @@ macho_get_executable_path (struct backtrace_state *state,
 
 #endif /* !HAVE_DECL__PGMPTR */
 
+#ifdef HAVE_WINDOWS_H
+
+#define FILENAME_BUF_SIZE (MAX_PATH)
+
+static char *
+windows_get_executable_path (char *buf, backtrace_error_callback error_callback,
+			     void *data)
+{
+  size_t got;
+  int error;
+
+  got = GetModuleFileNameA (NULL, buf, FILENAME_BUF_SIZE - 1);
+  error = GetLastError ();
+  if (got == 0
+      || (got == FILENAME_BUF_SIZE - 1 && error == ERROR_INSUFFICIENT_BUFFER))
+    {
+      error_callback (data,
+		      "could not get the filename of the current executable",
+		      error);
+      return NULL;
+    }
+  return buf;
+}
+
+#else /* !defined (HAVE_WINDOWS_H) */
+
+#define windows_get_executable_path(buf, error_callback, data) NULL
+#define FILENAME_BUF_SIZE 64
+
+#endif /* !defined (HAVE_WINDOWS_H) */
+
 /* Initialize the fileline information from the executable.  Returns 1
    on success, 0 on failure.  */
 
@@ -178,7 +221,7 @@ fileline_initialize (struct backtrace_state *state,
   int called_error_callback;
   int descriptor;
   const char *filename;
-  char buf[64];
+  char buf[FILENAME_BUF_SIZE];
 
   if (!state->threaded)
     failed = state->fileline_initialization_failed;
@@ -202,7 +245,7 @@ fileline_initialize (struct backtrace_state *state,
 
   descriptor = -1;
   called_error_callback = 0;
-  for (pass = 0; pass < 9; ++pass)
+  for (pass = 0; pass < 10; ++pass)
     {
       int does_not_exist;
 
@@ -239,6 +282,9 @@ fileline_initialize (struct backtrace_state *state,
 	case 8:
 	  filename = macho_get_executable_path (state, error_callback, data);
 	  break;
+	case 9:
+	  filename = windows_get_executable_path (buf, error_callback, data);
+	  break;
 	default:
 	  abort ();
 	}

Re: [PATCH 3/4] libbacktrace: work with aslr on windows

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

On Mon, Nov 20, 2023 at 11:58 AM Björn Schäpers <gcc@hazardy.de> wrote:
>
> An updated version, using neither A or W, but just the macro.

Thanks.  Committed as follows.

Ian

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1305 bytes --]

1017495bc91d40570f58c37e88ca013164782129
diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index 56af4828e27..f976a963bf3 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -39,6 +39,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #include "backtrace.h"
 #include "internal.h"
 
+#ifdef HAVE_WINDOWS_H
+#ifndef WIN32_MEAN_AND_LEAN
+#define WIN32_MEAN_AND_LEAN
+#endif
+
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+
+#include <windows.h>
+#endif
+
 /* Coff file header.  */
 
 typedef struct {
@@ -610,6 +622,7 @@ coff_add (struct backtrace_state *state, int descriptor,
   int debug_view_valid;
   int is_64;
   uintptr_t image_base;
+  uintptr_t base_address = 0;
   struct dwarf_sections dwarf_sections;
 
   *found_sym = 0;
@@ -856,7 +869,16 @@ coff_add (struct backtrace_state *state, int descriptor,
 				  + (sections[i].offset - min_offset));
     }
 
-  if (!backtrace_dwarf_add (state, /* base_address */ 0, &dwarf_sections,
+#ifdef HAVE_WINDOWS_H
+  {
+    uintptr_t module_handle;
+
+    module_handle = (uintptr_t) GetModuleHandle (NULL);
+    base_address = module_handle - image_base;
+  }
+#endif
+
+  if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
 			    0, /* FIXME: is_bigendian */
 			    NULL, /* altlink */
 			    error_callback, data, fileline_fn,

Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls

[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




> Tested on x86_64-linux and i686-w64-mingw32.
>
> -- >8 --
>
>         * pecoff.c (coff_add): New argument for the module handle of the
>         file, to get the base address.
>         * pecoff.c (backtrace_initialize): Iterate over loaded libraries
>         and call coff_add.
>
> Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
> ---
>  libbacktrace/pecoff.c | 76 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
> index 296f1357b5f..40395109e51 100644
> --- a/libbacktrace/pecoff.c
> +++ b/libbacktrace/pecoff.c
> @@ -49,6 +49,7 @@ POSSIBILITY OF SUCH DAMAGE.  */
>  #endif
>
>  #include <windows.h>
> +#include <psapi.h>
>  #endif
>
>  /* Coff file header.  */
> @@ -592,7 +593,8 @@ coff_syminfo (struct backtrace_state *state, uintptr_t addr,
>  static int
>  coff_add (struct backtrace_state *state, int descriptor,
>           backtrace_error_callback error_callback, void *data,
> -         fileline *fileline_fn, int *found_sym, int *found_dwarf)
> +         fileline *fileline_fn, int *found_sym, int *found_dwarf,
> +         uintptr_t module_handle ATTRIBUTE_UNUSED)
>  {
>    struct backtrace_view fhdr_view;
>    off_t fhdr_off;
> @@ -623,7 +625,6 @@ coff_add (struct backtrace_state *state, int descriptor,
>    int is_64;
>    uintptr_t image_base;
>    uintptr_t base_address = 0;
> -  uintptr_t module_handle;
>    struct dwarf_sections dwarf_sections;
>
>    *found_sym = 0;
> @@ -871,7 +872,6 @@ coff_add (struct backtrace_state *state, int descriptor,
>      }
>
>  #ifdef HAVE_WINDOWS_H
> -    module_handle = (uintptr_t) GetModuleHandleW (NULL);
>      base_address = module_handle - image_base;
>  #endif
>
> @@ -914,12 +914,80 @@ backtrace_initialize (struct backtrace_state *state,
>    int found_sym;
>    int found_dwarf;
>    fileline coff_fileline_fn;
> +  uintptr_t module_handle = 0;
> +
> +#ifdef HAVE_WINDOWS_H
> +  DWORD i;
> +  DWORD module_count;
> +  DWORD bytes_needed_for_modules;
> +  HMODULE *modules;
> +  char module_name[MAX_PATH];
> +  int module_found_sym;
> +  fileline module_fileline_fn;
> +
> +  module_handle = (uintptr_t) GetModuleHandleW (NULL);
> +#endif
>
>    ret = coff_add (state, descriptor, error_callback, data,
> -                 &coff_fileline_fn, &found_sym, &found_dwarf);
> +                 &coff_fileline_fn, &found_sym, &found_dwarf, module_handle);
>    if (!ret)
>      return 0;
>
> +#ifdef HAVE_WINDOWS_H
> +  module_count = 1000;
> + alloc_modules:
> +  modules = backtrace_alloc (state, module_count * sizeof(HMODULE),
> +                            error_callback, data);
> +  if (modules == NULL)
> +    goto skip_modules;
> +  if (!EnumProcessModules (GetCurrentProcess (), modules, module_count,
> +                          &bytes_needed_for_modules))
> +    {
> +      error_callback(data, "Could not enumerate process modules",
> +                    (int) GetLastError ());
> +      goto free_modules;
> +    }
> +  if (bytes_needed_for_modules > module_count * sizeof(HMODULE))
> +    {
> +      backtrace_free (state, modules, module_count * sizeof(HMODULE),
> +                     error_callback, data);
> +      // Add an extra of 2, if some module is loaded in another thread.
> +      module_count = bytes_needed_for_modules / sizeof(HMODULE) + 2;
> +      modules = NULL;
> +      goto alloc_modules;
> +    }
> +
> +  for (i = 0; i < bytes_needed_for_modules / sizeof(HMODULE); ++i)
> +    {
> +      if (GetModuleFileNameA (modules[i], module_name, MAX_PATH - 1))
> +       {
> +         if (strcmp (filename, module_name) == 0)
> +           continue;
> +
> +         module_handle = (uintptr_t) GetModuleHandleA (module_name);
> +         if (module_handle == 0)
> +           continue;
> +
> +         descriptor = backtrace_open (module_name, error_callback, data, NULL);
> +         if (descriptor < 0)
> +           continue;
> +
> +         coff_add (state, descriptor, error_callback, data,
> +                   &module_fileline_fn, &module_found_sym, &found_dwarf,
> +                   module_handle);
> +         if (module_found_sym)
> +           found_sym = 1;
> +       }
> +    }
> +
> + free_modules:
> +  if (modules)
> +    backtrace_free(state, modules, module_count * sizeof(HMODULE),
> +                  error_callback, data);
> +  modules = NULL;
> + skip_modules:
> +#endif
> +
>    if (!state->threaded)
>      {
>        if (found_sym)
> --
> 2.38.1
>

Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls

[Eli Zaretskii]

> Date: Thu, 30 Nov 2023 11:53:54 -0800
> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
> From: Ian Lance Taylor via Gcc <gcc@gcc.gnu.org>
> 
> Also starting with a module count of 1000 seems like a lot.  Do
> typical Windows programs load that many modules?

Unlikely.  I'd start with 100.

Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls

[Björn Schäpers]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

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.

[-- Attachment #2: 0003-libbacktrace-get-debug-information-for-loaded-dlls.patch --]
[-- Type: text/plain, Size: 5230 bytes --]

From bd552716ee7937cad9d54d4966532d6ea6dbc1bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de>
Date: Sun, 30 Apr 2023 23:54:32 +0200
Subject: [PATCH] libbacktrace: get debug information for loaded dlls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

Tested on x86_64-linux and i686-w64-mingw32.

-- >8 --

	* pecoff.c (coff_add): New argument for the module handle of the
	file, to get the base address.
	* pecoff.c (backtrace_initialize): Iterate over loaded libraries
	and call coff_add.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libbacktrace/pecoff.c | 104 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 96 insertions(+), 8 deletions(-)

diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index f976a963bf3..3eb9c4a4853 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -49,6 +49,7 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #endif
 
 #include <windows.h>
+#include <psapi.h>
 #endif
 
 /* Coff file header.  */
@@ -592,7 +593,8 @@ coff_syminfo (struct backtrace_state *state, uintptr_t addr,
 static int
 coff_add (struct backtrace_state *state, int descriptor,
 	  backtrace_error_callback error_callback, void *data,
-	  fileline *fileline_fn, int *found_sym, int *found_dwarf)
+	  fileline *fileline_fn, int *found_sym, int *found_dwarf,
+	  uintptr_t module_handle ATTRIBUTE_UNUSED)
 {
   struct backtrace_view fhdr_view;
   off_t fhdr_off;
@@ -870,12 +872,7 @@ coff_add (struct backtrace_state *state, int descriptor,
     }
 
 #ifdef HAVE_WINDOWS_H
-  {
-    uintptr_t module_handle;
-
-    module_handle = (uintptr_t) GetModuleHandle (NULL);
-    base_address = module_handle - image_base;
-  }
+  base_address = module_handle - image_base;
 #endif
 
   if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
@@ -903,6 +900,53 @@ coff_add (struct backtrace_state *state, int descriptor,
   return 0;
 }
 
+#ifdef HAVE_WINDOWS_H
+static void
+free_modules (struct backtrace_state *state,
+	      backtrace_error_callback error_callback, void *data,
+	      HMODULE **modules, DWORD bytes_allocated)
+{
+  backtrace_free (state, *modules, bytes_allocated, error_callback, data);
+  *modules = NULL;
+}
+
+static void
+get_all_modules (struct backtrace_state *state,
+		 backtrace_error_callback error_callback, void *data, 
+		 HMODULE **modules, DWORD *module_count, DWORD *bytes_allocated)
+{
+  DWORD bytes_needed = 0;
+
+  for (;;)
+    {
+      *bytes_allocated = *module_count * sizeof(HMODULE);
+      *modules = backtrace_alloc (state, *bytes_allocated, error_callback, data);
+
+      if (*modules == NULL)
+	return;
+
+      if (!EnumProcessModules (GetCurrentProcess (), *modules, *module_count,
+			       &bytes_needed))
+	{
+	  error_callback(data, "Could not enumerate process modules",
+			 (int) GetLastError ());
+	  free_modules (state, error_callback, data, modules, *bytes_allocated);
+	  return;
+	}
+
+      *module_count = bytes_needed / sizeof(HMODULE);
+      if (bytes_needed <= *bytes_allocated)
+	{
+	  return;
+	}
+
+      free_modules (state, error_callback, data, modules, *bytes_allocated);
+      // Add an extra of 2, of some module is loaded in another thread.
+      *module_count += 2;
+    }
+}
+#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
    sections.  */
@@ -917,12 +961,56 @@ backtrace_initialize (struct backtrace_state *state,
   int found_sym;
   int found_dwarf;
   fileline coff_fileline_fn;
+  uintptr_t module_handle = 0;
+
+#ifdef HAVE_WINDOWS_H
+  DWORD i;
+  DWORD module_count = 100;
+  DWORD bytes_allocated_for_modules = 0;
+  HMODULE *modules = NULL;
+  char module_name[MAX_PATH];
+  int module_found_sym;
+  fileline module_fileline_fn;
+
+  module_handle = (uintptr_t) GetModuleHandle (NULL);
+#endif
 
   ret = coff_add (state, descriptor, error_callback, data,
-		  &coff_fileline_fn, &found_sym, &found_dwarf);
+		  &coff_fileline_fn, &found_sym, &found_dwarf, module_handle);
   if (!ret)
     return 0;
 
+#ifdef HAVE_WINDOWS_H
+  get_all_modules (state, error_callback, data, &modules, 
+		   &module_count, &bytes_allocated_for_modules);
+
+  for (i = 0; i < module_count; ++i)
+    {
+      if (GetModuleFileNameA (modules[i], module_name, MAX_PATH - 1))
+	{
+	  if (strcmp (filename, module_name) == 0)
+	    continue;
+
+	  module_handle = (uintptr_t) GetModuleHandleA (module_name);
+	  if (module_handle == 0)
+	    continue;
+
+	  descriptor = backtrace_open (module_name, error_callback, data, NULL);
+	  if (descriptor < 0)
+	    continue;
+
+	  coff_add (state, descriptor, error_callback, data,
+		    &module_fileline_fn, &module_found_sym, &found_dwarf,
+		    module_handle);
+	  if (module_found_sym)
+	    found_sym = 1;
+	}
+    }
+
+  if (modules)
+    free_modules (state, error_callback, data, &modules, bytes_allocated_for_modules);
+#endif
+
   if (!state->threaded)
     {
       if (found_sym)
-- 
2.42.1

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

[Björn Schäpers]

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

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.

[-- Attachment #2: 0004-libbacktrace-improve-getting-debug-information-for-l.patch --]
[-- Type: text/plain, Size: 16922 bytes --]

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(-)

diff --git a/libbacktrace/config.h.in b/libbacktrace/config.h.in
index ee2616335c7..9b8ab88ab63 100644
--- a/libbacktrace/config.h.in
+++ b/libbacktrace/config.h.in
@@ -101,6 +101,9 @@
 /* Define to 1 if you have the <sys/types.h> header file. */
 #undef HAVE_SYS_TYPES_H
 
+/* Define to 1 if you have the <tlhelp32.h> header file. */
+#undef HAVE_TLHELP32_H
+
 /* Define to 1 if you have the <unistd.h> header file. */
 #undef HAVE_UNISTD_H
 
diff --git a/libbacktrace/configure b/libbacktrace/configure
index 7ade966b54d..ca52ee3bafb 100755
--- a/libbacktrace/configure
+++ b/libbacktrace/configure
@@ -1866,7 +1866,7 @@ else
 #define $2 innocuous_$2
 
 /* System header to define __stub macros and hopefully few prototypes,
-    which can conflict with char $2 (); below.
+    which can conflict with char $2 (void); below.
     Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
     <limits.h> exists even on freestanding compilers.  */
 
@@ -1884,7 +1884,7 @@ else
 #ifdef __cplusplus
 extern "C"
 #endif
-char $2 ();
+char $2 (void);
 /* The GNU C library defines this for functions which it implements
     to always fail with ENOSYS.  Some functions are actually named
     something starting with __ and the normal name is an alias.  */
@@ -1893,7 +1893,7 @@ choke me
 #endif
 
 int
-main ()
+main (void)
 {
 return $2 ();
   ;
@@ -1932,7 +1932,7 @@ else
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 if (sizeof ($2))
 	 return 0;
@@ -1945,7 +1945,7 @@ if ac_fn_c_try_compile "$LINENO"; then :
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 if (sizeof (($2)))
 	    return 0;
@@ -1983,7 +1983,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) >= 0)];
 test_array [0] = 0;
@@ -2000,7 +2000,7 @@ if ac_fn_c_try_compile "$LINENO"; then :
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) <= $ac_mid)];
 test_array [0] = 0;
@@ -2027,7 +2027,7 @@ else
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) < 0)];
 test_array [0] = 0;
@@ -2044,7 +2044,7 @@ if ac_fn_c_try_compile "$LINENO"; then :
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) >= $ac_mid)];
 test_array [0] = 0;
@@ -2079,7 +2079,7 @@ while test "x$ac_lo" != "x$ac_hi"; do
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) <= $ac_mid)];
 test_array [0] = 0;
@@ -2104,12 +2104,12 @@ esac
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 $4
-static long int longval () { return $2; }
-static unsigned long int ulongval () { return $2; }
+static long int longval (void) { return $2; }
+static unsigned long int ulongval (void) { return $2; }
 #include <stdio.h>
 #include <stdlib.h>
 int
-main ()
+main (void)
 {
 
   FILE *f = fopen ("conftest.val", "w");
@@ -2170,7 +2170,7 @@ else
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 #ifndef $as_decl_name
 #ifdef __cplusplus
@@ -3073,7 +3073,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3213,7 +3213,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #include <stdio.h>
 int
-main ()
+main (void)
 {
 FILE *f = fopen ("conftest.out", "w");
  return ferror (f) || fclose (f) != 0;
@@ -3277,7 +3277,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3328,7 +3328,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 #ifndef __GNUC__
        choke me
@@ -3369,7 +3369,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3384,7 +3384,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3400,7 +3400,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3449,9 +3449,7 @@ struct stat;
 /* Most of the following tests are stolen from RCS 5.7's src/conf.sh.  */
 struct buf { int x; };
 FILE * (*rcsopen) (struct buf *, struct stat *, int);
-static char *e (p, i)
-     char **p;
-     int i;
+static char *e (char **p, int i)
 {
   return p[i];
 }
@@ -3486,7 +3484,7 @@ int pairnames (int, char **, FILE *(*)(struct buf *, struct stat *, int), int, i
 int argc;
 char **argv;
 int
-main ()
+main (void)
 {
 return f (e, argv, 0) != argv[0]  ||  f (e, argv, 1) != argv[1];
   ;
@@ -3544,7 +3542,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3871,7 +3869,7 @@ else
 #include <float.h>
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3941,7 +3939,7 @@ else
 
 #define XOR(e, f) (((e) && !(f)) || (!(e) && (f)))
 int
-main ()
+main (void)
 {
   int i;
   for (i = 0; i < 256; i++)
@@ -4020,7 +4018,7 @@ else
 #         define __EXTENSIONS__ 1
           $ac_includes_default
 int
-main ()
+main (void)
 {
 
   ;
@@ -5002,7 +5000,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 #ifndef __GNUC__
        choke me
@@ -5043,7 +5041,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -5058,7 +5056,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -5074,7 +5072,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -5123,9 +5121,7 @@ struct stat;
 /* Most of the following tests are stolen from RCS 5.7's src/conf.sh.  */
 struct buf { int x; };
 FILE * (*rcsopen) (struct buf *, struct stat *, int);
-static char *e (p, i)
-     char **p;
-     int i;
+static char *e (char **p, int i)
 {
   return p[i];
 }
@@ -5160,7 +5156,7 @@ int pairnames (int, char **, FILE *(*)(struct buf *, struct stat *, int), int, i
 int argc;
 char **argv;
 int
-main ()
+main (void)
 {
 return f (e, argv, 0) != argv[0]  ||  f (e, argv, 1) != argv[1];
   ;
@@ -5218,7 +5214,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -7408,7 +7404,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -7994,7 +7990,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -9632,7 +9628,7 @@ _LT_EOF
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -9672,7 +9668,7 @@ if test -z "$aix_libpath"; then aix_libpath="/usr/lib:/lib"; fi
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -10947,7 +10943,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -11388,9 +11384,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dlopen ();
+char dlopen (void);
 int
-main ()
+main (void)
 {
 return dlopen ();
   ;
@@ -11441,9 +11437,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char shl_load ();
+char shl_load (void);
 int
-main ()
+main (void)
 {
 return shl_load ();
   ;
@@ -11484,9 +11480,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dlopen ();
+char dlopen (void);
 int
-main ()
+main (void)
 {
 return dlopen ();
   ;
@@ -11523,9 +11519,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dlopen ();
+char dlopen (void);
 int
-main ()
+main (void)
 {
 return dlopen ();
   ;
@@ -11562,9 +11558,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dld_link ();
+char dld_link (void);
 int
-main ()
+main (void)
 {
 return dld_link ();
   ;
@@ -11632,7 +11628,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11635 "configure"
+#line 11631 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11738,7 +11734,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11741 "configure"
+#line 11737 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12016,7 +12012,7 @@ else
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12062,7 +12058,7 @@ else
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12086,7 +12082,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12131,7 +12127,7 @@ else
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12155,7 +12151,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12228,7 +12224,7 @@ else
 /* end confdefs.h.  */
 static int f() { return 0; }
 int
-main ()
+main (void)
 {
 return f();
   ;
@@ -12259,7 +12255,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 return 0;
   ;
@@ -12312,7 +12308,7 @@ case "$host" in
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
 #if !defined(__SSE2__)
@@ -12339,7 +12335,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 asm ("setssbsy");
   ;
@@ -12401,7 +12397,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -12490,7 +12486,7 @@ $as_echo_n "checking for _Unwind_GetIPInfo... " >&6; }
 	struct _Unwind_Context *context;
 	int ip_before_insn = 0;
 int
-main ()
+main (void)
 {
 return _Unwind_GetIPInfo (context, &ip_before_insn);
   ;
@@ -12554,7 +12550,7 @@ case "$host" in
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
 #if !defined(__SSE2__)
@@ -12580,7 +12576,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 asm ("setssbsy");
   ;
@@ -12626,7 +12622,7 @@ if test x$may_have_cet = xyes; then
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 return 0;
   ;
@@ -12763,7 +12759,7 @@ else
 /* end confdefs.h.  */
 int i;
 int
-main ()
+main (void)
 {
 __sync_bool_compare_and_swap (&i, i, i);
                        __sync_lock_test_and_set (&i, 1);
@@ -12805,7 +12801,7 @@ else
 /* end confdefs.h.  */
 int i;
 int
-main ()
+main (void)
 {
 __atomic_load_n (&i, __ATOMIC_ACQUIRE);
 		       __atomic_store_n (&i, 1, __ATOMIC_RELEASE);
@@ -12843,7 +12839,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 int i;
 int
-main ()
+main (void)
 {
 int j;
   ;
@@ -13521,6 +13517,21 @@ fi
 
 done
 
+for ac_header in tlhelp32.h
+do :
+  ac_fn_c_check_header_compile "$LINENO" "tlhelp32.h" "ac_cv_header_tlhelp32_h" "#ifdef HAVE_WINDOWS_H
+#  include <windows.h>
+#endif
+"
+if test "x$ac_cv_header_tlhelp32_h" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_TLHELP32_H 1
+_ACEOF
+
+fi
+
+done
+
 
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
@@ -13625,7 +13636,7 @@ else
 #include <sys/sysctl.h>
 
 int
-main ()
+main (void)
 {
 int mib0 = CTL_KERN; int mib1 = KERN_PROC; int mib2 = KERN_PROC_PATHNAME;
   ;
@@ -13659,7 +13670,7 @@ else
 #include <sys/sysctl.h>
 
 int
-main ()
+main (void)
 {
 int mib0 = CTL_KERN; int mib1 = KERN_PROC_ARGS; int mib2 = KERN_PROC_PATHNAME;
   ;
@@ -13715,9 +13726,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char clock_gettime ();
+char clock_gettime (void);
 int
-main ()
+main (void)
 {
 return clock_gettime ();
   ;
@@ -13792,7 +13803,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 int i;
 int
-main ()
+main (void)
 {
 return 0;
   ;
@@ -13835,9 +13846,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char compress ();
+char compress (void);
 int
-main ()
+main (void)
 {
 return compress ();
   ;
@@ -13881,7 +13892,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -13919,7 +13930,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -13962,9 +13973,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char ZSTD_compress ();
+char ZSTD_compress (void);
 int
-main ()
+main (void)
 {
 return ZSTD_compress ();
   ;
@@ -14008,7 +14019,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -14338,9 +14349,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char lzma_auto_decoder ();
+char lzma_auto_decoder (void);
 int
-main ()
+main (void)
 {
 return lzma_auto_decoder ();
   ;
@@ -14385,7 +14396,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 return 0;
   ;
diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac
index 0f61f2b28ab..b9a695ab402 100644
--- a/libbacktrace/configure.ac
+++ b/libbacktrace/configure.ac
@@ -380,6 +380,10 @@ if test "$have_loadquery" = "yes"; then
 fi
 
 AC_CHECK_HEADERS(windows.h)
+AC_CHECK_HEADERS(tlhelp32.h, [], [],
+[#ifdef HAVE_WINDOWS_H
+#  include <windows.h>
+#endif])
 
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index 64787a18411..647baa39640 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -50,6 +50,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 
 #include <windows.h>
 #include <psapi.h>
+
+#ifdef HAVE_TLHELP32_H
+#include <tlhelp32.h>
+
+#ifdef UNICODE
+/* If UNICODE is defined, all the symbols are replaced by a macro to use the
+   wide variant. But we need the ansi variant, so undef the macros. */
+#undef MODULEENTRY32
+#undef Module32First
+#undef Module32Next
+#endif
+#endif
 #endif
 
 /* Coff file header.  */
@@ -900,7 +912,7 @@ coff_add (struct backtrace_state *state, int descriptor,
   return 0;
 }
 
-#ifdef HAVE_WINDOWS_H
+#if defined(HAVE_WINDOWS_H) && !defined(HAVE_TLHELP32_H)
 static void
 free_modules (struct backtrace_state *state,
 	      backtrace_error_callback error_callback, void *data,
@@ -964,13 +976,19 @@ backtrace_initialize (struct backtrace_state *state,
   uintptr_t module_handle = 0;
 
 #ifdef HAVE_WINDOWS_H
+  fileline module_fileline_fn;
+  int module_found_sym;
+
+#ifdef HAVE_TLHELP32_H
+  HANDLE snapshot;
+#else
   DWORD i;
   DWORD module_count = 100;
   DWORD bytes_allocated_for_modules = 0;
   HMODULE *modules = NULL;
   char module_name[MAX_PATH];
-  int module_found_sym;
-  fileline module_fileline_fn;
+
+#endif
 
   module_handle = (uintptr_t) GetModuleHandle (NULL);
 #endif
@@ -981,6 +999,43 @@ backtrace_initialize (struct backtrace_state *state,
     return 0;
 
 #ifdef HAVE_WINDOWS_H
+#ifdef HAVE_TLHELP32_H
+  do
+    {
+      snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPMODULE, 0);
+    }
+  while (snapshot == INVALID_HANDLE_VALUE
+	 && GetLastError () == ERROR_BAD_LENGTH);
+
+  if (snapshot != INVALID_HANDLE_VALUE)
+    {
+      MODULEENTRY32 entry;
+      BOOL ok;
+      entry.dwSize = sizeof(MODULEENTRY32);
+
+      for (ok = Module32First (snapshot, &entry); ok; ok =Module32Next (snapshot, &entry))
+	{
+	  if (strcmp (filename, entry.szExePath) == 0)
+	    continue;
+
+	  module_handle = (uintptr_t) entry.hModule;
+	  if (module_handle == 0)
+	    continue;
+
+	  descriptor = backtrace_open (entry.szExePath, error_callback, data, NULL);
+	  if (descriptor < 0)
+	    continue;
+
+	  coff_add (state, descriptor, error_callback, data,
+		    &module_fileline_fn, &module_found_sym, &found_dwarf,
+		    module_handle);
+	  if (module_found_sym)
+	    found_sym = 1;
+	}
+
+      CloseHandle (snapshot);
+    }
+#else
   get_all_modules (state, error_callback, data, &modules, 
 		   &module_count, &bytes_allocated_for_modules);
 
@@ -1009,6 +1064,7 @@ backtrace_initialize (struct backtrace_state *state,
 
   if (modules)
     free_modules (state, error_callback, data, &modules, bytes_allocated_for_modules);
+#endif
 #endif
 
   if (!state->threaded)
-- 
2.42.1

Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize

[Björn Schäpers]

[-- Attachment #1: Type: text/plain, Size: 1860 bytes --]

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.

[-- Attachment #2: 0005-libbacktrace-Add-loaded-dlls-after-initialize.patch --]
[-- Type: text/plain, Size: 4601 bytes --]

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)
-- 
2.42.1

Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize

[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

> +  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.

Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize

[Eli Zaretskii]

[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.

Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize

[Björn Schäpers]

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.

Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize

[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.

Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize

[Björn Schäpers]

[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]

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.

[-- Attachment #2: 0005-libbacktrace-Add-loaded-dlls-after-initialize.patch --]
[-- Type: text/plain, Size: 4645 bytes --]

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)
-- 
2.42.1

Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize

[Eli Zaretskii]

> 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).

Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls

[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.

A friendly ping.

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

[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

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

[Björn Schäpers]

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.

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

[Ian Lance Taylor]

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

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

[Björn Schäpers]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

Am 25.01.2024 um 23:04 schrieb Ian Lance Taylor:
> 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

Attached is the combined version of the two patches, only implementing the 
variant with the tlhelp32 API.

Tested on x86 and x86_64 windows.

Kind regards,
Björn.

[-- Attachment #2: 0003-libbacktrace-get-debug-information-for-loaded-dlls.patch --]
[-- Type: text/plain, Size: 17466 bytes --]

From 33a6380feff66e32ef0f0d7cbad6fb55319f4e2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de>
Date: Sun, 30 Apr 2023 23:54:32 +0200
Subject: [PATCH 1/2] libbacktrace: get debug information for loaded dlls
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

Tested on x86_64-linux and i686-w64-mingw32.

-- >8 --

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.
	 (coff_add): New argument for the module handle of the file,
	             to get the base address.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libbacktrace/config.h.in  |   3 +
 libbacktrace/configure    | 185 ++++++++++++++++++++------------------
 libbacktrace/configure.ac |   4 +
 libbacktrace/pecoff.c     |  74 +++++++++++++--
 4 files changed, 171 insertions(+), 95 deletions(-)

diff --git a/libbacktrace/config.h.in b/libbacktrace/config.h.in
index ee2616335c7..9b8ab88ab63 100644
--- a/libbacktrace/config.h.in
+++ b/libbacktrace/config.h.in
@@ -101,6 +101,9 @@
 /* Define to 1 if you have the <sys/types.h> header file. */
 #undef HAVE_SYS_TYPES_H
 
+/* Define to 1 if you have the <tlhelp32.h> header file. */
+#undef HAVE_TLHELP32_H
+
 /* Define to 1 if you have the <unistd.h> header file. */
 #undef HAVE_UNISTD_H
 
diff --git a/libbacktrace/configure b/libbacktrace/configure
index 7ade966b54d..ca52ee3bafb 100755
--- a/libbacktrace/configure
+++ b/libbacktrace/configure
@@ -1866,7 +1866,7 @@ else
 #define $2 innocuous_$2
 
 /* System header to define __stub macros and hopefully few prototypes,
-    which can conflict with char $2 (); below.
+    which can conflict with char $2 (void); below.
     Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
     <limits.h> exists even on freestanding compilers.  */
 
@@ -1884,7 +1884,7 @@ else
 #ifdef __cplusplus
 extern "C"
 #endif
-char $2 ();
+char $2 (void);
 /* The GNU C library defines this for functions which it implements
     to always fail with ENOSYS.  Some functions are actually named
     something starting with __ and the normal name is an alias.  */
@@ -1893,7 +1893,7 @@ choke me
 #endif
 
 int
-main ()
+main (void)
 {
 return $2 ();
   ;
@@ -1932,7 +1932,7 @@ else
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 if (sizeof ($2))
 	 return 0;
@@ -1945,7 +1945,7 @@ if ac_fn_c_try_compile "$LINENO"; then :
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 if (sizeof (($2)))
 	    return 0;
@@ -1983,7 +1983,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) >= 0)];
 test_array [0] = 0;
@@ -2000,7 +2000,7 @@ if ac_fn_c_try_compile "$LINENO"; then :
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) <= $ac_mid)];
 test_array [0] = 0;
@@ -2027,7 +2027,7 @@ else
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) < 0)];
 test_array [0] = 0;
@@ -2044,7 +2044,7 @@ if ac_fn_c_try_compile "$LINENO"; then :
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) >= $ac_mid)];
 test_array [0] = 0;
@@ -2079,7 +2079,7 @@ while test "x$ac_lo" != "x$ac_hi"; do
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 static int test_array [1 - 2 * !(($2) <= $ac_mid)];
 test_array [0] = 0;
@@ -2104,12 +2104,12 @@ esac
     cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 $4
-static long int longval () { return $2; }
-static unsigned long int ulongval () { return $2; }
+static long int longval (void) { return $2; }
+static unsigned long int ulongval (void) { return $2; }
 #include <stdio.h>
 #include <stdlib.h>
 int
-main ()
+main (void)
 {
 
   FILE *f = fopen ("conftest.val", "w");
@@ -2170,7 +2170,7 @@ else
 /* end confdefs.h.  */
 $4
 int
-main ()
+main (void)
 {
 #ifndef $as_decl_name
 #ifdef __cplusplus
@@ -3073,7 +3073,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3213,7 +3213,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #include <stdio.h>
 int
-main ()
+main (void)
 {
 FILE *f = fopen ("conftest.out", "w");
  return ferror (f) || fclose (f) != 0;
@@ -3277,7 +3277,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3328,7 +3328,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 #ifndef __GNUC__
        choke me
@@ -3369,7 +3369,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3384,7 +3384,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3400,7 +3400,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3449,9 +3449,7 @@ struct stat;
 /* Most of the following tests are stolen from RCS 5.7's src/conf.sh.  */
 struct buf { int x; };
 FILE * (*rcsopen) (struct buf *, struct stat *, int);
-static char *e (p, i)
-     char **p;
-     int i;
+static char *e (char **p, int i)
 {
   return p[i];
 }
@@ -3486,7 +3484,7 @@ int pairnames (int, char **, FILE *(*)(struct buf *, struct stat *, int), int, i
 int argc;
 char **argv;
 int
-main ()
+main (void)
 {
 return f (e, argv, 0) != argv[0]  ||  f (e, argv, 1) != argv[1];
   ;
@@ -3544,7 +3542,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3871,7 +3869,7 @@ else
 #include <float.h>
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -3941,7 +3939,7 @@ else
 
 #define XOR(e, f) (((e) && !(f)) || (!(e) && (f)))
 int
-main ()
+main (void)
 {
   int i;
   for (i = 0; i < 256; i++)
@@ -4020,7 +4018,7 @@ else
 #         define __EXTENSIONS__ 1
           $ac_includes_default
 int
-main ()
+main (void)
 {
 
   ;
@@ -5002,7 +5000,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 #ifndef __GNUC__
        choke me
@@ -5043,7 +5041,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -5058,7 +5056,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -5074,7 +5072,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -5123,9 +5121,7 @@ struct stat;
 /* Most of the following tests are stolen from RCS 5.7's src/conf.sh.  */
 struct buf { int x; };
 FILE * (*rcsopen) (struct buf *, struct stat *, int);
-static char *e (p, i)
-     char **p;
-     int i;
+static char *e (char **p, int i)
 {
   return p[i];
 }
@@ -5160,7 +5156,7 @@ int pairnames (int, char **, FILE *(*)(struct buf *, struct stat *, int), int, i
 int argc;
 char **argv;
 int
-main ()
+main (void)
 {
 return f (e, argv, 0) != argv[0]  ||  f (e, argv, 1) != argv[1];
   ;
@@ -5218,7 +5214,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -7408,7 +7404,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -7994,7 +7990,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -9632,7 +9628,7 @@ _LT_EOF
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -9672,7 +9668,7 @@ if test -z "$aix_libpath"; then aix_libpath="/usr/lib:/lib"; fi
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -10947,7 +10943,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -11388,9 +11384,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dlopen ();
+char dlopen (void);
 int
-main ()
+main (void)
 {
 return dlopen ();
   ;
@@ -11441,9 +11437,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char shl_load ();
+char shl_load (void);
 int
-main ()
+main (void)
 {
 return shl_load ();
   ;
@@ -11484,9 +11480,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dlopen ();
+char dlopen (void);
 int
-main ()
+main (void)
 {
 return dlopen ();
   ;
@@ -11523,9 +11519,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dlopen ();
+char dlopen (void);
 int
-main ()
+main (void)
 {
 return dlopen ();
   ;
@@ -11562,9 +11558,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dld_link ();
+char dld_link (void);
 int
-main ()
+main (void)
 {
 return dld_link ();
   ;
@@ -11632,7 +11628,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11635 "configure"
+#line 11631 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11738,7 +11734,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11741 "configure"
+#line 11737 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12016,7 +12012,7 @@ else
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12062,7 +12058,7 @@ else
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12086,7 +12082,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12131,7 +12127,7 @@ else
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12155,7 +12151,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
 int
-main ()
+main (void)
 {
 
   ;
@@ -12228,7 +12224,7 @@ else
 /* end confdefs.h.  */
 static int f() { return 0; }
 int
-main ()
+main (void)
 {
 return f();
   ;
@@ -12259,7 +12255,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 return 0;
   ;
@@ -12312,7 +12308,7 @@ case "$host" in
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
 #if !defined(__SSE2__)
@@ -12339,7 +12335,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 asm ("setssbsy");
   ;
@@ -12401,7 +12397,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -12490,7 +12486,7 @@ $as_echo_n "checking for _Unwind_GetIPInfo... " >&6; }
 	struct _Unwind_Context *context;
 	int ip_before_insn = 0;
 int
-main ()
+main (void)
 {
 return _Unwind_GetIPInfo (context, &ip_before_insn);
   ;
@@ -12554,7 +12550,7 @@ case "$host" in
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
 #if !defined(__SSE2__)
@@ -12580,7 +12576,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 asm ("setssbsy");
   ;
@@ -12626,7 +12622,7 @@ if test x$may_have_cet = xyes; then
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 return 0;
   ;
@@ -12763,7 +12759,7 @@ else
 /* end confdefs.h.  */
 int i;
 int
-main ()
+main (void)
 {
 __sync_bool_compare_and_swap (&i, i, i);
                        __sync_lock_test_and_set (&i, 1);
@@ -12805,7 +12801,7 @@ else
 /* end confdefs.h.  */
 int i;
 int
-main ()
+main (void)
 {
 __atomic_load_n (&i, __ATOMIC_ACQUIRE);
 		       __atomic_store_n (&i, 1, __ATOMIC_RELEASE);
@@ -12843,7 +12839,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 int i;
 int
-main ()
+main (void)
 {
 int j;
   ;
@@ -13521,6 +13517,21 @@ fi
 
 done
 
+for ac_header in tlhelp32.h
+do :
+  ac_fn_c_check_header_compile "$LINENO" "tlhelp32.h" "ac_cv_header_tlhelp32_h" "#ifdef HAVE_WINDOWS_H
+#  include <windows.h>
+#endif
+"
+if test "x$ac_cv_header_tlhelp32_h" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_TLHELP32_H 1
+_ACEOF
+
+fi
+
+done
+
 
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
@@ -13625,7 +13636,7 @@ else
 #include <sys/sysctl.h>
 
 int
-main ()
+main (void)
 {
 int mib0 = CTL_KERN; int mib1 = KERN_PROC; int mib2 = KERN_PROC_PATHNAME;
   ;
@@ -13659,7 +13670,7 @@ else
 #include <sys/sysctl.h>
 
 int
-main ()
+main (void)
 {
 int mib0 = CTL_KERN; int mib1 = KERN_PROC_ARGS; int mib2 = KERN_PROC_PATHNAME;
   ;
@@ -13715,9 +13726,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char clock_gettime ();
+char clock_gettime (void);
 int
-main ()
+main (void)
 {
 return clock_gettime ();
   ;
@@ -13792,7 +13803,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 int i;
 int
-main ()
+main (void)
 {
 return 0;
   ;
@@ -13835,9 +13846,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char compress ();
+char compress (void);
 int
-main ()
+main (void)
 {
 return compress ();
   ;
@@ -13881,7 +13892,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -13919,7 +13930,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -13962,9 +13973,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char ZSTD_compress ();
+char ZSTD_compress (void);
 int
-main ()
+main (void)
 {
 return ZSTD_compress ();
   ;
@@ -14008,7 +14019,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 
   ;
@@ -14338,9 +14349,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char lzma_auto_decoder ();
+char lzma_auto_decoder (void);
 int
-main ()
+main (void)
 {
 return lzma_auto_decoder ();
   ;
@@ -14385,7 +14396,7 @@ else
 /* end confdefs.h.  */
 
 int
-main ()
+main (void)
 {
 return 0;
   ;
diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac
index 0f61f2b28ab..b9a695ab402 100644
--- a/libbacktrace/configure.ac
+++ b/libbacktrace/configure.ac
@@ -380,6 +380,10 @@ if test "$have_loadquery" = "yes"; then
 fi
 
 AC_CHECK_HEADERS(windows.h)
+AC_CHECK_HEADERS(tlhelp32.h, [], [],
+[#ifdef HAVE_WINDOWS_H
+#  include <windows.h>
+#endif])
 
 # Check for the fcntl function.
 if test -n "${with_target_subdir}"; then
diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index 9e437d810c7..faa0be0b700 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -49,6 +49,18 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #endif
 
 #include <windows.h>
+
+#ifdef HAVE_TLHELP32_H
+#include <tlhelp32.h>
+
+#ifdef UNICODE
+/* If UNICODE is defined, all the symbols are replaced by a macro to use the
+   wide variant. But we need the ansi variant, so undef the macros. */
+#undef MODULEENTRY32
+#undef Module32First
+#undef Module32Next
+#endif
+#endif
 #endif
 
 /* Coff file header.  */
@@ -592,7 +604,8 @@ coff_syminfo (struct backtrace_state *state, uintptr_t addr,
 static int
 coff_add (struct backtrace_state *state, int descriptor,
 	  backtrace_error_callback error_callback, void *data,
-	  fileline *fileline_fn, int *found_sym, int *found_dwarf)
+	  fileline *fileline_fn, int *found_sym, int *found_dwarf,
+	  uintptr_t module_handle ATTRIBUTE_UNUSED)
 {
   struct backtrace_view fhdr_view;
   off_t fhdr_off;
@@ -870,12 +883,7 @@ coff_add (struct backtrace_state *state, int descriptor,
     }
 
 #ifdef HAVE_WINDOWS_H
-  {
-    uintptr_t module_handle;
-
-    module_handle = (uintptr_t) GetModuleHandle (NULL);
-    base_address = module_handle - image_base;
-  }
+  base_address = module_handle - image_base;
 #endif
 
   if (!backtrace_dwarf_add (state, base_address, &dwarf_sections,
@@ -917,12 +925,62 @@ backtrace_initialize (struct backtrace_state *state,
   int found_sym;
   int found_dwarf;
   fileline coff_fileline_fn;
+  uintptr_t module_handle = 0;
+
+#ifdef HAVE_TLHELP32_H
+  fileline module_fileline_fn;
+  int module_found_sym;
+
+  HANDLE snapshot;
+#endif
+
+#ifdef HAVE_WINDOWS_H
+  module_handle = (uintptr_t) GetModuleHandle (NULL);
+#endif
 
   ret = coff_add (state, descriptor, error_callback, data,
-		  &coff_fileline_fn, &found_sym, &found_dwarf);
+		  &coff_fileline_fn, &found_sym, &found_dwarf, module_handle);
   if (!ret)
     return 0;
 
+#ifdef HAVE_TLHELP32_H
+  do
+    {
+      snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPMODULE, 0);
+    }
+  while (snapshot == INVALID_HANDLE_VALUE
+	 && GetLastError () == ERROR_BAD_LENGTH);
+
+  if (snapshot != INVALID_HANDLE_VALUE)
+    {
+      MODULEENTRY32 entry;
+      BOOL ok;
+      entry.dwSize = sizeof(MODULEENTRY32);
+
+      for (ok = Module32First (snapshot, &entry); ok; ok =Module32Next (snapshot, &entry))
+	{
+	  if (strcmp (filename, entry.szExePath) == 0)
+	    continue;
+
+	  module_handle = (uintptr_t) entry.hModule;
+	  if (module_handle == 0)
+	    continue;
+
+	  descriptor = backtrace_open (entry.szExePath, error_callback, data, NULL);
+	  if (descriptor < 0)
+	    continue;
+
+	  coff_add (state, descriptor, error_callback, data,
+		    &module_fileline_fn, &module_found_sym, &found_dwarf,
+		    module_handle);
+	  if (module_found_sym)
+	    found_sym = 1;
+	}
+
+      CloseHandle (snapshot);
+    }
+#endif
+
   if (!state->threaded)
     {
       if (found_sym)
-- 
2.43.0

Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize

[Björn Schäpers]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

Am 10.01.2024 um 13:34 schrieb Eli Zaretskii:
>> 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).

Here an updated version. It is rebased on the combined approach of getting the 
loaded DLLs and two minor changes to suppress warnings.

[-- Attachment #2: 0004-libbacktrace-Add-loaded-dlls-after-initialize.patch --]
[-- Type: text/plain, Size: 4284 bytes --]

From 55d0f312c0a9c4e2305d72fa2329b37937a02e44 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 2/2] 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 | 106 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c
index faa0be0b700..455a5c2191d 100644
--- a/libbacktrace/pecoff.c
+++ b/libbacktrace/pecoff.c
@@ -61,6 +61,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.  */
@@ -911,6 +939,53 @@ coff_add (struct backtrace_state *state, int descriptor,
   return 0;
 }
 
+#ifdef HAVE_WINDOWS_H
+struct dll_notification_context
+{
+  struct backtrace_state *state;
+  backtrace_error_callback error_callback;
+  void *data;
+};
+
+static 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
    sections.  */
@@ -935,6 +1010,8 @@ backtrace_initialize (struct backtrace_state *state,
 #endif
 
 #ifdef HAVE_WINDOWS_H
+  HMODULE nt_dll_handle;
+
   module_handle = (uintptr_t) GetModuleHandle (NULL);
 #endif
 
@@ -981,6 +1058,35 @@ backtrace_initialize (struct backtrace_state *state,
     }
 #endif
 
+#ifdef HAVE_WINDOWS_H
+  nt_dll_handle = GetModuleHandleW (L"ntdll.dll");
+  if (nt_dll_handle)
+    {
+      LDR_REGISTER_FUNCTION register_func;
+      const char register_name[] = "LdrRegisterDllNotification";
+      register_func = (void*) 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)
     {
       if (found_sym)
-- 
2.43.0