[binutils-gdb] Fix problems with finishing a dummy function call on simulators.

[Luis Machado <luisgpm at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6ae8866180bf90e9ec76c2dd34c07fd826d11a83

commit 6ae8866180bf90e9ec76c2dd34c07fd826d11a83
Author: Luis Machado <lgustavo@codesourcery.com>
Date:   Wed Jun 17 16:50:57 2015 -0300

    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  <lgustavo@codesourcery.com>
    
    	* 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.

Diff:
---
 gdb/ChangeLog    | 12 ++++++++++++
 gdb/breakpoint.c | 44 ++++++++++++++------------------------------
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0e53882..17aa4da 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
 2015-06-17  Luis Machado  <lgustavo@codesourcery.com>
 
+	* 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  <lgustavo@codesourcery.com>
+
 	* breakpoint.c (make_breakpoint_permanent): Remove unused
 	function.
 	* breakpoint.h (make_breakpoint_permanent): Remove declaration.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7f9e161..1481112 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 ())