[regression] Do not read from catchpoint/watchpoint locations' addresses when checkin...

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  <lgustavo@codesourcery.com>

	* breakpoint.c (bp_loc_is_permanent): Return 0 when breakpoint
	location address is not meaningful.
	(breakpoint_address_is_meaningful): Update comment.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e48068c8144929b5e26afc13225484836c9ce0be..c67ae8aa572d0bd2d4534cd20b0081610a200c5b 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2015-08-12  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.
+
 2015-08-10  Doug Evans  <dje@google.com>
            Keith Seitz  <keiths@redhat.com>
 

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d06bef86a54f679e6069532751eb1f25102865a8..a3683d7c7087d8e539c3d7c72299e619e59a17ec 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6921,14 +6921,14 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
 \f
 
 /* 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
@@ -8965,6 +8965,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);