aboutsummaryrefslogtreecommitdiff
path: root/gdb/mem-break.c
diff options
context:
space:
mode:
authorPedro Alves <palves@redhat.com>2014-04-22 23:19:19 +0100
committerPedro Alves <palves@redhat.com>2014-04-23 15:09:27 +0100
commit08351840eabb44799e3d01026610420758f4fa40 (patch)
tree28e133aaf049499380c4d614ceda1c31c47248d0 /gdb/mem-break.c
parent076855f9e36ecfe8af325b197e9ecd46deb9fe6c (diff)
downloadgdb-08351840eabb44799e3d01026610420758f4fa40.zip
gdb-08351840eabb44799e3d01026610420758f4fa40.tar.gz
gdb-08351840eabb44799e3d01026610420758f4fa40.tar.bz2
Stale breakpoint instructions, spurious SIGTRAPS.
Without the code portion of the patch, we get these failures: FAIL: gdb.base/break-unload-file.exp: always-inserted on: break: continue FAIL: gdb.base/break-unload-file.exp: always-inserted on: hbreak: continue FAIL: gdb.base/sym-file.exp: stale bkpts: continue to breakpoint: end here They all looks like random SIGTRAPs: continue Continuing. Program received signal SIGTRAP, Trace/breakpoint trap. 0x0000000000400541 in foo () at ../../../src/gdb/testsuite/gdb.base/break-unload-file.c:21 21 } (gdb) FAIL: gdb.base/break-unload-file.exp: always-inserted on: break: continue (This is a regression caused by the remove-symbol-file command series.) break-unload-file.exp is about having breakpoints inserted, and then doing "file". I caught this while writing a test that does "file PROGRAM", while PROGRAM was already loaded, which internally does "file" first, because I wanted to force a breakpoint_re_set, but the test is more explicit in case GDB ever optimizes out that re-set. The problem is that unloading the file with "file" ends up in disable_breakpoints_in_freed_objfile, which marks all breakpoint locations of the objfile as both shlib_disabled, _and_ clears the inserted flag, without actually removing the breakpoints from the inferior. Now, usually, in all-stop, breakpoints will already be removed from the inferior before the user can issue the "file" command, but, with non-stop, or breakpoints always-inserted on mode, breakpoints stay inserted even while the user has the prompt. In the latter case, then, if we let the program continue, and it executes the address where we had previously set the breakpoint, it'll actually execute the breakpoint instruction that we left behind... Now, one issue is that the intent of disable_breakpoints_in_freed_objfile is really to handle the unloading of OBJF_USERLOADED objfiles. These are objfiles that were added with add-symbol-file and that are removed with remove-symbol-file. "add-symbol-file"'s docs in the manual clearly say these commands are used to let GDB know about dynamically loaded code: You would use this command when @var{filename} has been dynamically loaded (by some other means) into the program that is running. Similarly, the online help says: (gdb) help add-symbol-file Load symbols from FILE, assuming FILE has been dynamically loaded. So it makes sense to, like when shared libraries are unloaded through the generic solib machinery, mark the breakpoint locations as shlib_disabled. But, the "file" command is not about dynamically loaded code, it's about the main program. So the patch makes disable_breakpoints_in_freed_objfile skip all objfiles but OBJF_USERLOADED ones, thus skipping the main objfile. Then, the reason that disable_breakpoints_in_freed_objfile was clearing the inserted flag isn't clear, but likely to avoid breakpoint removal errors, assuming remove-symbol-file was called after the dynamic object was already unmapped from the inferior. In that case, it'd okay to simply clear the inserted flag, but not so if the user for example does remove-symbol-file to remove the library because he made a mistake in the library's address, and wants to re-do add-symbol-file with the correct address. To address all that, I propose an alternative implementation, that handles both cases. The patch includes changes to sym-file.exp to cover them. This implementation leaves the inserted flag alone, and handles breakpoint insertion/removal failure gracefully when the locations are in OBJF_USERLOADED objfiles, just like we handle insertion/removal failure gracefully for locations in shared libraries. To try to make sure we aren't patching back stale shadow memory contents into the inferior, in case the program mapped a different library at the same address where we had the breakpoint, without the user having had a chance of remove-symbol-file'ing before, this adds a new memory_validate_breakpoint function that checks if the breakpoint instruction is still in memory. ppc_linux_memory_remove_breakpoint does this unconditionally for all memory breakpoints, and questions whether memory_remove_breakpoint should be changed to do this for all breakpoints. Possibly yes, though I'm not certain, hence this baby-steps patch. Tested on x86_64 Fedora 17, native and gdbserver. gdb/ 2014-04-23 Pedro Alves <palves@redhat.com> * breakpoint.c (insert_bp_location): Tolerate errors if the breakpoint is set in a user-loaded objfile. (remove_breakpoint_1): Likewise. Also tolerate errors if the location is marked shlib_disabled. If the breakpoint is set in a user-loaded objfile is a GDB-side memory breakpoint, validate it before uninsertion. (disable_breakpoints_in_freed_objfile): Skip non-OBJF_USERLOADED objfiles. Don't clear the location's inserted flag. * mem-break.c (memory_validate_breakpoint): New function. * objfiles.c (userloaded_objfile_contains_address_p): New function. * objfiles.h (userloaded_objfile_contains_address_p): Declare. * target.h (memory_validate_breakpoint): New declaration. gdb/testsuite/ 2014-04-23 Pedro Alves <palves@redhat.com> * gdb.base/break-unload-file.c: New file. * gdb.base/break-unload-file.exp: New file. * gdb.base/sym-file-lib.c (baz): New function. * gdb.base/sym-file-loader.c (struct segment) <mapped_size>: New field. (load): Store the segment's mapped size. (unload): New function. (unload_shlib): New function. * gdb.base/sym-file-loader.h (unload_shlib): New declaration. * gdb.base/sym-file-main.c (main): Unload, and reload the library, set a breakpoint at baz, and call it. * gdb.base/sym-file.exp: New tests for stale breakpoint instructions.
Diffstat (limited to 'gdb/mem-break.c')
-rw-r--r--gdb/mem-break.c32
1 files changed, 32 insertions, 0 deletions
diff --git a/gdb/mem-break.c b/gdb/mem-break.c
index 1a057df..ad7296b 100644
--- a/gdb/mem-break.c
+++ b/gdb/mem-break.c
@@ -89,3 +89,35 @@ memory_remove_breakpoint (struct target_ops *ops, struct gdbarch *gdbarch,
{
return gdbarch_memory_remove_breakpoint (gdbarch, bp_tgt);
}
+
+int
+memory_validate_breakpoint (struct gdbarch *gdbarch,
+ struct bp_target_info *bp_tgt)
+{
+ CORE_ADDR addr = bp_tgt->placed_address;
+ const gdb_byte *bp;
+ int val;
+ int bplen;
+ gdb_byte cur_contents[BREAKPOINT_MAX];
+ struct cleanup *cleanup;
+ int ret;
+
+ /* Determine appropriate breakpoint contents and size for this
+ address. */
+ bp = gdbarch_breakpoint_from_pc (gdbarch, &addr, &bplen);
+
+ if (bp == NULL || bp_tgt->placed_size != bplen)
+ return 0;
+
+ /* Make sure we see the memory breakpoints. */
+ cleanup = make_show_memory_breakpoints_cleanup (1);
+ val = target_read_memory (addr, cur_contents, bplen);
+
+ /* If our breakpoint is no longer at the address, this means that
+ the program modified the code on us, so it is wrong to put back
+ the old value. */
+ ret = (val == 0 && memcmp (bp, cur_contents, bplen) == 0);
+
+ do_cleanups (cleanup);
+ return ret;
+}