From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8027 invoked by alias); 6 Dec 2011 20:08:10 -0000 Received: (qmail 7966 invoked by uid 22791); 6 Dec 2011 20:08:07 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,TW_CP X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 06 Dec 2011 20:07:51 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=EU1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RY1Is-00050t-M0 from pedro_alves@mentor.com for gdb-patches@sourceware.org; Tue, 06 Dec 2011 12:07:51 -0800 Received: from scottsdale.localnet ([172.16.63.104]) by EU1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Tue, 6 Dec 2011 20:07:47 +0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFC/WIP PATCH 02/14] Mask software breakpoints from memory writes too Date: Tue, 06 Dec 2011 20:40:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-13-generic; KDE/4.7.2; x86_64; ; ) References: <20111128153742.17761.21459.stgit@localhost6.localdomain6> <20111128153904.17761.45665.stgit@localhost6.localdomain6> In-Reply-To: <20111128153904.17761.45665.stgit@localhost6.localdomain6> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201112062007.46352.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-12/txt/msg00201.txt.bz2 On Monday 28 November 2011 15:39:04, Pedro Alves wrote: > Now that I've written this, I wonder if it's safe against a remote > target that implements software breakpoints itself... Duh, it is. Memory breakpoints don't get shadows (on gdb) in that case. I've now committed this updated version, which remembers to update the breakpoints' shadows on writes this time (duh2!), and also adds new simple tests that don't rely on displaced stepping. 2011-12-06 Pedro Alves gdb/ * breakpoint.c (breakpoint_restore_shadows): Rename to ... (breakpoint_xfer_memory): ... this. Change prototype. Handle memory writes too. * breakpoint.h (breakpoint_restore_shadows): Delete. (breakpoint_xfer_memory): Declare. * mem-break.c (default_memory_insert_breakpoint) (default_memory_remove_breakpoint): Use target_write_raw_memory. (memory_xfer_partial): Rename to ... (memory_xfer_partial_1): ... this. Don't mask out breakpoints here. (memory_xfer_partial): New. (target_write_raw_memory): New. * target.h (target_write_raw_memory): New. gdb/testsuite/ * gdb.base/break-always.exp: Test changing memory at addresses with breakpoints inserted. --- gdb/breakpoint.c | 32 +++++++++++- gdb/breakpoint.h | 15 ++++-- gdb/mem-break.c | 8 ++- gdb/target.c | 80 +++++++++++++++++++++++++------ gdb/target.h | 3 + gdb/testsuite/gdb.base/break-always.exp | 37 ++++++++++++++ 6 files changed, 148 insertions(+), 27 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index bd0a0e3..d9d5bbe 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1049,7 +1049,9 @@ bp_location_has_shadow (struct bp_location *bl) bl->address + bp_location_shadow_len_after_address_max <= memaddr */ void -breakpoint_restore_shadows (gdb_byte *buf, ULONGEST memaddr, LONGEST len) +breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf, + const gdb_byte *writebuf_org, + ULONGEST memaddr, LONGEST len) { /* Left boundary, right boundary and median element of our binary search. */ @@ -1161,8 +1163,32 @@ breakpoint_restore_shadows (gdb_byte *buf, ULONGEST memaddr, LONGEST len) bp_size -= (bp_addr + bp_size) - (memaddr + len); } - memcpy (buf + bp_addr - memaddr, - bl->target_info.shadow_contents + bptoffset, bp_size); + if (readbuf != NULL) + { + /* Update the read buffer with this inserted breakpoint's + shadow. */ + memcpy (readbuf + bp_addr - memaddr, + bl->target_info.shadow_contents + bptoffset, bp_size); + } + else + { + struct gdbarch *gdbarch = bl->gdbarch; + const unsigned char *bp; + CORE_ADDR placed_address = bl->target_info.placed_address; + unsigned placed_size = bl->target_info.placed_size; + + /* Update the shadow with what we want to write to memory. */ + memcpy (bl->target_info.shadow_contents + bptoffset, + writebuf_org + bp_addr - memaddr, bp_size); + + /* Determine appropriate breakpoint contents and size for this + address. */ + bp = gdbarch_breakpoint_from_pc (gdbarch, &placed_address, &placed_size); + + /* Update the final write buffer with this inserted + breakpoint's INSN. */ + memcpy (writebuf + bp_addr - memaddr, bp + bptoffset, bp_size); + } } } diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index ead3930..ddf1881 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1296,10 +1296,17 @@ extern int deprecated_remove_raw_breakpoint (struct gdbarch *, void *); target. */ int watchpoints_triggered (struct target_waitstatus *); -/* Update BUF, which is LEN bytes read from the target address MEMADDR, - by replacing any memory breakpoints with their shadowed contents. */ -void breakpoint_restore_shadows (gdb_byte *buf, ULONGEST memaddr, - LONGEST len); +/* Helper for transparent breakpoint hiding for memory read and write + routines. + + Update one of READBUF or WRITEBUF with either the shadows + (READBUF), or the breakpoint instructions (WRITEBUF) of inserted + breakpoints at the memory range defined by MEMADDR and extending + for LEN bytes. If writing, then WRITEBUF is a copy of WRITEBUF_ORG + on entry.*/ +extern void breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf, + const gdb_byte *writebuf_org, + ULONGEST memaddr, LONGEST len); extern int breakpoints_always_inserted_mode (void); diff --git a/gdb/mem-break.c b/gdb/mem-break.c index ba7dc24..31ca45c 100644 --- a/gdb/mem-break.c +++ b/gdb/mem-break.c @@ -60,8 +60,8 @@ default_memory_insert_breakpoint (struct gdbarch *gdbarch, /* Write the breakpoint. */ if (val == 0) - val = target_write_memory (bp_tgt->placed_address, bp, - bp_tgt->placed_size); + val = target_write_raw_memory (bp_tgt->placed_address, bp, + bp_tgt->placed_size); return val; } @@ -71,8 +71,8 @@ int default_memory_remove_breakpoint (struct gdbarch *gdbarch, struct bp_target_info *bp_tgt) { - return target_write_memory (bp_tgt->placed_address, bp_tgt->shadow_contents, - bp_tgt->placed_size); + return target_write_raw_memory (bp_tgt->placed_address, bp_tgt->shadow_contents, + bp_tgt->placed_size); } diff --git a/gdb/target.c b/gdb/target.c index 6358b00..5700066 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -1388,19 +1388,15 @@ memory_xfer_live_readonly_partial (struct target_ops *ops, For docs see target.h, to_xfer_partial. */ static LONGEST -memory_xfer_partial (struct target_ops *ops, enum target_object object, - void *readbuf, const void *writebuf, ULONGEST memaddr, - LONGEST len) +memory_xfer_partial_1 (struct target_ops *ops, enum target_object object, + void *readbuf, const void *writebuf, ULONGEST memaddr, + LONGEST len) { LONGEST res; int reg_len; struct mem_region *region; struct inferior *inf; - /* Zero length requests are ok and require no work. */ - if (len == 0) - return 0; - /* For accesses to unmapped overlay sections, read directly from files. Must do this first, as MEMADDR may need adjustment. */ if (readbuf != NULL && overlay_debugging) @@ -1551,11 +1547,7 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object, if (res <= 0) return -1; else - { - if (readbuf && !show_memory_breakpoints) - breakpoint_restore_shadows (readbuf, memaddr, reg_len); - return res; - } + return res; } /* If none of those methods found the memory we wanted, fall back @@ -1584,9 +1576,6 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object, } while (ops != NULL); - if (res > 0 && readbuf != NULL && !show_memory_breakpoints) - breakpoint_restore_shadows (readbuf, memaddr, reg_len); - /* Make sure the cache gets updated no matter what - if we are writing to the stack. Even if this write is not tagged as such, we still need to update the cache. */ @@ -1606,6 +1595,48 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object, return res; } +/* Perform a partial memory transfer. For docs see target.h, + to_xfer_partial. */ + +static LONGEST +memory_xfer_partial (struct target_ops *ops, enum target_object object, + void *readbuf, const void *writebuf, ULONGEST memaddr, + LONGEST len) +{ + int res; + + /* Zero length requests are ok and require no work. */ + if (len == 0) + return 0; + + /* Fill in READBUF with breakpoint shadows, or WRITEBUF with + breakpoint insns, thus hiding out from higher layers whether + there are software breakpoints inserted in the code stream. */ + if (readbuf != NULL) + { + res = memory_xfer_partial_1 (ops, object, readbuf, NULL, memaddr, len); + + if (res > 0 && !show_memory_breakpoints) + breakpoint_xfer_memory (readbuf, NULL, NULL, memaddr, res); + } + else + { + void *buf; + struct cleanup *old_chain; + + buf = xmalloc (len); + old_chain = make_cleanup (xfree, buf); + memcpy (buf, writebuf, len); + + breakpoint_xfer_memory (NULL, buf, writebuf, memaddr, len); + res = memory_xfer_partial_1 (ops, object, NULL, buf, memaddr, len); + + do_cleanups (old_chain); + } + + return res; +} + static void restore_show_memory_breakpoints (void *arg) { @@ -1761,6 +1792,25 @@ target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, int len) return EIO; } +/* Write LEN bytes from MYADDR to target raw memory at address + MEMADDR. Returns either 0 for success or an errno value if any + error occurs. If an error occurs, no guarantee is made about how + much data got written. Callers that can deal with partial writes + should call target_write. */ + +int +target_write_raw_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, int len) +{ + /* Dispatch to the topmost target, not the flattened current_target. + Memory accesses check target->to_has_(all_)memory, and the + flattened target doesn't inherit those. */ + if (target_write (current_target.beneath, TARGET_OBJECT_RAW_MEMORY, NULL, + myaddr, memaddr, len) == len) + return 0; + else + return EIO; +} + /* Fetch the target's memory map. */ VEC(mem_region_s) * diff --git a/gdb/target.h b/gdb/target.h index 1261d90..fd488d6 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -941,6 +941,9 @@ extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, int len); extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, int len); +extern int target_write_raw_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, + int len); + /* Fetches the target's memory map. If one is found it is sorted and returned, after some consistency checking. Otherwise, NULL is returned. */ diff --git a/gdb/testsuite/gdb.base/break-always.exp b/gdb/testsuite/gdb.base/break-always.exp index ce76af7..e20794e 100644 --- a/gdb/testsuite/gdb.base/break-always.exp +++ b/gdb/testsuite/gdb.base/break-always.exp @@ -49,7 +49,42 @@ gdb_test_no_output "enable 2" "enable 2.H" gdb_test_no_output "disable 2" "disable 2.I" gdb_test "info breakpoints" "keep n.*keep n.*keep y.*keep n.*keep n.*" "before re-enable check breakpoint state" gdb_test_no_output "enable" "re-enable all breakpoints" -gdb_continue_to_breakpoint "bar" ".*break-always.c:$bar_location.*" +set bp_address 0 +set test "set breakpoint on bar 2" +gdb_test_multiple "break bar" $test { + -re "Breakpoint 6 at ($hex).*$gdb_prompt $" { + set bp_address $expect_out(1,string) + pass $test + } +} + +# Save the original INSN under the breakpoint. +gdb_test "p /x \$shadow = *(char *) $bp_address" \ + " = $hex" \ + "save shadow" +# Overwrite memory where the breakpoint is planted. GDB should update +# its memory breakpoint's shadows, to account for the new contents, +# and still leave the breakpoint insn planted. Try twice with +# different values, in case we happen to be writting exactly what was +# there already. +gdb_test "p /x *(char *) $bp_address = 0" \ + " = 0x0" \ + "write 0 to breakpoint's address" +gdb_test "p /x *(char *) $bp_address" \ + " = 0x0" \ + "read back 0 from the breakpoint's address" +gdb_test "p /x *(char *) $bp_address = 1" \ + " = 0x1" \ + "write 1 to breakpoint's address" +gdb_test "p /x *(char *) $bp_address" \ + " = 0x1" \ + "read back 1 from the breakpoint's address" + +# Restore the original contents. +gdb_test "p /x *(char *) $bp_address = \$shadow" "" + +# Run to breakpoint. +gdb_continue_to_breakpoint "bar" ".*break-always.c:$bar_location.*"