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

["Björn Schäpers" <gcc@hazardy.de>]

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.