From patchwork Sat Jan 26 17:12:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 31218 Received: (qmail 126669 invoked by alias); 26 Jan 2019 17:12:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 126661 invoked by uid 89); 26 Jan 2019 17:12:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=am, shot, expensive, yes! X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 26 Jan 2019 17:12:20 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id x0QHCBcF018941 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 26 Jan 2019 12:12:16 -0500 Received: from [10.0.0.178] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 4BBA81E4A3; Sat, 26 Jan 2019 12:12:11 -0500 (EST) Subject: Re: [PATCH 1/2] Look for separate debug files in debug directories under a sysroot. From: Simon Marchi To: John Baldwin Cc: gdb-patches@sourceware.org References: <14397d0976052cba984ffbe8e0a54e37@polymtl.ca> Message-ID: <96b4fb01-d703-ad1b-5ea5-d7e5366f5f6b@polymtl.ca> Date: Sat, 26 Jan 2019 12:12:10 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <14397d0976052cba984ffbe8e0a54e37@polymtl.ca> X-IsSubscribed: yes On 2019-01-26 12:16 a.m., Simon Marchi wrote: > As discussed on IRC, we should probably add the same behavior for > build-id-based separate debug files. I gave a shot to this, here's what I have: From 377ef615ad6e2c57966c8853c61cbce95ea6c2b3 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Sat, 26 Jan 2019 11:34:45 -0500 Subject: [PATCH] Look for build-id-based separate debug files under the sysroot When looking for a separate debug file that matches a given build-id, GDB only looks in the host's debug dir (typically /usr/lib/debug). This patch makes it look in the sysroot as well. This is to match the behavior of GDB when using debuglink-based separate debug files. In the following example, my sysroot is "/tmp/sysroot" and I am trying to load symbols for /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so. This is the current behavior: (gdb) file /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so Reading symbols from /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so... Looking for separate debug info (build-id) for /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so Trying /usr/lib/debug/.build-id/f3/d6594d2600e985812cd4ba2ad083ac2aceae22.debug... no, unable to compute real path (No debugging symbols found in /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so) With this patch: (gdb) file /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so Reading symbols from /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so... Looking for separate debug info (build-id) for /tmp/sysroot/usr/lib/arm-linux-gnueabihf/gconv/EBCDIC-AT-DE.so Trying /usr/lib/debug/.build-id/f3/d6594d2600e985812cd4ba2ad083ac2aceae22.debug... no, unable to compute real path Trying /tmp/sysroot/usr/lib/debug/.build-id/f3/d6594d2600e985812cd4ba2ad083ac2aceae22.debug... yes! Reading symbols from /tmp/sysroot/usr/lib/debug/.build-id/f3/d6594d2600e985812cd4ba2ad083ac2aceae22.debug... In the original code, there is a suspicious "abfd.release ()" in build_id_to_debug_bfd, that I don't understand. If a file with the right name exists but its build-id note doesn't match, we release (leak) our reference, meaning the file will stay open? I removed it in the new code, so that the reference is dropped if we end up not using that file. I tested briefly by corrupting a separate debug file to trigger this code, nothing exploded. gdb/ChangeLog: * build-id.c (build_id_to_debug_bfd_1): New function. (build_id_to_debug_bfd): Look for separate debug file in sysroot. --- gdb/build-id.c | 115 +++++++++++++++++++++++++++++++------------------ 1 file changed, 72 insertions(+), 43 deletions(-) diff --git a/gdb/build-id.c b/gdb/build-id.c index e0c35579cd4a..27f29cd04423 100644 --- a/gdb/build-id.c +++ b/gdb/build-id.c @@ -65,13 +65,63 @@ build_id_verify (bfd *abfd, size_t check_len, const bfd_byte *check) return retval; } +/* Helper for build_id_to_debug_bfd. LINK is a path to a potential + build-id-based separate debug file, potentially a symlink to the real file. + If the file exists and matches BUILD_ID, return a BFD reference to it. */ + +static gdb_bfd_ref_ptr +build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len, + const bfd_byte *build_id) +{ + if (separate_debug_file_debug) + { + printf_unfiltered (_(" Trying %s..."), link.c_str ()); + gdb_flush (gdb_stdout); + } + + /* lrealpath() is expensive even for the usually non-existent files. */ + gdb::unique_xmalloc_ptr filename; + if (access (link.c_str (), F_OK) == 0) + filename.reset (lrealpath (link.c_str ())); + + if (filename == NULL) + { + if (separate_debug_file_debug) + printf_unfiltered (_(" no, unable to compute real path\n")); + + return {}; + } + + /* We expect to be silent on the non-existing files. */ + gdb_bfd_ref_ptr debug_bfd = gdb_bfd_open (filename.get (), gnutarget, -1); + + if (debug_bfd == NULL) + { + if (separate_debug_file_debug) + printf_unfiltered (_(" no, unable to open.\n")); + + return {}; + } + + if (!build_id_verify (debug_bfd.get(), build_id_len, build_id)) + { + if (separate_debug_file_debug) + printf_unfiltered (_(" no, build-id does not match.\n")); + + return {}; + } + + if (separate_debug_file_debug) + printf_unfiltered (_(" yes!\n")); + + return debug_bfd; +} + /* See build-id.h. */ gdb_bfd_ref_ptr build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id) { - gdb_bfd_ref_ptr abfd; - /* Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will cause "/.build-id/..." lookups. */ @@ -83,6 +133,10 @@ build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id) const gdb_byte *data = build_id; size_t size = build_id_len; + /* Compute where the file named after the build-id would be. + + If debugdir is "/usr/lib/debug" and the build-id is abcdef, this will + give "/usr/lib/debug/.build-id/ab/cdef.debug". */ std::string link = debugdir.get (); link += "/.build-id/"; @@ -97,53 +151,28 @@ build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id) link += ".debug"; - if (separate_debug_file_debug) - { - printf_unfiltered (_(" Trying %s..."), link.c_str ()); - gdb_flush (gdb_stdout); - } + gdb_bfd_ref_ptr debug_bfd + = build_id_to_debug_bfd_1 (link, build_id_len, build_id); + if (debug_bfd != NULL) + return debug_bfd; - /* lrealpath() is expensive even for the usually non-existent files. */ - gdb::unique_xmalloc_ptr filename; - if (access (link.c_str (), F_OK) == 0) - filename.reset (lrealpath (link.c_str ())); + /* Try to look under the sysroot as well. If the sysroot is + "/the/sysroot", it will give + "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug". - if (filename == NULL) + Don't do it if the sysroot is the target system ("target:"). It + could work in theory, but the lrealpath in build_id_to_debug_bfd_1 + only works with local paths. */ + if (strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0) { - if (separate_debug_file_debug) - printf_unfiltered (_(" no, unable to compute real path\n")); - - continue; + link = gdb_sysroot + link; + debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id); + if (debug_bfd != NULL) + return debug_bfd; } - - /* We expect to be silent on the non-existing files. */ - abfd = gdb_bfd_open (filename.get (), gnutarget, -1); - - if (abfd == NULL) - { - if (separate_debug_file_debug) - printf_unfiltered (_(" no, unable to open.\n")); - - continue; - } - - if (build_id_verify (abfd.get(), build_id_len, build_id)) - { - if (separate_debug_file_debug) - printf_unfiltered (_(" yes!\n")); - - break; - } - else - { - if (separate_debug_file_debug) - printf_unfiltered (_(" no, build-id does not match.\n")); - } - - abfd.release (); } - return abfd; + return {}; } /* See build-id.h. */