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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ messages in thread

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-11-20 19:57                 ` Björn Schäpers
  2023-11-20 20:07                   ` Eli Zaretskii
@ 2023-11-30 19:25                   ` Ian Lance Taylor
  1 sibling, 0 replies; 19+ messages in thread
From: Ian Lance Taylor @ 2023-11-30 19:25 UTC (permalink / raw)
  To: Björn Schäpers; +Cc: Eli Zaretskii, Gabriel Ravier, gcc-patches, gcc

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

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-11-21 19:35                     ` Björn Schäpers
@ 2023-11-22  1:13                       ` LIU Hao
  0 siblings, 0 replies; 19+ messages in thread
From: LIU Hao @ 2023-11-22  1:13 UTC (permalink / raw)
  To: Björn Schäpers, Eli Zaretskii; +Cc: gabravier, gcc-patches, gcc


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

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-11-20 20:07                   ` Eli Zaretskii
@ 2023-11-21 19:35                     ` Björn Schäpers
  2023-11-22  1:13                       ` LIU Hao
  0 siblings, 1 reply; 19+ messages in thread
From: Björn Schäpers @ 2023-11-21 19:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gabravier, gcc-patches, gcc

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


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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-11-20 19:57                 ` Björn Schäpers
@ 2023-11-20 20:07                   ` Eli Zaretskii
  2023-11-21 19:35                     ` Björn Schäpers
  2023-11-30 19:25                   ` Ian Lance Taylor
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-11-20 20:07 UTC (permalink / raw)
  To: Björn Schäpers; +Cc: gabravier, gcc-patches, gcc

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

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-21 11:42               ` Eli Zaretskii
@ 2023-11-20 19:57                 ` Björn Schäpers
  2023-11-20 20:07                   ` Eli Zaretskii
  2023-11-30 19:25                   ` Ian Lance Taylor
  0 siblings, 2 replies; 19+ messages in thread
From: Björn Schäpers @ 2023-11-20 19:57 UTC (permalink / raw)
  To: Eli Zaretskii, Gabriel Ravier; +Cc: gcc-patches, gcc

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


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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-21 10:47             ` Gabriel Ravier
@ 2023-01-21 11:42               ` Eli Zaretskii
  2023-11-20 19:57                 ` Björn Schäpers
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-01-21 11:42 UTC (permalink / raw)
  To: Gabriel Ravier; +Cc: gcc, gcc-patches, gcc

> 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

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-21  4:05           ` Eli Zaretskii
  2023-01-21  9:18             ` LIU Hao
@ 2023-01-21 10:47             ` Gabriel Ravier
  2023-01-21 11:42               ` Eli Zaretskii
  1 sibling, 1 reply; 19+ messages in thread
From: Gabriel Ravier @ 2023-01-21 10:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gcc, gcc-patches, gcc


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.

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-21  9:18             ` LIU Hao
@ 2023-01-21  9:25               ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2023-01-21  9:25 UTC (permalink / raw)
  To: LIU Hao; +Cc: gabravier, gcc, gcc-patches, gcc

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

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-21  4:05           ` Eli Zaretskii
@ 2023-01-21  9:18             ` LIU Hao
  2023-01-21  9:25               ` Eli Zaretskii
  2023-01-21 10:47             ` Gabriel Ravier
  1 sibling, 1 reply; 19+ messages in thread
From: LIU Hao @ 2023-01-21  9:18 UTC (permalink / raw)
  To: Eli Zaretskii, Gabriel Ravier; +Cc: gcc, gcc-patches, gcc


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

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-20 20:39         ` Gabriel Ravier
@ 2023-01-21  4:05           ` Eli Zaretskii
  2023-01-21  9:18             ` LIU Hao
  2023-01-21 10:47             ` Gabriel Ravier
  0 siblings, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2023-01-21  4:05 UTC (permalink / raw)
  To: Gabriel Ravier; +Cc: gcc, gcc-patches, gcc

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

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-20 19:19       ` Eli Zaretskii
@ 2023-01-20 20:39         ` Gabriel Ravier
  2023-01-21  4:05           ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Gabriel Ravier @ 2023-01-20 20:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gcc, gcc-patches, gcc

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)


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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-20 16:46     ` Gabriel Ravier
@ 2023-01-20 19:19       ` Eli Zaretskii
  2023-01-20 20:39         ` Gabriel Ravier
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-01-20 19:19 UTC (permalink / raw)
  To: Gabriel Ravier; +Cc: gcc, gcc-patches, gcc

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

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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-20 13:39   ` Eli Zaretskii
@ 2023-01-20 16:46     ` Gabriel Ravier
  2023-01-20 19:19       ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Gabriel Ravier @ 2023-01-20 16:46 UTC (permalink / raw)
  To: Eli Zaretskii, Björn Schäpers; +Cc: gcc-patches, gcc

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


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

* Re: [PATCH 3/4] libbacktrace: work with aslr on windows
  2023-01-20 10:54 ` [PATCH 3/4] libbacktrace: work with aslr on windows Björn Schäpers
@ 2023-01-20 13:39   ` Eli Zaretskii
  2023-01-20 16:46     ` Gabriel Ravier
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-01-20 13:39 UTC (permalink / raw)
  To: Björn Schäpers; +Cc: gcc-patches, gcc

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

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

* [PATCH 3/4] libbacktrace: work with aslr on windows
  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-01-20 13:39   ` Eli Zaretskii
  0 siblings, 1 reply; 19+ 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>

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] 19+ messages in thread

end of thread, other threads:[~2023-11-30 19:25 UTC | newest]

Thread overview: 19+ 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 3/4] libbacktrace: work with aslr on windows Björn Schäpers
2023-01-20 13:39   ` Eli Zaretskii
2023-01-20 16:46     ` Gabriel Ravier
2023-01-20 19:19       ` Eli Zaretskii
2023-01-20 20:39         ` Gabriel Ravier
2023-01-21  4:05           ` Eli Zaretskii
2023-01-21  9:18             ` LIU Hao
2023-01-21  9:25               ` Eli Zaretskii
2023-01-21 10:47             ` Gabriel Ravier
2023-01-21 11:42               ` Eli Zaretskii
2023-11-20 19:57                 ` Björn Schäpers
2023-11-20 20:07                   ` Eli Zaretskii
2023-11-21 19:35                     ` Björn Schäpers
2023-11-22  1:13                       ` LIU Hao
2023-11-30 19:25                   ` Ian Lance Taylor

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