From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.hazardy.de (mail.hazardy.de [78.94.181.132]) by sourceware.org (Postfix) with ESMTPS id 2C28F3858D35; Sun, 7 Jan 2024 16:07:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2C28F3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=hazardy.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=hazardy.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2C28F3858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=78.94.181.132 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704643629; cv=none; b=JpJ3ep4WIjey7rijEs4qVRq/fAj0ftfPGHqeX9QRSdTmS9VT4csXzTVYSkylh+V2EZYIniMPEg16fxIucvfB5V/YltYHOhh/oxxYTwpJ2KPYAdAIXxPjmhwrk2w4lnlJ+CHLXfQsFG+5HkBLhwMgSknplY1F/ZL8TJaEW3c1D/o= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704643629; c=relaxed/simple; bh=BGVApAjWZdXyYyHus5rlbb3sRAiA8uVGbY+RwfbThJg=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=H6J0WN6dOOJPi3V06If+GMwZV1Q/tamts/SbToLVyYJ9x5dtR9K/PXzxgjKSTAhAEIEAXzQv6jl3NsnGMwFd2jGqr6YWqi1QakeOTxN5xG++xJ5q3U7/CikjP/8Am/+HtB5plYsuJuT+DxNvSSf4F4DofdcJuUjl0eX8kyouK9o= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from [10.0.1.129] (hades.hazardy.de [10.0.1.129]) by mail.hazardy.de (Postfix) with ESMTPSA id EC01A700276; Sun, 7 Jan 2024 17:07:05 +0100 (CET) Message-ID: <4630f8bb-5859-483e-9db3-e09e4cb46ecf@hazardy.de> Date: Sun, 7 Jan 2024 17:07:06 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 6/4] libbacktrace: Add loaded dlls after initialize To: Eli Zaretskii Cc: iant@google.com, gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org References: <20230120105409.54949-1-gcc@hazardy.de> <20230120105409.54949-4-gcc@hazardy.de> <0c08e584-499f-473f-8699-a41c6a967536@hazardy.de> <863fd658-11e9-455e-9def-8a95217bc98f@hazardy.de> <83bk9xsklk.fsf@gnu.org> <4cb3a2a5-c0b3-40c8-b460-f21d65a9aea2@hazardy.de> <83ttnpqk0g.fsf@gnu.org> Content-Language: de-DE From: =?UTF-8?Q?Bj=C3=B6rn_Sch=C3=A4pers?= In-Reply-To: <83ttnpqk0g.fsf@gnu.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.6 required=5.0 tests=BAYES_00,KAM_DMARC_STATUS,KAM_SHORT,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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-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.