From patchwork Wed Apr 10 12:10:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 32246 Received: (qmail 49873 invoked by alias); 10 Apr 2019 12:11:05 -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 49800 invoked by uid 89); 10 Apr 2019 12:11:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=H*r:15.20.1771.19, sk:build_e, 6323, didn X-HELO: EUR01-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr140040.outbound.protection.outlook.com (HELO EUR01-VE1-obe.outbound.protection.outlook.com) (40.107.14.40) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 10 Apr 2019 12:11:01 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=R6g5YORVxyo3TSvpPAnx2mRl4Ifn79cmcJx5/rAHTAU=; b=Lj8GgxVbwEnBeXNvQnI4HBvA8d8IA1betZ7r7l+k1JfPZ+xm1RGo3IMBmjsRLAl9XduUECbMpV7GyrL2RkklnViBxq036SywuDlYPgTgr4j2aHJrImcJMhwG7oIlvcmkDMM5qQD4sZIy3IZJBxrfGdNcB7n6Am4YN1KmpBgiICg= Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com (10.172.227.22) by DB6PR0802MB2486.eurprd08.prod.outlook.com (10.172.251.21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1771.19; Wed, 10 Apr 2019 12:10:58 +0000 Received: from DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::399b:3a32:bff9:827e]) by DB6PR0802MB2133.eurprd08.prod.outlook.com ([fe80::399b:3a32:bff9:827e%10]) with mapi id 15.20.1771.016; Wed, 10 Apr 2019 12:10:57 +0000 From: Alan Hayward To: Pedro Alves CC: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH] Testsuite: Add gdbserver sysroot test Date: Wed, 10 Apr 2019 12:10:57 +0000 Message-ID: <55920B72-A248-4908-A748-CE6E4DE4D49A@arm.com> References: <20190408153440.39250-1-alan.hayward@arm.com> <2c8f233b-0c53-ed95-b556-6932c5b077fd@redhat.com> In-Reply-To: <2c8f233b-0c53-ed95-b556-6932c5b077fd@redhat.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-exchange-purlcount: 2 received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 Content-ID: <95505EF5E5E45A4BADB9B977BE20EAE5@eurprd08.prod.outlook.com> MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-IsSubscribed: yes > On 9 Apr 2019, at 15:35, Pedro Alves wrote: > > On 4/9/19 12:04 PM, Alan Hayward wrote: > >> I’ve overriden gdb_target_cmd locally as I didn’t think it was worth moving the >> changed version into the library. > > That doesn't seem like a good idea to me. Likely people will forget to keep the > new copy in sync if/when the main copy changes. Agreed. But was balancing that against adding the additional param for just the single use, which I doubted anything else would need. > > Plus, try --target_board=native-extended-remote, and you'll see: > > Running /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/sysroot.exp ... > ERROR: tcl error sourcing /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.server/sysroot.exp. > ERROR: wrong # args: should be "gdb_target_cmd targetname serialport additional_text" > while executing > "gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport" > (procedure "gdbserver_start_multi" line 12) > invoked from within > "gdbserver_start_multi" > (procedure "gdb_start" line 6) > invoked from within > ... > ....but given this errors, then yes, I’ll change the common version. > Why not add the new parameter to the lib proc, but make it optional, like? > > proc gdb_target_cmd { targetname serialport {additional_text ""} } { > Done. >> >>> >>> But the thing is, even if you don't have debug info for shared libraries, if you >>> debug info for the main program, you'll be able to set a breakpoint on "printf". >>> The result is you end up with a breakpoint at printf@plt. So I'm thinking that >>> the test would pass even if we failed to load the shared libraries from the target. >>> >> >> I just wanted to make sure we could stop somewhere outside the binary. >> Any suggestions? Or is is best to just remove that part. > > Maybe just add an empty space after "printf", like: > > - gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf" > + gdb_test "continue" "Breakpoint $decimal.* printf .*" "continue to printf" > > so that it doesn't match printf@plt. Done. > >> >> >>> I tried to do that manually, by issuing a "nosharedlibrary" after connecting >>> to gdbserver, and then running to the breakpoint, but unfortunately, that >>> runs into a nasty gdb bug: >>> >>> (gdb) nosharedlibrary >>> (gdb) c >>> Continuing. >>> pure virtual method called >>> terminate called without an active exception >>> Aborted (core dumped) >>> $ >>> >>> I'm looking into that… >> >> I get the same error for that. > > Fix for that posted: > https://sourceware.org/ml/gdb-patches/2019-04/msg00141.html Excellent. I would give it a review, but don’t know anything in that area. > >> >>> >>> Also, the test as is fails for me, on x86-64: >>> >>> set sysroot / >>> warning: Could not load shared library symbols for linux-vdso.so.1. >>> Do you need "set solib-search-path" or "set sysroot"? >>> (gdb) FAIL: gdb.server/sysroot.exp: sysroot=/: set sysroot / >> >> Works for me on: >> Ubuntu 16.04, both AArch64 and x86-64 >> >> Just tried it on: >> Centos 7.4 AArch64 >> OpenSuse 13.3 AArch64 >> >> And works there too. But I don’t see anything with “vdso” in any of the logs. >> >> What OS are you running? >> > > x86-64 Fedora 27. But the new version passes, except for the > extended-remote issue above. Hmmm, odd. >> + >> +int >> +main () >> +{ >> + printf("Hello World!\n"); > > Missing space before parens. Done. > >> + return 0; >> +} >> diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp >> new file mode 100644 > >> + >> +# Test GDB can correct read the binary with different sysroot setups. > > "Test THAT GDB can correctLY" ? > > I'd suggest extending that a bit, to mention shared libraries, and "target:": > > Test that GDB can correctly read the binary and shared libraries > with different sysroot setups: local, and "target:". > >> + >> +# Run once with sysroot set to the local filesystem and once set to the remote target. > > Hit M-q in your emacs here. Done (assuming you just meant to wrap before column 80). > >> +foreach_with_prefix sysroot { "local" "remote" } { >> + global srcdir >> + global subdir >> + global binfile >> + >> + if { $sysroot == "local" } { >> + set sysroot_command "/" >> + set reading_symbols "Reading symbols from $binfile..." >> + } else { >> + set sysroot_command "target:" >> + set reading_symbols "Reading $binfile from remote target..." >> + } >> + >> + # Restart GDB >> + gdb_exit >> + gdb_start >> + gdb_reinitialize_dir $srcdir/$subdir > > Use clean_restart, and add period: > > # Restart GDB. > clean_restart This was intentional to avoid the “file” command at the end of clean_restart. > >> + >> + # Make sure we're disconnected, in case we're testing with an >> + # extended-remote board, therefore already connected. >> + gdb_test "disconnect $reading_symbols" ".*" >> + >> + # Start GDBserver. >> + set res [gdbserver_start "" $binfile] >> + set gdbserver_protocol [lindex $res 0] >> + set gdbserver_gdbport [lindex $res 1] >> + >> + # Set the sysroot. >> + gdb_test_no_output "set sysroot $sysroot_command" >> + >> + # Connect to gdbsever, making sure GDB reads in the binary correctly. >> + gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols >> + >> + gdb_breakpoint main >> + gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main" >> + >> + # Test we can stop inside a library. > > "Test that” ? Done. > >> + gdb_breakpoint printf >> + gdb_test "continue" "Breakpoint $decimal.* printf.*" "continue to printf" >> +} > Thanks, > Pedro Alves With latest changes: gdb/testsuite/ChangeLog: 2019-04-10 Alan Hayward * gdb.server/sysroot.c: New test. * gdb.server/sysroot.exp: New file. * lib/gdbserver-support.exp (gdb_target_cmd): Add additional text matching param. diff --git a/gdb/testsuite/gdb.server/sysroot.c b/gdb/testsuite/gdb.server/sysroot.c new file mode 100644 index 0000000000..7db1a138d1 --- /dev/null +++ b/gdb/testsuite/gdb.server/sysroot.c @@ -0,0 +1,25 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2019 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +int +main () +{ + printf ("Hello World!\n"); + return 0; +} diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp new file mode 100644 index 0000000000..dbd548ba2b --- /dev/null +++ b/gdb/testsuite/gdb.server/sysroot.exp @@ -0,0 +1,79 @@ +# This testcase is part of GDB, the GNU debugger. +# +# Copyright 2019 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test that GDB can correctly read the binary and shared libraries +# with different sysroot setups: local and "target:". + +load_lib gdbserver-support.exp + +if { [skip_gdbserver_tests] } { + verbose "skipping gdbserver tests" + return -1 +} + +standard_testfile +if {[build_executable "failed to prepare" $testfile $srcfile "additional_flags=--no-builtin"] == -1} { + return -1 +} + +# Run once with sysroot set to the local filesystem and once set to the remote +# target. +foreach_with_prefix sysroot { "local" "remote" } { + global srcdir + global subdir + global binfile + + if { $sysroot == "local" } { + set sysroot_command "/" + set reading_symbols "Reading symbols from $binfile..." + } else { + set sysroot_command "target:" + set reading_symbols "Reading $binfile from remote target..." + } + + # Restart GDB + gdb_exit + gdb_start + gdb_reinitialize_dir $srcdir/$subdir + + # Make sure we're disconnected, in case we're testing with an + # extended-remote board, therefore already connected. + gdb_test "disconnect" ".*" + + # Start GDBserver. + set res [gdbserver_start "" $binfile] + set gdbserver_protocol [lindex $res 0] + set gdbserver_gdbport [lindex $res 1] + + # Set the sysroot. + gdb_test_no_output "set sysroot $sysroot_command" + + # Connect to gdbsever, making sure GDB reads in the binary correctly. + set test "connect to remote and read binary" + if {[gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols] == 0} { + pass $test + } else { + fail $test + } + + gdb_breakpoint main + gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main" + + # Test that we can stop inside a library. + gdb_breakpoint printf + gdb_test "continue" "Breakpoint $decimal.* printf .*" "continue to printf" +} diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp index dbd885aa22..1343169488 100644 --- a/gdb/testsuite/lib/gdbserver-support.exp +++ b/gdb/testsuite/lib/gdbserver-support.exp @@ -42,9 +42,11 @@ # # gdb_target_cmd -# Send gdb the "target" command +# Send gdb the "target" command. Returns 0 on success, 1 on failure. +# If specified, then additional_text must match the text which must comes after +# the connection message in order for procedure to succeed. # -proc gdb_target_cmd { targetname serialport } { +proc gdb_target_cmd { targetname serialport {additional_text ""} } { global gdb_prompt set serialport_re [string_to_regexp $serialport] @@ -61,23 +63,23 @@ proc gdb_target_cmd { targetname serialport } { -re "Couldn't establish connection to remote.*$gdb_prompt $" { verbose "Connection failed" } - -re "Remote MIPS debugging.*$gdb_prompt" { + -re "Remote MIPS debugging.*$additional_text.*$gdb_prompt" { verbose "Set target to $targetname" return 0 } - -re "Remote debugging using .*$serialport_re.*$gdb_prompt $" { + -re "Remote debugging using .*$serialport_re.*$additional_text.*$gdb_prompt $" { verbose "Set target to $targetname" return 0 } - -re "Remote debugging using stdio.*$gdb_prompt $" { + -re "Remote debugging using stdio.*$additional_text.*$gdb_prompt $" { verbose "Set target to $targetname" return 0 } - -re "Remote target $targetname connected to.*$gdb_prompt $" { + -re "Remote target $targetname connected to.*$additional_text.*$gdb_prompt $" { verbose "Set target to $targetname" return 0 } - -re "Connected to.*$gdb_prompt $" { + -re "Connected to.*$additional_text.*$gdb_prompt $" { verbose "Set target to $targetname" return 0 }