Handle the case of a remote target supporting target side commands, but not on software breakpoints.

Although we can tell upfront whether a remote target supports target
side commands, we can only tell whether the target supports that in
combination with a given breakpoint kind (software, hardware,
watchpoints, etc.) when we go and try to insert such a breakpoint kind
the first time.  It's not desirable to make remote_insert_breakpoint
simply return -1 in this case, because if the breakpoint was set in a
shared library, insert_bp_location will assume that the breakpoint
insertion failed because the library wasn't mapped in.

insert_bp_location already handles errors/exceptions thrown from the
target_insert_xxx methods, exactly so the backend can tell the user
the detailed reason the insertion of hw breakpoints failed.  But, in
the case of software breakpoints, it discards the detailed error
message.

So the patch makes insert_bp_location use the error's message for SW
breakpoints too, and, introduces a NOT_SUPPORTED_ERROR error code so
that insert_bp_location doesn't confuse the error for failure due to a
shared library disappearing.

The result is:

(gdb) c
Warning:
Cannot insert breakpoint 2: Target doesn't support breakpoints that have target side commands.

2014-01-09  Pedro Alves  <palves@redhat.com>
	    Hui Zhu  <hui@codesourcery.com>

	PR gdb/16101
	* breakpoint.c (insert_bp_location): Rename hw_bp_err_string to
	bp_err_string.  Don't mark the location shlib_disabled if the
	error thrown wasn't a generic or memory error.  Catch errors
	thrown while inserting breakpoints in overlayed code.  Output
	error message of software breakpoints.
	* remote.c (remote_insert_breakpoint): If this breakpoint has
	target-side commands but this stub doesn't support Z0 packets,
	throw NOT_SUPPORTED_ERROR error.
	* exceptions.h (enum errors) <NOT_SUPPORTED_ERROR>: New error.
	* target.h (target_insert_breakpoint): Extend comment.
	(target_insert_hw_breakpoint): Add comment.

1 files changed, 77 insertions, 28 deletions

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 66a69b5..152f638 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2394,8 +2394,8 @@ insert_bp_location (struct bp_location *bl,
 		    int *hw_breakpoint_error,
 		    int *hw_bp_error_explained_already)
 {
-  int val = 0;
-  const char *hw_bp_err_string = NULL;
+  enum errors bp_err = GDB_NO_ERROR;
+  const char *bp_err_message = NULL;
   struct gdb_exception e;
 
   if (!should_be_inserted (bl) || (bl->inserted && !bl->needs_update))
@@ -2495,12 +2495,16 @@ insert_bp_location (struct bp_location *bl,
 	  /* No overlay handling: just set the breakpoint.  */
 	  TRY_CATCH (e, RETURN_MASK_ALL)
 	    {
+	      int val;
+
 	      val = bl->owner->ops->insert_location (bl);
+	      if (val)
+		bp_err = GENERIC_ERROR;
 	    }
 	  if (e.reason < 0)
 	    {
-	      val = 1;
-	      hw_bp_err_string = e.message;
+	      bp_err = e.error;
+	      bp_err_message = e.message;
 	    }
 	}
       else
@@ -2522,9 +2526,24 @@ insert_bp_location (struct bp_location *bl,
 		  /* Set a software (trap) breakpoint at the LMA.  */
 		  bl->overlay_target_info = bl->target_info;
 		  bl->overlay_target_info.placed_address = addr;
-		  val = target_insert_breakpoint (bl->gdbarch,
-						  &bl->overlay_target_info);
-		  if (val != 0)
+
+		  /* No overlay handling: just set the breakpoint.  */
+		  TRY_CATCH (e, RETURN_MASK_ALL)
+		    {
+		      int val;
+
+		      val = target_insert_breakpoint (bl->gdbarch,
+						      &bl->overlay_target_info);
+		      if (val)
+			bp_err = GENERIC_ERROR;
+		    }
+		  if (e.reason < 0)
+		    {
+		      bp_err = e.error;
+		      bp_err_message = e.message;
+		    }
+
+		  if (bp_err != GDB_NO_ERROR)
 		    fprintf_unfiltered (tmp_error_stream,
 					"Overlay breakpoint %d "
 					"failed: in ROM?\n",
@@ -2537,12 +2556,16 @@ insert_bp_location (struct bp_location *bl,
 	      /* Yes.  This overlay section is mapped into memory.  */
 	      TRY_CATCH (e, RETURN_MASK_ALL)
 	        {
+		  int val;
+
 	          val = bl->owner->ops->insert_location (bl);
+		  if (val)
+		    bp_err = GENERIC_ERROR;
 	        }
 	      if (e.reason < 0)
 	        {
-	          val = 1;
-	          hw_bp_err_string = e.message;
+		  bp_err = e.error;
+		  bp_err_message = e.message;
 	        }
 	    }
 	  else
@@ -2553,13 +2576,23 @@ insert_bp_location (struct bp_location *bl,
 	    }
 	}
 
-      if (val)
+      if (bp_err != GDB_NO_ERROR)
 	{
 	  /* Can't set the breakpoint.  */
-	  if (solib_name_from_address (bl->pspace, bl->address))
+
+	  /* In some cases, we might not be able to insert a
+	     breakpoint in a shared library that has already been
+	     removed, but we have not yet processed the shlib unload
+	     event.  Unfortunately, some targets that implement
+	     breakpoint insertion themselves (necessary if this is a
+	     HW breakpoint, but SW breakpoints likewise) can't tell
+	     why the breakpoint insertion failed (e.g., the remote
+	     target doesn't define error codes), so we must treat
+	     generic errors as memory errors.  */
+	  if ((bp_err == GENERIC_ERROR || bp_err == MEMORY_ERROR)
+	      && solib_name_from_address (bl->pspace, bl->address))
 	    {
 	      /* See also: disable_breakpoints_in_shlibs.  */
-	      val = 0;
 	      bl->shlib_disabled = 1;
 	      observer_notify_breakpoint_modified (bl->owner);
 	      if (!*disabled_breaks)
@@ -2574,39 +2607,51 @@ insert_bp_location (struct bp_location *bl,
 	      *disabled_breaks = 1;
 	      fprintf_unfiltered (tmp_error_stream,
 				  "breakpoint #%d\n", bl->owner->number);
+	      return 0;
 	    }
 	  else
 	    {
 	      if (bl->loc_type == bp_loc_hardware_breakpoint)
 		{
-                  *hw_breakpoint_error = 1;
-                  *hw_bp_error_explained_already = hw_bp_err_string != NULL;
+		  *hw_breakpoint_error = 1;
+		  *hw_bp_error_explained_already = bp_err_message != NULL;
                   fprintf_unfiltered (tmp_error_stream,
                                       "Cannot insert hardware breakpoint %d%s",
-                                      bl->owner->number, hw_bp_err_string ? ":" : ".\n");
-                  if (hw_bp_err_string)
-                    fprintf_unfiltered (tmp_error_stream, "%s.\n", hw_bp_err_string);
+                                      bl->owner->number, bp_err_message ? ":" : ".\n");
+                  if (bp_err_message != NULL)
+                    fprintf_unfiltered (tmp_error_stream, "%s.\n", bp_err_message);
 		}
 	      else
 		{
-		  char *message = memory_error_message (TARGET_XFER_E_IO,
-							bl->gdbarch, bl->address);
-		  struct cleanup *old_chain = make_cleanup (xfree, message);
-
-		  fprintf_unfiltered (tmp_error_stream, 
-				      "Cannot insert breakpoint %d.\n"
-				      "%s\n",
-				      bl->owner->number, message);
-
-		  do_cleanups (old_chain);
+		  if (bp_err_message == NULL)
+		    {
+		      char *message
+			= memory_error_message (TARGET_XFER_E_IO,
+						bl->gdbarch, bl->address);
+		      struct cleanup *old_chain = make_cleanup (xfree, message);
+
+		      fprintf_unfiltered (tmp_error_stream,
+					  "Cannot insert breakpoint %d.\n"
+					  "%s\n",
+					  bl->owner->number, message);
+		      do_cleanups (old_chain);
+		    }
+		  else
+		    {
+		      fprintf_unfiltered (tmp_error_stream,
+					  "Cannot insert breakpoint %d: %s\n",
+					  bl->owner->number,
+					  bp_err_message);
+		    }
 		}
+	      return 1;
 
 	    }
 	}
       else
 	bl->inserted = 1;
 
-      return val;
+      return 0;
     }
 
   else if (bl->loc_type == bp_loc_hardware_watchpoint
@@ -2614,6 +2659,8 @@ insert_bp_location (struct bp_location *bl,
 	      watchpoints.  It's not clear that it's necessary...  */
 	   && bl->owner->disposition != disp_del_at_next_stop)
     {
+      int val;
+
       gdb_assert (bl->owner->ops != NULL
 		  && bl->owner->ops->insert_location != NULL);
 
@@ -2657,6 +2704,8 @@ insert_bp_location (struct bp_location *bl,
 
   else if (bl->owner->type == bp_catchpoint)
     {
+      int val;
+
       gdb_assert (bl->owner->ops != NULL
 		  && bl->owner->ops->insert_location != NULL);