From 6ae8866180bf90e9ec76c2dd34c07fd826d11a83 Mon Sep 17 00:00:00 2001 From: Luis Machado Date: Wed, 17 Jun 2015 16:50:57 -0300 Subject: [PATCH] Fix problems with finishing a dummy function call on simulators. Some simulators don't handle permanent breakpoints properly and will sometimes terminate when hitting such a breakpoint instruction or have unwanted effects. When a permanent breakpoint is inserted, GDB will not attempt to insert other breakpoint locations on top of it, leading to the problem described above. By not marking permanent breakpoint locations as inserted, we allow the insertion of breakpoint locations on top of the permanent ones, preventing the simulators from running into that situation. gdb/ChangeLog: 2015-06-17 Luis Machado * breakpoint.c (add_location_to_breakpoint): Don't mark permanent locations as inserted. Update and expand comment about permanent locations. (bp_loc_is_permanent): Don't return 0 for bp_call_dummy. Move comment to add_location_to_breakpoint. (update_global_location_list): Don't error out if a permanent breakpoint is not marked inserted. Don't error out if a non-permanent breakpoint location is inserted on top of a permanent breakpoint. --- gdb/ChangeLog | 12 ++++++++++++ gdb/breakpoint.c | 44 ++++++++++++++------------------------------ 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0e53882cc6b..17aa4dabf11 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2015-06-17 Luis Machado + + * breakpoint.c (add_location_to_breakpoint): Don't mark permanent + locations as inserted. + Update and expand comment about permanent locations. + (bp_loc_is_permanent): Don't return 0 for bp_call_dummy. + Move comment to add_location_to_breakpoint. + (update_global_location_list): Don't error out if a permanent + breakpoint is not marked inserted. + Don't error out if a non-permanent breakpoint location is inserted on + top of a permanent breakpoint. + 2015-06-17 Luis Machado * breakpoint.c (make_breakpoint_permanent): Remove unused diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 7f9e161b571..1481112ea5c 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8898,11 +8898,21 @@ add_location_to_breakpoint (struct breakpoint *b, set_breakpoint_location_function (loc, sal->explicit_pc || sal->explicit_line); + /* While by definition, permanent breakpoints are already present in the + code, we don't mark the location as inserted. Normally one would expect + that GDB could rely on that breakpoint instruction to stop the program, + thus removing the need to insert its own breakpoint, except that executing + the breakpoint instruction can kill the target instead of reporting a + SIGTRAP. E.g., on SPARC, when interrupts are disabled, executing the + instruction resets the CPU, so QEMU 2.0.0 for SPARC correspondingly dies + with "Trap 0x02 while interrupts disabled, Error state". Letting the + breakpoint be inserted normally results in QEMU knowing about the GDB + breakpoint, and thus trap before the breakpoint instruction is executed. + (If GDB later needs to continue execution past the permanent breakpoint, + it manually increments the PC, thus avoiding executing the breakpoint + instruction.) */ if (bp_loc_is_permanent (loc)) - { - loc->inserted = 1; - loc->permanent = 1; - } + loc->permanent = 1; return loc; } @@ -8954,20 +8964,6 @@ bp_loc_is_permanent (struct bp_location *loc) gdb_assert (loc != NULL); - /* bp_call_dummy breakpoint locations are usually memory locations - where GDB just wrote a breakpoint instruction, making it look - as if there is a permanent breakpoint at that location. Considering - it permanent makes GDB rely on that breakpoint instruction to stop - the program, thus removing the need to insert its own breakpoint - there. This is normally expected to work, except that some versions - of QEMU (Eg: QEMU 2.0.0 for SPARC) just report a fatal problem (Trap - 0x02 while interrupts disabled, Error state) instead of reporting - a SIGTRAP. QEMU should probably be fixed, but in the interest of - compatibility with versions that behave this way, we always consider - bp_call_dummy breakpoint locations as non-permanent. */ - if (loc->owner->type == bp_call_dummy) - return 0; - cleanup = save_current_space_and_thread (); switch_to_program_space_and_thread (loc->pspace); @@ -12419,12 +12415,6 @@ update_global_location_list (enum ugll_insert_mode insert_mode) continue; } - /* Permanent breakpoint should always be inserted. */ - if (loc->permanent && ! loc->inserted) - internal_error (__FILE__, __LINE__, - _("allegedly permanent breakpoint is not " - "actually inserted")); - if (b->type == bp_hardware_watchpoint) loc_first_p = &wp_loc_first; else if (b->type == bp_read_watchpoint) @@ -12460,12 +12450,6 @@ update_global_location_list (enum ugll_insert_mode insert_mode) /* Clear the condition modification flag. */ loc->condition_changed = condition_unchanged; - - if (loc->inserted && !loc->permanent - && (*loc_first_p)->permanent) - internal_error (__FILE__, __LINE__, - _("another breakpoint was inserted on top of " - "a permanent breakpoint")); } if (insert_mode == UGLL_INSERT || breakpoints_should_be_inserted_now ()) -- 2.30.2