Really fail inserting software breakpoints on read-only regions

Currently, with "set breakpoint auto-hw off", we'll still try to
insert a software breakpoint at addresses covered by supposedly
read-only or inacessible regions:

 (top-gdb) mem 0x443000 0x450000 ro
 (top-gdb) set mem inaccessible-by-default off
 (top-gdb) disassemble
 Dump of assembler code for function main:
    0x0000000000443956 <+34>:    movq   $0x0,0x10(%rax)
 => 0x000000000044395e <+42>:    movq   $0x0,0x18(%rax)
    0x0000000000443966 <+50>:    mov    -0x24(%rbp),%eax
    0x0000000000443969 <+53>:    mov    %eax,-0x20(%rbp)
 End of assembler dump.
 (top-gdb) b *0x0000000000443969
 Breakpoint 5 at 0x443969: file ../../src/gdb/gdb.c, line 29.
 (top-gdb) c
 Continuing.
 warning: cannot set software breakpoint at readonly address 0x443969

 Breakpoint 5, 0x0000000000443969 in main (argc=1, argv=0x7fffffffd918) at ../../src/gdb/gdb.c:29
 29        args.argc = argc;
 (top-gdb)

We warn, saying that the insertion can't be done, but then proceed
attempting the insertion anyway, and in case of manually added
regions, the insert actually succeeds.

This is a regression; GDB used to fail inserting the breakpoint.  More
below.

I stumbled on this as I wrote a test that manually sets up a read-only
memory region with the "mem" command, in order to test GDB's behavior
with breakpoints set on read-only regions, even when the real memory
the breakpoints are set at isn't really read-only.  I wanted that in
order to add a test that exercises software single-stepping through
read-only regions.

Note that the memory regions that target_memory_map returns aren't
like e.g., what would expect to see in /proc/PID/maps on Linux.
Instead, they're the physical memory map from the _debuggers_
perspective.  E.g., a read-only region would be real ROM or flash
memory, while a read-only+execute mapping in /proc/PID/maps is still
read-write to the debugger (otherwise the debugger wouldn't be able to
set software breakpoints in the code segment).

If one tries to manually write to memory that falls within a memory
region that is known to be read-only, with e.g., "p foo = 1", then we
hit a check in memory_xfer_partial_1 before the write mananges to make
it to the target side.

But writing a software/memory breakpoint nowadays goes through
target_write_raw_memory, and unlike when writing memory with
TARGET_OBJECT_MEMORY, nothing on the TARGET_OBJECT_RAW_MEMORY path
checks whether we're trying to write to a read-only region.

At the time "breakpoint auto-hw" was added, we didn't have the
TARGET_OBJECT_MEMORY vs TARGET_OBJECT_RAW_MEMORY target object
distinction yet, and the code path in memory_xfer_partial that blocks
writes to read-only memory was hit for memory breakpoints too.  With
GDB 6.8 we had:

 warning: cannot set software breakpoint at readonly address 0000000000443943
 Warning:
 Cannot insert breakpoint 1.
 Error accessing memory address 0x443943: Input/output error.

So I started out by fixing this by adding the memory region validation
to TARGET_OBJECT_RAW_MEMORY too.

But later, when testing against GDBserver, I realized that that would
only block software/memory breakpoints GDB itself inserts with
gdb/mem-break.c.  If a target has a to_insert_breakpoint method, the
insertion request will still pass through to the target.  So I ended
up converting the "cannot set breakpoint" warning in breakpoint.c to a
real error return, thus blocking the insertion sooner.

With that, we'll end up no longer needing the TARGET_OBJECT_RAW_MEMORY
changes once software single-step breakpoints are converted to real
breakpoints.  We need them today as software single-step breakpoints
bypass insert_bp_location.  But, it'll be best to leave that in as
safeguard anyway, for other direct uses of TARGET_OBJECT_RAW_MEMORY.

Tested on x86_64 Fedora 20, native and gdbserver.

gdb/
2014-10-01  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (insert_bp_location): Error out if inserting a
	software breakpoint at a read-only address.
	* target.c (memory_xfer_check_region): New function, factored out
	from ...
	(memory_xfer_partial_1): ... this.  Make the 'reg_len' local a
	ULONGEST.
	(target_xfer_partial) <TARGET_OBJECT_RAW_MEMORY>: Check the access
	against the memory region attributes.

gdb/testsuite/
2014-10-01  Pedro Alves  <palves@redhat.com>

	* gdb.base/breakpoint-in-ro-region.c: New file.
	* gdb.base/breakpoint-in-ro-region.exp: New file.

1 files changed, 67 insertions, 28 deletions

diff --git a/gdb/target.c b/gdb/target.c
index c1b5182..4606d17 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -909,6 +909,59 @@ target_section_by_addr (struct target_ops *target, CORE_ADDR addr)
   return NULL;
 }
 
+
+/* Helper for the memory xfer routines.  Checks the attributes of the
+   memory region of MEMADDR against the read or write being attempted.
+   If the access is permitted returns true, otherwise returns false.
+   REGION_P is an optional output parameter.  If not-NULL, it is
+   filled with a pointer to the memory region of MEMADDR.  REG_LEN
+   returns LEN trimmed to the end of the region.  This is how much the
+   caller can continue requesting, if the access is permitted.  A
+   single xfer request must not straddle memory region boundaries.  */
+
+static int
+memory_xfer_check_region (gdb_byte *readbuf, const gdb_byte *writebuf,
+			  ULONGEST memaddr, ULONGEST len, ULONGEST *reg_len,
+			  struct mem_region **region_p)
+{
+  struct mem_region *region;
+
+  region = lookup_mem_region (memaddr);
+
+  if (region_p != NULL)
+    *region_p = region;
+
+  switch (region->attrib.mode)
+    {
+    case MEM_RO:
+      if (writebuf != NULL)
+	return 0;
+      break;
+
+    case MEM_WO:
+      if (readbuf != NULL)
+	return 0;
+      break;
+
+    case MEM_FLASH:
+      /* We only support writing to flash during "load" for now.  */
+      if (writebuf != NULL)
+	error (_("Writing to flash memory forbidden in this context"));
+      break;
+
+    case MEM_NONE:
+      return 0;
+    }
+
+  /* region->hi == 0 means there's no upper bound.  */
+  if (memaddr + len < region->hi || region->hi == 0)
+    *reg_len = len;
+  else
+    *reg_len = region->hi - memaddr;
+
+  return 1;
+}
+
 /* Read memory from more than one valid target.  A core file, for
    instance, could have some of memory but delegate other bits to
    the target below it.  So, we must manually try all targets.  */
@@ -970,7 +1023,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
 		       ULONGEST len, ULONGEST *xfered_len)
 {
   enum target_xfer_status res;
-  int reg_len;
+  ULONGEST reg_len;
   struct mem_region *region;
   struct inferior *inf;
 
@@ -1017,34 +1070,10 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object,
     }
 
   /* Try GDB's internal data cache.  */
-  region = lookup_mem_region (memaddr);
-  /* region->hi == 0 means there's no upper bound.  */
-  if (memaddr + len < region->hi || region->hi == 0)
-    reg_len = len;
-  else
-    reg_len = region->hi - memaddr;
-
-  switch (region->attrib.mode)
-    {
-    case MEM_RO:
-      if (writebuf != NULL)
-	return TARGET_XFER_E_IO;
-      break;
 
-    case MEM_WO:
-      if (readbuf != NULL)
-	return TARGET_XFER_E_IO;
-      break;
-
-    case MEM_FLASH:
-      /* We only support writing to flash during "load" for now.  */
-      if (writebuf != NULL)
-	error (_("Writing to flash memory forbidden in this context"));
-      break;
-
-    case MEM_NONE:
-      return TARGET_XFER_E_IO;
-    }
+  if (!memory_xfer_check_region (readbuf, writebuf, memaddr, len, &reg_len,
+				 &region))
+    return TARGET_XFER_E_IO;
 
   if (!ptid_equal (inferior_ptid, null_ptid))
     inf = find_inferior_pid (ptid_get_pid (inferior_ptid));
@@ -1184,6 +1213,16 @@ target_xfer_partial (struct target_ops *ops,
 				  writebuf, offset, len, xfered_len);
   else if (object == TARGET_OBJECT_RAW_MEMORY)
     {
+      /* Skip/avoid accessing the target if the memory region
+	 attributes block the access.  Check this here instead of in
+	 raw_memory_xfer_partial as otherwise we'd end up checking
+	 this twice in the case of the memory_xfer_partial path is
+	 taken; once before checking the dcache, and another in the
+	 tail call to raw_memory_xfer_partial.  */
+      if (!memory_xfer_check_region (readbuf, writebuf, offset, len, &len,
+				     NULL))
+	return TARGET_XFER_E_IO;
+
       /* Request the normal memory object from other layers.  */
       retval = raw_memory_xfer_partial (ops, readbuf, writebuf, offset, len,
 					xfered_len);