Message ID | 1460027557-20679-1-git-send-email-palves@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 70333 invoked by alias); 7 Apr 2016 11:12:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 70256 invoked by uid 89); 7 Apr 2016 11:12:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=1767 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 07 Apr 2016 11:12:42 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 16BC5C056819; Thu, 7 Apr 2016 11:12:39 +0000 (UTC) Received: from cascais.lan (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u37BCbTc012925; Thu, 7 Apr 2016 07:12:38 -0400 From: Pedro Alves <palves@redhat.com> To: gdb-patches@sourceware.org Cc: Simon Marchi <simon.marchi@ericsson.com> Subject: [PATCH] Fix gdb.server/solib-list.exp regression Date: Thu, 7 Apr 2016 12:12:37 +0100 Message-Id: <1460027557-20679-1-git-send-email-palves@redhat.com> |
Commit Message
Pedro Alves
April 7, 2016, 11:12 a.m. UTC
Commit 7817ea46148d (Improve gdb_remote_download, remove gdb_download) caused: FAIL: gdb.server/solib-list.exp: non-stop 0: target extended-remote (timeout) FAIL: gdb.server/solib-list.exp: non-stop 0: continue (the program is no longer running) FAIL: gdb.server/solib-list.exp: non-stop 0: p libvar FAIL: gdb.server/solib-list.exp: non-stop 1: target extended-remote (timeout) FAIL: gdb.server/solib-list.exp: non-stop 1: continue (the program is no longer running) FAIL: gdb.server/solib-list.exp: non-stop 1: p libvar gdb.log shows: system interpreter is: /lib64/ld-linux-x86-64.so.2 ... spawn ../gdbserver/gdbserver --once :2347 /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.server/solib-list/ld-linux-x86-64.so.2 /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.server/solib-list/solib-list Process /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.server/solib-list/ld-linux-x86-64.so.2 created; pid = 18637 Cannot exec /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.server/solib-list/ld-linux-x86-64.so.2: No such file or directory. ... The test copied the interpreter to the outputs directory, however ld-linux-x86-64.so.2 is a relative symlink that when copied points nowhere: $ ls -l testsuite/outputs/gdb.server/solib-list/ total 52 -rwxrwxr-x. 1 pedro pedro 13450 Apr 7 10:52 gdb.log -rw-rw-r--. 1 pedro pedro 1512 Apr 7 10:52 gdb.sum lrwxrwxrwx. 1 pedro pedro 10 Apr 7 11:39 ld-linux-x86-64.so.2 -> ld-2.22.so -rwxrwxr-x. 1 pedro pedro 9464 Apr 7 11:39 solib-list -rw-rw-r--. 1 pedro pedro 3472 Apr 7 11:39 solib-list-lib.c.o -rw-rw-r--. 1 pedro pedro 2760 Apr 7 11:39 solib-list.o -rwxrwxr-x. 1 pedro pedro 9232 Apr 7 11:39 solib-list.so The copying comes from gdbserver_spawn -> gdbserver_download_current_prog -> gdb_remote_download. My first attempt at a fix made solib-list.exp resolve the interpreter symlink, so that the copying works, and that did make the test pass. However, I noticed something else, which can be reduced to this: gdb_load ${binfile} gdbserver_spawn gdb_load passes ${binfile} unmodified to the "file" command. If $binfile needs to be run through standard_output_file, that should be done before calling gdb_load, and it normally is. In solib-list.exp's case, the ${binfile} is the system interpreter, outside the outputs directory. Since gdbserver_download_current_prog copies the file to the outputs directory and runs it there, we'd end up with a mismatch between what gdbserver starts, and gdb loads. So to address that we'd have to have the test do: +proc gdb_realpath {filename} { + ... +} set interp_system [section_get ${binfile} .interp] +set interp_system [gdb_realpath $interp_system] +set interp_system [standard_output_file $interp_system] But that all seems like pointless copying/work in the first place. I think that instead, when local testing, we should make what gdbserver runs be exactly what was passed to gdb_load / gdb_file_cmd, which is what we used to do before commit 7817ea46148d (Improve gdb_remote_download, remove gdb_download), which did: set gdbserver_host_mtime [file mtime $host_exec] - if [is_remote target] { - set gdbserver_server_exec [gdb_download $host_exec] - } else { - set gdbserver_server_exec $host_exec - } + set gdbserver_server_exec [gdb_remote_download target $host_exec] gdb/ChangeLog: 2016-04-07 Pedro Alves <palves@redhat.com> * lib/gdbserver-support.exp (gdbserver_download_current_prog): Skip download if the target is local. --- gdb/testsuite/lib/gdbserver-support.exp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
Comments
On 16-04-07 07:12 AM, Pedro Alves wrote: > Commit 7817ea46148d (Improve gdb_remote_download, remove gdb_download) > caused: > > FAIL: gdb.server/solib-list.exp: non-stop 0: target extended-remote (timeout) > FAIL: gdb.server/solib-list.exp: non-stop 0: continue (the program is no longer running) > FAIL: gdb.server/solib-list.exp: non-stop 0: p libvar > FAIL: gdb.server/solib-list.exp: non-stop 1: target extended-remote (timeout) > FAIL: gdb.server/solib-list.exp: non-stop 1: continue (the program is no longer running) > FAIL: gdb.server/solib-list.exp: non-stop 1: p libvar > > gdb.log shows: > > system interpreter is: /lib64/ld-linux-x86-64.so.2 > ... > spawn ../gdbserver/gdbserver --once :2347 /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.server/solib-list/ld-linux-x86-64.so.2 /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.server/solib-list/solib-list > Process /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.server/solib-list/ld-linux-x86-64.so.2 created; pid = 18637 > Cannot exec /home/pedro/brno/pedro/gdb/mygit/build/gdb/testsuite/outputs/gdb.server/solib-list/ld-linux-x86-64.so.2: No such file or directory. > ... > > The test copied the interpreter to the outputs directory, however > ld-linux-x86-64.so.2 is a relative symlink that when copied points > nowhere: > > $ ls -l testsuite/outputs/gdb.server/solib-list/ > total 52 > -rwxrwxr-x. 1 pedro pedro 13450 Apr 7 10:52 gdb.log > -rw-rw-r--. 1 pedro pedro 1512 Apr 7 10:52 gdb.sum > lrwxrwxrwx. 1 pedro pedro 10 Apr 7 11:39 ld-linux-x86-64.so.2 -> ld-2.22.so > -rwxrwxr-x. 1 pedro pedro 9464 Apr 7 11:39 solib-list > -rw-rw-r--. 1 pedro pedro 3472 Apr 7 11:39 solib-list-lib.c.o > -rw-rw-r--. 1 pedro pedro 2760 Apr 7 11:39 solib-list.o > -rwxrwxr-x. 1 pedro pedro 9232 Apr 7 11:39 solib-list.so At least I have an excuse for not seeing this one :). The link on Debian-based distros seems to be absolute, so it works even when copying the link. $ ls -l /lib64 lrwxrwxrwx 1 root root 32 Feb 16 14:29 ld-linux-x86-64.so.2 -> /lib/x86_64-linux-gnu/ld-2.19.so $ ls -l testsuite/outputs/gdb.server/solib-list lrwxrwxrwx 1 emaisin camorndithub 32 Apr 7 09:58 ld-linux-x86-64.so.2 -> /lib/x86_64-linux-gnu/ld-2.19.so ... > The copying comes from gdbserver_spawn -> > gdbserver_download_current_prog -> gdb_remote_download. > > My first attempt at a fix made solib-list.exp resolve the interpreter > symlink, so that the copying works, and that did make the test pass. > > However, I noticed something else, which can be reduced to this: > > gdb_load ${binfile} > gdbserver_spawn > > gdb_load passes ${binfile} unmodified to the "file" command. If > $binfile needs to be run through standard_output_file, that should be > done before calling gdb_load, and it normally is. > > In solib-list.exp's case, the ${binfile} is the system interpreter, > outside the outputs directory. Since gdbserver_download_current_prog > copies the file to the outputs directory and runs it there, we'd end > up with a mismatch between what gdbserver starts, and gdb loads. So > to address that we'd have to have the test do: > > +proc gdb_realpath {filename} { > + ... > +} > > set interp_system [section_get ${binfile} .interp] > +set interp_system [gdb_realpath $interp_system] > +set interp_system [standard_output_file $interp_system] > > But that all seems like pointless copying/work in the first place. I > think that instead, when local testing, we should make what gdbserver > runs be exactly what was passed to gdb_load / gdb_file_cmd, which is > what we used to do before commit 7817ea46148d (Improve > gdb_remote_download, remove gdb_download), which did: > > set gdbserver_host_mtime [file mtime $host_exec] > - if [is_remote target] { > - set gdbserver_server_exec [gdb_download $host_exec] > - } else { > - set gdbserver_server_exec $host_exec > - } > + set gdbserver_server_exec [gdb_remote_download target $host_exec] > > gdb/ChangeLog: > 2016-04-07 Pedro Alves <palves@redhat.com> > > * lib/gdbserver-support.exp (gdbserver_download_current_prog): > Skip download if the target is local. > --- > gdb/testsuite/lib/gdbserver-support.exp | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp > index 67a8333..817331b 100644 > --- a/gdb/testsuite/lib/gdbserver-support.exp > +++ b/gdb/testsuite/lib/gdbserver-support.exp > @@ -176,7 +176,16 @@ proc gdbserver_download_current_prog { } { > if { $reuse == 0 } { > set gdbserver_host_exec $host_exec > set gdbserver_host_mtime [file mtime $host_exec] > - set gdbserver_server_exec [gdb_remote_download target $host_exec] > + > + # If local testing, don't use gdb_remote_download, which would > + # copy the file to the testcase's output directory. Use > + # LAST_LOADED_FILE as is, so that gdbserver starts the exact > + # same filename that will be handed to gdb's file command. > + if [is_remote target] { > + set gdbserver_server_exec [gdb_remote_download target $host_exec] > + } else { > + set gdbserver_server_exec $host_exec > + } > } > > return $gdbserver_server_exec I am not sure how I feel about this. Here is some brain dump, but in the end it's your call. I wouldn't see it as a problem that gdb and gdbserver are not started with the same filename, as long as the files are identical. After all, when they run on different systems, they are not loading the same file. So another solution could be to make gdb_remote_download get the realpath of its source path, so that we don't copy a symlink in the output directory (it doesn't sound like something we would ever want to do). Maybe it's more generic to handle it at this level? This way, if there is another instance of a symlink being copied, we won't have to add such a workaround. Also, it follows the idea (which I like) of having all the artifacts involved in a test case in the test's output directory. Similarly, we wouldn't need to copy Python .py files over to the output directory, but I find it nice that everything that is used gets copied at the same place. Another idea, I see that the test doesn't support remote testing right now: # This test case (currently) does not support remote targets, since it # assumes the ELF interpreter can be found on the host system if [is_remote target] then { return } Maybe the more "definitive" solution would be to make it work with a real remote target? Then, we would have to do a remote_upload of the interpreter from the target to the output directory on the build machine. We would just have to make sure that DejaGnu's remote_upload handles symlinks as we want to (i.e. that it doesn't just copy the link), and if it doesn't, then we write a gdb_remote_upload wrapper. Simon
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp index 67a8333..817331b 100644 --- a/gdb/testsuite/lib/gdbserver-support.exp +++ b/gdb/testsuite/lib/gdbserver-support.exp @@ -176,7 +176,16 @@ proc gdbserver_download_current_prog { } { if { $reuse == 0 } { set gdbserver_host_exec $host_exec set gdbserver_host_mtime [file mtime $host_exec] - set gdbserver_server_exec [gdb_remote_download target $host_exec] + + # If local testing, don't use gdb_remote_download, which would + # copy the file to the testcase's output directory. Use + # LAST_LOADED_FILE as is, so that gdbserver starts the exact + # same filename that will be handed to gdb's file command. + if [is_remote target] { + set gdbserver_server_exec [gdb_remote_download target $host_exec] + } else { + set gdbserver_server_exec $host_exec + } } return $gdbserver_server_exec