public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t
@ 2022-12-06 19:50 Björn Schäpers
  2022-12-06 19:50 ` [PATCH 2/4] libbacktrace: detect executable path on windows Björn Schäpers
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Björn Schäpers @ 2022-12-06 19:50 UTC (permalink / raw)
  To: gcc-patches, iant

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/4] libbacktrace: detect executable path on windows
  2022-12-06 19:50 [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t Björn Schäpers
@ 2022-12-06 19:50 ` Björn Schäpers
  2022-12-06 19:50 ` [PATCH 3/4] libbacktrace: work with aslr " Björn Schäpers
  2022-12-06 19:50 ` [PATCH 4/4] libbacktrace: get debug information for loaded dlls Björn Schäpers
  2 siblings, 0 replies; 9+ messages in thread
From: Björn Schäpers @ 2022-12-06 19:50 UTC (permalink / raw)
  To: gcc-patches, iant

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/configure.ac |  2 ++
 libbacktrace/fileline.c   | 43 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 60 insertions(+), 1 deletion(-)

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/4] libbacktrace: work with aslr on windows
  2022-12-06 19:50 [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t Björn Schäpers
  2022-12-06 19:50 ` [PATCH 2/4] libbacktrace: detect executable path on windows Björn Schäpers
@ 2022-12-06 19:50 ` Björn Schäpers
  2022-12-06 19:50 ` [PATCH 4/4] libbacktrace: get debug information for loaded dlls Björn Schäpers
  2 siblings, 0 replies; 9+ messages in thread
From: Björn Schäpers @ 2022-12-06 19:50 UTC (permalink / raw)
  To: gcc-patches, iant

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] libbacktrace: get debug information for loaded dlls
  2022-12-06 19:50 [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t Björn Schäpers
  2022-12-06 19:50 ` [PATCH 2/4] libbacktrace: detect executable path on windows Björn Schäpers
  2022-12-06 19:50 ` [PATCH 3/4] libbacktrace: work with aslr " Björn Schäpers
@ 2022-12-06 19:50 ` Björn Schäpers
  2 siblings, 0 replies; 9+ messages in thread
From: Björn Schäpers @ 2022-12-06 19:50 UTC (permalink / raw)
  To: gcc-patches, iant

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls
  2024-01-02 23:12     ` Björn Schäpers
@ 2024-01-23 21:24       ` Björn Schäpers
  0 siblings, 0 replies; 9+ messages in thread
From: Björn Schäpers @ 2024-01-23 21:24 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc-patches, gcc

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls
  2023-11-30 19:53   ` Ian Lance Taylor
  2023-11-30 20:16     ` Eli Zaretskii
@ 2024-01-02 23:12     ` Björn Schäpers
  2024-01-23 21:24       ` Björn Schäpers
  1 sibling, 1 reply; 9+ messages in thread
From: Björn Schäpers @ 2024-01-02 23:12 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc-patches, gcc

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls
  2023-11-30 19:53   ` Ian Lance Taylor
@ 2023-11-30 20:16     ` Eli Zaretskii
  2024-01-02 23:12     ` Björn Schäpers
  1 sibling, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2023-11-30 20:16 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc, gcc-patches, gcc

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls
  2023-01-20 10:54 ` [PATCH 4/4] libbacktrace: get debug information for loaded dlls Björn Schäpers
@ 2023-11-30 19:53   ` Ian Lance Taylor
  2023-11-30 20:16     ` Eli Zaretskii
  2024-01-02 23:12     ` Björn Schäpers
  0 siblings, 2 replies; 9+ messages in thread
From: Ian Lance Taylor @ 2023-11-30 19:53 UTC (permalink / raw)
  To: Björn Schäpers; +Cc: gcc-patches, gcc

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
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/4] libbacktrace: get debug information for loaded dlls
  2023-01-20 10:54 [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t Björn Schäpers
@ 2023-01-20 10:54 ` Björn Schäpers
  2023-11-30 19:53   ` Ian Lance Taylor
  0 siblings, 1 reply; 9+ messages in thread
From: Björn Schäpers @ 2023-01-20 10:54 UTC (permalink / raw)
  To: gcc-patches, gcc

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-01-23 21:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-06 19:50 [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t Björn Schäpers
2022-12-06 19:50 ` [PATCH 2/4] libbacktrace: detect executable path on windows Björn Schäpers
2022-12-06 19:50 ` [PATCH 3/4] libbacktrace: work with aslr " Björn Schäpers
2022-12-06 19:50 ` [PATCH 4/4] libbacktrace: get debug information for loaded dlls Björn Schäpers
2023-01-20 10:54 [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t Björn Schäpers
2023-01-20 10:54 ` [PATCH 4/4] libbacktrace: get debug information for loaded dlls Björn Schäpers
2023-11-30 19:53   ` Ian Lance Taylor
2023-11-30 20:16     ` Eli Zaretskii
2024-01-02 23:12     ` Björn Schäpers
2024-01-23 21:24       ` Björn Schäpers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).