diff mbox

[regression] Do not read from catchpoint/watchpoint locations' addresses when checking for a permanent breakpoint

Message ID 1439303444-720-1-git-send-email-lgustavo@codesourcery.com
State New, archived
Headers

Commit Message

Luis Machado Aug. 11, 2015, 2:30 p.m. UTC 
  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.

Is this OK?

Luis

2015-08-11  Luis Machado  <lgustavo@codesourcery.com>

	* breakpoint.c (bp_loc_is_permanent): Return 0 when breakpoint
	location address is not meaningful.
	(breakpoint_address_is_meaningful): Update comment.
---
 gdb/breakpoint.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Pedro Alves Aug. 11, 2015, 6:03 p.m. UTC | #1 
On 08/11/2015 03:30 PM, Luis Machado wrote:

> Is this OK?

OK for master and 7.10.

Thanks,
Pedro Alves
Luis Machado Aug. 12, 2015, 8:56 a.m. UTC | #2 
On 08/11/2015 03:03 PM, Pedro Alves wrote:
> On 08/11/2015 03:30 PM, Luis Machado wrote:
>
>> Is this OK?
>
> OK for master and 7.10.
>
> Thanks,
> Pedro Alves
>

Thanks Pedro.

Pushed to both:

master: 244558af868d5427903c35c5105bf5499639f81f
7.10: 835001623351330d49678d215d1338c0ce35c1f9
diff mbox

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 91a53b9..94f4ee6 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6930,14 +6930,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 it to other
+   addresses (or use it 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
@@ -8974,6 +8974,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);