From 244558af868d5427903c35c5105bf5499639f81f Mon Sep 17 00:00:00 2001 From: Luis Machado Date: Wed, 12 Aug 2015 05:36:09 -0300 Subject: [PATCH] [regression] Do not read from catchpoint/watchpoint locations' addresses when checking for a permanent breakpoint While running bare-metal tests with GDB i noticed some failures in gdb.base/break.exp, related to the use of the catch commands. It turns out GDB tries to access memory address 0x0 whenever one tries to insert a catchpoint, which should obviously not happen. This was introduced with the changes for permanent breakpoints. In special, bp_loc_is_permanent tries to check if there is a breakpoint inserted at the same address as the current breakpoint's location's address. In the case of catchpoints, this is 0x0. (top-gdb) catch fork Sending packet: $m0,1#fa...Packet received: E01 Catchpoint 4 (fork) (top-gdb) catch vfork Sending packet: $m0,1#fa...Packet received: E01 Catchpoint 5 (vfork) It is not obvious to detect because this fails silently for Linux. For our bare-metal testing, though, this fails with a clear error message from the target about not being able to read such address. The attached patch addresses this by bailing out of bp_loc_is_permanent (...) if the location address is not meaningful. I also took the opportunity to update the comment for breakpoint_address_is_meaningful, which mentioned breakpoint addresses as opposed to their locations' addresses. gdb/ChangeLog: 2015-08-11 Luis Machado * breakpoint.c (bp_loc_is_permanent): Return 0 when breakpoint location address is not meaningful. (breakpoint_address_is_meaningful): Update comment. --- gdb/ChangeLog | 6 ++++++ gdb/breakpoint.c | 17 ++++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 01e9938fa56..4d60123197b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2015-08-12 Luis Machado + + * breakpoint.c (bp_loc_is_permanent): Return 0 when breakpoint + location address is not meaningful. + (breakpoint_address_is_meaningful): Update comment. + 2015-08-11 Keith Seitz * NEWS: Mention explicit locations. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index bffff23d526..a6994c77dad 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -6963,14 +6963,14 @@ describe_other_breakpoints (struct gdbarch *gdbarch, /* Return true iff it is meaningful to use the address member of - BPT. For some breakpoint types, the address member is irrelevant - and it makes no sense to attempt to compare it to other addresses - (or use it for any other purpose either). + BPT locations. For some breakpoint types, the locations' address members + are irrelevant and it makes no sense to attempt to compare them to other + addresses (or use them for any other purpose either). More specifically, each of the following breakpoint types will - always have a zero valued address and we don't want to mark + always have a zero valued location address and we don't want to mark breakpoints of any of these types to be a duplicate of an actual - breakpoint at address zero: + breakpoint location at address zero: bp_watchpoint bp_catchpoint @@ -9007,6 +9007,13 @@ bp_loc_is_permanent (struct bp_location *loc) gdb_assert (loc != NULL); + /* If we have a catchpoint or a watchpoint, just return 0. We should not + attempt to read from the addresses the locations of these breakpoint types + point to. program_breakpoint_here_p, below, will attempt to read + memory. */ + if (!breakpoint_address_is_meaningful (loc->owner)) + return 0; + cleanup = save_current_space_and_thread (); switch_to_program_space_and_thread (loc->pspace); -- 2.30.2