From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49787 invoked by alias); 17 Sep 2018 10:34:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 49772 invoked by uid 89); 17 Sep 2018 10:34:15 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=AWL,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=fire, timing, watchpoints, prefixes X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Sep 2018 10:34:14 +0000 Received: by mail-wr1-f66.google.com with SMTP id w11-v6so16722682wrc.5 for ; Mon, 17 Sep 2018 03:34:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ph8XyegG/UinFhUTMcS9GvESQg14vND9U3fZ/2ASzKQ=; b=MKXzq0sfsvS7SS+ArKaVuNhE1PoAz4atd0cPmYfKZPNOsTONT3xCQER+a8bG/5urs1 hUKGWxFOvvg5/VwI5lUcexkMTPaxzmbYhyadicYiHeHCTd9mk0zlzHHXgZ6aWYaajiHT 6RTpCSiIlJgk+HD3TqYvLkueCE4lsYhJK9S64ERhD+8ziFQDDe39nOFBjr0Vx/wuVAZN B9e8OQiKVTzzlm/6Z6cSQS0iKPf1NPEI89zUqBVUmxayZ4Ip6o+JZXUajP9nAmeLvAev YM2vu61neueKTZwjPvoYSAKYVhfiiJVbHNVf86qKGv8uqrcp4Sek+Cm9iaI7pMLQcvW9 pqXw== Return-Path: Received: from localhost (host81-158-205-250.range81-158.btcentralplus.com. [81.158.205.250]) by smtp.gmail.com with ESMTPSA id b10-v6sm11686007wrr.88.2018.09.17.03.34.11 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 17 Sep 2018 03:34:11 -0700 (PDT) Date: Mon, 17 Sep 2018 10:34:00 -0000 From: Andrew Burgess To: Craig Blackmore Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] RISC-V: enable have_nonsteppable_watchpoint by default Message-ID: <20180917103409.GJ5952@embecosm.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Fortune: My, how you've changed since I've changed. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00563.txt.bz2 * Craig Blackmore [2018-09-16 01:13:08 +0100]: > 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 1 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 1 to match the recommended behaviour. If a target does not follow > this timing, then 'set riscv have_nonsteppable_watchpoint 0' will need > to be issued on the command line. Thanks for this. Just a few minor formatting issues which I've pointed out below. Thanks, Andrew > > gdb/ChangeLog: > > * riscv-tdep.c (set_have_nonsteppable_watchpoint): add > callback for 'set riscv have_nonsteppable_watchpoint' > (riscv_gdbarch_init): initialise gdbarch setting for > have_nonesteppable_watchpoint Proper sentences in ChangeLogs please, capital letters and full-stops. > > --- > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index 254914c9c7..8f301d8b01 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -226,6 +226,20 @@ show_use_compressed_breakpoints (struct ui_file *file, int from_tty, > "to %s%s.\n"), value, additional_info); > } > > +static int riscv_have_nonsteppable_watchpoint = 1; You need to add a comment for this variable. > + > +/* The set callback for 'set riscv have-nonsteppable-watchpoint'. */ Two whitespace after the full-stop and before the comment close please. > + > +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 set and show lists for 'set riscv' and 'show riscv' prefixes. */ > > static struct cmd_list_element *setriscvcmdlist = NULL; > @@ -2736,6 +2750,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 +2996,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 1 (default) for\n\ Two whitespace after full-stop again please. > +targets that follow this behaviour, otherwise set to 0."), As this is a boolean command can you replace references to 1 and 0 both here and in the commit message with on and off. > + set_have_nonsteppable_watchpoint, > + NULL, You need to implement the show method too to allow for internationalisation. > + &setriscvcmdlist, > + &showriscvcmdlist); > } > >