From patchwork Mon Jun 18 14:40:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Bunt X-Patchwork-Id: 27905 Received: (qmail 5536 invoked by alias); 18 Jun 2018 14:41:09 -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 5518 invoked by uid 89); 18 Jun 2018 14:41:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_NUMSUBJECT, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=attaching, borrowed, sk:gdb_tes, launch X-HELO: EUR01-HE1-obe.outbound.protection.outlook.com Received: from mail-he1eur01on0067.outbound.protection.outlook.com (HELO EUR01-HE1-obe.outbound.protection.outlook.com) (104.47.0.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 18 Jun 2018 14:41:02 +0000 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Richard.Bunt@arm.com; Received: from sles-12-aarch64.lan.allinea.com (217.140.96.140) by AM0PR08MB3187.eurprd08.prod.outlook.com (2603:10a6:208:5d::28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.863.16; Mon, 18 Jun 2018 14:40:58 +0000 From: Richard Bunt To: gdb-patches@sourceware.org Cc: Dirk.Schubert@arm.com, nd@arm.com, Richard Bunt Subject: [PATCH] Enable hardware watchpoints on attach for aarch64 Date: Mon, 18 Jun 2018 15:40:31 +0100 Message-Id: <1529332831-72455-1-git-send-email-richard.bunt@arm.com> In-Reply-To: <942ba9f74d67b3857396d461353aae1a@polymtl.ca> References: <942ba9f74d67b3857396d461353aae1a@polymtl.ca> MIME-Version: 1.0 X-ClientProxiedBy: LO2P265CA0007.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:62::19) To AM0PR08MB3187.eurprd08.prod.outlook.com (2603:10a6:208:5d::28) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 7683c6a7-624e-4eff-295e-08d5d52981ad X-MS-Office365-Filtering-HT: Tenant X-MS-TrafficTypeDiagnostic: AM0PR08MB3187: NoDisclaimer: True X-Exchange-Antispam-Report-Test: UriScan:(250305191791016)(180628864354917)(22074186197030); X-MS-Exchange-SenderADCheck: 1 X-Forefront-PRVS: 0707248B64 Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jun 2018 14:40:58.7527 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 7683c6a7-624e-4eff-295e-08d5d52981ad X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB3187 X-IsSubscribed: yes This commit fixes a bug whereby hardware watchpoints are not used on aarch64 when attaching to a target. The fix adds an aarch64 specialization of post_attach which records the number of available hardware debug registers using aarch64_linux_get_debug_reg_capacity. This implementation mirrors that of aarch64_linux_child_post_startup_inferior which successfully enables the use of hardware watchpoints when launching the target under the debugger. 2018-05-14 Richard Bunt Dirk Schubert * aarch64-linux-nat.c (post_attach): New. (aarch64_linux_nat_target::post_attach): Override post_attach to record the number of hardware debug registers. gdb/testsuite/ChangeLog: 2018-05-14 Richard Bunt * gdb.base/watchpoint-hw-attach.c: New test. * gdb.base/watchpoint-hw-attach.exp: New file. --- Updated based on Simon's feedback; thanks for the review. The feedback has been addressed as detailed below. >On 2018-06-07 09:19, Alan Hayward wrote: >> Aarch64 changes look good to me. >> I'm not up to speed enough with .exp files to comment on that part. >> I tested it on an aarch64 box, and it works fine for me. > >Hi, > >I looked at the test, if Alan is fine with the code changes then I am >fine with them. I noted some nits, but otherwise it looks good. Please >push after addressing them. > >>>> +# Get the PID of the test process. >>>> +set testpid "" >>>> +set test "get inferior process ID" >>>> + gdb_test_multiple "p mypid" $test { >>>> + -re " = ($decimal)\r\n$gdb_prompt $" { >>>> + set testpid $expect_out(1,string) >>>> + pass $test >>>> + } >>>> +} > >You can use get_integer_valueof for this. > Fixed, and good to know about this one. >>>> +set test "attach $testpid" >>>> +gdb_test_multiple $test $test { >>>> + -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt $" >>>> { >>>> + pass "$test" >>>> + } >>>> + timeout { >>>> + fail "$test" >>>> + } >>>> +} > >Just wondering, can't simple gdb_test be used here? > Changed. There is no reason that I am aware of that this cannot be a gdb_test. I think this was just the structure used in the attach test that I borrowed from. >>>> + >>>> +gdb_test_no_output "set should_continue = 1" >>>> +# Ensure the test program is in the top frame so the required >>>> +# variables are in scope. >>>> +gdb_breakpoint [gdb_get_line_number "prewatchtrigger"] >>>> +gdb_continue_to_breakpoint "prewatchtrigger" >>>> + >>>> +gdb_test "watch watched_variable" \ >>>> + "Hardware watchpoint $decimal: watched_variable" >>>> + >>>> +gdb_test "continue" \ >>>> + "continue.*Continuing.*\.Hardware watchpoint $decimal:\ >>>> + watched_variable.*Old value = 0.*New value = 4.*" > >8 spaces -> tab on the last line. Though for test patterns, it's fine >to go over 80 columns, IMO. > Fixed, that certainly makes it easier to read. >Simon Will push (as per the review) in a few days if there are no more objections. Many thanks, Rich gdb/aarch64-linux-nat.c | 18 +++++++ gdb/testsuite/gdb.base/watchpoint-hw-attach.c | 44 +++++++++++++++ gdb/testsuite/gdb.base/watchpoint-hw-attach.exp | 71 +++++++++++++++++++++++++ 3 files changed, 133 insertions(+) create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-attach.c create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-attach.exp diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index 1e7db29..c9fd062 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -77,6 +77,9 @@ public: /* Override the GNU/Linux inferior startup hook. */ void post_startup_inferior (ptid_t) override; + /* Override the GNU/Linux post attach hook. */ + void post_attach (int pid) override; + /* These three defer to common nat/ code. */ void low_new_thread (struct lwp_info *lp) override { aarch64_linux_new_thread (lp); } @@ -568,6 +571,21 @@ aarch64_linux_nat_target::post_startup_inferior (ptid_t ptid) linux_nat_target::post_startup_inferior (ptid); } +/* Implement the "post_attach" target_ops method. */ + +void +aarch64_linux_nat_target::post_attach (int pid) +{ + low_forget_process (pid); + /* Set the hardware debug register capacity. If + aarch64_linux_get_debug_reg_capacity is not called + (as it is in aarch64_linux_child_post_startup_inferior) then + software watchpoints will be used instead of hardware + watchpoints when attaching to a target. */ + aarch64_linux_get_debug_reg_capacity (pid); + linux_nat_target::post_attach (pid); +} + extern struct target_desc *tdesc_arm_with_neon; /* Implement the "read_description" target_ops method. */ diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-attach.c b/gdb/testsuite/gdb.base/watchpoint-hw-attach.c new file mode 100644 index 0000000..9d55b28 --- /dev/null +++ b/gdb/testsuite/gdb.base/watchpoint-hw-attach.c @@ -0,0 +1,44 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2018 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 +#include +#include + +/* This is set to 1 by the debugger post attach to continue to the + watchpoint trigger. */ +volatile int should_continue = 0; +/* The variable to place a watchpoint on. */ +volatile int watched_variable = 0; + +int +main (void) +{ + unsigned int counter = 1; + int mypid = getpid (); + + /* Wait for the debugger to attach, but not indefinitely so this + test program is not left hanging around. */ + for (counter = 0; !should_continue && counter < 100; counter++) + sleep (1); /* pidacquired */ + + watched_variable = 0; /* prewatchtrigger */ + /* Trigger a watchpoint. */ + watched_variable = 4; + printf ("My variable is %d\n", watched_variable); + return 0; +} diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-attach.exp b/gdb/testsuite/gdb.base/watchpoint-hw-attach.exp new file mode 100644 index 0000000..0c5037c --- /dev/null +++ b/gdb/testsuite/gdb.base/watchpoint-hw-attach.exp @@ -0,0 +1,71 @@ +# Copyright 2018 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 . + +# watchpoint-hw-attach.exp -- Test if hardware watchpoints are used +# when attaching to a target. + +if {[skip_hw_watchpoint_tests]} { + return 0 +} + +if {![can_spawn_for_attach]} { + return 0 +} + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +clean_restart $binfile + +if ![runto_main] { + untested "can't run to main" + return -1 +} + +# Run to the point where mypid in the test program has been +# populated. +gdb_breakpoint [gdb_get_line_number "pidacquired"] +gdb_continue_to_breakpoint "pidacquired" + +# Get the PID of the test process. +set testpid [get_integer_valueof "mypid" 0] + +gdb_test "detach" "Detaching from program: .*, process $testpid\r\n\\\[Inferior $decimal \\(process $testpid\\) detached\\\]" + +if {$testpid == ""} { + return -1 +} + +# A clean restart is needed to force the hardware watchpoint setup +# logic to run post attach rather than post inferior launch. +clean_restart $binfile + +gdb_test "attach $testpid" "Attaching to program: .*, process $testpid.*" + +gdb_test_no_output "set should_continue = 1" + +# Ensure the test program is in the top frame so the required +# variables are in scope. +gdb_breakpoint [gdb_get_line_number "prewatchtrigger"] +gdb_continue_to_breakpoint "prewatchtrigger" + +gdb_test "watch watched_variable" \ + "Hardware watchpoint $decimal: watched_variable" + +gdb_test "continue" \ + "continue.*Continuing.*\.Hardware watchpoint $decimal: watched_variable.*Old value = 0.*New value = 4.*watched_variable\\);"