From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by sourceware.org (Postfix) with ESMTP id 8380638708F2 for ; Sat, 16 May 2020 20:10:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8380638708F2 Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-371-my2FQxg0Mw6qR8pqdfYLVQ-1; Sat, 16 May 2020 16:10:12 -0400 X-MC-Unique: my2FQxg0Mw6qR8pqdfYLVQ-1 Received: by mail-wr1-f70.google.com with SMTP id r7so3050633wrc.13 for ; Sat, 16 May 2020 13:10:12 -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=iB+FZmWc01EhVuXfsjWfW8tt8AZuyfNgFVyG8KFObIk=; b=ejsIZkrKU9PGMfLbzKjd0bN2m2EbnjsmCOpsGlh+o235QloTSIXH495G0X87Zun4PY LFVE6Ap8hn/s2F6OZq2WY/unOV6/OCqRm3rsgDfUFc8QzP3ukUEyYozr5LSDaVhn5cMa NU7eXCDRCTyqMM/2hjbws9yChlxB0iK/8Av7bi9a3nTFrUCo4SbaLgHc/uz+0SGEMOh7 gl98LJxFYXY8P6ZwySdEuTKdgSm2qyf1axpGCmX7FyHrnFC9yhem7XOzBPIjoy7GO/Z7 qvWxcaFsaSR4MWocg5U8CjeUoX63cCLp7lpBzmBNsGW9wFD14vlb3wO0QaLOKWs2kgaD wZBw== X-Gm-Message-State: AOAM533NsXnFzk9lfyJP2c4V7VC7T05I1qpw5wJWJf1BGyVBn3KHwIHQ s9mKXrcWJgGIIoBvxts/y2DNy8fSMjOTZ7Dilt/vTT6+qbjf2K0qm87jrX2M/5dOO95Zse1KmjI 8Xb8417iJ/W0= X-Received: by 2002:a5d:6a01:: with SMTP id m1mr11573789wru.64.1589659810835; Sat, 16 May 2020 13:10:10 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzAm86PtDzBFq88rrKnEsTFu3elk77VOxAiM0O+NQpoHeERgLQS6u3LINW5tnoJEic5T0SjlA== X-Received: by 2002:a5d:6a01:: with SMTP id m1mr11573767wru.64.1589659810482; Sat, 16 May 2020 13:10:10 -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 w82sm8929179wmg.28.2020.05.16.13.10.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 16 May 2020 13:10:09 -0700 (PDT) Subject: Re: exec-file-mismatch and native-gdbserver testing To: "Metzger, Markus T" , GDB References: From: Pedro Alves Message-ID: Date: Sat, 16 May 2020 21:10:08 +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: Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.0 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@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 16 May 2020 20:10:18 -0000 On 4/8/20 3:02 PM, Metzger, Markus T via Gdb wrote: > Hello, > > I noticed an issue when running tests that use standard_temp_file with the native-gdbserver board. > > In gdb_remote_download, when called without tofile argument, as it is, for example, when starting gdbserver via gdb_reload, we set the filename to standard_output_file [file tail $fromfile] and copy the file. > > GDB and gdbserver now use a copy of the same file at different locations. > > This triggers an exec-file-mismatch warning and, with the default “ask” setting, a user prompt that isn’t handled by the tests and eventually leads to a timeout. This can be seen with all tests that use gdb_simple_compile, e.g. via skip_*_test calls. An example would be gdb.btrace/*.exp. > > In exec.c:validate_exec_file (), we check the filenames and, if they differ, print a warning and re-load the symbol file. > > Should validate_exec_file () check more than just the filenames? > Should gdb_simple_compile use standard_output_file instead of standard_temp_file? It probably can't hurt to make gdb_simple_compile use standard_output_file instead of standard_temp_file. I've tried it locally and ran the testsuite with that, and it indeed fixes most of the problem. It avoids copying the binary when testing with local gdbserver boards, so seems like a win regardless. Looking at the testresults made me realize that the new feature exposed at least one bug, fixed here: https://sourceware.org/pipermail/gdb-patches/2020-May/168680.html So I've been looking at the remaining failures that the gdb_simple_compile change wouldn't fix, to look for other bugs. The next one I'm looking at is gdb.base/argv0-symlink.exp. When tested with native-extended-gdbserver, we get: (gdb) run Starting program: /home/pedro/brno/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink-filelink warning: Mismatch between current exec-file /home/pedro/brno/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink-filelink and automatically determined exec-file /home/pedro/brno/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/brno/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink"? (y or n) I first thought that a quick fix at least for local boards could be to make validate_exec_file resolve symlinks before comparing the filename. But then, I thought -- how hard can it be to validate the build ids? So I cooked up something. Below's the resulting preliminary patch. Seems to work nicely -- it fixes gdb.base/argv0-symlink.exp at least. I haven't run the testsuite yet. There's (at least) one issue that I'll need to fix. It's to get rid of the "transfers from remote targets can be slow" warning when we open the remote file to read the build id: (gdb) r Starting program: /home/pedro/brno/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/break/break Reading /home/pedro/brno/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/brno/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/break/break and automatically determined exec-file /home/pedro/brno/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/brno/pedro/gdb/binutils-gdb/build/gdb/testsuite/outputs/gdb.base/argv0-symlink/argv0-symlink"? (y or n) For that, I'll need to pass down a struct with a warn_if_slow field as closure to gdb_bfd_openr_iovec, I think. 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 is the read-ahead cache, I think. 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 that. >From 51124c1ae7dde0c7ba033b4f841777b7d523c864 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Sat, 16 May 2020 20:56:54 +0100 Subject: [PATCH] validate_exec_file & build-id --- gdb/exec.c | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/gdb/exec.c b/gdb/exec.c index a2added5e22..04d94994894 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,11 +248,45 @@ 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; + /* 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)); + if (abfd != nullptr) + { + const bfd_build_id *target_exec_file_build_id + = build_id_bfd_get (abfd.get ()); + + 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; + } + } + } + std::string exec_file_target (pid_exec_file); /* In case the exec file is not local, exec_file_target has to point at @@ -259,7 +294,9 @@ validate_exec_file (int from_tty) if (is_target_filename (current_exec_file) && !target_filesystem_is_local ()) exec_file_target = TARGET_SYSROOT_PREFIX + exec_file_target; - if (exec_file_target != current_exec_file) + /* If build-id validation wasn't possible, fallback to validating by + filename. */ + if (build_id_mismatch || exec_file_target != current_exec_file) { warning (_("Mismatch between current exec-file %ps\n" base-commit: 7cfd74cfc6e14034779e6cc048c68877b7a08f88 -- 2.14.5