diff options
| -rw-r--r-- | gdb/ChangeLog | 16 | ||||
| -rw-r--r-- | gdb/breakpoint.c | 71 | ||||
| -rw-r--r-- | gdb/mem-break.c | 32 | ||||
| -rw-r--r-- | gdb/objfiles.c | 16 | ||||
| -rw-r--r-- | gdb/objfiles.h | 7 | ||||
| -rw-r--r-- | gdb/target.h | 6 | ||||
| -rw-r--r-- | gdb/testsuite/ChangeLog | 16 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/break-unload-file.c | 35 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/break-unload-file.exp | 128 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/sym-file-lib.c | 6 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/sym-file-loader.c | 28 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/sym-file-loader.h | 4 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/sym-file-main.c | 26 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/sym-file.exp | 49 |
14 files changed, 425 insertions, 15 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4066217..262d709 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,21 @@ 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. + +2014-04-23 Pedro Alves <palves@redhat.com> + * breakpoint.c (insert_bp_location, remove_breakpoint_1): If the breakpoint is set in a shared library, only suppress errors for software breakpoints, not hardware breakpoints. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 19a7376..f422998 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2648,7 +2648,9 @@ insert_bp_location (struct bp_location *bl, errors as memory errors. */ if ((bp_err == GENERIC_ERROR || bp_err == MEMORY_ERROR) && bl->loc_type == bp_loc_software_breakpoint - && solib_name_from_address (bl->pspace, bl->address)) + && (solib_name_from_address (bl->pspace, bl->address) + || userloaded_objfile_contains_address_p (bl->pspace, + bl->address))) { /* See also: disable_breakpoints_in_shlibs. */ bl->shlib_disabled = 1; @@ -3778,7 +3780,31 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is) || !(section_is_overlay (bl->section))) { /* No overlay handling: just remove the breakpoint. */ - val = bl->owner->ops->remove_location (bl); + + /* If we're trying to uninsert a memory breakpoint that we + know is set in a dynamic object that is marked + shlib_disabled, then either the dynamic object was + removed with "remove-symbol-file" or with + "nosharedlibrary". In the former case, we don't know + whether another dynamic object might have loaded over the + breakpoint's address -- the user might well let us know + about it next with add-symbol-file (the whole point of + OBJF_USERLOADED is letting the user manually maintain a + list of dynamically loaded objects). If we have the + breakpoint's shadow memory, that is, this is a software + breakpoint managed by GDB, check whether the breakpoint + is still inserted in memory, to avoid overwriting wrong + code with stale saved shadow contents. Note that HW + breakpoints don't have shadow memory, as they're + implemented using a mechanism that is not dependent on + being able to modify the target's memory, and as such + they should always be removed. */ + if (bl->shlib_disabled + && bl->target_info.shadow_len != 0 + && !memory_validate_breakpoint (bl->gdbarch, &bl->target_info)) + val = 0; + else + val = bl->owner->ops->remove_location (bl); } else { @@ -3823,12 +3849,21 @@ remove_breakpoint_1 (struct bp_location *bl, insertion_state_t is) } } - /* In some cases, we might not be able to remove a breakpoint - in a shared library that has already been removed, but we - have not yet processed the shlib unload event. */ + /* In some cases, we might not be able to remove a breakpoint in + a shared library that has already been removed, but we have + not yet processed the shlib unload event. Similarly for an + unloaded add-symbol-file object - the user might not yet have + had the chance to remove-symbol-file it. shlib_disabled will + be set if the library/object has already been removed, but + the breakpoint hasn't been uninserted yet, e.g., after + "nosharedlibrary" or "remove-symbol-file" with breakpoints + always-inserted mode. */ if (val - && bl->loc_type == bp_loc_software_breakpoint - && solib_name_from_address (bl->pspace, bl->address)) + && (bl->loc_type == bp_loc_software_breakpoint + && (bl->shlib_disabled + || solib_name_from_address (bl->pspace, bl->address) + || userloaded_objfile_contains_address_p (bl->pspace, + bl->address)))) val = 0; if (val) @@ -7665,10 +7700,18 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile) if (objfile == NULL) return; - /* If the file is a shared library not loaded by the user then - solib_unloaded was notified and disable_breakpoints_in_unloaded_shlib - was called. In that case there is no need to take action again. */ - if ((objfile->flags & OBJF_SHARED) && !(objfile->flags & OBJF_USERLOADED)) + /* OBJF_USERLOADED are dynamic modules manually managed by the user + with add-symbol-file/remove-symbol-file. Similarly to how + breakpoints in shared libraries are handled in response to + "nosharedlibrary", mark breakpoints in OBJF_USERLOADED modules + shlib_disabled so they end up uninserted on the next global + location list update. Shared libraries not loaded by the user + aren't handled here -- they're already handled in + disable_breakpoints_in_unloaded_shlib, called by solib.c's + solib_unloaded observer. We skip objfiles that are not + OBJF_USERLOADED (nor OBJF_SHARED) as those aren't considered + dynamic objects (e.g. the main objfile). */ + if ((objfile->flags & OBJF_USERLOADED) == 0) return; ALL_BREAKPOINTS (b) @@ -7700,7 +7743,11 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile) if (is_addr_in_objfile (loc_addr, objfile)) { loc->shlib_disabled = 1; - loc->inserted = 0; + /* At this point, we don't know whether the object was + unmapped from the inferior or not, so leave the + inserted flag alone. We'll handle failure to + uninsert quietly, in case the object was indeed + unmapped. */ mark_breakpoint_location_modified (loc); 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; +} diff --git a/gdb/objfiles.c b/gdb/objfiles.c index e212c70..81bbf24 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -1453,6 +1453,22 @@ is_addr_in_objfile (CORE_ADDR addr, const struct objfile *objfile) return 0; } +int +userloaded_objfile_contains_address_p (struct program_space *pspace, + CORE_ADDR address) +{ + struct objfile *objfile; + + ALL_PSPACE_OBJFILES (pspace, objfile) + { + if ((objfile->flags & OBJF_USERLOADED) != 0 + && is_addr_in_objfile (address, objfile)) + return 1; + } + + return 0; +} + /* The default implementation for the "iterate_over_objfiles_in_search_order" gdbarch method. It is equivalent to use the ALL_OBJFILES macro, searching the objfiles in the order they are stored internally, diff --git a/gdb/objfiles.h b/gdb/objfiles.h index dc06c7b..6684278 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -515,6 +515,13 @@ extern void objfiles_changed (void); extern int is_addr_in_objfile (CORE_ADDR addr, const struct objfile *objfile); +/* Return true if ADDRESS maps into one of the sections of the + userloaded ("add-symbol-file") objfiles of PSPACE and false + otherwise. */ + +extern int userloaded_objfile_contains_address_p (struct program_space *pspace, + CORE_ADDR address); + /* This operation deletes all objfile entries that represent solibs that weren't explicitly loaded by the user, via e.g., the add-symbol-file command. */ diff --git a/gdb/target.h b/gdb/target.h index d7c6c3d..492ecb5 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -2110,6 +2110,12 @@ extern int memory_remove_breakpoint (struct target_ops *, struct gdbarch *, extern int memory_insert_breakpoint (struct target_ops *, struct gdbarch *, struct bp_target_info *); +/* Check whether the memory at the breakpoint's placed address still + contains the expected breakpoint instruction. */ + +extern int memory_validate_breakpoint (struct gdbarch *gdbarch, + struct bp_target_info *bp_tgt); + extern int default_memory_remove_breakpoint (struct gdbarch *, struct bp_target_info *); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 5502a08..ecf2147 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,21 @@ 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. + +2014-04-23 Pedro Alves <palves@redhat.com> + * gdb.base/hbreak-in-shr-unsupported-shr.c: New file. * gdb.base/hbreak-in-shr-unsupported.c: New file. * gdb.base/hbreak-in-shr-unsupported.exp: New file. diff --git a/gdb/testsuite/gdb.base/break-unload-file.c b/gdb/testsuite/gdb.base/break-unload-file.c new file mode 100644 index 0000000..b0524f2 --- /dev/null +++ b/gdb/testsuite/gdb.base/break-unload-file.c @@ -0,0 +1,35 @@ +/* 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/>. */ + +void +foo (void) +{ +} + +void +bar (void) +{ +} + +int +main (void) +{ + foo (); + bar (); + + return 0; +} diff --git a/gdb/testsuite/gdb.base/break-unload-file.exp b/gdb/testsuite/gdb.base/break-unload-file.exp new file mode 100644 index 0000000..35c0f5f --- /dev/null +++ b/gdb/testsuite/gdb.base/break-unload-file.exp @@ -0,0 +1,128 @@ +# 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/>. */ + +# Test that "file" doesn't leave stale breakpoints planted in the +# target. + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +if ![runto_main] then { + fail "Can't run to main" + return 0 +} + +# Run the test proper. ALWAYS_INSERT determines whether +# always-inserted mode is on/off, and BREAK_COMMAND is the break +# command being tested. +# +proc test_break { always_inserted break_command } { + global gdb_prompt binfile hex + + with_test_prefix "always-inserted $always_inserted: $break_command" { + clean_restart $binfile + + if ![runto_main] then { + fail "Can't run to main" + return + } + + delete_breakpoints + + gdb_test_no_output "set breakpoint always-inserted $always_inserted" + + set test "$break_command foo" + gdb_test_multiple "$break_command foo" $test { + -re "No hardware breakpoint support in the target.*$gdb_prompt $" { + unsupported $test + return + } + -re "Hardware breakpoints used exceeds limit.*$gdb_prompt $" { + unsupported $test + return + } + -re "Cannot insert hardware breakpoint.*$gdb_prompt $" { + unsupported $test + return + } + -re ".*reakpoint .* at .*$gdb_prompt $" { + pass $test + } + } + + # The breakpoint shouldn't be pending now. + gdb_test "info break" "y.*$hex.*in foo at.*" \ + "breakpoint is not pending" + + # Remove the file, while the breakpoint above is inserted in a + # function in the main objfile. GDB used to have a bug where + # it would mark the breakpoint as uninserted, but actually + # would leave it inserted in the target. + set test "file" + gdb_test_multiple "file" $test { + -re "Are you sure you want to change the file. .*y or n. $" { + send_gdb "y\n" + exp_continue + } + -re "Discard symbol table from `.*'? .y or n. $" { + send_gdb "y\n" + exp_continue + } + -re "No symbol file now\\.\r\n$gdb_prompt $" { + pass $test + } + } + + gdb_test "info break" "y.*PENDING.*foo" \ + "breakpoint is not pending" + + # Now delete the breakpoint from GDB's tables, to make sure + # GDB doesn't reinsert it, masking the bug (with the bug, on + # re-insert, GDB would fill the shadow buffer with a + # breakpoint instruction). Avoid delete_breakpoints as that + # doesn't record a pass/fail. + gdb_test "delete" "" "delete all breakpoints" \ + "Delete all breakpoints.*y or n.*$" "y" + + # Re-add symbols back. + set test "file \$binfile" + gdb_test_multiple "file $binfile" $test { + -re "Are you sure you want to change the file. .*y or n. $" { + send_gdb "y\n" + exp_continue + } + -re "Reading symbols from.*done.*$gdb_prompt $" { + pass $test + } + } + + # Run to another function now. With the bug, GDB would trip + # on a spurious trap at foo. + gdb_test "b bar" ".*reakpoint .* at .*" + gdb_test "continue" "Breakpoint .*, bar .*" + } +} + +# While it doesn't trigger the original bug this is a regression test +# for, test with breakpoint always-inserted off for extra coverage. +foreach always_inserted { "off" "on" } { + test_break $always_inserted "break" + if {![skip_hw_breakpoint_tests]} { + test_break $always_inserted "hbreak" + } +} diff --git a/gdb/testsuite/gdb.base/sym-file-lib.c b/gdb/testsuite/gdb.base/sym-file-lib.c index d0c8847..67ac9ad 100644 --- a/gdb/testsuite/gdb.base/sym-file-lib.c +++ b/gdb/testsuite/gdb.base/sym-file-lib.c @@ -24,3 +24,9 @@ foo (int a) { return a; /* gdb break at foo */ } + +extern int +baz (int a) +{ + return a; /* gdb break at baz */ +} diff --git a/gdb/testsuite/gdb.base/sym-file-loader.c b/gdb/testsuite/gdb.base/sym-file-loader.c index d10065e..0b0039d 100644 --- a/gdb/testsuite/gdb.base/sym-file-loader.c +++ b/gdb/testsuite/gdb.base/sym-file-loader.c @@ -60,6 +60,7 @@ sizeof ((hdr)->field) == sizeof (Elf_Addr) ? *(Elf_Addr *) (hdr)->field : \ struct segment { uint8_t *mapped_addr; + size_t mapped_size; Elf_External_Phdr *phdr; struct segment *next; }; @@ -101,6 +102,7 @@ load (uint8_t *addr, Elf_External_Phdr *phdr, struct segment *tail_seg) { struct segment *seg = NULL; uint8_t *mapped_addr = NULL; + size_t mapped_size = 0; void *from = NULL; void *to = NULL; @@ -110,6 +112,7 @@ load (uint8_t *addr, Elf_External_Phdr *phdr, struct segment *tail_seg) mapped_addr = (uint8_t *) mmap ((void *) GETADDR (phdr, p_vaddr), GET (phdr, p_memsz), perm, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + mapped_size = GET (phdr, p_memsz); from = (void *) (addr + GET (phdr, p_offset)); to = (void *) mapped_addr; @@ -122,6 +125,7 @@ load (uint8_t *addr, Elf_External_Phdr *phdr, struct segment *tail_seg) return 0; seg->mapped_addr = mapped_addr; + seg->mapped_size = mapped_size; seg->phdr = phdr; seg->next = 0; @@ -173,6 +177,30 @@ get_origin (void) return self_path; } +/* Unload/unmap a segment. */ + +static void +unload (struct segment *seg) +{ + munmap (seg->mapped_addr, seg->mapped_size); + free (seg); +} + +void +unload_shlib (struct library *lib) +{ + struct segment *seg, *next_seg; + + for (seg = lib->segments; seg != NULL; seg = next_seg) + { + next_seg = seg->next; + unload (seg); + } + + close (lib->fd); + free (lib); +} + /* Mini shared library loader. No reallocation is performed for the sake of simplicity. */ diff --git a/gdb/testsuite/gdb.base/sym-file-loader.h b/gdb/testsuite/gdb.base/sym-file-loader.h index c6b1af3..7798a40 100644 --- a/gdb/testsuite/gdb.base/sym-file-loader.h +++ b/gdb/testsuite/gdb.base/sym-file-loader.h @@ -25,6 +25,10 @@ struct library; struct library *load_shlib (const char *file); +/* Unload a library. */ + +void unload_shlib (struct library *lib); + /* Lookup the address of FUNC. */ int lookup_function (struct library *lib, const char *func, void **addr); diff --git a/gdb/testsuite/gdb.base/sym-file-main.c b/gdb/testsuite/gdb.base/sym-file-main.c index b48a1c2..b6e36df 100644 --- a/gdb/testsuite/gdb.base/sym-file-main.c +++ b/gdb/testsuite/gdb.base/sym-file-main.c @@ -42,6 +42,8 @@ main (int argc, const char *argv[]) char *text_addr = NULL; int (*pbar) () = NULL; int (*pfoo) (int) = NULL; + int (*pbaz) () = NULL; + int i; lib = load_shlib (file); if (lib == NULL) @@ -64,8 +66,28 @@ main (int argc, const char *argv[]) (*pfoo) (2); - /* Notify GDB to remove the symbol file. */ + /* Unload the library, invalidating all memory breakpoints. */ + unload_shlib (lib); + + /* Notify GDB to remove the symbol file. Also check that GDB + doesn't complain that it can't remove breakpoints from the + unmapped library. */ gdb_remove_symbol_file (text_addr); - return 0; + /* Reload the library. */ + lib = load_shlib (file); /* reload lib here */ + if (lib == NULL) + return 1; + + if (get_text_addr (lib, (void **) &text_addr) != 0) + return 1; + + gdb_add_symbol_file (text_addr, file); + + if (lookup_function (lib, "baz", (void *) &pbaz) != 0) + return 1; + + (*pbaz) (); + + return 0; /* end here */ } diff --git a/gdb/testsuite/gdb.base/sym-file.exp b/gdb/testsuite/gdb.base/sym-file.exp index c87c3c7..48fb193 100644 --- a/gdb/testsuite/gdb.base/sym-file.exp +++ b/gdb/testsuite/gdb.base/sym-file.exp @@ -27,6 +27,7 @@ # 11) 'info files' must not display ${lib_basename}, anymore. # 12) Check that the breakpoints at foo and bar are pending. # 13) Check that the execution can continue without error. +# 14) Regression test for a stale breakpoints bug. if {![is_elf_target]} { return 0 @@ -159,4 +160,50 @@ gdb_test "info breakpoints 4" \ "breakpoint at bar is pending" # 13) Check that the execution can continue without error. -gdb_continue_to_end +set lnum_reload [gdb_get_line_number "reload lib here"] +gdb_breakpoint $lnum_reload +gdb_continue_to_breakpoint reload ".*${srcfile}:$lnum_reload.*" + +# 14) Regression test for a stale breakpoints bug. Check whether +# unloading symbols manually without the program actually unloading +# the library, when breakpoints are inserted doesn't leave stale +# breakpoints behind. +with_test_prefix "stale bkpts" { + # Force breakpoints always inserted. + gdb_test_no_output "set breakpoint always-inserted on" + + # Get past the library reload. + gdb_continue_to_breakpoint gdb_add_symbol_file + + # Load the library's symbols. + gdb_test "add-symbol-file ${lib_syms} addr" \ + "Reading symbols from .*${lib_syms}\\.\\.\\.done\\." \ + "add-symbol-file ${lib_basename}.so addr" \ + "add symbol table from file \".*${lib_syms}\"\ +at.*\\(y or n\\) " \ + "y" + + # Set a breakpoint at baz, in the library. + gdb_breakpoint baz + + gdb_test "info breakpoints 7" ".*y.*0x.*in baz.*" \ + "breakpoint at baz is resolved" + + # Unload symbols manually without the program actually unloading + # the library. + gdb_test "remove-symbol-file -a addr" \ + "" \ + "remove-symbol-file -a addr" \ + "Remove symbol table from file \".*${lib_basename}\\.so\"\\?\ +.*\\(y or n\\) " \ + "y" + + gdb_test "info breakpoints 7" ".*PENDING.*" \ + "breakpoint at baz is pending" + + # Check that execution can continue without error. If GDB leaves + # breakpoints behind, we'll get back a spurious SIGTRAP. + set lnum_end [gdb_get_line_number "end here"] + gdb_breakpoint $lnum_end + gdb_continue_to_breakpoint "end here" ".*end here.*" +} |