| Message ID | d8ae8f6f-97c3-4c93-b253-db8d653b560e@hazardy.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] libbacktrace: change all pc related variables to uintptr_t | expand |
Am 04.01.2024 um 23:33 schrieb Björn Schäpers: > Am 03.01.2024 um 00:12 schrieb Björn Schäpers: >> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: >>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote: >>>> >>>> From: Björn Schäpers <bjoern@hazardy.de> >>>> >>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except >>>> that libraries loaded after the backtrace_initialize are not handled. >>>> But as far as I can see that's the same for elf. >>> >>> Thanks, but I don't want a patch that loops using goto statements. >>> Please rewrite to avoid that. It may be simpler to call a function. >>> >>> Also starting with a module count of 1000 seems like a lot. Do >>> typical Windows programs load that many modules? >>> >>> Ian >>> >>> >> >> Rewritten using a function. >> >> If that is commited, could you attribute that commit to me (--author="Björn >> Schäpers <bjoern@hazardy.de>")? >> >> Thanks and kind regards, >> Björn. > > I noticed that under 64 bit libraries loaded with LoadLibrary were missing. > EnumProcessModules stated the correct number of modules, but did not fill the > the HMODULEs, but set them to 0. While trying to investigate I noticed if I do > the very same thing from main (in C++) I even got fewer module HMODULEs. > > So I went a different way. This detects all libraries correctly, in 32 and 64 > bit. The question is, if it should be a patch on top of the previous, or should > they be merged, or even only this solution and drop the EnumProcessModules variant? > > Kind regards, > Björn. This patch adds libraries which are loaded after backtrace_initialize, like plugins or similar. I don't know what style is preferred for the Win32 typedefs, should the code use PVOID or void*? And I'm not sure I wrapped every long line correctly. Kind regards, Björn. From 02e76e727b95dc21dc07d1fe8424b68e37e91a83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de> Date: Sat, 6 Jan 2024 22:53:54 +0100 Subject: [PATCH 3/3] libbacktrace: Add loaded dlls after initialize libbacktrace/Changelog: * pecoff.c [HAVE_WINDOWS_H]: (dll_notification_data): Added (dll_notification_context): Added (dll_notification): Added (backtrace_initialize): Use LdrRegisterDllNotification to load debug information of later loaded dlls. --- libbacktrace/pecoff.c | 102 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c index 647baa39640..bfe12cc2a0a 100644 --- a/libbacktrace/pecoff.c +++ b/libbacktrace/pecoff.c @@ -62,6 +62,32 @@ POSSIBILITY OF SUCH DAMAGE. */ #undef Module32Next #endif #endif + +#if defined(_ARM_) +#define NTAPI +#else +#define NTAPI __stdcall +#endif + +/* This is a simplified (but binary compatible) version of what Microsoft + defines in their documentation. */ +struct dll_notifcation_data +{ + ULONG reserved; + /* The name as UNICODE_STRING struct. */ + PVOID full_dll_name; + PVOID base_dll_name; + PVOID dll_base; + ULONG size_of_image; +}; + +typedef LONG NTSTATUS; +typedef VOID CALLBACK (*LDR_DLL_NOTIFICATION)(ULONG, + struct dll_notifcation_data*, + PVOID); +typedef NTSTATUS NTAPI (*LDR_REGISTER_FUNCTION)(ULONG, + LDR_DLL_NOTIFICATION, PVOID, + PVOID*); #endif /* Coff file header. */ @@ -912,7 +938,8 @@ coff_add (struct backtrace_state *state, int descriptor, return 0; } -#if defined(HAVE_WINDOWS_H) && !defined(HAVE_TLHELP32_H) +#ifdef HAVE_WINDOWS_H +#ifndef HAVE_TLHELP32_H static void free_modules (struct backtrace_state *state, backtrace_error_callback error_callback, void *data, @@ -958,6 +985,51 @@ get_all_modules (struct backtrace_state *state, } } #endif +struct dll_notification_context +{ + struct backtrace_state *state; + backtrace_error_callback error_callback; + void *data; +}; + +VOID CALLBACK +dll_notification (ULONG reason, +struct dll_notifcation_data *notification_data, +PVOID context) +{ + char module_name[MAX_PATH]; + int descriptor; + struct dll_notification_context* dll_context = + (struct dll_notification_context*) context; + struct backtrace_state *state = dll_context->state; + void *data = dll_context->data; + backtrace_error_callback error_callback = dll_context->data; + fileline fileline; + int found_sym; + int found_dwarf; + HMODULE module_handle; + + if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1) + return; + + if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + (TCHAR*) notification_data->dll_base, + &module_handle)) + return; + + if (!GetModuleFileNameA ((HMODULE) module_handle, module_name, MAX_PATH - 1)) + return; + + descriptor = backtrace_open (module_name, error_callback, data, NULL); + + if (descriptor < 0) + return; + + coff_add (state, descriptor, error_callback, data, &fileline, &found_sym, + &found_dwarf, (uintptr_t) module_handle); +} +#endif /* Initialize the backtrace data we need from an ELF executable. At the ELF level, all we need to do is find the debug info @@ -978,6 +1050,7 @@ backtrace_initialize (struct backtrace_state *state, #ifdef HAVE_WINDOWS_H fileline module_fileline_fn; int module_found_sym; + HMODULE nt_dll_handle; #ifdef HAVE_TLHELP32_H HANDLE snapshot; @@ -1065,6 +1138,33 @@ backtrace_initialize (struct backtrace_state *state, if (modules) free_modules (state, error_callback, data, &modules, bytes_allocated_for_modules); #endif + + nt_dll_handle = GetModuleHandle (TEXT ("ntdll.dll")); + if (nt_dll_handle) + { + LDR_REGISTER_FUNCTION register_func; + const char register_name[] = "LdrRegisterDllNotification"; + register_func = (LDR_REGISTER_FUNCTION) GetProcAddress (nt_dll_handle, + register_name); + + if (register_func) + { + PVOID cookie; + struct dll_notification_context *context + = backtrace_alloc (state, + sizeof(struct dll_notification_context), + error_callback, data); + + if (context) + { + context->state = state; + context->data = data; + context->error_callback = error_callback; + + register_func (0, &dll_notification, context, &cookie); + } + } + } #endif if (!state->threaded)
> Date: Sat, 6 Jan 2024 23:15:24 +0100 > From: Björn Schäpers <gcc@hazardy.de> > Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org > > This patch adds libraries which are loaded after backtrace_initialize, like > plugins or similar. > > I don't know what style is preferred for the Win32 typedefs, should the code use > PVOID or void*? It doesn't matter, at least not if the source file includes the Windows header files (where PVOID is defined). > + if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1) IMO, it would be better to supply a #define if undefined: #ifndef LDR_DLL_NOTIFICATION_REASON_LOADED # define LDR_DLL_NOTIFICATION_REASON_LOADED 1 #endif > + if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS > + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, > + (TCHAR*) notification_data->dll_base, Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies on compile-time definition of UNICODE? (I'm not familiar with the internals of libbacktrace, so apologies if this is a silly question.) Thanks.
[I re-added the other addressees, as I don' think you meant to make this discussion private between the two of us.] > Date: Sun, 7 Jan 2024 12:58:29 +0100 > From: Björn Schäpers <gcc@hazardy.de> > > Am 07.01.2024 um 07:50 schrieb Eli Zaretskii: > >> Date: Sat, 6 Jan 2024 23:15:24 +0100 > >> From: Björn Schäpers <gcc@hazardy.de> > >> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org > >> > >> This patch adds libraries which are loaded after backtrace_initialize, like > >> plugins or similar. > >> > >> I don't know what style is preferred for the Win32 typedefs, should the code use > >> PVOID or void*? > > > > It doesn't matter, at least not if the source file includes the > > Windows header files (where PVOID is defined). > > > >> + if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1) > > > > IMO, it would be better to supply a #define if undefined: > > > > #ifndef LDR_DLL_NOTIFICATION_REASON_LOADED > > # define LDR_DLL_NOTIFICATION_REASON_LOADED 1 > > #endif > > > > I surely can define it. But the ifndef is not needed, since there are no headers > containing the function signatures, structures or the defines: > https://learn.microsoft.com/en-us/windows/win32/devnotes/ldrregisterdllnotification OK, I wasn't sure about that. > >> + if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS > >> + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, > >> + (TCHAR*) notification_data->dll_base, > > > > Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies > > on compile-time definition of UNICODE? (I'm not familiar with the > > internals of libbacktrace, so apologies if this is a silly question.) > > > > Thanks. > > As far as I can see it's the first time for TCHAR, I would've gone for > GetModuleHandleExW, but https://gcc.gnu.org/pipermail/gcc/2023-January/240534.html That was about GetModuleHandle, not about GetModuleHandleEx. For the latter, all Windows versions that support it also support "wide" APIs. So my suggestion is to use GetModuleHandleExW here. However, you will need to make sure that notification_data->dll_base is declared as 'wchar_t *', not 'char *'. If dll_base is declared as 'char *', then only GetModuleHandleExA will work, and you will lose the ability to support file names with non-ASCII characters outside of the current system codepage. > But I didn't want to force GetModuleHandleExA, so I went for TCHAR and > GetModuleHandleEx so it automatically chooses which to use. Same for > GetModuleHandle of ntdll.dll. The considerations for GetModuleHandle and for GetModuleHandleEx are different: the former is also available on old versions of Windows that doesn't support "wide" APIs.
Am 07.01.2024 um 15:46 schrieb Eli Zaretskii: > [I re-added the other addressees, as I don' think you meant to make > this discussion private between the two of us.] > Yeah, that was a mistake. >> Date: Sun, 7 Jan 2024 12:58:29 +0100 >> From: Björn Schäpers <gcc@hazardy.de> >> >> Am 07.01.2024 um 07:50 schrieb Eli Zaretskii: >>>> Date: Sat, 6 Jan 2024 23:15:24 +0100 >>>> From: Björn Schäpers <gcc@hazardy.de> >>>> Cc: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org >>>> >>>> This patch adds libraries which are loaded after backtrace_initialize, like >>>> plugins or similar. >>>> >>>> I don't know what style is preferred for the Win32 typedefs, should the code use >>>> PVOID or void*? >>> >>> It doesn't matter, at least not if the source file includes the >>> Windows header files (where PVOID is defined). >>> >>>> + if (reason != /*LDR_DLL_NOTIFICATION_REASON_LOADED*/1) >>> >>> IMO, it would be better to supply a #define if undefined: >>> >>> #ifndef LDR_DLL_NOTIFICATION_REASON_LOADED >>> # define LDR_DLL_NOTIFICATION_REASON_LOADED 1 >>> #endif >>> >> >> I surely can define it. But the ifndef is not needed, since there are no headers >> containing the function signatures, structures or the defines: >> https://learn.microsoft.com/en-us/windows/win32/devnotes/ldrregisterdllnotification > > OK, I wasn't sure about that. > >>>> + if (!GetModuleHandleEx (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS >>>> + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, >>>> + (TCHAR*) notification_data->dll_base, >>> >>> Is TCHAR correct here? does libbacktrace indeed use TCHAR and relies >>> on compile-time definition of UNICODE? (I'm not familiar with the >>> internals of libbacktrace, so apologies if this is a silly question.) >>> >>> Thanks. >> >> As far as I can see it's the first time for TCHAR, I would've gone for >> GetModuleHandleExW, but https://gcc.gnu.org/pipermail/gcc/2023-January/240534.html > > That was about GetModuleHandle, not about GetModuleHandleEx. For the > latter, all Windows versions that support it also support "wide" APIs. > So my suggestion is to use GetModuleHandleExW here. However, you will > need to make sure that notification_data->dll_base is declared as > 'wchar_t *', not 'char *'. If dll_base is declared as 'char *', then > only GetModuleHandleExA will work, and you will lose the ability to > support file names with non-ASCII characters outside of the current > system codepage. The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag GetModuleHandleEx does not look for a name, but uses an adress in the module to get the HMODULE, so you cast it to char* or wchar_t* depending on which function you call. Actually one could just cast the dll_base to HMODULE, at least in win32 on x86 the HMODULE of a dll is always its base adress. But to make it safer and future proof I went the way through GetModuleHandeEx. > >> But I didn't want to force GetModuleHandleExA, so I went for TCHAR and >> GetModuleHandleEx so it automatically chooses which to use. Same for >> GetModuleHandle of ntdll.dll. > > The considerations for GetModuleHandle and for GetModuleHandleEx are > different: the former is also available on old versions of Windows > that doesn't support "wide" APIs.
> Date: Sun, 7 Jan 2024 17:07:06 +0100 > Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org > From: Björn Schäpers <gcc@hazardy.de> > > > That was about GetModuleHandle, not about GetModuleHandleEx. For the > > latter, all Windows versions that support it also support "wide" APIs. > > So my suggestion is to use GetModuleHandleExW here. However, you will > > need to make sure that notification_data->dll_base is declared as > > 'wchar_t *', not 'char *'. If dll_base is declared as 'char *', then > > only GetModuleHandleExA will work, and you will lose the ability to > > support file names with non-ASCII characters outside of the current > > system codepage. > > The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag > GetModuleHandleEx does not look for a name, but uses an adress in the module to > get the HMODULE, so you cast it to char* or wchar_t* depending on which function > you call. Actually one could just cast the dll_base to HMODULE, at least in > win32 on x86 the HMODULE of a dll is always its base adress. But to make it > safer and future proof I went the way through GetModuleHandeEx. In that case, you an call either GetModuleHandeExA or GetModuleHandeExW, the difference is minor.
Am 07.01.2024 um 18:03 schrieb Eli Zaretskii: >> Date: Sun, 7 Jan 2024 17:07:06 +0100 >> Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org >> From: Björn Schäpers <gcc@hazardy.de> >> >>> That was about GetModuleHandle, not about GetModuleHandleEx. For the >>> latter, all Windows versions that support it also support "wide" APIs. >>> So my suggestion is to use GetModuleHandleExW here. However, you will >>> need to make sure that notification_data->dll_base is declared as >>> 'wchar_t *', not 'char *'. If dll_base is declared as 'char *', then >>> only GetModuleHandleExA will work, and you will lose the ability to >>> support file names with non-ASCII characters outside of the current >>> system codepage. >> >> The dll_base is a PVOID. With the GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS flag >> GetModuleHandleEx does not look for a name, but uses an adress in the module to >> get the HMODULE, so you cast it to char* or wchar_t* depending on which function >> you call. Actually one could just cast the dll_base to HMODULE, at least in >> win32 on x86 the HMODULE of a dll is always its base adress. But to make it >> safer and future proof I went the way through GetModuleHandeEx. > > In that case, you an call either GetModuleHandeExA or > GetModuleHandeExW, the difference is minor. Here an updated version without relying on TEXT or TCHAR, directly calling GetModuleHandleExW. Kind regards, Björn. From a8e1e64ccb56158ec8a7e5de0d5228f3f6f7e5c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de> Date: Sat, 6 Jan 2024 22:53:54 +0100 Subject: [PATCH 3/3] libbacktrace: Add loaded dlls after initialize libbacktrace/Changelog: * pecoff.c [HAVE_WINDOWS_H]: (dll_notification_data): Added (dll_notification_context): Added (dll_notification): Added (backtrace_initialize): Use LdrRegisterDllNotification to load debug information of later loaded dlls. --- libbacktrace/pecoff.c | 104 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-) diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c index 647baa39640..d973a26f05a 100644 --- a/libbacktrace/pecoff.c +++ b/libbacktrace/pecoff.c @@ -62,6 +62,34 @@ POSSIBILITY OF SUCH DAMAGE. */ #undef Module32Next #endif #endif + +#if defined(_ARM_) +#define NTAPI +#else +#define NTAPI __stdcall +#endif + +/* This is a simplified (but binary compatible) version of what Microsoft + defines in their documentation. */ +struct dll_notifcation_data +{ + ULONG reserved; + /* The name as UNICODE_STRING struct. */ + PVOID full_dll_name; + PVOID base_dll_name; + PVOID dll_base; + ULONG size_of_image; +}; + +#define LDR_DLL_NOTIFICATION_REASON_LOADED 1 + +typedef LONG NTSTATUS; +typedef VOID CALLBACK (*LDR_DLL_NOTIFICATION)(ULONG, + struct dll_notifcation_data*, + PVOID); +typedef NTSTATUS NTAPI (*LDR_REGISTER_FUNCTION)(ULONG, + LDR_DLL_NOTIFICATION, PVOID, + PVOID*); #endif /* Coff file header. */ @@ -912,7 +940,8 @@ coff_add (struct backtrace_state *state, int descriptor, return 0; } -#if defined(HAVE_WINDOWS_H) && !defined(HAVE_TLHELP32_H) +#ifdef HAVE_WINDOWS_H +#ifndef HAVE_TLHELP32_H static void free_modules (struct backtrace_state *state, backtrace_error_callback error_callback, void *data, @@ -958,6 +987,51 @@ get_all_modules (struct backtrace_state *state, } } #endif +struct dll_notification_context +{ + struct backtrace_state *state; + backtrace_error_callback error_callback; + void *data; +}; + +VOID CALLBACK +dll_notification (ULONG reason, +struct dll_notifcation_data *notification_data, +PVOID context) +{ + char module_name[MAX_PATH]; + int descriptor; + struct dll_notification_context* dll_context = + (struct dll_notification_context*) context; + struct backtrace_state *state = dll_context->state; + void *data = dll_context->data; + backtrace_error_callback error_callback = dll_context->data; + fileline fileline; + int found_sym; + int found_dwarf; + HMODULE module_handle; + + if (reason != LDR_DLL_NOTIFICATION_REASON_LOADED) + return; + + if (!GetModuleHandleExW (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + (wchar_t*) notification_data->dll_base, + &module_handle)) + return; + + if (!GetModuleFileNameA ((HMODULE) module_handle, module_name, MAX_PATH - 1)) + return; + + descriptor = backtrace_open (module_name, error_callback, data, NULL); + + if (descriptor < 0) + return; + + coff_add (state, descriptor, error_callback, data, &fileline, &found_sym, + &found_dwarf, (uintptr_t) module_handle); +} +#endif /* Initialize the backtrace data we need from an ELF executable. At the ELF level, all we need to do is find the debug info @@ -978,6 +1052,7 @@ backtrace_initialize (struct backtrace_state *state, #ifdef HAVE_WINDOWS_H fileline module_fileline_fn; int module_found_sym; + HMODULE nt_dll_handle; #ifdef HAVE_TLHELP32_H HANDLE snapshot; @@ -1065,6 +1140,33 @@ backtrace_initialize (struct backtrace_state *state, if (modules) free_modules (state, error_callback, data, &modules, bytes_allocated_for_modules); #endif + + nt_dll_handle = GetModuleHandleW (L"ntdll.dll"); + if (nt_dll_handle) + { + LDR_REGISTER_FUNCTION register_func; + const char register_name[] = "LdrRegisterDllNotification"; + register_func = (LDR_REGISTER_FUNCTION) GetProcAddress (nt_dll_handle, + register_name); + + if (register_func) + { + PVOID cookie; + struct dll_notification_context *context + = backtrace_alloc (state, + sizeof(struct dll_notification_context), + error_callback, data); + + if (context) + { + context->state = state; + context->data = data; + context->error_callback = error_callback; + + register_func (0, &dll_notification, context, &cookie); + } + } + } #endif if (!state->threaded)
> Date: Tue, 9 Jan 2024 21:02:44 +0100 > Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org > From: Björn Schäpers <gcc@hazardy.de> > > Am 07.01.2024 um 18:03 schrieb Eli Zaretskii: > > In that case, you an call either GetModuleHandeExA or > > GetModuleHandeExW, the difference is minor. > > Here an updated version without relying on TEXT or TCHAR, directly calling > GetModuleHandleExW. Thanks, this LGTM (but I couldn't test it, I just looked at the sour ce code).
On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers <gcc@hazardy.de> wrote: > > Am 03.01.2024 um 00:12 schrieb Björn Schäpers: > > Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: > >> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote: > >>> > >>> From: Björn Schäpers <bjoern@hazardy.de> > >>> > >>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except > >>> that libraries loaded after the backtrace_initialize are not handled. > >>> But as far as I can see that's the same for elf. > >> > >> Thanks, but I don't want a patch that loops using goto statements. > >> Please rewrite to avoid that. It may be simpler to call a function. > >> > >> Also starting with a module count of 1000 seems like a lot. Do > >> typical Windows programs load that many modules? > >> > >> Ian > >> > >> > > > > Rewritten using a function. > > > > If that is commited, could you attribute that commit to me (--author="Björn > > Schäpers <bjoern@hazardy.de>")? > > > > Thanks and kind regards, > > Björn. > > I noticed that under 64 bit libraries loaded with LoadLibrary were missing. > EnumProcessModules stated the correct number of modules, but did not fill the > the HMODULEs, but set them to 0. While trying to investigate I noticed if I do > the very same thing from main (in C++) I even got fewer module HMODULEs. > > So I went a different way. This detects all libraries correctly, in 32 and 64 > bit. The question is, if it should be a patch on top of the previous, or should > they be merged, or even only this solution and drop the EnumProcessModules variant? Is there any reason to use both patches? Seems simpler to just use this one if it works. Thanks. Ian
Am 23.01.2024 um 23:37 schrieb Ian Lance Taylor: > On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers <gcc@hazardy.de> wrote: >> >> Am 03.01.2024 um 00:12 schrieb Björn Schäpers: >>> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: >>>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote: >>>>> >>>>> From: Björn Schäpers <bjoern@hazardy.de> >>>>> >>>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except >>>>> that libraries loaded after the backtrace_initialize are not handled. >>>>> But as far as I can see that's the same for elf. >>>> >>>> Thanks, but I don't want a patch that loops using goto statements. >>>> Please rewrite to avoid that. It may be simpler to call a function. >>>> >>>> Also starting with a module count of 1000 seems like a lot. Do >>>> typical Windows programs load that many modules? >>>> >>>> Ian >>>> >>>> >>> >>> Rewritten using a function. >>> >>> If that is commited, could you attribute that commit to me (--author="Björn >>> Schäpers <bjoern@hazardy.de>")? >>> >>> Thanks and kind regards, >>> Björn. >> >> I noticed that under 64 bit libraries loaded with LoadLibrary were missing. >> EnumProcessModules stated the correct number of modules, but did not fill the >> the HMODULEs, but set them to 0. While trying to investigate I noticed if I do >> the very same thing from main (in C++) I even got fewer module HMODULEs. >> >> So I went a different way. This detects all libraries correctly, in 32 and 64 >> bit. The question is, if it should be a patch on top of the previous, or should >> they be merged, or even only this solution and drop the EnumProcessModules variant? > > Is there any reason to use both patches? Seems simpler to just use > this one if it works. Thanks. > > Ian This one needs the tlhelp32 header and its functions, which are (accoridng to the microsoft documentation) are only available since Windows XP rsp. Windows Server 2003. If that's no problem, and in my opinion it shouldn't be, then I can basically drop patch 4 and rebase this one. Kind regards, Björn.
On Thu, Jan 25, 2024 at 11:53 AM Björn Schäpers <gcc@hazardy.de> wrote: > > Am 23.01.2024 um 23:37 schrieb Ian Lance Taylor: > > On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers <gcc@hazardy.de> wrote: > >> > >> Am 03.01.2024 um 00:12 schrieb Björn Schäpers: > >>> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: > >>>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote: > >>>>> > >>>>> From: Björn Schäpers <bjoern@hazardy.de> > >>>>> > >>>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except > >>>>> that libraries loaded after the backtrace_initialize are not handled. > >>>>> But as far as I can see that's the same for elf. > >>>> > >>>> Thanks, but I don't want a patch that loops using goto statements. > >>>> Please rewrite to avoid that. It may be simpler to call a function. > >>>> > >>>> Also starting with a module count of 1000 seems like a lot. Do > >>>> typical Windows programs load that many modules? > >>>> > >>>> Ian > >>>> > >>>> > >>> > >>> Rewritten using a function. > >>> > >>> If that is commited, could you attribute that commit to me (--author="Björn > >>> Schäpers <bjoern@hazardy.de>")? > >>> > >>> Thanks and kind regards, > >>> Björn. > >> > >> I noticed that under 64 bit libraries loaded with LoadLibrary were missing. > >> EnumProcessModules stated the correct number of modules, but did not fill the > >> the HMODULEs, but set them to 0. While trying to investigate I noticed if I do > >> the very same thing from main (in C++) I even got fewer module HMODULEs. > >> > >> So I went a different way. This detects all libraries correctly, in 32 and 64 > >> bit. The question is, if it should be a patch on top of the previous, or should > >> they be merged, or even only this solution and drop the EnumProcessModules variant? > > > > Is there any reason to use both patches? Seems simpler to just use > > this one if it works. Thanks. > > > > Ian > > This one needs the tlhelp32 header and its functions, which are (accoridng to > the microsoft documentation) are only available since Windows XP rsp. Windows > Server 2003. > > If that's no problem, and in my opinion it shouldn't be, then I can basically > drop patch 4 and rebase this one. I don't see that as a problem. It seems like the patch will fall back cleanly on ancient Windows and simply fail to pick up other loaded DLLs. I think that is fine. I'll look for an updated patch. Thanks. Ian
Am 25.01.2024 um 23:04 schrieb Ian Lance Taylor: > On Thu, Jan 25, 2024 at 11:53 AM Björn Schäpers <gcc@hazardy.de> wrote: >> >> Am 23.01.2024 um 23:37 schrieb Ian Lance Taylor: >>> On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers <gcc@hazardy.de> wrote: >>>> >>>> Am 03.01.2024 um 00:12 schrieb Björn Schäpers: >>>>> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: >>>>>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote: >>>>>>> >>>>>>> From: Björn Schäpers <bjoern@hazardy.de> >>>>>>> >>>>>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except >>>>>>> that libraries loaded after the backtrace_initialize are not handled. >>>>>>> But as far as I can see that's the same for elf. >>>>>> >>>>>> Thanks, but I don't want a patch that loops using goto statements. >>>>>> Please rewrite to avoid that. It may be simpler to call a function. >>>>>> >>>>>> Also starting with a module count of 1000 seems like a lot. Do >>>>>> typical Windows programs load that many modules? >>>>>> >>>>>> Ian >>>>>> >>>>>> >>>>> >>>>> Rewritten using a function. >>>>> >>>>> If that is commited, could you attribute that commit to me (--author="Björn >>>>> Schäpers <bjoern@hazardy.de>")? >>>>> >>>>> Thanks and kind regards, >>>>> Björn. >>>> >>>> I noticed that under 64 bit libraries loaded with LoadLibrary were missing. >>>> EnumProcessModules stated the correct number of modules, but did not fill the >>>> the HMODULEs, but set them to 0. While trying to investigate I noticed if I do >>>> the very same thing from main (in C++) I even got fewer module HMODULEs. >>>> >>>> So I went a different way. This detects all libraries correctly, in 32 and 64 >>>> bit. The question is, if it should be a patch on top of the previous, or should >>>> they be merged, or even only this solution and drop the EnumProcessModules variant? >>> >>> Is there any reason to use both patches? Seems simpler to just use >>> this one if it works. Thanks. >>> >>> Ian >> >> This one needs the tlhelp32 header and its functions, which are (accoridng to >> the microsoft documentation) are only available since Windows XP rsp. Windows >> Server 2003. >> >> If that's no problem, and in my opinion it shouldn't be, then I can basically >> drop patch 4 and rebase this one. > > I don't see that as a problem. It seems like the patch will fall back > cleanly on ancient Windows and simply fail to pick up other loaded > DLLs. I think that is fine. I'll look for an updated patch. Thanks. > > Ian Attached is the combined version of the two patches, only implementing the variant with the tlhelp32 API. Tested on x86 and x86_64 windows. Kind regards, Björn. From 33a6380feff66e32ef0f0d7cbad6fb55319f4e2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de> Date: Sun, 30 Apr 2023 23:54:32 +0200 Subject: [PATCH 1/2] libbacktrace: get debug information for loaded dlls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except that libraries loaded after the backtrace_initialize are not handled. But as far as I can see that's the same for elf. Tested on x86_64-linux and i686-w64-mingw32. -- >8 -- libbacktrace/Changelog: * configure.ac: Checked for tlhelp32.h * configure: Regenerate. * config.h.in: Regenerate. * pecoff.c: Include <tlhelp32.h> if available. (backtrace_initialize): Use tlhelp32 api for a snapshot to detect loaded modules. (coff_add): New argument for the module handle of the file, to get the base address. Signed-off-by: Björn Schäpers <bjoern@hazardy.de> --- libbacktrace/config.h.in | 3 + libbacktrace/configure | 185 ++++++++++++++++++++------------------ libbacktrace/configure.ac | 4 + libbacktrace/pecoff.c | 74 +++++++++++++-- 4 files changed, 171 insertions(+), 95 deletions(-) diff --git a/libbacktrace/config.h.in b/libbacktrace/config.h.in index ee2616335c7..9b8ab88ab63 100644 --- a/libbacktrace/config.h.in +++ b/libbacktrace/config.h.in @@ -101,6 +101,9 @@ /* Define to 1 if you have the <sys/types.h> header file. */ #undef HAVE_SYS_TYPES_H +/* Define to 1 if you have the <tlhelp32.h> header file. */ +#undef HAVE_TLHELP32_H + /* Define to 1 if you have the <unistd.h> header file. */ #undef HAVE_UNISTD_H diff --git a/libbacktrace/configure b/libbacktrace/configure index 7ade966b54d..ca52ee3bafb 100755 --- a/libbacktrace/configure +++ b/libbacktrace/configure @@ -1866,7 +1866,7 @@ else #define $2 innocuous_$2 /* System header to define __stub macros and hopefully few prototypes, - which can conflict with char $2 (); below. + which can conflict with char $2 (void); below. Prefer <limits.h> to <assert.h> if __STDC__ is defined, since <limits.h> exists even on freestanding compilers. */ @@ -1884,7 +1884,7 @@ else #ifdef __cplusplus extern "C" #endif -char $2 (); +char $2 (void); /* The GNU C library defines this for functions which it implements to always fail with ENOSYS. Some functions are actually named something starting with __ and the normal name is an alias. */ @@ -1893,7 +1893,7 @@ choke me #endif int -main () +main (void) { return $2 (); ; @@ -1932,7 +1932,7 @@ else /* end confdefs.h. */ $4 int -main () +main (void) { if (sizeof ($2)) return 0; @@ -1945,7 +1945,7 @@ if ac_fn_c_try_compile "$LINENO"; then : /* end confdefs.h. */ $4 int -main () +main (void) { if (sizeof (($2))) return 0; @@ -1983,7 +1983,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) >= 0)]; test_array [0] = 0; @@ -2000,7 +2000,7 @@ if ac_fn_c_try_compile "$LINENO"; then : /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) <= $ac_mid)]; test_array [0] = 0; @@ -2027,7 +2027,7 @@ else /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) < 0)]; test_array [0] = 0; @@ -2044,7 +2044,7 @@ if ac_fn_c_try_compile "$LINENO"; then : /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) >= $ac_mid)]; test_array [0] = 0; @@ -2079,7 +2079,7 @@ while test "x$ac_lo" != "x$ac_hi"; do /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) <= $ac_mid)]; test_array [0] = 0; @@ -2104,12 +2104,12 @@ esac cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ $4 -static long int longval () { return $2; } -static unsigned long int ulongval () { return $2; } +static long int longval (void) { return $2; } +static unsigned long int ulongval (void) { return $2; } #include <stdio.h> #include <stdlib.h> int -main () +main (void) { FILE *f = fopen ("conftest.val", "w"); @@ -2170,7 +2170,7 @@ else /* end confdefs.h. */ $4 int -main () +main (void) { #ifndef $as_decl_name #ifdef __cplusplus @@ -3073,7 +3073,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { ; @@ -3213,7 +3213,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ #include <stdio.h> int -main () +main (void) { FILE *f = fopen ("conftest.out", "w"); return ferror (f) || fclose (f) != 0; @@ -3277,7 +3277,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3328,7 +3328,7 @@ else /* end confdefs.h. */ int -main () +main (void) { #ifndef __GNUC__ choke me @@ -3369,7 +3369,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3384,7 +3384,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3400,7 +3400,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3449,9 +3449,7 @@ struct stat; /* Most of the following tests are stolen from RCS 5.7's src/conf.sh. */ struct buf { int x; }; FILE * (*rcsopen) (struct buf *, struct stat *, int); -static char *e (p, i) - char **p; - int i; +static char *e (char **p, int i) { return p[i]; } @@ -3486,7 +3484,7 @@ int pairnames (int, char **, FILE *(*)(struct buf *, struct stat *, int), int, i int argc; char **argv; int -main () +main (void) { return f (e, argv, 0) != argv[0] || f (e, argv, 1) != argv[1]; ; @@ -3544,7 +3542,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3871,7 +3869,7 @@ else #include <float.h> int -main () +main (void) { ; @@ -3941,7 +3939,7 @@ else #define XOR(e, f) (((e) && !(f)) || (!(e) && (f))) int -main () +main (void) { int i; for (i = 0; i < 256; i++) @@ -4020,7 +4018,7 @@ else # define __EXTENSIONS__ 1 $ac_includes_default int -main () +main (void) { ; @@ -5002,7 +5000,7 @@ else /* end confdefs.h. */ int -main () +main (void) { #ifndef __GNUC__ choke me @@ -5043,7 +5041,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -5058,7 +5056,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -5074,7 +5072,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -5123,9 +5121,7 @@ struct stat; /* Most of the following tests are stolen from RCS 5.7's src/conf.sh. */ struct buf { int x; }; FILE * (*rcsopen) (struct buf *, struct stat *, int); -static char *e (p, i) - char **p; - int i; +static char *e (char **p, int i) { return p[i]; } @@ -5160,7 +5156,7 @@ int pairnames (int, char **, FILE *(*)(struct buf *, struct stat *, int), int, i int argc; char **argv; int -main () +main (void) { return f (e, argv, 0) != argv[0] || f (e, argv, 1) != argv[1]; ; @@ -5218,7 +5214,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -7408,7 +7404,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu /* end confdefs.h. */ int -main () +main (void) { ; @@ -7994,7 +7990,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -9632,7 +9628,7 @@ _LT_EOF /* end confdefs.h. */ int -main () +main (void) { ; @@ -9672,7 +9668,7 @@ if test -z "$aix_libpath"; then aix_libpath="/usr/lib:/lib"; fi /* end confdefs.h. */ int -main () +main (void) { ; @@ -10947,7 +10943,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -11388,9 +11384,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char dlopen (); +char dlopen (void); int -main () +main (void) { return dlopen (); ; @@ -11441,9 +11437,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char shl_load (); +char shl_load (void); int -main () +main (void) { return shl_load (); ; @@ -11484,9 +11480,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char dlopen (); +char dlopen (void); int -main () +main (void) { return dlopen (); ; @@ -11523,9 +11519,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char dlopen (); +char dlopen (void); int -main () +main (void) { return dlopen (); ; @@ -11562,9 +11558,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char dld_link (); +char dld_link (void); int -main () +main (void) { return dld_link (); ; @@ -11632,7 +11628,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11635 "configure" +#line 11631 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -11738,7 +11734,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11741 "configure" +#line 11737 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -12016,7 +12012,7 @@ else && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12062,7 +12058,7 @@ else && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12086,7 +12082,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12131,7 +12127,7 @@ else && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12155,7 +12151,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12228,7 +12224,7 @@ else /* end confdefs.h. */ static int f() { return 0; } int -main () +main (void) { return f(); ; @@ -12259,7 +12255,7 @@ else /* end confdefs.h. */ int -main () +main (void) { return 0; ; @@ -12312,7 +12308,7 @@ case "$host" in /* end confdefs.h. */ int -main () +main (void) { #if !defined(__SSE2__) @@ -12339,7 +12335,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { asm ("setssbsy"); ; @@ -12401,7 +12397,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -12490,7 +12486,7 @@ $as_echo_n "checking for _Unwind_GetIPInfo... " >&6; } struct _Unwind_Context *context; int ip_before_insn = 0; int -main () +main (void) { return _Unwind_GetIPInfo (context, &ip_before_insn); ; @@ -12554,7 +12550,7 @@ case "$host" in /* end confdefs.h. */ int -main () +main (void) { #if !defined(__SSE2__) @@ -12580,7 +12576,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { asm ("setssbsy"); ; @@ -12626,7 +12622,7 @@ if test x$may_have_cet = xyes; then /* end confdefs.h. */ int -main () +main (void) { return 0; ; @@ -12763,7 +12759,7 @@ else /* end confdefs.h. */ int i; int -main () +main (void) { __sync_bool_compare_and_swap (&i, i, i); __sync_lock_test_and_set (&i, 1); @@ -12805,7 +12801,7 @@ else /* end confdefs.h. */ int i; int -main () +main (void) { __atomic_load_n (&i, __ATOMIC_ACQUIRE); __atomic_store_n (&i, 1, __ATOMIC_RELEASE); @@ -12843,7 +12839,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int i; int -main () +main (void) { int j; ; @@ -13521,6 +13517,21 @@ fi done +for ac_header in tlhelp32.h +do : + ac_fn_c_check_header_compile "$LINENO" "tlhelp32.h" "ac_cv_header_tlhelp32_h" "#ifdef HAVE_WINDOWS_H +# include <windows.h> +#endif +" +if test "x$ac_cv_header_tlhelp32_h" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_TLHELP32_H 1 +_ACEOF + +fi + +done + # Check for the fcntl function. if test -n "${with_target_subdir}"; then @@ -13625,7 +13636,7 @@ else #include <sys/sysctl.h> int -main () +main (void) { int mib0 = CTL_KERN; int mib1 = KERN_PROC; int mib2 = KERN_PROC_PATHNAME; ; @@ -13659,7 +13670,7 @@ else #include <sys/sysctl.h> int -main () +main (void) { int mib0 = CTL_KERN; int mib1 = KERN_PROC_ARGS; int mib2 = KERN_PROC_PATHNAME; ; @@ -13715,9 +13726,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char clock_gettime (); +char clock_gettime (void); int -main () +main (void) { return clock_gettime (); ; @@ -13792,7 +13803,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int i; int -main () +main (void) { return 0; ; @@ -13835,9 +13846,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char compress (); +char compress (void); int -main () +main (void) { return compress (); ; @@ -13881,7 +13892,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { ; @@ -13919,7 +13930,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { ; @@ -13962,9 +13973,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char ZSTD_compress (); +char ZSTD_compress (void); int -main () +main (void) { return ZSTD_compress (); ; @@ -14008,7 +14019,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { ; @@ -14338,9 +14349,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char lzma_auto_decoder (); +char lzma_auto_decoder (void); int -main () +main (void) { return lzma_auto_decoder (); ; @@ -14385,7 +14396,7 @@ else /* end confdefs.h. */ int -main () +main (void) { return 0; ; diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac index 0f61f2b28ab..b9a695ab402 100644 --- a/libbacktrace/configure.ac +++ b/libbacktrace/configure.ac @@ -380,6 +380,10 @@ if test "$have_loadquery" = "yes"; then fi AC_CHECK_HEADERS(windows.h) +AC_CHECK_HEADERS(tlhelp32.h, [], [], +[#ifdef HAVE_WINDOWS_H +# include <windows.h> +#endif]) # Check for the fcntl function. if test -n "${with_target_subdir}"; then diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c index 9e437d810c7..faa0be0b700 100644 --- a/libbacktrace/pecoff.c +++ b/libbacktrace/pecoff.c @@ -49,6 +49,18 @@ POSSIBILITY OF SUCH DAMAGE. */ #endif #include <windows.h> + +#ifdef HAVE_TLHELP32_H +#include <tlhelp32.h> + +#ifdef UNICODE +/* If UNICODE is defined, all the symbols are replaced by a macro to use the + wide variant. But we need the ansi variant, so undef the macros. */ +#undef MODULEENTRY32 +#undef Module32First +#undef Module32Next +#endif +#endif #endif /* Coff file header. */ @@ -592,7 +604,8 @@ coff_syminfo (struct backtrace_state *state, uintptr_t addr, static int coff_add (struct backtrace_state *state, int descriptor, backtrace_error_callback error_callback, void *data, - fileline *fileline_fn, int *found_sym, int *found_dwarf) + fileline *fileline_fn, int *found_sym, int *found_dwarf, + uintptr_t module_handle ATTRIBUTE_UNUSED) { struct backtrace_view fhdr_view; off_t fhdr_off; @@ -870,12 +883,7 @@ coff_add (struct backtrace_state *state, int descriptor, } #ifdef HAVE_WINDOWS_H - { - uintptr_t module_handle; - - module_handle = (uintptr_t) GetModuleHandle (NULL); - base_address = module_handle - image_base; - } + base_address = module_handle - image_base; #endif if (!backtrace_dwarf_add (state, base_address, &dwarf_sections, @@ -917,12 +925,62 @@ backtrace_initialize (struct backtrace_state *state, int found_sym; int found_dwarf; fileline coff_fileline_fn; + uintptr_t module_handle = 0; + +#ifdef HAVE_TLHELP32_H + fileline module_fileline_fn; + int module_found_sym; + + HANDLE snapshot; +#endif + +#ifdef HAVE_WINDOWS_H + module_handle = (uintptr_t) GetModuleHandle (NULL); +#endif ret = coff_add (state, descriptor, error_callback, data, - &coff_fileline_fn, &found_sym, &found_dwarf); + &coff_fileline_fn, &found_sym, &found_dwarf, module_handle); if (!ret) return 0; +#ifdef HAVE_TLHELP32_H + do + { + snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPMODULE, 0); + } + while (snapshot == INVALID_HANDLE_VALUE + && GetLastError () == ERROR_BAD_LENGTH); + + if (snapshot != INVALID_HANDLE_VALUE) + { + MODULEENTRY32 entry; + BOOL ok; + entry.dwSize = sizeof(MODULEENTRY32); + + for (ok = Module32First (snapshot, &entry); ok; ok =Module32Next (snapshot, &entry)) + { + if (strcmp (filename, entry.szExePath) == 0) + continue; + + module_handle = (uintptr_t) entry.hModule; + if (module_handle == 0) + continue; + + descriptor = backtrace_open (entry.szExePath, error_callback, data, NULL); + if (descriptor < 0) + continue; + + coff_add (state, descriptor, error_callback, data, + &module_fileline_fn, &module_found_sym, &found_dwarf, + module_handle); + if (module_found_sym) + found_sym = 1; + } + + CloseHandle (snapshot); + } +#endif + if (!state->threaded) { if (found_sym)
Am 10.01.2024 um 13:34 schrieb Eli Zaretskii: >> Date: Tue, 9 Jan 2024 21:02:44 +0100 >> Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org >> From: Björn Schäpers <gcc@hazardy.de> >> >> Am 07.01.2024 um 18:03 schrieb Eli Zaretskii: >>> In that case, you an call either GetModuleHandeExA or >>> GetModuleHandeExW, the difference is minor. >> >> Here an updated version without relying on TEXT or TCHAR, directly calling >> GetModuleHandleExW. > > Thanks, this LGTM (but I couldn't test it, I just looked at the > sour ce code). Here an updated version. It is rebased on the combined approach of getting the loaded DLLs and two minor changes to suppress warnings. From 55d0f312c0a9c4e2305d72fa2329b37937a02e44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern@hazardy.de> Date: Sat, 6 Jan 2024 22:53:54 +0100 Subject: [PATCH 2/2] libbacktrace: Add loaded dlls after initialize libbacktrace/Changelog: * pecoff.c [HAVE_WINDOWS_H]: (dll_notification_data): Added (dll_notification_context): Added (dll_notification): Added (backtrace_initialize): Use LdrRegisterDllNotification to load debug information of later loaded dlls. --- libbacktrace/pecoff.c | 106 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c index faa0be0b700..455a5c2191d 100644 --- a/libbacktrace/pecoff.c +++ b/libbacktrace/pecoff.c @@ -61,6 +61,34 @@ POSSIBILITY OF SUCH DAMAGE. */ #undef Module32Next #endif #endif + +#if defined(_ARM_) +#define NTAPI +#else +#define NTAPI __stdcall +#endif + +/* This is a simplified (but binary compatible) version of what Microsoft + defines in their documentation. */ +struct dll_notifcation_data +{ + ULONG reserved; + /* The name as UNICODE_STRING struct. */ + PVOID full_dll_name; + PVOID base_dll_name; + PVOID dll_base; + ULONG size_of_image; +}; + +#define LDR_DLL_NOTIFICATION_REASON_LOADED 1 + +typedef LONG NTSTATUS; +typedef VOID CALLBACK (*LDR_DLL_NOTIFICATION)(ULONG, + struct dll_notifcation_data*, + PVOID); +typedef NTSTATUS NTAPI (*LDR_REGISTER_FUNCTION)(ULONG, + LDR_DLL_NOTIFICATION, PVOID, + PVOID*); #endif /* Coff file header. */ @@ -911,6 +939,53 @@ coff_add (struct backtrace_state *state, int descriptor, return 0; } +#ifdef HAVE_WINDOWS_H +struct dll_notification_context +{ + struct backtrace_state *state; + backtrace_error_callback error_callback; + void *data; +}; + +static VOID CALLBACK +dll_notification (ULONG reason, + struct dll_notifcation_data *notification_data, + PVOID context) +{ + char module_name[MAX_PATH]; + int descriptor; + struct dll_notification_context* dll_context = + (struct dll_notification_context*) context; + struct backtrace_state *state = dll_context->state; + void *data = dll_context->data; + backtrace_error_callback error_callback = dll_context->data; + fileline fileline; + int found_sym; + int found_dwarf; + HMODULE module_handle; + + if (reason != LDR_DLL_NOTIFICATION_REASON_LOADED) + return; + + if (!GetModuleHandleExW (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + (wchar_t*) notification_data->dll_base, + &module_handle)) + return; + + if (!GetModuleFileNameA ((HMODULE) module_handle, module_name, MAX_PATH - 1)) + return; + + descriptor = backtrace_open (module_name, error_callback, data, NULL); + + if (descriptor < 0) + return; + + coff_add (state, descriptor, error_callback, data, &fileline, &found_sym, + &found_dwarf, (uintptr_t) module_handle); +} +#endif + /* Initialize the backtrace data we need from an ELF executable. At the ELF level, all we need to do is find the debug info sections. */ @@ -935,6 +1010,8 @@ backtrace_initialize (struct backtrace_state *state, #endif #ifdef HAVE_WINDOWS_H + HMODULE nt_dll_handle; + module_handle = (uintptr_t) GetModuleHandle (NULL); #endif @@ -981,6 +1058,35 @@ backtrace_initialize (struct backtrace_state *state, } #endif +#ifdef HAVE_WINDOWS_H + nt_dll_handle = GetModuleHandleW (L"ntdll.dll"); + if (nt_dll_handle) + { + LDR_REGISTER_FUNCTION register_func; + const char register_name[] = "LdrRegisterDllNotification"; + register_func = (void*) GetProcAddress (nt_dll_handle, + register_name); + + if (register_func) + { + PVOID cookie; + struct dll_notification_context *context + = backtrace_alloc (state, + sizeof(struct dll_notification_context), + error_callback, data); + + if (context) + { + context->state = state; + context->data = data; + context->error_callback = error_callback; + + register_func (0, &dll_notification, context, &cookie); + } + } + } +#endif + if (!state->threaded) { if (found_sym)
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 <gcc@hazardy.de> wrote: >>> >>> Am 23.01.2024 um 23:37 schrieb Ian Lance Taylor: >>>> On Thu, Jan 4, 2024 at 2:33 PM Björn Schäpers <gcc@hazardy.de> wrote: >>>>> >>>>> Am 03.01.2024 um 00:12 schrieb Björn Schäpers: >>>>>> Am 30.11.2023 um 20:53 schrieb Ian Lance Taylor: >>>>>>> On Fri, Jan 20, 2023 at 2:55 AM Björn Schäpers <gcc@hazardy.de> wrote: >>>>>>>> >>>>>>>> From: Björn Schäpers <bjoern@hazardy.de> >>>>>>>> >>>>>>>> Fixes https://github.com/ianlancetaylor/libbacktrace/issues/53, except >>>>>>>> that libraries loaded after the backtrace_initialize are not handled. >>>>>>>> But as far as I can see that's the same for elf. >>>>>>> >>>>>>> Thanks, but I don't want a patch that loops using goto statements. >>>>>>> Please rewrite to avoid that. It may be simpler to call a function. >>>>>>> >>>>>>> Also starting with a module count of 1000 seems like a lot. Do >>>>>>> typical Windows programs load that many modules? >>>>>>> >>>>>>> Ian >>>>>>> >>>>>>> >>>>>> >>>>>> Rewritten using a function. >>>>>> >>>>>> If that is commited, could you attribute that commit to me (--author="Björn >>>>>> Schäpers <bjoern@hazardy.de>")? >>>>>> >>>>>> Thanks and kind regards, >>>>>> Björn. >>>>> >>>>> I noticed that under 64 bit libraries loaded with LoadLibrary were missing. >>>>> EnumProcessModules stated the correct number of modules, but did not fill the >>>>> the HMODULEs, but set them to 0. While trying to investigate I noticed if I do >>>>> the very same thing from main (in C++) I even got fewer module HMODULEs. >>>>> >>>>> So I went a different way. This detects all libraries correctly, in 32 and 64 >>>>> bit. The question is, if it should be a patch on top of the previous, or >>>>> should >>>>> they be merged, or even only this solution and drop the EnumProcessModules >>>>> variant? >>>> >>>> Is there any reason to use both patches? Seems simpler to just use >>>> this one if it works. Thanks. >>>> >>>> Ian >>> >>> This one needs the tlhelp32 header and its functions, which are (accoridng to >>> the microsoft documentation) are only available since Windows XP rsp. Windows >>> Server 2003. >>> >>> If that's no problem, and in my opinion it shouldn't be, then I can basically >>> drop patch 4 and rebase this one. >> >> I don't see that as a problem. It seems like the patch will fall back >> cleanly on ancient Windows and simply fail to pick up other loaded >> DLLs. I think that is fine. I'll look for an updated patch. Thanks. >> >> Ian > > Attached is the combined version of the two patches, only implementing the > variant with the tlhelp32 API. > > Tested on x86 and x86_64 windows. > > Kind regards, > Björn. A friendly ping.
On Thu, Apr 25, 2024 at 1:15 PM Björn Schäpers <gcc@hazardy.de> 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 942a9cf2a958113d2ab46f5b015c36e569abedcf diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac index 3e0075a2b79..59e9c415db8 100644 --- a/libbacktrace/configure.ac +++ b/libbacktrace/configure.ac @@ -380,6 +380,10 @@ if test "$have_loadquery" = "yes"; then fi AC_CHECK_HEADERS(windows.h) +AC_CHECK_HEADERS(tlhelp32.h, [], [], +[#ifdef HAVE_WINDOWS_H +# include <windows.h> +#endif]) # Check for the fcntl function. if test -n "${with_target_subdir}"; then diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c index 9e437d810c7..4f267841178 100644 --- a/libbacktrace/pecoff.c +++ b/libbacktrace/pecoff.c @@ -49,6 +49,18 @@ POSSIBILITY OF SUCH DAMAGE. */ #endif #include <windows.h> + +#ifdef HAVE_TLHELP32_H +#include <tlhelp32.h> + +#ifdef UNICODE +/* If UNICODE is defined, all the symbols are replaced by a macro to use the + wide variant. But we need the ansi variant, so undef the macros. */ +#undef MODULEENTRY32 +#undef Module32First +#undef Module32Next +#endif +#endif #endif /* Coff file header. */ @@ -592,7 +604,8 @@ coff_syminfo (struct backtrace_state *state, uintptr_t addr, static int coff_add (struct backtrace_state *state, int descriptor, backtrace_error_callback error_callback, void *data, - fileline *fileline_fn, int *found_sym, int *found_dwarf) + fileline *fileline_fn, int *found_sym, int *found_dwarf, + uintptr_t module_handle ATTRIBUTE_UNUSED) { struct backtrace_view fhdr_view; off_t fhdr_off; @@ -870,12 +883,7 @@ coff_add (struct backtrace_state *state, int descriptor, } #ifdef HAVE_WINDOWS_H - { - uintptr_t module_handle; - - module_handle = (uintptr_t) GetModuleHandle (NULL); - base_address = module_handle - image_base; - } + base_address = module_handle - image_base; #endif if (!backtrace_dwarf_add (state, base_address, &dwarf_sections, @@ -917,12 +925,61 @@ backtrace_initialize (struct backtrace_state *state, int found_sym; int found_dwarf; fileline coff_fileline_fn; + uintptr_t module_handle = 0; +#ifdef HAVE_TLHELP32_H + fileline module_fileline_fn; + int module_found_sym; + HANDLE snapshot; +#endif + +#ifdef HAVE_WINDOWS_H + module_handle = (uintptr_t) GetModuleHandle (NULL); +#endif ret = coff_add (state, descriptor, error_callback, data, - &coff_fileline_fn, &found_sym, &found_dwarf); + &coff_fileline_fn, &found_sym, &found_dwarf, module_handle); if (!ret) return 0; +#ifdef HAVE_TLHELP32_H + do + { + snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPMODULE, 0); + } + while (snapshot == INVALID_HANDLE_VALUE + && GetLastError () == ERROR_BAD_LENGTH); + + if (snapshot != INVALID_HANDLE_VALUE) + { + MODULEENTRY32 entry; + BOOL ok; + entry.dwSize = sizeof (MODULEENTRY32); + + for (ok = Module32First (snapshot, &entry); ok; ok = Module32Next (snapshot, &entry)) + { + if (strcmp (filename, entry.szExePath) == 0) + continue; + + module_handle = (uintptr_t) entry.hModule; + if (module_handle == 0) + continue; + + descriptor = backtrace_open (entry.szExePath, error_callback, data, + NULL); + if (descriptor < 0) + continue; + + coff_add (state, descriptor, error_callback, data, + &module_fileline_fn, &module_found_sym, &found_dwarf, + module_handle); + if (module_found_sym) + found_sym = 1; + } + + CloseHandle (snapshot); + } +#endif + if (!state->threaded) { if (found_sym)
Am 28.04.2024 um 20:16 schrieb Ian Lance Taylor: > On Thu, Apr 25, 2024 at 1:15 PM Björn Schäpers <gcc@hazardy.de> 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?= <bjoern@hazardy.de> Date: Sat, 6 Jan 2024 22:53:54 +0100 Subject: [PATCH 2/2] libbacktrace: Add loaded dlls after initialize libbacktrace/Changelog: * pecoff.c [HAVE_WINDOWS_H]: (dll_notification_data): Added (dll_notification_context): Added (dll_notification): Added (backtrace_initialize): Use LdrRegisterDllNotification to load debug information of later loaded dlls. --- libbacktrace/pecoff.c | 106 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c index faa0be0b700..455a5c2191d 100644 --- a/libbacktrace/pecoff.c +++ b/libbacktrace/pecoff.c @@ -61,6 +61,34 @@ POSSIBILITY OF SUCH DAMAGE. */ #undef Module32Next #endif #endif + +#if defined(_ARM_) +#define NTAPI +#else +#define NTAPI __stdcall +#endif + +/* This is a simplified (but binary compatible) version of what Microsoft + defines in their documentation. */ +struct dll_notifcation_data +{ + ULONG reserved; + /* The name as UNICODE_STRING struct. */ + PVOID full_dll_name; + PVOID base_dll_name; + PVOID dll_base; + ULONG size_of_image; +}; + +#define LDR_DLL_NOTIFICATION_REASON_LOADED 1 + +typedef LONG NTSTATUS; +typedef VOID CALLBACK (*LDR_DLL_NOTIFICATION)(ULONG, + struct dll_notifcation_data*, + PVOID); +typedef NTSTATUS NTAPI (*LDR_REGISTER_FUNCTION)(ULONG, + LDR_DLL_NOTIFICATION, PVOID, + PVOID*); #endif /* Coff file header. */ @@ -911,6 +939,53 @@ coff_add (struct backtrace_state *state, int descriptor, return 0; } +#ifdef HAVE_WINDOWS_H +struct dll_notification_context +{ + struct backtrace_state *state; + backtrace_error_callback error_callback; + void *data; +}; + +static VOID CALLBACK +dll_notification (ULONG reason, + struct dll_notifcation_data *notification_data, + PVOID context) +{ + char module_name[MAX_PATH]; + int descriptor; + struct dll_notification_context* dll_context = + (struct dll_notification_context*) context; + struct backtrace_state *state = dll_context->state; + void *data = dll_context->data; + backtrace_error_callback error_callback = dll_context->data; + fileline fileline; + int found_sym; + int found_dwarf; + HMODULE module_handle; + + if (reason != LDR_DLL_NOTIFICATION_REASON_LOADED) + return; + + if (!GetModuleHandleExW (GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS + | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, + (wchar_t*) notification_data->dll_base, + &module_handle)) + return; + + if (!GetModuleFileNameA ((HMODULE) module_handle, module_name, MAX_PATH - 1)) + return; + + descriptor = backtrace_open (module_name, error_callback, data, NULL); + + if (descriptor < 0) + return; + + coff_add (state, descriptor, error_callback, data, &fileline, &found_sym, + &found_dwarf, (uintptr_t) module_handle); +} +#endif + /* Initialize the backtrace data we need from an ELF executable. At the ELF level, all we need to do is find the debug info sections. */ @@ -935,6 +1010,8 @@ backtrace_initialize (struct backtrace_state *state, #endif #ifdef HAVE_WINDOWS_H + HMODULE nt_dll_handle; + module_handle = (uintptr_t) GetModuleHandle (NULL); #endif @@ -981,6 +1058,35 @@ backtrace_initialize (struct backtrace_state *state, } #endif +#ifdef HAVE_WINDOWS_H + nt_dll_handle = GetModuleHandleW (L"ntdll.dll"); + if (nt_dll_handle) + { + LDR_REGISTER_FUNCTION register_func; + const char register_name[] = "LdrRegisterDllNotification"; + register_func = (void*) GetProcAddress (nt_dll_handle, + register_name); + + if (register_func) + { + PVOID cookie; + struct dll_notification_context *context + = backtrace_alloc (state, + sizeof(struct dll_notification_context), + error_callback, data); + + if (context) + { + context->state = state; + context->data = data; + context->error_callback = error_callback; + + register_func (0, &dll_notification, context, &cookie); + } + } + } +#endif + if (!state->threaded) { if (found_sym)
On Thu, May 2, 2024 at 12:23 PM Björn Schäpers <gcc@hazardy.de> wrote: > > Am 28.04.2024 um 20:16 schrieb Ian Lance Taylor: > > > > Which of your other patches are still relevant? Thanks. > > > only this one. Thanks. Committed. Ian
On Fri, Mar 15, 2024 at 1:41 PM Björn Schäpers <gcc@hazardy.de> wrote: > > Am 10.01.2024 um 13:34 schrieb Eli Zaretskii: > >> Date: Tue, 9 Jan 2024 21:02:44 +0100 > >> Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org > >> From: Björn Schäpers <gcc@hazardy.de> > >> > >> Am 07.01.2024 um 18:03 schrieb Eli Zaretskii: > >>> In that case, you an call either GetModuleHandeExA or > >>> GetModuleHandeExW, the difference is minor. > >> > >> Here an updated version without relying on TEXT or TCHAR, directly calling > >> GetModuleHandleExW. > > > > Thanks, this LGTM (but I couldn't test it, I just looked at the > > sour ce code). > > Here an updated version. It is rebased on the combined approach of getting the > loaded DLLs and two minor changes to suppress warnings. 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? Thanks. Ian
> From: Ian Lance Taylor <iant@google.com> > Date: Mon, 29 Jul 2024 09:46:46 -0700 > Cc: Eli Zaretskii <eliz@gnu.org>, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org > > On Fri, Mar 15, 2024 at 1:41 PM Björn Schäpers <gcc@hazardy.de> wrote: > > > > Am 10.01.2024 um 13:34 schrieb Eli Zaretskii: > > >> Date: Tue, 9 Jan 2024 21:02:44 +0100 > > >> Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org > > >> From: Björn Schäpers <gcc@hazardy.de> > > >> > > >> Am 07.01.2024 um 18:03 schrieb Eli Zaretskii: > > >>> In that case, you an call either GetModuleHandeExA or > > >>> GetModuleHandeExW, the difference is minor. > > >> > > >> Here an updated version without relying on TEXT or TCHAR, directly calling > > >> GetModuleHandleExW. > > > > > > Thanks, this LGTM (but I couldn't test it, I just looked at the > > > sour ce code). > > > > Here an updated version. It is rebased on the combined approach of getting the > > loaded DLLs and two minor changes to suppress warnings. > > 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 <ntdef.h>, 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.
Am 29.07.2024 um 19:58 schrieb Eli Zaretskii: >> From: Ian Lance Taylor <iant@google.com> >> Date: Mon, 29 Jul 2024 09:46:46 -0700 >> Cc: Eli Zaretskii <eliz@gnu.org>, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org >> >> On Fri, Mar 15, 2024 at 1:41 PM Björn Schäpers <gcc@hazardy.de> wrote: >>> >>> Am 10.01.2024 um 13:34 schrieb Eli Zaretskii: >>>>> Date: Tue, 9 Jan 2024 21:02:44 +0100 >>>>> Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org >>>>> From: Björn Schäpers <gcc@hazardy.de> >>>>> >>>>> Am 07.01.2024 um 18:03 schrieb Eli Zaretskii: >>>>>> In that case, you an call either GetModuleHandeExA or >>>>>> GetModuleHandeExW, the difference is minor. >>>>> >>>>> Here an updated version without relying on TEXT or TCHAR, directly calling >>>>> GetModuleHandleExW. >>>> >>>> Thanks, this LGTM (but I couldn't test it, I just looked at the >>>> sour ce code). >>> >>> Here an updated version. It is rebased on the combined approach of getting the >>> loaded DLLs and two minor changes to suppress warnings. >> >> 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 <ntdef.h>, 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.
On Mon, Jul 29, 2024 at 12:41 PM Björn Schäpers <gcc@hazardy.de> wrote: > > > 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 <ntdef.h>, 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. Thanks. I committed this patch. Ian * pecoff.c (LDR_DLL_NOTIFICATION): Put function modifier inside parentheses. (LDR_REGISTER_FUNCTION): Likewise. 338a93ce71ccfd435c0f392af483cc946b2c26fc diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c index 636e1b11296..ccd5ccbce2c 100644 --- a/libbacktrace/pecoff.c +++ b/libbacktrace/pecoff.c @@ -83,10 +83,10 @@ struct dll_notification_data #define LDR_DLL_NOTIFICATION_REASON_LOADED 1 typedef LONG NTSTATUS; -typedef VOID CALLBACK (*LDR_DLL_NOTIFICATION)(ULONG, +typedef VOID (CALLBACK *LDR_DLL_NOTIFICATION)(ULONG, struct dll_notification_data*, PVOID); -typedef NTSTATUS NTAPI (*LDR_REGISTER_FUNCTION)(ULONG, +typedef NTSTATUS (NTAPI *LDR_REGISTER_FUNCTION)(ULONG, LDR_DLL_NOTIFICATION, PVOID, PVOID*); #endif
diff --git a/libbacktrace/config.h.in b/libbacktrace/config.h.in index ee2616335c7..9b8ab88ab63 100644 --- a/libbacktrace/config.h.in +++ b/libbacktrace/config.h.in @@ -101,6 +101,9 @@ /* Define to 1 if you have the <sys/types.h> header file. */ #undef HAVE_SYS_TYPES_H +/* Define to 1 if you have the <tlhelp32.h> header file. */ +#undef HAVE_TLHELP32_H + /* Define to 1 if you have the <unistd.h> header file. */ #undef HAVE_UNISTD_H diff --git a/libbacktrace/configure b/libbacktrace/configure index 7ade966b54d..ca52ee3bafb 100755 --- a/libbacktrace/configure +++ b/libbacktrace/configure @@ -1866,7 +1866,7 @@ else #define $2 innocuous_$2 /* System header to define __stub macros and hopefully few prototypes, - which can conflict with char $2 (); below. + which can conflict with char $2 (void); below. Prefer <limits.h> to <assert.h> if __STDC__ is defined, since <limits.h> exists even on freestanding compilers. */ @@ -1884,7 +1884,7 @@ else #ifdef __cplusplus extern "C" #endif -char $2 (); +char $2 (void); /* The GNU C library defines this for functions which it implements to always fail with ENOSYS. Some functions are actually named something starting with __ and the normal name is an alias. */ @@ -1893,7 +1893,7 @@ choke me #endif int -main () +main (void) { return $2 (); ; @@ -1932,7 +1932,7 @@ else /* end confdefs.h. */ $4 int -main () +main (void) { if (sizeof ($2)) return 0; @@ -1945,7 +1945,7 @@ if ac_fn_c_try_compile "$LINENO"; then : /* end confdefs.h. */ $4 int -main () +main (void) { if (sizeof (($2))) return 0; @@ -1983,7 +1983,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) >= 0)]; test_array [0] = 0; @@ -2000,7 +2000,7 @@ if ac_fn_c_try_compile "$LINENO"; then : /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) <= $ac_mid)]; test_array [0] = 0; @@ -2027,7 +2027,7 @@ else /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) < 0)]; test_array [0] = 0; @@ -2044,7 +2044,7 @@ if ac_fn_c_try_compile "$LINENO"; then : /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) >= $ac_mid)]; test_array [0] = 0; @@ -2079,7 +2079,7 @@ while test "x$ac_lo" != "x$ac_hi"; do /* end confdefs.h. */ $4 int -main () +main (void) { static int test_array [1 - 2 * !(($2) <= $ac_mid)]; test_array [0] = 0; @@ -2104,12 +2104,12 @@ esac cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ $4 -static long int longval () { return $2; } -static unsigned long int ulongval () { return $2; } +static long int longval (void) { return $2; } +static unsigned long int ulongval (void) { return $2; } #include <stdio.h> #include <stdlib.h> int -main () +main (void) { FILE *f = fopen ("conftest.val", "w"); @@ -2170,7 +2170,7 @@ else /* end confdefs.h. */ $4 int -main () +main (void) { #ifndef $as_decl_name #ifdef __cplusplus @@ -3073,7 +3073,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { ; @@ -3213,7 +3213,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ #include <stdio.h> int -main () +main (void) { FILE *f = fopen ("conftest.out", "w"); return ferror (f) || fclose (f) != 0; @@ -3277,7 +3277,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3328,7 +3328,7 @@ else /* end confdefs.h. */ int -main () +main (void) { #ifndef __GNUC__ choke me @@ -3369,7 +3369,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3384,7 +3384,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3400,7 +3400,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3449,9 +3449,7 @@ struct stat; /* Most of the following tests are stolen from RCS 5.7's src/conf.sh. */ struct buf { int x; }; FILE * (*rcsopen) (struct buf *, struct stat *, int); -static char *e (p, i) - char **p; - int i; +static char *e (char **p, int i) { return p[i]; } @@ -3486,7 +3484,7 @@ int pairnames (int, char **, FILE *(*)(struct buf *, struct stat *, int), int, i int argc; char **argv; int -main () +main (void) { return f (e, argv, 0) != argv[0] || f (e, argv, 1) != argv[1]; ; @@ -3544,7 +3542,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -3871,7 +3869,7 @@ else #include <float.h> int -main () +main (void) { ; @@ -3941,7 +3939,7 @@ else #define XOR(e, f) (((e) && !(f)) || (!(e) && (f))) int -main () +main (void) { int i; for (i = 0; i < 256; i++) @@ -4020,7 +4018,7 @@ else # define __EXTENSIONS__ 1 $ac_includes_default int -main () +main (void) { ; @@ -5002,7 +5000,7 @@ else /* end confdefs.h. */ int -main () +main (void) { #ifndef __GNUC__ choke me @@ -5043,7 +5041,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -5058,7 +5056,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -5074,7 +5072,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -5123,9 +5121,7 @@ struct stat; /* Most of the following tests are stolen from RCS 5.7's src/conf.sh. */ struct buf { int x; }; FILE * (*rcsopen) (struct buf *, struct stat *, int); -static char *e (p, i) - char **p; - int i; +static char *e (char **p, int i) { return p[i]; } @@ -5160,7 +5156,7 @@ int pairnames (int, char **, FILE *(*)(struct buf *, struct stat *, int), int, i int argc; char **argv; int -main () +main (void) { return f (e, argv, 0) != argv[0] || f (e, argv, 1) != argv[1]; ; @@ -5218,7 +5214,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -7408,7 +7404,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu /* end confdefs.h. */ int -main () +main (void) { ; @@ -7994,7 +7990,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -9632,7 +9628,7 @@ _LT_EOF /* end confdefs.h. */ int -main () +main (void) { ; @@ -9672,7 +9668,7 @@ if test -z "$aix_libpath"; then aix_libpath="/usr/lib:/lib"; fi /* end confdefs.h. */ int -main () +main (void) { ; @@ -10947,7 +10943,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -11388,9 +11384,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char dlopen (); +char dlopen (void); int -main () +main (void) { return dlopen (); ; @@ -11441,9 +11437,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char shl_load (); +char shl_load (void); int -main () +main (void) { return shl_load (); ; @@ -11484,9 +11480,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char dlopen (); +char dlopen (void); int -main () +main (void) { return dlopen (); ; @@ -11523,9 +11519,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char dlopen (); +char dlopen (void); int -main () +main (void) { return dlopen (); ; @@ -11562,9 +11558,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char dld_link (); +char dld_link (void); int -main () +main (void) { return dld_link (); ; @@ -11632,7 +11628,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11635 "configure" +#line 11631 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -11738,7 +11734,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11741 "configure" +#line 11737 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -12016,7 +12012,7 @@ else && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12062,7 +12058,7 @@ else && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12086,7 +12082,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12131,7 +12127,7 @@ else && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12155,7 +12151,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext && LARGE_OFF_T % 2147483647 == 1) ? 1 : -1]; int -main () +main (void) { ; @@ -12228,7 +12224,7 @@ else /* end confdefs.h. */ static int f() { return 0; } int -main () +main (void) { return f(); ; @@ -12259,7 +12255,7 @@ else /* end confdefs.h. */ int -main () +main (void) { return 0; ; @@ -12312,7 +12308,7 @@ case "$host" in /* end confdefs.h. */ int -main () +main (void) { #if !defined(__SSE2__) @@ -12339,7 +12335,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { asm ("setssbsy"); ; @@ -12401,7 +12397,7 @@ else /* end confdefs.h. */ int -main () +main (void) { ; @@ -12490,7 +12486,7 @@ $as_echo_n "checking for _Unwind_GetIPInfo... " >&6; } struct _Unwind_Context *context; int ip_before_insn = 0; int -main () +main (void) { return _Unwind_GetIPInfo (context, &ip_before_insn); ; @@ -12554,7 +12550,7 @@ case "$host" in /* end confdefs.h. */ int -main () +main (void) { #if !defined(__SSE2__) @@ -12580,7 +12576,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { asm ("setssbsy"); ; @@ -12626,7 +12622,7 @@ if test x$may_have_cet = xyes; then /* end confdefs.h. */ int -main () +main (void) { return 0; ; @@ -12763,7 +12759,7 @@ else /* end confdefs.h. */ int i; int -main () +main (void) { __sync_bool_compare_and_swap (&i, i, i); __sync_lock_test_and_set (&i, 1); @@ -12805,7 +12801,7 @@ else /* end confdefs.h. */ int i; int -main () +main (void) { __atomic_load_n (&i, __ATOMIC_ACQUIRE); __atomic_store_n (&i, 1, __ATOMIC_RELEASE); @@ -12843,7 +12839,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int i; int -main () +main (void) { int j; ; @@ -13521,6 +13517,21 @@ fi done +for ac_header in tlhelp32.h +do : + ac_fn_c_check_header_compile "$LINENO" "tlhelp32.h" "ac_cv_header_tlhelp32_h" "#ifdef HAVE_WINDOWS_H +# include <windows.h> +#endif +" +if test "x$ac_cv_header_tlhelp32_h" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_TLHELP32_H 1 +_ACEOF + +fi + +done + # Check for the fcntl function. if test -n "${with_target_subdir}"; then @@ -13625,7 +13636,7 @@ else #include <sys/sysctl.h> int -main () +main (void) { int mib0 = CTL_KERN; int mib1 = KERN_PROC; int mib2 = KERN_PROC_PATHNAME; ; @@ -13659,7 +13670,7 @@ else #include <sys/sysctl.h> int -main () +main (void) { int mib0 = CTL_KERN; int mib1 = KERN_PROC_ARGS; int mib2 = KERN_PROC_PATHNAME; ; @@ -13715,9 +13726,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char clock_gettime (); +char clock_gettime (void); int -main () +main (void) { return clock_gettime (); ; @@ -13792,7 +13803,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int i; int -main () +main (void) { return 0; ; @@ -13835,9 +13846,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char compress (); +char compress (void); int -main () +main (void) { return compress (); ; @@ -13881,7 +13892,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { ; @@ -13919,7 +13930,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { ; @@ -13962,9 +13973,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char ZSTD_compress (); +char ZSTD_compress (void); int -main () +main (void) { return ZSTD_compress (); ; @@ -14008,7 +14019,7 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext /* end confdefs.h. */ int -main () +main (void) { ; @@ -14338,9 +14349,9 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext #ifdef __cplusplus extern "C" #endif -char lzma_auto_decoder (); +char lzma_auto_decoder (void); int -main () +main (void) { return lzma_auto_decoder (); ; @@ -14385,7 +14396,7 @@ else /* end confdefs.h. */ int -main () +main (void) { return 0; ; diff --git a/libbacktrace/configure.ac b/libbacktrace/configure.ac index 0f61f2b28ab..b9a695ab402 100644 --- a/libbacktrace/configure.ac +++ b/libbacktrace/configure.ac @@ -380,6 +380,10 @@ if test "$have_loadquery" = "yes"; then fi AC_CHECK_HEADERS(windows.h) +AC_CHECK_HEADERS(tlhelp32.h, [], [], +[#ifdef HAVE_WINDOWS_H +# include <windows.h> +#endif]) # Check for the fcntl function. if test -n "${with_target_subdir}"; then diff --git a/libbacktrace/pecoff.c b/libbacktrace/pecoff.c index 64787a18411..647baa39640 100644 --- a/libbacktrace/pecoff.c +++ b/libbacktrace/pecoff.c @@ -50,6 +50,18 @@ POSSIBILITY OF SUCH DAMAGE. */ #include <windows.h> #include <psapi.h> + +#ifdef HAVE_TLHELP32_H +#include <tlhelp32.h> + +#ifdef UNICODE +/* If UNICODE is defined, all the symbols are replaced by a macro to use the + wide variant. But we need the ansi variant, so undef the macros. */ +#undef MODULEENTRY32 +#undef Module32First +#undef Module32Next +#endif +#endif #endif /* Coff file header. */ @@ -900,7 +912,7 @@ coff_add (struct backtrace_state *state, int descriptor, return 0; } -#ifdef HAVE_WINDOWS_H +#if defined(HAVE_WINDOWS_H) && !defined(HAVE_TLHELP32_H) static void free_modules (struct backtrace_state *state, backtrace_error_callback error_callback, void *data, @@ -964,13 +976,19 @@ backtrace_initialize (struct backtrace_state *state, uintptr_t module_handle = 0; #ifdef HAVE_WINDOWS_H + fileline module_fileline_fn; + int module_found_sym; + +#ifdef HAVE_TLHELP32_H + HANDLE snapshot; +#else DWORD i; DWORD module_count = 100; DWORD bytes_allocated_for_modules = 0; HMODULE *modules = NULL; char module_name[MAX_PATH]; - int module_found_sym; - fileline module_fileline_fn; + +#endif module_handle = (uintptr_t) GetModuleHandle (NULL); #endif @@ -981,6 +999,43 @@ backtrace_initialize (struct backtrace_state *state, return 0; #ifdef HAVE_WINDOWS_H +#ifdef HAVE_TLHELP32_H + do + { + snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPMODULE, 0); + } + while (snapshot == INVALID_HANDLE_VALUE + && GetLastError () == ERROR_BAD_LENGTH); + + if (snapshot != INVALID_HANDLE_VALUE) + { + MODULEENTRY32 entry; + BOOL ok; + entry.dwSize = sizeof(MODULEENTRY32); + + for (ok = Module32First (snapshot, &entry); ok; ok =Module32Next (snapshot, &entry)) + { + if (strcmp (filename, entry.szExePath) == 0) + continue; + + module_handle = (uintptr_t) entry.hModule; + if (module_handle == 0) + continue; + + descriptor = backtrace_open (entry.szExePath, error_callback, data, NULL); + if (descriptor < 0) + continue; + + coff_add (state, descriptor, error_callback, data, + &module_fileline_fn, &module_found_sym, &found_dwarf, + module_handle); + if (module_found_sym) + found_sym = 1; + } + + CloseHandle (snapshot); + } +#else get_all_modules (state, error_callback, data, &modules, &module_count, &bytes_allocated_for_modules); @@ -1009,6 +1064,7 @@ backtrace_initialize (struct backtrace_state *state, if (modules) free_modules (state, error_callback, data, &modules, bytes_allocated_for_modules); +#endif #endif if (!state->threaded)