From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id gAmYOJuiFGGNWQAAWB0awg (envelope-from ) for ; Thu, 12 Aug 2021 00:24:59 -0400 Received: by simark.ca (Postfix, from userid 112) id E582B1EDFB; Thu, 12 Aug 2021 00:24:59 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.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 75B401E813 for ; Thu, 12 Aug 2021 00:24:59 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 32F763839C51 for ; Thu, 12 Aug 2021 04:24:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 32F763839C51 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1628742299; bh=IAMxGcjC8oouhFrbUzLyVVrUfo1pryLn5s/VSThXMdY=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=WNCaZSjC1Jvfy2ZO72IhPb56Q+GY/yjPQ9TPkY6vXi5s1TQiBMBFw+BTB0Br25uRm FA1dYEqCzG0lWrC4aVDYQSDr4cpKXpeicQnz7cpy3qMfSaNqkDnnxfn51uWfQxlGGU UxqWRfi3mX+vyplizQmqWGmB8Wfg+JcDsxiycFJE= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id CDDB73877014 for ; Thu, 12 Aug 2021 04:24:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CDDB73877014 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-590-jm7yHEsVNumwETwxJ6Tnjg-1; Thu, 12 Aug 2021 00:24:08 -0400 X-MC-Unique: jm7yHEsVNumwETwxJ6Tnjg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6F9941082921; Thu, 12 Aug 2021 04:24:07 +0000 (UTC) Received: from localhost.localdomain.com (unknown [10.22.16.110]) by smtp.corp.redhat.com (Postfix) with ESMTP id D7F025D9C6; Thu, 12 Aug 2021 04:24:06 +0000 (UTC) To: gdb-patches@sourceware.org, simon.marchi@polymtl.ca, tom@tromey.com Subject: [PATCH v3 0/3] Add debuginfod core file support Date: Thu, 12 Aug 2021 00:24:03 -0400 Message-Id: <20210812042406.75637-1-amerey@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" 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: Aaron Merey via Gdb-patches Reply-To: Aaron Merey Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Hello, This patch series has been updated based on feedback from https://sourceware.org/pipermail/gdb-patches/2021-July/180956.html I want to address some points from Simon's review. > Instead of duplicating scan_dyntag, can we please extract a common > base? It is some non-trivial code, so it would be really good to > avoid having two copies. > > Everything until setting the dynptr looks like it could be > commonized. The common function could work like scan_dyntag, where you > pass a desired dyntag. But since you are actually looking for two > values, that would require two calls, so two scans. So instead I would > suggest making a "foreach_dyntag" function that takes a callback (a > gdb::function_view), where you invoke the callback for each dyntag. > The rest could then easily be implemented on top of that, and dyntag > parsing code would be at a single place. The first patch in this series moves the multiple implementations of scan_dyntag to solib.c so that duplication is avoided. I decided not to add a foreach_dyntag function because the reason for having more than one call to scan_dyntag was just to get the value of the DT_STRTAB tag. I don't think it's really necessary to get this value because, as far as I know, the value of DT_STRTAB always refers to the .dynstr section, which is easily found with bfd_get_section_by_name. Unless there are cases where DT_STRTAB doesn't refer to .dynstr, a foreach_dyntag shouldn't be needed. > What's the point of calling bfd_check_format without checking the > result? It looks like a function without side-effects. For some reason this function appears to update the flags and build-id fields of the bfd with the correct values. > In fact, that soname to buildid mapping doesn't seem related to > debuginfod at all, it's just a generic soname to buildid mapping. That > could exist and be useful even if debuginfod didn't exist, right? I'm > not sure what is the best place. But that information is produced by > the core target and consumed by the solib subsystem... so it should > probably be close to one of them. The second patch in this series implements a per-program_space soname to build-id mapping that is independent of debuginfod. > Not related to your patch but... would it be a good idea to show the > build-ids in "info proc mappings"? It might sound useful to have that > information in order to determine / find the right binaries that your > process was using. I think it would be useful in some circumstances. The changes to linux_read_core_file_mappings in the second patch of this series should make it easy to include build-ids in 'info proc mappings'. > > @@ -536,6 +538,23 @@ solib_map_sections (struct so_list *so) > > gdb::unique_xmalloc_ptr filename (tilde_expand (so->so_name)); > > gdb_bfd_ref_ptr abfd (ops->bfd_open (filename.get ())); > > > > + if (abfd == NULL) > > + { > > + gdb::unique_xmalloc_ptr build_id_hex; > > + > > + /* If a build-id was previously associated with this soname > > + then use it to query debuginfod for the library. */ > > + if (debuginfod_get_soname_buildid (so->so_name, &build_id_hex)) > > + { > > + scoped_fd fd = debuginfod_exec_query > > + ((const unsigned char*) build_id_hex.get (), > > + 0, so->so_name, &filename); > > + > > + if (fd.get () >= 0) > > + abfd = ops->bfd_open (filename.get ()); > > + } > > + } > > I have a question about the order of the operations here. Let's say I > generate a core on my ARM machine and bring it back to debug it on my > x86-64 machine. And let's say I have a /usr/lib/foo.so on both > machines. Will the first ops->bfd_open manage to open the one of my > development machine (the wrong one), and abfd will be != NULL? > > I am not sure what happens in that case, but if we could somehow > determine that we didn't open the right library (for example, seeing > that the lib's build-id doesn't match the expected build-id), and > instead try downloading the right one using debuginfod... would that > make sense? If the solib installed at the time of debugging has the same soname mentioned in the core file but a different build-id, then gdb will silently use this installed solib even though it's the wrong build. At least this is what happens when the arch stays the same. I have not tested this senario when using multiple architectures. The third patch in this series prints a warning message when a build-id mismatch happens. If debuginfod is enabled then gdb will also query debuginfod for the correct build of the solib. One thing I should point out is that if we have the wrong build of the solib installed locally and the debuginfod query fails because of some error, the user will see something like the following: warning: Build-id of /lib64/libc.so.6 does not match core file. Download failed: No route to host. Continuing without executable for /lib64/libc.so.6. However gdb is continuing with libc.so.6, although it is the wrong build. We could modify solib_map_sections so that if there is a build-id mismatch gdb refuses to use the solib. Or we could add a parameter to debuginfod_exec_query that replaces "Continuing without executable..." with some other string indicating that gdb will use the local solib despite the mismatch. Thanks, Aaron Aaron Merey (3): gdb/solib: Refactor scan_dyntag gdb: Add soname to build-id mapping for corefiles PR gdb/27570: missing support for debuginfod in core_target::build_file_mappings gdb/arch-utils.c | 21 ++- gdb/arch-utils.h | 21 ++- gdb/build-id.h | 2 + gdb/corelow.c | 46 ++++- gdb/debuginfod-support.c | 46 +++++ gdb/debuginfod-support.h | 16 ++ gdb/gcore.in | 3 + gdb/gdbarch.c | 2 +- gdb/gdbarch.h | 4 +- gdb/gdbarch.sh | 2 +- gdb/linux-tdep.c | 52 ++++-- gdb/progspace.c | 36 ++++ gdb/progspace.h | 17 ++ gdb/solib-dsbt.c | 104 +---------- gdb/solib-svr4.c | 118 +----------- gdb/solib.c | 174 ++++++++++++++++++ gdb/solib.h | 11 ++ .../gdb.debuginfod/fetch_src_and_symbols.exp | 25 ++- 18 files changed, 451 insertions(+), 249 deletions(-) -- 2.31.1