Message ID | 20190413204242.16305-1-jimw@sifive.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 9504 invoked by alias); 13 Apr 2019 20:42:49 -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 9489 invoked by uid 89); 13 Apr 2019 20:42:49 -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.1 spammy=Wrap, HX-Languages-Length:1652, H*RU:209.85.215.196, HX-Spam-Relays-External:209.85.215.196 X-HELO: mail-pg1-f196.google.com Received: from mail-pg1-f196.google.com (HELO mail-pg1-f196.google.com) (209.85.215.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 13 Apr 2019 20:42:48 +0000 Received: by mail-pg1-f196.google.com with SMTP id q1so6747256pgv.13 for <gdb-patches@sourceware.org>; Sat, 13 Apr 2019 13:42:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=from:to:cc:subject:date:message-id; bh=MJkNoCqGwDRmFu4DIGJr/yfcHW15odi2KpuXIuJfEo0=; b=hNanDEP5C3AazMywutcFDg54Y33jyj/JV7AQA4Y7kveHDlKZrte5/pkS18X9+iw1TM iVOKeaOj0r8jKZu24AErR0Pa6wIwMibSX6tH4s6/qZwBn5QkqMpqxWtUtuI9xQTAJ07W o+LPITq4IcLglMc967l4WZHyZ/1yZPu791AJPRl67EqRzqrY7YOiBV/WfdwynxbHjqf4 GY87R6+Qg3zwPEzcQrRzrdZCoXpKh3u67tBfReXZKhM4ixXJX+GMYds4sBKj81rlXCMZ uVjBeX5v2gCVOMnvEnC2UOa7SUJHFpYQ9PgC6tW63XoyIpMEwN7NmNaSkYO8YWpDQWkV /gRA== Return-Path: <jimw@sifive.com> Received: from rohan.hsd1.ca.comcast.net ([2601:646:c100:8240:a9d0:1708:9640:790e]) by smtp.gmail.com with ESMTPSA id a8sm74645363pfo.144.2019.04.13.13.42.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 13 Apr 2019 13:42:45 -0700 (PDT) From: Jim Wilson <jimw@sifive.com> To: gdb-patches@sourceware.org Cc: Jim Wilson <jimw@sifive.com> Subject: [PATCH] RISC-V: Allow setting breakpoints at invalid addresses. Date: Sat, 13 Apr 2019 13:42:42 -0700 Message-Id: <20190413204242.16305-1-jimw@sifive.com> X-IsSubscribed: yes |
Commit Message
Jim Wilson
April 13, 2019, 8:42 p.m. UTC
Some testsuite testcases construct dwarf2 debug info for fake functions to test that this debug info is handled correctly. We get an error trying to read from an invalid address when setting a breakpoint on these fake functions. Other targets allow setting breakpoints on invalid addresses, and only error when a command forces us to write the breakpoint to memory. This matches that behavior by wrapping the read in a try/catch. Tested with a riscv64-linux native testsuite run. I get 55 fewer unexpected failures with the patch, and there are no regressions. gdb/ * riscv-tdep.c (riscv_breakpoint_kind_from_pc): Wrap read_code call in try/catch. Set buf[0] to 0 in catch clause. --- gdb/riscv-tdep.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
Comments
* Jim Wilson <jimw@sifive.com> [2019-04-13 13:42:42 -0700]: > Some testsuite testcases construct dwarf2 debug info for fake functions to > test that this debug info is handled correctly. We get an error trying to > read from an invalid address when setting a breakpoint on these fake functions. > Other targets allow setting breakpoints on invalid addresses, and only error > when a command forces us to write the breakpoint to memory. This matches that > behavior by wrapping the read in a try/catch. > > Tested with a riscv64-linux native testsuite run. I get 55 fewer unexpected > failures with the patch, and there are no regressions. > > gdb/ > * riscv-tdep.c (riscv_breakpoint_kind_from_pc): Wrap read_code call > in try/catch. Set buf[0] to 0 in catch clause. > --- > gdb/riscv-tdep.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index 6370bc268f..38dc578add 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -430,8 +430,19 @@ riscv_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr) > unaligned_p = true; > else > { > - /* Read the opcode byte to determine the instruction length. */ > - read_code (*pcptr, buf, 1); > + try > + { > + /* Read the opcode byte to determine the instruction length. */ > + read_code (*pcptr, buf, 1); > + } > + catch (const gdb_exception_error &ex) > + { > + /* We may have tried to set a breakpoint at a invalid address. > + Defer the error until we try to write the breakpoint to > + memory to match how other gdb targets work. Also, testsuite > + testcases like gdb.cp/nsalias.exp require this behavior. */ > + buf[0] = 0; > + } I think that you can just use something like: /* Read the opcode byte to determine the instruction length. If the read fails this may be because we tried to set the breakpoint at an invalid address. We provide a fake result which will give a breakpoint length of 4, hopefully when we try to actually insert the breakpoint we will see a failure then too. */ if (target_read_code (*pcptr, buf, 1) == -1) buf[0] = 0; and avoid the try/catch. Annoyingly my QEMU/RISC-V/Linux setup got broken so I'm unable to test the above right now ... hopefully I'll get some time next week to fix my infrastructure issue :-/ Thanks, Andrew > } > > if (riscv_debug_breakpoints) > -- > 2.17.1 >
On Sat, Apr 13, 2019 at 4:16 PM Andrew Burgess <andrew.burgess@embecosm.com> wrote: > I think that you can just use something like: > > /* Read the opcode byte to determine the instruction length. If > the read fails this may be because we tried to set the > breakpoint at an invalid address. We provide a fake result > which will give a breakpoint length of 4, hopefully when we > try to actually insert the breakpoint we will see a failure > then too. */ > if (target_read_code (*pcptr, buf, 1) == -1) > buf[0] = 0; > > and avoid the try/catch. Yes, thanks, this looks a little simpler than what I suggested. I confirmed that this works with a riscv64-linux gdb build and make check. Jim
* Jim Wilson <jimw@sifive.com> [2019-04-14 16:23:40 -0700]: > On Sat, Apr 13, 2019 at 4:16 PM Andrew Burgess > <andrew.burgess@embecosm.com> wrote: > > I think that you can just use something like: > > > > /* Read the opcode byte to determine the instruction length. If > > the read fails this may be because we tried to set the > > breakpoint at an invalid address. We provide a fake result > > which will give a breakpoint length of 4, hopefully when we > > try to actually insert the breakpoint we will see a failure > > then too. */ > > if (target_read_code (*pcptr, buf, 1) == -1) > > buf[0] = 0; > > > > and avoid the try/catch. > > Yes, thanks, this looks a little simpler than what I suggested. I > confirmed that this works with a riscv64-linux gdb build and make > check. Thanks for testing this. I pushed a patch with the above fix in. Thanks, Andrew
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index 6370bc268f..38dc578add 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -430,8 +430,19 @@ riscv_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr) unaligned_p = true; else { - /* Read the opcode byte to determine the instruction length. */ - read_code (*pcptr, buf, 1); + try + { + /* Read the opcode byte to determine the instruction length. */ + read_code (*pcptr, buf, 1); + } + catch (const gdb_exception_error &ex) + { + /* We may have tried to set a breakpoint at a invalid address. + Defer the error until we try to write the breakpoint to + memory to match how other gdb targets work. Also, testsuite + testcases like gdb.cp/nsalias.exp require this behavior. */ + buf[0] = 0; + } } if (riscv_debug_breakpoints)