From patchwork Sun Oct 27 09:10:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 99670 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1355E3858027 for ; Sun, 27 Oct 2024 09:11:47 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id C2D703858405 for ; Sun, 27 Oct 2024 09:11:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C2D703858405 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C2D703858405 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730020274; cv=none; b=wdLGTUt8KxfwSYq4K0SBC/1qBnUwK0MXFrn9P+KxhwJfbA+7epJzU+KItXTVhGKRYfTxx0h425i6xW+puGco6e2OuJQOBbCbSHrl9AYDHqdXHEgs6qO4P3JKPqIeJTulEz8EKRKbMlAB3+IiSwDMXFBe4dmLue29bzBerBE+Mtk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730020274; c=relaxed/simple; bh=Ywy9k14gDEETjwjNVlGMw6piJtp37NDqzGhz/50e8Vw=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=InGBJ+V0huACEE3jY94+p1vxSjJVqMxUItHUSsyIByKcngDWYLUgVksKQhc7VxlRIELMm6NBQZ0NcVIkmO2tuQXV0FROjLPVghL0+U4CWHJp1wNXCdXeiGNSSN6BB5gZCql7VdoQdsbtikE71Uedeg4xIgbUXHo0UE87VKBZzjU= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730020272; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uBGuSkRu1JxDe7G4omALM7Mowc+1OHkfZ8q1ll1JY4c=; b=cfRM/hFu3t4LNxxFaANO1EyHwTiSIK+IMcHo4ZuITM0dG2lxOiyd77NGWstNj1EdW+uGYE +lsXPBLe6WZ3zXDSgrJRRFFrCN+8IM0/hEiNItrPbz/2iJH2Bp27DtjU/lxqNNhH/LQyME U8ddtx+gMec3p3ToDMfKFtxCYEyQ1yo= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-441-HO8uWvCvMU-fgOXuWNKF_A-1; Sun, 27 Oct 2024 05:11:03 -0400 X-MC-Unique: HO8uWvCvMU-fgOXuWNKF_A-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-4316300bb15so24270115e9.2 for ; Sun, 27 Oct 2024 02:11:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730020261; x=1730625061; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=uBGuSkRu1JxDe7G4omALM7Mowc+1OHkfZ8q1ll1JY4c=; b=UiAiUcJsCV1h5Wk/OQpcDjBH2yOh85mHWmFX2rc8cm/gH3MOOXSBn0Du6W8QRDegG9 oJasfDGj3TeDfRt6VY+rD4H5ECUnyXhmQUWD/SgFyPdNxuv+BbqyoHzYU+yREB5NbjQW YQGDrjdSflK+tND/H712sWxixrFEWYgvG7s91LfHBgp+Av3KcQk56gnpwCk8Mvgc14oE c5WSYbfXFwLbvKoZpkUIvihcANXzv/6VUVZjSt4ezmnQjbPul3QCMCmk9uHUmoupnjWp ToPZecgL/T4pMpXQiInPncIlj13Q2Yuyipo0fRjFB0SCkvc60gIJb4lOFRdCAZ+4EDgC A1sA== X-Gm-Message-State: AOJu0Yzjcc23d1CwZA/Og/4k3PAqP2Di3ykCKBBFYvmcuB19SLOc+QaE PduTfxB+hGNbX37Ac1S6SgQf2IGr7fTRasQSPe6dU40/lnkLTXyS3iP9zBekU571ET9/z3Ed06b n9DzLFpk5+S/OQiqcPNdBUjeb5K5F3q+fxZ0EvzoIrCDJA9HAdZnNNPMMO6nZMetlKkHcEsnp/T VNvVHWkCs3pzT4RCRwZGEDOkFVnS/pT6Ju/bhxaRnCr3I= X-Received: by 2002:a05:600c:3b1f:b0:42f:310f:de9 with SMTP id 5b1f17b1804b1-4319acac7fdmr32564255e9.15.1730020261012; Sun, 27 Oct 2024 02:11:01 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEtpTnc+1nGE/KZqc5eW6u0M0/PgIg7tIqs4/6GvYHCV+7a6iyeQQ0TkXmgjAg8N/IS6qXJQg== X-Received: by 2002:a05:600c:3b1f:b0:42f:310f:de9 with SMTP id 5b1f17b1804b1-4319acac7fdmr32564025e9.15.1730020260425; Sun, 27 Oct 2024 02:11:00 -0700 (PDT) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4319360c0besm69264165e9.41.2024.10.27.02.10.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 27 Oct 2024 02:10:58 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Tom Tromey Subject: [PATCHv2 1/4] gdb: use mapped file information to improve debuginfod text Date: Sun, 27 Oct 2024 09:10:52 +0000 Message-Id: <70e631978c2cee367380b20984c86ed59a3ec96b.1730020155.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org When opening a core-file GDB is able to use debuginfod to download the executable that matches the core-file if GDB can find a build-id for the executable in the core-file. In this case GDB calls debuginfod_exec_query to download the executable and GDB prints a message like: Downloading executable for /path/to/core-file... which makes sense in that case. For a long time GDB has also had the ability to download memory-mapped files and shared libraries when opening a core-file. However, recent commits have made these cases more likely to trigger, which is a good thing, but the messaging from GDB in these cases is not ideal. When downloading a memory-mapped file GDB prints: Downloading executable for /path/to/memory-mapped-file And for a shared library: Downloading executable for /path/to/libfoo.so These last two messages could, I think, be improved. I propose making two changes. First, I suggest instead of using /path/to/core-file in the first case, we use the name of the executable that GDB is fetching. This makes the messaging consistent in that we print the name of the file we're fetching rather than the name of the file we're fetching something for. I further propose that we replace 'executable for' with the more generic word 'file'. The messages will then become: Downloading file /path/to/exec-file... Downloading file /path/to/memory-mapped-file... Downloading file /path/to/libfoo.so... I think these messages are clearer than what we used to have, and they are consistent in that we name the thing being downloaded in all cases. There is one tiny problem. The first case relies on GDB knowing the name of the executable it wants to download. The only place we can currently get that from is, I think, the memory-mapped file list. [ ASIDE: There is `bfd_core_file_failing_command` which reports the executable and argument list from the core file, but this information is not ideal for this task. First, the executable and arguments are merged into a single string, and second, the string is a relatively short, fixed length string, so the executable name is often truncated. For these reasons I don't consider fetching the executable name using this bfd function as a solution. ] We do have to consider the case that the core file does not have any mapped file information. This shouldn't ever be the case for a Linux target, but it's worth considering. [ ASIDE: I mention Linux specifically because this only becomes a problem if we try to do a lookup via debuginfod, which requires that we have build-ids available. Linux has special support for embedding build-ids into the core file, but I'm not sure if other kernels do this. ] For the unlikely edge case of a core-file that has build-ids, but doesn't have any mapped file information then I propose that we synthesis a filename like: 'with build-id xxxxxx'. We would then see a message like: Downloading file with build-id xxxxxx... Where 'xxxxxx' would be replaced by the actual build-id. This isn't ideal, but I think is good enough, and, as I said, I think this case is not going to be hit very often, or maybe at all. We already had some tests that emitted two of the above messages, which I've updated, these cover the mapped-file and shared library case. The message about downloading the exec for the core-file is actually really hard to trigger now as usually the exec will also appear in the memory-mapped file list and GDB will download the file at this stage. Then when GDB needs the executable for loading the symbols it'll ask debuginfod, and debuginfod will find the file in its cache, and so no message will be printed. If anyone has any ideas about how to trigger this case then I'm happy to add additional tests. Approved-By: Tom Tromey --- gdb/corelow.c | 59 ++++++++++++++++++- gdb/debuginfod-support.c | 4 +- .../gdb.debuginfod/corefile-mapped-file.exp | 2 +- .../gdb.debuginfod/solib-with-soname.exp | 2 +- 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/gdb/corelow.c b/gdb/corelow.c index 5820ffed332..87ce04d6852 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -255,6 +255,27 @@ class core_target final : public process_stratum_target return m_mapped_file_info.lookup (filename, addr); } + /* Return a string containing the expected executable filename obtained + from the mapped file information within the core file. The filename + returned will be for the mapped file whose ELF headers are mapped at + the lowest address (i.e. which GDB encounters first). + + If no suitable filename can be found then the returned string will be + empty. + + If there are no build-ids embedded into the core file then the + returned string will be empty. + + If a non-empty string is returned then there is no guarantee that the + named file exists on disk, or if it does exist on disk, then the + on-disk file might have a different build-id to the desired + build-id. */ + const std::string & + expected_exec_filename () const + { + return m_expected_exec_filename; + } + private: /* per-core data */ /* Get rid of the core inferior. */ @@ -289,6 +310,11 @@ class core_target final : public process_stratum_target /* FIXME: kettenis/20031023: Eventually this field should disappear. */ struct gdbarch *m_core_gdbarch = NULL; + + /* If not empty then this contains the name of the executable discovered + when processing the memory-mapped file information. This will only + be set if we find a mapped with a suitable build-id. */ + std::string m_expected_exec_filename; }; core_target::core_target () @@ -418,11 +444,23 @@ core_target::build_file_mappings () } }); + /* Get the build-id of the core file. */ + const bfd_build_id *core_build_id + = build_id_bfd_get (current_program_space->core_bfd ()); + for (const auto &iter : mapped_files) { const std::string &filename = iter.first; const mapped_file &file_data = iter.second; + /* If this mapped file has the same build-id as was discovered for + the core-file itself, then we assume this is the main + executable. Record the filename as we can use this later. */ + if (file_data.build_id != nullptr + && m_expected_exec_filename.empty () + && build_id_equal (file_data.build_id, core_build_id)) + m_expected_exec_filename = filename; + /* Use exec_file_find() to do sysroot expansion. It'll also strip the potential sysroot "target:" prefix. If there is no sysroot, an equivalent (possibly more @@ -832,14 +870,29 @@ rename_vmcore_idle_reg_sections (bfd *abfd, inferior *inf) BFD ABFD. */ static void -locate_exec_from_corefile_build_id (bfd *abfd, int from_tty) +locate_exec_from_corefile_build_id (bfd *abfd, core_target *target, + int from_tty) { const bfd_build_id *build_id = build_id_bfd_get (abfd); if (build_id == nullptr) return; + /* The filename used for the find_objfile_by_build_id call. */ + std::string filename; + + if (!target->expected_exec_filename ().empty ()) + filename = target->expected_exec_filename (); + else + { + /* We didn't find an executable name from the mapped file + information, so as a stand-in build a string based on the + build-id. */ + std::string build_id_hex_str = bin2hex (build_id->data, build_id->size); + filename = string_printf ("with build-id %s", build_id_hex_str.c_str ()); + } + gdb_bfd_ref_ptr execbfd - = find_objfile_by_build_id (build_id, abfd->filename); + = find_objfile_by_build_id (build_id, filename.c_str ()); if (execbfd != nullptr) { @@ -972,7 +1025,7 @@ core_target_open (const char *arg, int from_tty) if (current_program_space->exec_bfd () == nullptr) locate_exec_from_corefile_build_id (current_program_space->core_bfd (), - from_tty); + target, from_tty); post_create_inferior (from_tty); diff --git a/gdb/debuginfod-support.c b/gdb/debuginfod-support.c index 841b6f2078c..9460ae18dd6 100644 --- a/gdb/debuginfod-support.c +++ b/gdb/debuginfod-support.c @@ -409,7 +409,7 @@ debuginfod_exec_query (const unsigned char *build_id, std::optional term_state; { - user_data data ("executable for", filename); + user_data data ("file", filename); debuginfod_set_user_data (c, &data); if (target_supports_terminal_ours ()) @@ -423,7 +423,7 @@ debuginfod_exec_query (const unsigned char *build_id, debuginfod_set_user_data (c, nullptr); } - print_outcome (fd.get (), "executable for", filename); + print_outcome (fd.get (), "file", filename); if (fd.get () >= 0) destname->reset (dname); diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp index cf96b41ac9a..cad70aaa3c6 100644 --- a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp +++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp @@ -370,7 +370,7 @@ with_debuginfod_env $cache { gdb_load $binfile load_core_file "load corefile, download library from debuginfod" \ - "Downloading\[^\r\n\]* executable for [string_to_regexp $library_filename]\\.\\.\\." + "Downloading\[^\r\n\]* file [string_to_regexp $library_filename]\\.\\.\\." set ptr_value [read_ptr_value] gdb_assert { $ptr_value == $ptr_expected_value } \ diff --git a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp index 9ef12041dc6..31ca7181af6 100644 --- a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp +++ b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp @@ -161,7 +161,7 @@ proc load_exec_and_core_file { expect_warning expect_download testname \ set saw_warning true exp_continue } - -re "^Downloading executable for \[^\r\n\]+/libfoo_1\\.so\\.\\.\\.\r\n" { + -re "^Downloading file \[^\r\n\]+/libfoo_1\\.so\\.\\.\\.\r\n" { set saw_download true exp_continue }