[PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t
From: Björn Schäpers 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 --- 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_section
[PATCH 4/4] libbacktrace: get debug information for loaded dlls
From: Björn Schäpers 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 --- 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 +#include #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
[PATCH 3/4] libbacktrace: work with aslr on windows
From: Björn Schäpers 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 --- 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 +#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 2/4] libbacktrace: detect executable path on windows
From: Björn Schäpers This is actually needed so that libstdc++'s 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 --- 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 header file. */ #undef HAVE_UNISTD_H +/* Define to 1 if you have the 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 #endif +#ifdef HAVE_WINDOWS_H +#ifndef WIN32_MEAN_AND_LEAN +#define WIN32_MEAN_AND_LEAN +#endif + +#ifndef NOMINMAX +#define NOMINMAX +#endif + +#include +#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
Re: [PATCH 1/4] libbacktrace: change all pc related variables to uintptr_t
Am 20.01.2023 um 23:25 schrieb Ian Lance Taylor: On Fri, Jan 20, 2023 at 2:54 AM Björn Schäpers wrote: From: Björn Schäpers 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
Am 24.01.2023 um 17:52 schrieb Eli Zaretskii: From: Ian Lance Taylor Date: Tue, 24 Jan 2023 06:35:21 -0800 Cc: g...@hazardy.de, gcc-patc...@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
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 wrote: From: Ian Lance Taylor Date: Tue, 24 Jan 2023 09:58:10 -0800 Cc: g...@hazardy.de, gcc-patc...@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
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 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 wrote: From: Ian Lance Taylor Date: Tue, 24 Jan 2023 09:58:10 -0800 Cc: g...@hazardy.de, gcc-patc...@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 From e0ee58b71f726606205aa1f0168a724859162c21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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 --- 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 a4f5bf6..ee2616335c7 100644 --- a/libbacktrace/config.h.in +++ b/libbacktrace/config.h.in @@ -104,6 +104,9 @@ /* Define to 1 if you have the header file. */ #undef HAVE_UNISTD_H +/* Define to 1 if you have the 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 #endif +#ifdef HAVE_WINDOWS_H +#ifndef WIN32_MEAN_AND_LEAN +#define WIN32_MEAN_AND_LEAN +#endif + +#ifndef NOMINMAX +#define NOMINMAX +#endif + +#include +#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
Re: [PATCH 3/4] libbacktrace: work with aslr on windows
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: g...@hazardy.de, gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org From: Gabriel Ravier On 1/21/23 05:05, Eli Zaretskii wrote: Date: Fri, 20 Jan 2023 21:39:56 +0100 Cc: g...@hazardy.de, gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org From: Gabriel Ravier - 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 From 52cbe06b1c165172191f66ff7e55a49adecf661d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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 --- 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 +#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
I'll guess it is not needed here, but otherwise 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 . Am 20.11.2023 um 21:07 schrieb Eli Zaretskii: Date: Mon, 20 Nov 2023 20:57:38 +0100 Cc: gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org From: Björn Schäpers +#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 4/4] libbacktrace: get debug information for loaded dlls
Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers wrote: From: Björn Schäpers 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 ")? Thanks and kind regards, Björn. From bd552716ee7937cad9d54d4966532d6ea6dbc1bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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 --- 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 +#include #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
[PATCH 5/4] libbacktrace: improve getting debug information for loaded dlls
Am 03.01.2024 um 00:12 schrieb Björn Schäpers: Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers wrote: From: Björn Schäpers 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 ")? Thanks and kind regards, Björn. I noticed that under 64 bit libraries loaded with LoadLibrary were missing. EnumProcessModules stated the correct number of modules, but did not fill the the HMODULEs, but set them to 0. While trying to investigate I noticed if I do the very same thing from main (in C++) I even got fewer module HMODULEs. So I went a different way. This detects all libraries correctly, in 32 and 64 bit. The question is, if it should be a patch on top of the previous, or should they be merged, or even only this solution and drop the EnumProcessModules variant? Kind regards, Björn. From 784e01f1baf92c23c819aeb9e77010412023700f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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 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 header file. */ #undef HAVE_SYS_TYPES_H +/* Define to 1 if you have the header file. */ +#undef HAVE_TLHELP32_H + /* Define to 1 if you have the 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 to if __STDC__ is defined, since 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
Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
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 wrote: From: Björn Schäpers 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 ")? Thanks and kind regards, Björn. I noticed that under 64 bit libraries loaded with LoadLibrary were missing. EnumProcessModules stated the correct number of modules, but did not fill the the HMODULEs, but set them to 0. While trying to investigate I noticed if I do the very same thing from main (in C++) I even got fewer module HMODULEs. So I went a different way. This detects all libraries correctly, in 32 and 64 bit. The question is, if it should be a patch on top of the previous, or should they be merged, or even only this solution and drop the EnumProcessModules variant? Kind regards, Björn. This patch adds libraries which are loaded after backtrace_initialize, like plugins or similar. I don't know what style is preferred for the Win32 typedefs, should the code use PVOID or void*? And I'm not sure I wrapped every long line correctly. Kind regards, Björn.From 02e76e727b95dc21dc07d1fe8424b68e37e91a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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)) +
Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
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 Am 07.01.2024 um 07:50 schrieb Eli Zaretskii: Date: Sat, 6 Jan 2024 23:15:24 +0100 From: Björn Schäpers Cc: gcc-patc...@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
Am 07.01.2024 um 18:03 schrieb Eli Zaretskii: Date: Sun, 7 Jan 2024 17:07:06 +0100 Cc: i...@google.com, gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org From: Björn Schäpers That was about GetModuleHandle, not about GetModuleHandleEx. For the latter, all Windows versions that support it also support "wide" APIs. So my suggestion is to use GetModuleHandleExW here. However, you will need to make sure that notification_data->dll_base is declared as 'wchar_t *', not 'char *'. If dll_base is declared as 'char *', then only GetModuleHandleExA will work, and you will lose the ability to support file names with non-ASCII characters outside of the current system codepage. The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag GetModuleHandleEx does not look for a name, but uses an adress in the module to get the HMODULE, so you cast it to char* or wchar_t* depending on which function you call. Actually one could just cast the dll_base to HMODULE, at least in win32 on x86 the HMODULE of a dll is always its base adress. But to make it safer and future proof I went the way through GetModuleHandeEx. In that case, you an call either GetModuleHandeExA or GetModuleHandeExW, the difference is minor. Here an updated version without relying on TEXT or TCHAR, directly calling GetModuleHandleExW. Kind regards, Björn.From a8e1e64ccb56158ec8a7e5de0d5228f3f6f7e5c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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
Re: [PATCH 4/4] libbacktrace: get debug information for loaded dlls
Am 03.01.2024 um 00:12 schrieb Björn Schäpers: Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers wrote: From: Björn Schäpers 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 ")? Thanks and kind regards, Björn. A friendly ping.
Re: [PATCH 5/4] libbacktrace: improve getting debug information for loaded dlls
Am 23.01.2024 um 23:37 schrieb Ian Lance Taylor: On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers 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 wrote: From: Björn Schäpers 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 ")? 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
Am 25.01.2024 um 23:04 schrieb Ian Lance Taylor: On Thu, Jan 25, 2024 at 11:53 AM Björn Schäpers 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 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 wrote: From: Björn Schäpers 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 ")? 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.From 33a6380feff66e32ef0f0d7cbad6fb55319f4e2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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 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 --- 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 header file. */ #undef HAVE_SYS_TYPES_H +/* Define to 1 if you have the header file. */ +#undef HAVE_TLHELP32_H + /* Define to 1 if you have the 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 to if __STDC__ is defined, since 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 -m
Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
Am 10.01.2024 um 13:34 schrieb Eli Zaretskii: Date: Tue, 9 Jan 2024 21:02:44 +0100 Cc: i...@google.com, gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org From: Björn Schäpers 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. From 55d0f312c0a9c4e2305d72fa2329b37937a02e44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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_conte
Re: [PATCH 5/4] libbacktrace: improve getting debug information for loaded dlls
Am 15.03.2024 um 21:40 schrieb Björn Schäpers: Am 25.01.2024 um 23:04 schrieb Ian Lance Taylor: On Thu, Jan 25, 2024 at 11:53 AM Björn Schäpers 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 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 wrote: From: Björn Schäpers 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 ")? 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. A friendly ping.
Re: [PATCH 5/4] libbacktrace: improve getting debug information for loaded dlls
Am 28.04.2024 um 20:16 schrieb Ian Lance Taylor: On Thu, Apr 25, 2024 at 1:15 PM Björn Schäpers wrote: 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. A friendly ping. Thanks. Committed as follows. Which of your other patches are still relevant? Thanks. Ian Hi, only this one. Kind regards, Björn.From 55d0f312c0a9c4e2305d72fa2329b37937a02e44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= 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
Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize
Am 29.07.2024 um 19:58 schrieb Eli Zaretskii: From: Ian Lance Taylor Date: Mon, 29 Jul 2024 09:46:46 -0700 Cc: Eli Zaretskii , gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org On Fri, Mar 15, 2024 at 1:41 PM Björn Schäpers wrote: Am 10.01.2024 um 13:34 schrieb Eli Zaretskii: Date: Tue, 9 Jan 2024 21:02:44 +0100 Cc: i...@google.com, gcc-patc...@gcc.gnu.org, gcc@gcc.gnu.org From: Björn Schäpers 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. This bug report was filed about this patch: https://github.com/ianlancetaylor/libbacktrace/issues/131 src\pecoff.c(86): error C2059: syntax error: '(' src\pecoff.c(89): error C2059: syntax error: '(' It works fine if deleting CALLBACK and NTAPI. Any ideas? Instead of deleting those, move them inside the parentheses: typedef VOID (CALLBACK *LDR_DLL_NOTIFICATION)(ULONG, struct dll_notification_data*, PVOID); typedef NTSTATUS (NTAPI *LDR_REGISTER_FUNCTION)(ULONG, LDR_DLL_NOTIFICATION, PVOID, PVOID*); and also I think you need to include , for the definition of the NTSTATUS type. Caveat: I don't have MSVC, so I couldn't verify that these measures fix the problem, sorry. Moving into the parentheses does fix the issue: https://godbolt.org/z/Pe558ofYz NTSTATUS is typedefed directly before, so that no additional include is needed.