From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Hag3A99lcWCrRwAAWB0awg (envelope-from ) for ; Sat, 10 Apr 2021 04:46:23 -0400 Received: by simark.ca (Postfix, from userid 112) id F41631EE1B; Sat, 10 Apr 2021 04:46:22 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 3E8FE1E54D for ; Sat, 10 Apr 2021 04:46:21 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3954C3857C44; Sat, 10 Apr 2021 08:46:20 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3954C3857C44 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1618044380; bh=dKe3V3sFfV6udTdo9H1tRy1j9Fj+ZSmQ4lJ81CfMBr0=; h=Date:To:In-Reply-To:Subject:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=Qr86YmlJQQkSQ0b7RjmipGqpPsi/SmwXgzfqEPRkmZlrCQezERmSYQkhpErqVhYWm rCjtrXwsBrcoeoyx5b3V3eJxan5iIHdjUHLPp7Mrm+XOjxE6UbvrUN173Yh1RSonjK NsmE4vthZ5ThzqS1mLJeZLJI0WMbcSMtwz6YxZmc= Received: from eggs.gnu.org (eggs.gnu.org [IPv6:2001:470:142:3::10]) by sourceware.org (Postfix) with ESMTPS id C1E8C3857C44 for ; Sat, 10 Apr 2021 08:46:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C1E8C3857C44 Received: from fencepost.gnu.org ([2001:470:142:3::e]:41918) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lV9Fw-0002rI-RI; Sat, 10 Apr 2021 04:46:17 -0400 Received: from 84.94.185.95.cable.012.net.il ([84.94.185.95]:2645 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lV9Fu-0004LO-Qe; Sat, 10 Apr 2021 04:46:15 -0400 Date: Sat, 10 Apr 2021 11:46:01 +0300 Message-Id: <83y2dq5xyu.fsf@gnu.org> To: Simon Marchi In-Reply-To: (message from Simon Marchi on Thu, 8 Apr 2021 09:57:27 -0400) Subject: Re: Subtle problems with "info sharedlibrary" on MS-Windows References: <83czw7p4nd.fsf@gnu.org> <777379173.1335754.1615393830518@mail.yahoo.com> <83mtvbne96.fsf@gnu.org> <259022839.1083386.1615397702855@mail.yahoo.com> <83y2dwbow6.fsf@gnu.org> <83lf9vbljw.fsf@gnu.org> <83czv59rw1.fsf@gnu.org> X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Eli Zaretskii via Gdb-patches Reply-To: Eli Zaretskii Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" > Cc: ssbssa@yahoo.de, gdb-patches@sourceware.org > From: Simon Marchi > Date: Thu, 8 Apr 2021 09:57:27 -0400 > > >> According to our style guideline, we would use > >> > >> dll_name != nullptr > > > > Does this also mean the GDB style prefers, e.g., > > > > if (load_addr == nullptr) > > > > to > > > > if (!load_addr) > > > > ? Because you didn't comment on those lines, only on those where the > > value is tested for NOT being null. > > Exactly, sorry for not being clear. We always want explicit comparison > with nullptr for pointers, whether that is == or !=. > > Same for integers that are not used as booleans, we want > > if (item_count == 0) > > and not > > if (!item_count) > > This is described here: > > https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_nullptr_And_Zero > > >>> + if (!(load_addr && mi.lpBaseOfDll != load_addr)) > >> > >> Perhaps matter of personal preference, but I would understand it better > >> (less mental steps) as > >> > >> if (!load_addr || mi.lpBaseOfDll == load_addr) > > > > I feel the other way around, but maybe I'm the odd one out here. > > Pedro, Joel: what say you? > > > > In any case, I guess I could add a comment there explaining the logic > > in plain English, so everyone would understand the intent. > > That's alright, leave it the way it is. But a comment explaining the > intent is always welcome, in my book. OK, thanks. Below is what I actually pushed: commit b3885679dd71bed069e7e6fc2ae8b9eb05f90d62 Author: Eli Zaretskii AuthorDate: Sat Apr 10 11:33:08 2021 +0300 Commit: Eli Zaretskii CommitDate: Sat Apr 10 11:42:54 2021 +0300 Fix handling DLL loads at run time This patch makes handling a DLL load at run time (using LoadLibrary) much more reliable when its file name cannot be obtained using the lpImageName pointer provided by the DLL load debug event. The solution is to enumerate all the DLLs loaded by the inferior, looking for the DLL that's loaded at base address provided by the lpBaseOfDll pointer of the debug event. Correctly resolving the DLL file name is important, because without that GDB doesn't record the DLL in the list of solibs, and then later is unable to show functions in that DLL in the backtraces, which produces corrupted and truncated backtraces. See this thread for the problems that causes: https://sourceware.org/pipermail/gdb-patches/2021-March/177022.html gdb/ChangeLog: 2021-04-10 Eli Zaretskii * windows-nat.c (windows_nat::handle_load_dll): Call windows_add_dll if get_image_name failed to glean the name of the DLL by using the lpImageName pointer. (windows_add_all_dlls): Now a thin wrapper around windows_add_dll. (windows_add_dll): Now does what windows_add_all_dlls did before, but also accepts an argument LOAD_ADDR, which, if non-NULL, specifies the address where the DLL was loaded into the inferior, and looks for the single DLL loaded at that address. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index c5ed593..5c38a4e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2021-04-10 Eli Zaretskii + + * windows-nat.c (windows_nat::handle_load_dll): Call + windows_add_dll if get_image_name failed to glean the name of the + DLL by using the lpImageName pointer. + (windows_add_all_dlls): Now a thin wrapper around windows_add_dll. + (windows_add_dll): Now does what windows_add_all_dlls did before, + but also accepts an argument LOAD_ADDR, which, if non-NULL, + specifies the address where the DLL was loaded into the inferior, + and looks for the single DLL loaded at that address. + 2021-04-09 Luis Machado * nat/aarch64-mte-linux-ptrace.c: Update include file order. diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 00f8353..c0f64f8 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -869,6 +869,8 @@ windows_make_so (const char *name, LPVOID load_addr) return so; } +static bool windows_add_dll (LPVOID); + /* See nat/windows-nat.h. */ void @@ -884,11 +886,21 @@ windows_nat::handle_load_dll () (source: MSDN LOAD_DLL_DEBUG_INFO structure). */ dll_name = get_image_name (current_process_handle, event->lpImageName, event->fUnicode); - if (!dll_name) - return; + /* If the DLL name could not be gleaned via lpImageName, try harder + by enumerating all the DLLs loaded into the inferior, looking for + one that is loaded at base address = lpBaseOfDll. */ + if (dll_name != nullptr) + { - solib_end->next = windows_make_so (dll_name, event->lpBaseOfDll); - solib_end = solib_end->next; + solib_end->next = windows_make_so (dll_name, event->lpBaseOfDll); + solib_end = solib_end->next; + } + else if (event->lpBaseOfDll != nullptr + && windows_add_dll (event->lpBaseOfDll)) + dll_name = solib_end->so_name; + + if (dll_name == nullptr) + return; lm_info_windows *li = (lm_info_windows *) solib_end->lm_info; @@ -1899,6 +1911,19 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, static void windows_add_all_dlls (void) { + windows_add_dll (NULL); +} + +/* Iterate over all DLLs currently mapped by our inferior, looking for + a DLL which is loaded at LOAD_ADDR. If found, add the DLL to our + list of solibs and return 'true'; otherwise do nothing and return + 'false'. LOAD_ADDR NULL means add all DLLs to the list of solibs; + this is used when the inferior finishes its initialization, and all + the DLLs it statically depends on are presumed loaded. */ + +static bool +windows_add_dll (LPVOID load_addr) +{ HMODULE dummy_hmodule; DWORD cb_needed; HMODULE *hmodules; @@ -1910,18 +1935,18 @@ windows_add_all_dlls (void) if (EnumProcessModulesEx (current_process_handle, &dummy_hmodule, sizeof (HMODULE), &cb_needed, LIST_MODULES_32BIT) == 0) - return; + return false; } else #endif { if (EnumProcessModules (current_process_handle, &dummy_hmodule, sizeof (HMODULE), &cb_needed) == 0) - return; + return false; } if (cb_needed < 1) - return; + return false; hmodules = (HMODULE *) alloca (cb_needed); #ifdef __x86_64__ @@ -1930,14 +1955,14 @@ windows_add_all_dlls (void) if (EnumProcessModulesEx (current_process_handle, hmodules, cb_needed, &cb_needed, LIST_MODULES_32BIT) == 0) - return; + return false; } else #endif { if (EnumProcessModules (current_process_handle, hmodules, cb_needed, &cb_needed) == 0) - return; + return false; } char system_dir[__PMAX]; @@ -1983,6 +2008,7 @@ windows_add_all_dlls (void) if (GetModuleInformation (current_process_handle, hmodules[i], &mi, sizeof (mi)) == 0) continue; + if (GetModuleFileNameEx (current_process_handle, hmodules[i], dll_name, sizeof (dll_name)) == 0) continue; @@ -2005,9 +2031,17 @@ windows_add_all_dlls (void) name = syswow_dll_path.c_str(); } - solib_end->next = windows_make_so (name, mi.lpBaseOfDll); - solib_end = solib_end->next; + /* Record the DLL if either LOAD_ADDR is NULL or the address + at which the DLL was loaded is equal to LOAD_ADDR. */ + if (!(load_addr != nullptr && mi.lpBaseOfDll != load_addr)) + { + solib_end->next = windows_make_so (name, mi.lpBaseOfDll); + solib_end = solib_end->next; + if (load_addr != nullptr) + return true; + } } + return load_addr == nullptr ? true : false; } void