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.
This commit is contained in:
Pedro Alves 2014-10-01 23:31:55 +01:00
parent 2ddf430110
commit 0fec99e8be
6 changed files with 263 additions and 32 deletions

View File

@ -1,3 +1,14 @@
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.
2014-10-01 Simon Marchi <simon.marchi@ericsson.com>
* NEWS: Announce new exit-code field in -list-thread-groups

View File

@ -2674,10 +2674,16 @@ insert_bp_location (struct bp_location *bl,
}
}
else if (bl->loc_type == bp_loc_software_breakpoint
&& mr->attrib.mode != MEM_RW)
warning (_("cannot set software breakpoint "
"at readonly address %s"),
paddress (bl->gdbarch, bl->address));
&& mr->attrib.mode != MEM_RW)
{
fprintf_unfiltered (tmp_error_stream,
_("Cannot insert breakpoint %d.\n"
"Cannot set software breakpoint "
"at read-only address %s\n"),
bl->owner->number,
paddress (bl->gdbarch, bl->address));
return 1;
}
}
}

View File

@ -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);

View File

@ -1,3 +1,8 @@
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.
2014-10-01 Simon Marchi <simon.marchi@ericsson.com>
* gdb.mi/mi-exit-code.exp: New file.

View File

@ -0,0 +1,28 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2014 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
volatile int i;
int
main (void)
{
i = 0;
i = 0;
i = 0;
return 0;
}

View File

@ -0,0 +1,142 @@
# Copyright 2014 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# This file is part of the gdb testsuite
standard_testfile
if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
return -1
}
if ![runto main] {
return -1
}
delete_breakpoints
# Get the bounds of a function, and write them to FUNC_LO (inclusive),
# FUNC_HI (exclusive). Return true on success and false on failure.
proc get_function_bounds {function func_lo func_hi} {
global gdb_prompt
global hex decimal
upvar $func_lo lo
upvar $func_hi hi
set lo ""
set size ""
set test "get lo address of $function"
gdb_test_multiple "disassemble $function" $test {
-re "($hex) .*$hex <\\+($decimal)>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" {
set lo $expect_out(1,string)
set size $expect_out(2,string)
pass $test
}
}
if { $lo == "" || $size == "" } {
return false
}
# Account for the size of the last instruction.
set test "get hi address of $function"
gdb_test_multiple "x/2i $function+$size" $test {
-re ".*$hex <$function\\+$size>:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
set hi $expect_out(1,string)
pass $test
}
}
if { $hi == "" } {
return false
}
# Remove unnecessary leading 0's (0x00000ADDR => 0xADDR) so we can
# easily do matches. Disassemble includes leading zeros, while
# x/i doesn't.
regsub -all "0x0\+" $lo "0x" lo
regsub -all "0x0\+" $hi "0x" hi
return true
}
# Get the address where the thread is currently stopped.
proc get_curr_insn {} {
global gdb_prompt
global hex
set pc ""
set test "get current insn"
gdb_test_multiple "p /x \$pc" $test {
-re " = ($hex)\r\n$gdb_prompt $" {
set pc $expect_out(1,string)
pass $test
}
}
return $pc
}
# Get the address of where a single-step should land.
proc get_next_insn {} {
global gdb_prompt
global hex
set next ""
set test "get next insn"
gdb_test_multiple "x/2i \$pc" $test {
-re "$hex .*:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
set next $expect_out(1,string)
pass $test
}
}
return $next
}
if ![get_function_bounds "main" main_lo main_hi] {
# Can't do the following tests if main's bounds are unknown.
return -1
}
# Manually create a read-only memory region that covers 'main'.
gdb_test_no_output "mem $main_lo $main_hi ro" \
"create read-only mem region covering main"
# So that we don't fail inserting breakpoints on addresses outside
# main, like the internal event breakpoints.
gdb_test_no_output "set mem inaccessible-by-default off"
# So we get an immediate warning/error without needing to resume the
# target.
gdb_test_no_output "set breakpoint always-inserted on"
# Disable the automatic fallback to HW breakpoints. We want a
# software breakpoint to be attempted, and to fail.
gdb_test_no_output "set breakpoint auto-hw off"
# Confirm manual writes to the read-only memory region fail.
gdb_test "p /x *(char *) $main_lo = 1" \
"Cannot access memory at address $main_lo" \
"writing to read-only memory fails"
# Ensure that inserting a software breakpoint in a known-read-only
# region fails.
gdb_test "break *$main_lo" \
"Cannot insert breakpoint .*Cannot set software breakpoint at read-only address $main_lo.*" \
"inserting software breakpoint in read-only memory fails"