From patchwork Thu Oct 4 16:25:54 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Craig Blackmore X-Patchwork-Id: 29646 Received: (qmail 88496 invoked by alias); 4 Oct 2018 16:26:16 -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 88480 invoked by uid 89); 4 Oct 2018 16:26:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=H*f:sk:2018091 X-HELO: mail-wm1-f68.google.com Received: from mail-wm1-f68.google.com (HELO mail-wm1-f68.google.com) (209.85.128.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 04 Oct 2018 16:26:14 +0000 Received: by mail-wm1-f68.google.com with SMTP id 185-v6so9696496wmt.2 for ; Thu, 04 Oct 2018 09:26:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=subject:to:cc:references:from:openpgp:autocrypt:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=XZWcjNC3o1lfxixPuNhr623xZXNjZcl7tDeIHG02Xkc=; b=gwxDj05Zh/rHo3DwoKInfWjGR5EkxzmipUetDHTT/gVgAdqNV7hLTLAhv3biSp9FQV lS1PyEigP7S0h+hc7zplDRhn/3+LaKG3tCIOWRj9/98Ohf9uxRF6IE0azyct9wGn1YeC mQXtMdEniBv8ob+Mj0kp06vYVwugfbJSbEo2Kj8ZADxSXikEluUjEduZe7OmpIjJeiVd mxLTDpX/iysqhQ7rKXCmhh7TKmIP7DXUQ1FfSuRTLrk5D+4vZOlPrhErNIScSYe/q2Dc fSB54Tavl/+BtQWWlG4ar72uk85FhbvQlpwsPgXrPPuGlIfwnqY9vSbu2PfBkv5yjJb1 0qqQ== Return-Path: Received: from [192.168.43.141] ([85.255.234.84]) by smtp.gmail.com with ESMTPSA id k7-v6sm6152078wmf.41.2018.10.04.09.26.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Oct 2018 09:26:11 -0700 (PDT) Subject: Re: [PATCH] RISC-V: enable have_nonsteppable_watchpoint by default To: Joel Brobecker Cc: Andrew Burgess , gdb-patches@sourceware.org References: <20180917103409.GJ5952@embecosm.com> <77978648-c391-0011-6c03-c7fd38429914@embecosm.com> <20181003223703.GA22933@adacore.com> From: Craig Blackmore Openpgp: preference=signencrypt Message-ID: Date: Thu, 4 Oct 2018 17:25:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181003223703.GA22933@adacore.com> On 03/10/18 23:37, Joel Brobecker wrote: > I assume this patch is waiting for review from Andrew? > > I took a look, since I am interested in it as well. Here are my > comments. > > You forgot to document the introduction of riscv_have_nonsteppable_watchpoint > in your ChangeLog above; eg: > > * riscv-tdep.c (riscv_have_nonsteppable_watchpoint): New static > global. > > You also forgot to document that you're adding new subcommands. > > The rest looks good to me. Hi Joel, Thanks for the review. I have updated the ChangeLog in the patch below. Craig --- The RISC-V debug spec 0.13 recommends that write triggers fire before the write is committed. If the target follows this behaviour, then have_nonsteppable_watchpoint needs to be set to 'on' so that GDB will step over the watchpoint before checking if the value has changed. This patch adds a setshow for have_nonsteppable_watchpoint which defaults to 'on' to match the recommended behaviour. If a target does not follow this timing, then 'set riscv have_nonsteppable_watchpoint off' will need to be issued on the command line. gdb/ChangeLog: * riscv-tdep.c (riscv_have_nonsteppable_watchpoint): New static global. (set_have_nonsteppable_watchpoint): Add callback for 'set riscv have_nonsteppable_watchpoint'. (show_have_nonsteppable_watchpoint): Add callback for 'show riscv have_nonsteppable_watchpoint'. (riscv_gdbarch_init): Initialise gdbarch setting for have_nonesteppable_watchpoint. (_initialize_riscv_tdep): Add 'set/show riscv have_nonsteppable_watchpoint' subcommands. --- diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 254914c..857c5d1 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -226,6 +226,36 @@ show_use_compressed_breakpoints (struct ui_file *file, int from_tty, "to %s%s.\n"), value, additional_info); } +/* Controls whether the debugger should step over hardware watchpoints before + checking if the watched variable has changed. If true, then the debugger + will step over the watchpoint. */ + +static int riscv_have_nonsteppable_watchpoint = 1; + +/* The set callback for 'set riscv have-nonsteppable-watchpoint'. */ + +static void +set_have_nonsteppable_watchpoint (const char *args, int from_tty, + struct cmd_list_element *c) +{ + struct gdbarch *gdbarch = target_gdbarch (); + + set_gdbarch_have_nonsteppable_watchpoint (gdbarch, + riscv_have_nonsteppable_watchpoint); +} + +/* The show callback for 'show riscv have-nonsteppable-watchpoint'. */ + +static void +show_have_nonsteppable_watchpoint (struct ui_file *file, int from_tty, + struct cmd_list_element *c, + const char *value) +{ + fprintf_filtered (file, + _("Debugger must step over hardware watchpoints is set to " + "%s.\n"), value); +} + /* The set and show lists for 'set riscv' and 'show riscv' prefixes. */ static struct cmd_list_element *setriscvcmdlist = NULL; @@ -2736,6 +2766,8 @@ riscv_gdbarch_init (struct gdbarch_info info, set_gdbarch_return_value (gdbarch, riscv_return_value); set_gdbarch_breakpoint_kind_from_pc (gdbarch, riscv_breakpoint_kind_from_pc); set_gdbarch_sw_breakpoint_from_kind (gdbarch, riscv_sw_breakpoint_from_kind); + set_gdbarch_have_nonsteppable_watchpoint (gdbarch, + riscv_have_nonsteppable_watchpoint); /* Register architecture. */ set_gdbarch_num_regs (gdbarch, RISCV_LAST_REGNUM + 1); @@ -2980,4 +3012,20 @@ can be used."), show_use_compressed_breakpoints, &setriscvcmdlist, &showriscvcmdlist); + + add_setshow_boolean_cmd ("have-nonsteppable-watchpoint", no_class, + &riscv_have_nonsteppable_watchpoint, + _("\ +Set whether debugger must step over hardware watchpoints"), + _("\ +Show whether debugger must step over hardware watchpoints"), + _("\ +The RISC-V debug spec recommends that hardware write watchpoints fire before\n\ +the write is committed, in which case, GDB must step over the watchpoint\n\ +before checking the old and new values. Set this option to 'on' (default)\n\ +for targets that follow this behaviour, otherwise set to 'off'."), + set_have_nonsteppable_watchpoint, + show_have_nonsteppable_watchpoint, + &setriscvcmdlist, + &showriscvcmdlist); }