From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by sourceware.org (Postfix) with ESMTP id 04236388A83E for ; Mon, 18 May 2020 13:55:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 04236388A83E Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-12--ZMQCa33P4iP5kyotCKtzg-1; Mon, 18 May 2020 09:55:31 -0400 X-MC-Unique: -ZMQCa33P4iP5kyotCKtzg-1 Received: by mail-wr1-f71.google.com with SMTP id r14so5689928wrw.8 for ; Mon, 18 May 2020 06:55:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=tiT2xFqsLgYZ/W8r6yyi2+SGNpz2uRwoicB7Dp8O7zg=; b=kSq+8izQB9y1gm3eInP4oh0VgFl7rxICcdi3KDujSRWlGUpFJrIDMckT4BUU1whdO5 D6UL92JdkzCv0DLtZMBJ006n+GFdfD6mP/bBA6Hazz2Bz9T3ciBR/GTJggZ/H6wKGGZk 72vk4R5buIHZMEMRvD6ry4aFQUmMgJZSfiKuRlK+ZWSRV7G0lu0mR7wbMMI8FHjhlgDc ZF+0Q5/wfCu8tQCIXHueZGtZCIMVRnGrQDEF6YlIZeui2ti/YJn6nXxeFv6cXmIDl65r x2N2OHIOYExcNxlc7revtW/e1oixI6UtP0vpxBDgVyCHb/shJbVbuHYE9b7QO/T84+zL tUvw== X-Gm-Message-State: AOAM532yQKzy3SRe9eyxamaICC5IYTryY7U5ZRJkie8hAJl/yPJTMJsr m/d+LdsEEPAXP3RlPkBukG3zg/kEBeXROCiNfcQ/c9xqHjTul0I7HuHWffpsTT8wTPE0xpYR8ZZ PUgdUWXtmhVMEW6Br0kuM+A== X-Received: by 2002:adf:b1c1:: with SMTP id r1mr19740648wra.76.1589810130230; Mon, 18 May 2020 06:55:30 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzOvV6rBb5396KFvVmREEJgiIIiiIj9YHowMHIO0tc/jRUaJj1uqoPX4xcR6pQmWJSztvlwGw== X-Received: by 2002:adf:b1c1:: with SMTP id r1mr19740629wra.76.1589810129936; Mon, 18 May 2020 06:55:29 -0700 (PDT) Received: from ?IPv6:2001:8a0:f909:7b00:56ee:75ff:fe8d:232b? ([2001:8a0:f909:7b00:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id y4sm17070484wro.91.2020.05.18.06.55.28 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 May 2020 06:55:29 -0700 (PDT) Subject: [PATCH v2 3/3] Make exec-file-mismatch compare build IDs To: gdb-patches@sourceware.org References: <20200517180450.14925-1-palves@redhat.com> <20200517180450.14925-4-palves@redhat.com> From: Pedro Alves Message-ID: Date: Mon, 18 May 2020 14:55:28 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200517180450.14925-4-palves@redhat.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.7 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_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.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: , X-List-Received-Date: Mon, 18 May 2020 13:55:38 -0000 On 5/17/20 7:04 PM, Pedro Alves via Gdb-patches wrote: > The patch makes GDB first try exec-file-mismatch validation via build > IDs, and then if that isn't possible, fallback to validating using the > old method of comparing filenames. I'd argue that we should remove > the filename validation for causing too many false positives, though. I've argued about that yesterday over at gdb@, in this thread: https://sourceware.org/pipermail/gdb/2020-May/048544.html Here's an updated version that removes the filename comparison. I've kept the structure of the code the same, in case we add some form of fallback later on. >From 771c3a52fd88c351bd4e93491f591f9b942ce217 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 18 May 2020 14:49:34 +0100 Subject: [PATCH] Make exec-file-mismatch compare build IDs The patch makes GDB do exec-file-mismatch validation by comparing build IDs instead of the current method of comparing filenames. Currently, the exec-file-mismatch feature simply compares filenames to decide whether the exec file loaded in gdb and the exec file the target reports is running match. This causes false positives when remote debugging, because it'll often be the case that the paths in the host and the target won't match. And of course misses the case of the files having the same name but being actually different files (e.g., different builds). This also broke many testcases when running against gdbserver, causing tests to be skipped like (here native-extended-gdbserver): (gdb) run Starting program: /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink-filelink warning: Mismatch between current exec-file /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink-filelink and automatically determined exec-file /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink exec-file-mismatch handling is currently "ask" Load new symbol table from "/home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink"? (y or n) UNTESTED: gdb.base/argv0-symlink.exp: could not run to main or to fail like (here native-gdbserver): (gdb) spawn /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/../../gdbserver/gdbserver --once localhost:2346 /home/pedro/gdb/binutils-gdb/build/gdb/te stsuite/outputs/gdb.btrace/buffer-size/skip_btrace_tests-19968.x Process /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.btrace/buffer-size/skip_btrace_tests-19968.x created; pid = 20040 Listening on port 2346 target remote localhost:2346 Remote debugging using localhost:2346 warning: Mismatch between current exec-file /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/temp/19968/skip_btrace_tests-19968.x and automatically determined exec-file /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.btrace/buffer-size/skip_btrace_tests-19968.x exec-file-mismatch handling is currently "ask" Load new symbol table from "/home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.btrace/buffer-size/skip_btrace_tests-19968.x"? (y or n) Quit (gdb) UNSUPPORTED: gdb.btrace/buffer-size.exp: target does not support record-btrace The former case is about GDB not realizing the two files are the same, because one of the them is a symlink to the other. The latter case is about GDB realizing that one file is a copy of the other. Over the years, the toolchain has settled on build ID matching being the canonical method to match core dumps to executables, and executables with no debug info to their debug info. This patch makes us use build IDs to match the running image of a binary with its version loaded in gdb, which may or may not have debug info. This is very much like the core dump/executable matching. The change to gdb_bfd_open is necessary to get rid of the "transfers from remote targets can be slow" warning when we open the remote file to read its build ID: (gdb) r Starting program: /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/break/break Reading /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink from remote target... warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: Mismatch between current exec-file /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/break/break and automatically determined exec-file /home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink exec-file-mismatch handling is currently "ask" Load new symbol table from "/home/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink"? (y or n) While trying this out, I was worried that bfd would read a lot of stuff from the binary in order to extract the build ID, making it potentially slow, but turns out we don't read all that much. Maybe a couple hundred bytes, and most of it seemingly is the read-ahead cache. So I'm not worried about that. Otherwise I'd consider whether a new qXfer:buildid:read would be better. But I'm happy that we seemingly don't need to worry about it. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * NEWS (set exec-file-mismatch): Adjust entry. * exec.c: Include "build-id.h". (validate_exec_file): Try to match build IDs instead of filenames. * gdb_bfd.c (struct gdb_bfd_open_closure): New. (gdb_bfd_iovec_fileio_open): Adjust to use gdb_bfd_open_closure and pass down 'warn_if_slow'. (gdb_bfd_open): Add 'warn_if_slow' parameter. Use gdb_bfd_open_closure to pass it down. * gdb_bfd.h (gdb_bfd_open): Add 'warn_if_slow' parameter. gdb/doc/ChangeLog: yyyy-mm-dd Pedro Alves * gdb.texinfo (Attach): Update exec-file-mismatch description to mention build IDs. (Separate Debug Files): Add "build id" anchor. --- gdb/doc/gdb.texinfo | 21 +++++++++++---------- gdb/NEWS | 15 +++++++-------- gdb/exec.c | 49 ++++++++++++++++++++++++++++++++++++++++++------- gdb/gdb_bfd.c | 23 ++++++++++++++++------- gdb/gdb_bfd.h | 6 ++++-- 5 files changed, 80 insertions(+), 34 deletions(-) diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 8f3301259af..64181628495 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -2909,22 +2909,22 @@ the @code{file} command to load the program. @xref{Files, ,Commands to Specify Files}. @anchor{set exec-file-mismatch} -If the debugger can determine the name of the executable file running -in the process it is attaching to, and this file name does not match -the name of the current exec-file loaded by @value{GDBN}, the option -@code{exec-file-mismatch} specifies how to handle the mismatch. +If the debugger can determine that the executable file running in the +process it is attaching to does not match the current exec-file loaded +by @value{GDBN}, the option @code{exec-file-mismatch} specifies how to +handle the mismatch. @value{GDBN} tries to compare the files by +comparing their build IDs (@pxref{build ID}), if available. @table @code @kindex exec-file-mismatch @cindex set exec-file-mismatch @item set exec-file-mismatch @samp{ask|warn|off} -Whether to detect mismatch between the name of the current executable -file loaded by @value{GDBN} and the name of the executable file used to -start the process. If @samp{ask}, the default, display a warning -and ask the user whether to load the process executable file; if -@samp{warn}, just display a warning; if @samp{off}, don't attempt to -detect a mismatch. +Whether to detect mismatch between the current executable file loaded +by @value{GDBN} and the executable file used to start the process. If +@samp{ask}, the default, display a warning and ask the user whether to +load the process executable file; if @samp{warn}, just display a +warning; if @samp{off}, don't attempt to detect a mismatch. @cindex show exec-file-mismatch @item show exec-file-mismatch @@ -20874,6 +20874,7 @@ checksum for the debug file, which @value{GDBN} uses to validate that the executable and the debug file came from the same build. @item +@anchor{build ID} The executable contains a @dfn{build ID}, a unique bit string that is also present in the corresponding debug info file. (This is supported only on some operating systems, when using the ELF or PE file formats diff --git a/gdb/NEWS b/gdb/NEWS index a059fc7aa0e..2a9c8b8ee19 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -54,14 +54,13 @@ set exec-file-mismatch -- Set exec-file-mismatch handling (ask|warn|off). show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off). - Set or show the option 'exec-file-mismatch'. When GDB attaches to - a running process and can determine the name of the executable file - the process runs, this new option indicates whether to detect mismatch - between the name of the current executable file loaded by GDB - and the name of the executable file used to start the process. - If 'ask', the default, display a warning and ask the user - whether to load the process executable file; if 'warn', just display - a warning; if 'off', don't attempt to detect a mismatch. + Set or show the option 'exec-file-mismatch'. When GDB attaches to a + running process, this new option indicates whether to detect + a mismatch between the current executable file loaded by GDB and the + executable file used to start the process. If 'ask', the default, + display a warning and ask the user whether to load the process + executable file; if 'warn', just display a warning; if 'off', don't + attempt to detect a mismatch. tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]... Define a new TUI layout, specifying its name and the windows that diff --git a/gdb/exec.c b/gdb/exec.c index a2added5e22..8b89828ddda 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -37,6 +37,7 @@ #include "gdb_bfd.h" #include "gcore.h" #include "source.h" +#include "build-id.h" #include #include "readline/tilde.h" @@ -247,20 +248,54 @@ validate_exec_file (int from_tty) struct inferior *inf = current_inferior (); /* Try to determine a filename from the process itself. */ const char *pid_exec_file = target_pid_to_exec_file (inf->pid); + bool build_id_mismatch = false; - /* If wee cannot validate the exec file, return. */ + /* If we cannot validate the exec file, return. */ if (current_exec_file == NULL || pid_exec_file == NULL) return; - std::string exec_file_target (pid_exec_file); + /* Try validating via build-id, if available. This is the most + reliable check. */ + const bfd_build_id *exec_file_build_id = build_id_bfd_get (exec_bfd); + if (exec_file_build_id != nullptr) + { + /* Prepend the target prefix, to force gdb_bfd_open to open the + file on the remote file system (if indeed remote). */ + std::string target_pid_exec_file + = std::string (TARGET_SYSROOT_PREFIX) + pid_exec_file; + + gdb_bfd_ref_ptr abfd (gdb_bfd_open (target_pid_exec_file.c_str (), + gnutarget, -1, false)); + if (abfd != nullptr) + { + const bfd_build_id *target_exec_file_build_id + = build_id_bfd_get (abfd.get ()); - /* In case the exec file is not local, exec_file_target has to point at - the target file system. */ - if (is_target_filename (current_exec_file) && !target_filesystem_is_local ()) - exec_file_target = TARGET_SYSROOT_PREFIX + exec_file_target; + if (target_exec_file_build_id != nullptr) + { + if (exec_file_build_id->size == target_exec_file_build_id->size + && memcmp (exec_file_build_id->data, + target_exec_file_build_id->data, + exec_file_build_id->size) == 0) + { + /* Match. */ + return; + } + else + build_id_mismatch = true; + } + } + } - if (exec_file_target != current_exec_file) + if (build_id_mismatch) { + std::string exec_file_target (pid_exec_file); + + /* In case the exec file is not local, exec_file_target has to point at + the target file system. */ + if (is_target_filename (current_exec_file) && !target_filesystem_is_local ()) + exec_file_target = TARGET_SYSROOT_PREFIX + exec_file_target; + warning (_("Mismatch between current exec-file %ps\n" "and automatically determined exec-file %ps\n" diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c index 61872a0bf3d..25e0178a8b8 100644 --- a/gdb/gdb_bfd.c +++ b/gdb/gdb_bfd.c @@ -271,22 +271,29 @@ fileio_errno_to_host (int errnum) return -1; } +/* bfd_openr_iovec OPEN_CLOSURE data for gdb_bfd_open. */ +struct gdb_bfd_open_closure +{ + inferior *inf; + bool warn_if_slow; +}; + /* Wrapper for target_fileio_open suitable for passing as the - OPEN_FUNC argument to gdb_bfd_openr_iovec. The supplied - OPEN_CLOSURE is unused. */ + OPEN_FUNC argument to gdb_bfd_openr_iovec. */ static void * -gdb_bfd_iovec_fileio_open (struct bfd *abfd, void *inferior) +gdb_bfd_iovec_fileio_open (struct bfd *abfd, void *open_closure) { const char *filename = bfd_get_filename (abfd); int fd, target_errno; int *stream; + gdb_bfd_open_closure *oclosure = (gdb_bfd_open_closure *) open_closure; gdb_assert (is_target_filename (filename)); - fd = target_fileio_open ((struct inferior *) inferior, + fd = target_fileio_open (oclosure->inf, filename + strlen (TARGET_SYSROOT_PREFIX), - FILEIO_O_RDONLY, 0, true, + FILEIO_O_RDONLY, 0, oclosure->warn_if_slow, &target_errno); if (fd == -1) { @@ -378,7 +385,8 @@ gdb_bfd_iovec_fileio_fstat (struct bfd *abfd, void *stream, /* See gdb_bfd.h. */ gdb_bfd_ref_ptr -gdb_bfd_open (const char *name, const char *target, int fd) +gdb_bfd_open (const char *name, const char *target, int fd, + bool warn_if_slow) { hashval_t hash; void **slot; @@ -392,9 +400,10 @@ gdb_bfd_open (const char *name, const char *target, int fd) { gdb_assert (fd == -1); + gdb_bfd_open_closure open_closure { current_inferior (), warn_if_slow }; return gdb_bfd_openr_iovec (name, target, gdb_bfd_iovec_fileio_open, - current_inferior (), + &open_closure, gdb_bfd_iovec_fileio_pread, gdb_bfd_iovec_fileio_close, gdb_bfd_iovec_fileio_fstat); diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h index 532520e0f7d..a941b79fe73 100644 --- a/gdb/gdb_bfd.h +++ b/gdb/gdb_bfd.h @@ -78,10 +78,12 @@ typedef gdb::ref_ptr gdb_bfd_ref_ptr; If the BFD was not accessed using target fileio operations then the filename associated with the BFD and accessible with bfd_get_filename will not be exactly NAME but rather NAME with - TARGET_SYSROOT_PREFIX stripped. */ + TARGET_SYSROOT_PREFIX stripped. If WARN_IF_SLOW is true, print a + warning message if the file is being accessed over a link that may + be slow. */ gdb_bfd_ref_ptr gdb_bfd_open (const char *name, const char *target, - int fd = -1); + int fd = -1, bool warn_if_slow = true); /* Mark the CHILD BFD as being a member of PARENT. Also, increment the reference count of CHILD. Calling this function ensures that base-commit: 7cfd74cfc6e14034779e6cc048c68877b7a08f88 prerequisite-patch-id: b945b532dec51eb5ae7fdb9b8fe9007deca8d276 prerequisite-patch-id: 6730601601e24970f31193c533a7c257c7e1c48c -- 2.14.5