[PATCH/DOC,RFA] Fix "breakpoint always-inserted off"; remove "breakpoint always-inserted auto"

Message ID 1410913576-4928-1-git-send-email-palves@redhat.com
State Superseded
Headers

Commit Message

Pedro Alves Sept. 17, 2014, 12:26 a.m. UTC
  By default, GDB removes all breakpoints from the target when the
target stops and the prompt is given back to the user.  This is useful
in case GDB crashes while the user is interacting, as otherwise,
there's a higher chance breakpoints would be left planted on the
target.

But, as long as any thread is running free, we need to make sure to
keep breakpoints inserted, lest a thread misses a breakpoint.  With
that in mind, in preparation for non-stop mode, we added a "breakpoint
always-inserted on" mode.  This traded off the extra crash protection
for never having threads miss breakpoints, and in addition is more
efficient if there's a ton of breakpoints to remove/insert at each
user command (e.g., at each "step").

When we added non-stop mode, and for a period, we required users to
manually set "always-inserted on" when they enabled non-stop mode, as
otherwise GDB removes all breakpoints from the target as soon as any
thread stops, even if other threads are still running.  That soon
revealed a nuisance, and so later we added an extra "breakpoint
always-inserted auto" mode, that made GDB behave like "always-inserted
on" when non-stop was enabled, and "always-inserted off" when non-stop
was disabled.  "auto" was made the default at the same time.

In hindsight, this "auto" setting was unnecessary, and not the ideal
solution.  Non-stop mode does depends on breakpoints always-inserted
mode, but only as long as any thread is running.  If no thread is
running, no breakpoint can be missed.  The same is true for all-stop
too.  E.g., if, in all-stop mode, and the user does:

 (gdb) c&
 (gdb) b foo

That breakpoint at "foo" should be inserted immediately, but it
currently isn't -- currently it'll end up inserted only if the target
happens to trip on some event, and is re-resumed, e.g., an internal
breakpoint triggers that doesn't cause a user-visible stop, and so we
end up in keep_going calling insert_breakpoints.

IOW, no matter whether in non-stop or all-stop, if the target fully
stops, we can remove breakpoints.  And no matter whether in all-stop
or non-stop, if any thread is running in the target, then we need
breakpoints to be immediately inserted.  And then, if the target has
global breakpoints, we need to keep breakpoints even when the target
is stopped.

So with that in mind, and aiming at reducing all-stop vs non-stop
differences for all-stop-on-stop-of-non-stop, this patch fixes
"breakpoint always-inserted off" to not remove breakpoints from the
target until it fully stops, and then removes the "auto" setting as
unnecessary.  I propose removing it straight away rather than keeping
it as an alias, unless someone complains they have scripts that need
it and that can't adjust.

Tested on x86_64 Fedora 20.

gdb/
2014-09-16  Pedro Alves  <palves@redhat.com>

	* NEWS: Mention merge of "breakpoint always-inserted" modes "off"
	and "auto" merged.
	* breakpoint.c (enum ugll_insert_mode): New enum.
	(always_inserted_mode): Now a plain boolean.
	(show_always_inserted_mode): No longer handle AUTO_BOOLEAN_AUTO.
	(breakpoints_always_inserted_mode): Delete.
	(breakpoints_should_be_inserted_now): New function.
	(insert_breakpoints): Pass UGLL_INSERT to
	update_global_location_list instead of calling
	insert_breakpoint_locations manually.
	(create_solib_event_breakpoint_1): New, factored out from ...
	(create_solib_event_breakpoint): ... this.
	(create_and_insert_solib_event_breakpoint): Use
	create_solib_event_breakpoint_1 instead of calling
	insert_breakpoint_locations manually.
	(update_global_location_list): Change parameter type from boolean
	to enum ugll_insert_mode.  All callers adjusted.  Adjust to use
	breakpoints_should_be_inserted_now and handle UGLL_INSERT.
	(update_global_location_list_nothrow): Change parameter type from
	boolean to enum ugll_insert_mode.
	(_initialize_breakpoint): "breakpoint always-inserted" option is
	now a boolean command.  Update help text.
	* breakpoint.h (breakpoints_always_inserted_mode): Delete declaration.
	(breakpoints_should_be_inserted_now): New declaration.
	* infrun.c (handle_inferior_event) <TARGET_WAITKIND_LOADED>:
	Remove breakpoints_always_inserted_mode check.
	(normal_stop): Adjust to use breakpoints_should_be_inserted_now.
	* remote.c (remote_start_remote): Likewise.

gdb/doc/
2014-09-16  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Set Breaks): Document that "set breakpoint
	always-inserted off" is the default mode now.  Delete
	documentation of "set breakpoint always-inserted auto".
---
 gdb/NEWS            |   7 ++
 gdb/breakpoint.c    | 224 ++++++++++++++++++++++++++++++----------------------
 gdb/breakpoint.h    |   2 +-
 gdb/doc/gdb.texinfo |  13 +--
 gdb/infrun.c        |   5 +-
 gdb/remote.c        |   5 +-
 6 files changed, 142 insertions(+), 114 deletions(-)
  

Comments

Eli Zaretskii Sept. 17, 2014, 5:14 a.m. UTC | #1
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 17 Sep 2014 01:26:16 +0100
> 
> So with that in mind, and aiming at reducing all-stop vs non-stop
> differences for all-stop-on-stop-of-non-stop, this patch fixes
> "breakpoint always-inserted off" to not remove breakpoints from the
> target until it fully stops, and then removes the "auto" setting as
> unnecessary.  I propose removing it straight away rather than keeping
> it as an alias, unless someone complains they have scripts that need
> it and that can't adjust.

OK for the documentation parts.

Thanks.
  
Yao Qi Sept. 17, 2014, 1:12 p.m. UTC | #2
Pedro Alves <palves@redhat.com> writes:

> In hindsight, this "auto" setting was unnecessary, and not the ideal
> solution.  Non-stop mode does depends on breakpoints always-inserted
> mode, but only as long as any thread is running.  If no thread is
> running, no breakpoint can be missed.  The same is true for all-stop
> too.  E.g., if, in all-stop mode, and the user does:
>
>  (gdb) c&
>  (gdb) b foo
>
> That breakpoint at "foo" should be inserted immediately, but it
> currently isn't -- currently it'll end up inserted only if the target
> happens to trip on some event, and is re-resumed, e.g., an internal
> breakpoint triggers that doesn't cause a user-visible stop, and so we
> end up in keep_going calling insert_breakpoints.
>
> IOW, no matter whether in non-stop or all-stop, if the target fully
> stops, we can remove breakpoints.  And no matter whether in all-stop
> or non-stop, if any thread is running in the target, then we need
> breakpoints to be immediately inserted.  And then, if the target has

Could we add some test cases for this? for example, breakpoint should be
inserted immediately if any thread is running.

> +/* update_global_location_list's modes of operation wrt to whether to
> +   insert locations now.  */
> +enum ugll_insert_mode
> +{
> +  /* Don't insert any breakpoint locations into the inferior, only
> +     remove already-inserted locations that no longer should be
> +     inserted.  Functions that delete a breakpoint or breakpoints
> +     should specify this mode, so that deleting a breakpoint doesn't
> +     have the side effect of inserting the locations of other
> +     breakpoints that are marked not-inserted, but should_be_inserted
> +     returns true on them.
> +
> +     This behavior is useful is situations close to tear-down -- e.g.,
> +     after an exec, while the target still has execution, but
> +     breakpoint shadows of the previous executable image should *NOT*
> +     be restored to the new image; or before detaching, where the
> +     target still has execution and wants to delete breakpoints from
> +     GDB's lists, and all breakpoints had already been removed from
> +     the inferior.  */
> +  UGLL_DONT_INSERT,
> +
> +  /* May insert breakpoints iff breakpoints_should_be_inserted_now
> +     claims breakpoints should be inserted now.  */
> +  UGLL_MAY_INSERT,
> +
> +  /* Insert locations now, even if breakpoints_should_be_inserted_now
> +     would return false.  For example, no thread is resumed yet, but
> +     we're just about to resume the target.  */
> +  UGLL_INSERT,
> +};
> +

What does the last sentence in comments mean?  "no thread is resumed
yet, but we're just about to resume the target."

The patch looks good to me.  However, IWBN to split it to two, patch 1
is about refactor update_global_location_list and add enum ugll_insert_mode
with only UGLL_DONT_INSERT and UGLL_MAY_INSERT.  patch 2 does the rest.
  
Pedro Alves Sept. 17, 2014, 1:49 p.m. UTC | #3
On 09/17/2014 02:12 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> In hindsight, this "auto" setting was unnecessary, and not the ideal
>> solution.  Non-stop mode does depends on breakpoints always-inserted
>> mode, but only as long as any thread is running.  If no thread is
>> running, no breakpoint can be missed.  The same is true for all-stop
>> too.  E.g., if, in all-stop mode, and the user does:
>>
>>  (gdb) c&
>>  (gdb) b foo
>>
>> That breakpoint at "foo" should be inserted immediately, but it
>> currently isn't -- currently it'll end up inserted only if the target
>> happens to trip on some event, and is re-resumed, e.g., an internal
>> breakpoint triggers that doesn't cause a user-visible stop, and so we
>> end up in keep_going calling insert_breakpoints.
>>
>> IOW, no matter whether in non-stop or all-stop, if the target fully
>> stops, we can remove breakpoints.  And no matter whether in all-stop
>> or non-stop, if any thread is running in the target, then we need
>> breakpoints to be immediately inserted.  And then, if the target has
> 
> Could we add some test cases for this? for example, breakpoint should be
> inserted immediately if any thread is running.

I'll try writing one.  May need to skip all-stop/remote, as we can't
talk to the target if anything is running (blocked waiting for the vCont
reply).

>> +/* update_global_location_list's modes of operation wrt to whether to
>> +   insert locations now.  */
>> +enum ugll_insert_mode
>> +{
>> +  /* Don't insert any breakpoint locations into the inferior, only
>> +     remove already-inserted locations that no longer should be
>> +     inserted.  Functions that delete a breakpoint or breakpoints
>> +     should specify this mode, so that deleting a breakpoint doesn't
>> +     have the side effect of inserting the locations of other
>> +     breakpoints that are marked not-inserted, but should_be_inserted
>> +     returns true on them.
>> +
>> +     This behavior is useful is situations close to tear-down -- e.g.,
>> +     after an exec, while the target still has execution, but
>> +     breakpoint shadows of the previous executable image should *NOT*
>> +     be restored to the new image; or before detaching, where the
>> +     target still has execution and wants to delete breakpoints from
>> +     GDB's lists, and all breakpoints had already been removed from
>> +     the inferior.  */
>> +  UGLL_DONT_INSERT,
>> +
>> +  /* May insert breakpoints iff breakpoints_should_be_inserted_now
>> +     claims breakpoints should be inserted now.  */
>> +  UGLL_MAY_INSERT,
>> +
>> +  /* Insert locations now, even if breakpoints_should_be_inserted_now
>> +     would return false.  For example, no thread is resumed yet, but
>> +     we're just about to resume the target.  */
>> +  UGLL_INSERT,
>> +};
>> +
> 
> What does the last sentence in comments mean?  "no thread is resumed
> yet, but we're just about to resume the target."

E.g., say everything all threads are stopped right now, and the user
does "continue".  We need to insert breakpoints _before_ resuming the
target, but breakpoints_should_be_inserted_now returns false at
that point, as no thread is running.  See where it's used:

@@ -2928,13 +2968,7 @@ insert_breakpoints (void)
 	update_watchpoint (w, 0 /* don't reparse.  */);
       }

-  update_global_location_list (1);
-
-  /* update_global_location_list does not insert breakpoints when
-     always_inserted_mode is not enabled.  Explicitly insert them
-     now.  */
-  if (!breakpoints_always_inserted_mode ())
-    insert_breakpoint_locations ();
+  update_global_location_list (UGLL_INSERT);
 }

I'll try to clarify the comment of UGLL_INSERT, and I'll preserve this
comment in insert_breakpoints somehow instead of removing it completely.

> The patch looks good to me.  However, IWBN to split it to two, patch 1
> is about refactor update_global_location_list and add enum ugll_insert_mode
> with only UGLL_DONT_INSERT and UGLL_MAY_INSERT.  patch 2 does the rest.

I'll do that.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index 343ee49..ac94874 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -31,6 +31,13 @@  queue-signal signal-name-or-number
   confirmation if the program had stopped for a signal and the user
   switched threads meanwhile.
 
+* "breakpoint always-inserted" modes "off" and "auto" merged.
+
+  Now, when 'breakpoint always-inserted mode' is set to "off", GDB
+  won't remove breakpoints from the target until all threads stop,
+  even in non-stop mode.  The "auto" mode has been removed, and "off"
+  is now the default mode.
+
 *** Changes in GDB 7.8
 
 * New command line options
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 94b55c3..6adc12a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -237,9 +237,40 @@  static void decref_bp_location (struct bp_location **loc);
 
 static struct bp_location *allocate_bp_location (struct breakpoint *bpt);
 
-static void update_global_location_list (int);
+/* update_global_location_list's modes of operation wrt to whether to
+   insert locations now.  */
+enum ugll_insert_mode
+{
+  /* Don't insert any breakpoint locations into the inferior, only
+     remove already-inserted locations that no longer should be
+     inserted.  Functions that delete a breakpoint or breakpoints
+     should specify this mode, so that deleting a breakpoint doesn't
+     have the side effect of inserting the locations of other
+     breakpoints that are marked not-inserted, but should_be_inserted
+     returns true on them.
+
+     This behavior is useful is situations close to tear-down -- e.g.,
+     after an exec, while the target still has execution, but
+     breakpoint shadows of the previous executable image should *NOT*
+     be restored to the new image; or before detaching, where the
+     target still has execution and wants to delete breakpoints from
+     GDB's lists, and all breakpoints had already been removed from
+     the inferior.  */
+  UGLL_DONT_INSERT,
+
+  /* May insert breakpoints iff breakpoints_should_be_inserted_now
+     claims breakpoints should be inserted now.  */
+  UGLL_MAY_INSERT,
+
+  /* Insert locations now, even if breakpoints_should_be_inserted_now
+     would return false.  For example, no thread is resumed yet, but
+     we're just about to resume the target.  */
+  UGLL_INSERT,
+};
+
+static void update_global_location_list (enum ugll_insert_mode);
 
-static void update_global_location_list_nothrow (int);
+static void update_global_location_list_nothrow (enum ugll_insert_mode);
 
 static int is_hardware_watchpoint (const struct breakpoint *bpt);
 
@@ -421,34 +452,43 @@  show_automatic_hardware_breakpoints (struct ui_file *file, int from_tty,
 		    value);
 }
 
-/* If on, gdb will keep breakpoints inserted even as inferior is
-   stopped, and immediately insert any new breakpoints.  If off, gdb
-   will insert breakpoints into inferior only when resuming it, and
-   will remove breakpoints upon stop.  If auto, GDB will behave as ON
-   if in non-stop mode, and as OFF if all-stop mode.*/
-
-static enum auto_boolean always_inserted_mode = AUTO_BOOLEAN_AUTO;
+/* If on, GDB keeps breakpoints inserted even if the inferior is
+   stopped, and immediately inserts any new breakpoints as soon as
+   they're created.  If off (default), GDB keeps breakpoints off of
+   the target as long as possible.  That is, it delays inserting
+   breakpoints until the next resume, and removes them again when the
+   target fully stops.  This is a bit safer in case GDB crashes while
+   processing user input.  */
+static int always_inserted_mode = 0;
 
 static void
 show_always_inserted_mode (struct ui_file *file, int from_tty,
 		     struct cmd_list_element *c, const char *value)
 {
-  if (always_inserted_mode == AUTO_BOOLEAN_AUTO)
-    fprintf_filtered (file,
-		      _("Always inserted breakpoint "
-			"mode is %s (currently %s).\n"),
-		      value,
-		      breakpoints_always_inserted_mode () ? "on" : "off");
-  else
-    fprintf_filtered (file, _("Always inserted breakpoint mode is %s.\n"),
-		      value);
+  fprintf_filtered (file, _("Always inserted breakpoint mode is %s.\n"),
+		    value);
 }
 
 int
-breakpoints_always_inserted_mode (void)
+breakpoints_should_be_inserted_now (void)
 {
-  return (always_inserted_mode == AUTO_BOOLEAN_TRUE
-	  || (always_inserted_mode == AUTO_BOOLEAN_AUTO && non_stop));
+  if (gdbarch_has_global_breakpoints (target_gdbarch ()))
+    {
+      /* If breakpoints are global, they should be inserted even if no
+	 thread under gdb's control is running.  */
+      return 1;
+    }
+  else if (target_has_execution)
+    {
+      struct thread_info *tp;
+
+      ALL_NON_EXITED_THREADS (tp)
+        {
+	  if (tp->executing)
+	    return 1;
+	}
+    }
+  return 0;
 }
 
 static const char condition_evaluation_both[] = "host or target";
@@ -835,7 +875,7 @@  set_condition_evaluation_mode (char *args, int from_tty,
 	}
 
       /* Do the update.  */
-      update_global_location_list (1);
+      update_global_location_list (UGLL_MAY_INSERT);
     }
 
   return;
@@ -1063,7 +1103,7 @@  condition_command (char *arg, int from_tty)
 	set_breakpoint_condition (b, p, from_tty);
 
 	if (is_breakpoint (b))
-	  update_global_location_list (1);
+	  update_global_location_list (UGLL_MAY_INSERT);
 
 	return;
       }
@@ -2908,7 +2948,7 @@  breakpoint_program_space_exit (struct program_space *pspace)
 
   /* Now update the global location list to permanently delete the
      removed locations above.  */
-  update_global_location_list (0);
+  update_global_location_list (UGLL_DONT_INSERT);
 }
 
 /* Make sure all breakpoints are inserted in inferior.
@@ -2928,13 +2968,7 @@  insert_breakpoints (void)
 	update_watchpoint (w, 0 /* don't reparse.  */);
       }
 
-  update_global_location_list (1);
-
-  /* update_global_location_list does not insert breakpoints when
-     always_inserted_mode is not enabled.  Explicitly insert them
-     now.  */
-  if (!breakpoints_always_inserted_mode ())
-    insert_breakpoint_locations ();
+  update_global_location_list (UGLL_INSERT);
 }
 
 /* Invoke CALLBACK for each of bp_location.  */
@@ -3391,7 +3425,7 @@  create_overlay_event_breakpoint (void)
          overlay_events_enabled = 0;
        }
     }
-  update_global_location_list (1);
+  update_global_location_list (UGLL_MAY_INSERT);
 }
 
 static void
@@ -3501,7 +3535,7 @@  create_longjmp_master_breakpoint (void)
 	}
     }
   }
-  update_global_location_list (1);
+  update_global_location_list (UGLL_MAY_INSERT);
 
   do_cleanups (old_chain);
 }
@@ -3557,7 +3591,7 @@  create_std_terminate_master_breakpoint (void)
     }
   }
 
-  update_global_location_list (1);
+  update_global_location_list (UGLL_MAY_INSERT);
 
   do_cleanups (old_chain);
 }
@@ -3659,7 +3693,7 @@  create_exception_master_breakpoint (void)
       b->enable_state = bp_disabled;
     }
 
-  update_global_location_list (1);
+  update_global_location_list (UGLL_MAY_INSERT);
 }
 
 void
@@ -5654,9 +5688,9 @@  bpstat_stop_status (struct address_space *aspace,
 	}
 
   if (need_remove_insert)
-    update_global_location_list (1);
+    update_global_location_list (UGLL_MAY_INSERT);
   else if (removed_any)
-    update_global_location_list (0);
+    update_global_location_list (UGLL_DONT_INSERT);
 
   return bs_head;
 }
@@ -7565,7 +7599,7 @@  enable_overlay_breakpoints (void)
     if (b->type == bp_overlay_event)
     {
       b->enable_state = bp_enabled;
-      update_global_location_list (1);
+      update_global_location_list (UGLL_MAY_INSERT);
       overlay_events_enabled = 1;
     }
 }
@@ -7579,7 +7613,7 @@  disable_overlay_breakpoints (void)
     if (b->type == bp_overlay_event)
     {
       b->enable_state = bp_disabled;
-      update_global_location_list (0);
+      update_global_location_list (UGLL_DONT_INSERT);
       overlay_events_enabled = 0;
     }
 }
@@ -7624,7 +7658,7 @@  create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
   b->addr_string
     = xstrprintf ("*%s", paddress (b->loc->gdbarch, b->loc->address));
 
-  update_global_location_list_nothrow (1);
+  update_global_location_list_nothrow (UGLL_MAY_INSERT);
 
   return b;
 }
@@ -7655,7 +7689,7 @@  create_jit_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 
   b = create_internal_breakpoint (gdbarch, address, bp_jit_event,
 				  &internal_breakpoint_ops);
-  update_global_location_list_nothrow (1);
+  update_global_location_list_nothrow (UGLL_MAY_INSERT);
   return b;
 }
 
@@ -7696,17 +7730,28 @@  remove_solib_event_breakpoints_at_next_stop (void)
       b->disposition = disp_del_at_next_stop;
 }
 
-struct breakpoint *
-create_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
+/* Helper for create_solib_event_breakpoint /
+   create_and_insert_solib_event_breakpoint.  Allows specifying which
+   INSERT_MODE to pass through to update_global_location_list.  */
+
+static struct breakpoint *
+create_solib_event_breakpoint_1 (struct gdbarch *gdbarch, CORE_ADDR address,
+				 enum ugll_insert_mode insert_mode)
 {
   struct breakpoint *b;
 
   b = create_internal_breakpoint (gdbarch, address, bp_shlib_event,
 				  &internal_breakpoint_ops);
-  update_global_location_list_nothrow (1);
+  update_global_location_list_nothrow (insert_mode);
   return b;
 }
 
+struct breakpoint *
+create_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
+{
+  return create_solib_event_breakpoint_1 (gdbarch, address, UGLL_MAY_INSERT);
+}
+
 /* See breakpoint.h.  */
 
 struct breakpoint *
@@ -7714,9 +7759,7 @@  create_and_insert_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR add
 {
   struct breakpoint *b;
 
-  b = create_solib_event_breakpoint (gdbarch, address);
-  if (!breakpoints_always_inserted_mode ())
-    insert_breakpoint_locations ();
+  b = create_solib_event_breakpoint_1 (gdbarch, address, UGLL_INSERT);
   if (!b->loc->inserted)
     {
       delete_breakpoint (b);
@@ -8838,7 +8881,7 @@  install_breakpoint (int internal, struct breakpoint *b, int update_gll)
   observer_notify_breakpoint_created (b);
 
   if (update_gll)
-    update_global_location_list (1);
+    update_global_location_list (UGLL_MAY_INSERT);
 }
 
 static void
@@ -9080,7 +9123,7 @@  disable_watchpoints_before_interactive_call_start (void)
     if (is_watchpoint (b) && breakpoint_enabled (b))
       {
 	b->enable_state = bp_call_disabled;
-	update_global_location_list (0);
+	update_global_location_list (UGLL_DONT_INSERT);
       }
   }
 }
@@ -9095,7 +9138,7 @@  enable_watchpoints_after_interactive_call_stop (void)
     if (is_watchpoint (b) && b->enable_state == bp_call_disabled)
       {
 	b->enable_state = bp_enabled;
-	update_global_location_list (1);
+	update_global_location_list (UGLL_MAY_INSERT);
       }
   }
 }
@@ -9104,7 +9147,7 @@  void
 disable_breakpoints_before_startup (void)
 {
   current_program_space->executing_startup = 1;
-  update_global_location_list (0);
+  update_global_location_list (UGLL_DONT_INSERT);
 }
 
 void
@@ -9140,7 +9183,7 @@  set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
   if (in_thread_list (inferior_ptid))
     b->thread = pid_to_thread_id (inferior_ptid);
 
-  update_global_location_list_nothrow (1);
+  update_global_location_list_nothrow (UGLL_MAY_INSERT);
 
   return b;
 }
@@ -9178,7 +9221,7 @@  momentary_breakpoint_from_master (struct breakpoint *orig,
   copy->disposition = disp_donttouch;
   copy->number = internal_breakpoint_number--;
 
-  update_global_location_list_nothrow (0);
+  update_global_location_list_nothrow (UGLL_DONT_INSERT);
   return copy;
 }
 
@@ -10133,7 +10176,7 @@  create_breakpoint (struct gdbarch *gdbarch,
   do_cleanups (old_chain);
 
   /* error call may happen here - have BKPT_CHAIN already discarded.  */
-  update_global_location_list (1);
+  update_global_location_list (UGLL_MAY_INSERT);
 
   return 1;
 }
@@ -10645,7 +10688,7 @@  break_range_command (char *arg, int from_tty)
 
   mention (b);
   observer_notify_breakpoint_created (b);
-  update_global_location_list (1);
+  update_global_location_list (UGLL_MAY_INSERT);
 }
 
 /*  Return non-zero if EXP is verified as constant.  Returned zero
@@ -12548,24 +12591,16 @@  force_breakpoint_reinsertion (struct bp_location *bl)
 	}
     }
 }
+/* Called whether new breakpoints are created, or existing breakpoints
+   deleted, to update the global location list and recompute which
+   locations are duplicate of which.
 
-/* If SHOULD_INSERT is false, do not insert any breakpoint locations
-   into the inferior, only remove already-inserted locations that no
-   longer should be inserted.  Functions that delete a breakpoint or
-   breakpoints should pass false, so that deleting a breakpoint
-   doesn't have the side effect of inserting the locations of other
-   breakpoints that are marked not-inserted, but should_be_inserted
-   returns true on them.
-
-   This behaviour is useful is situations close to tear-down -- e.g.,
-   after an exec, while the target still has execution, but breakpoint
-   shadows of the previous executable image should *NOT* be restored
-   to the new image; or before detaching, where the target still has
-   execution and wants to delete breakpoints from GDB's lists, and all
-   breakpoints had already been removed from the inferior.  */
+   The INSERT_MODE flag determines whether locations may not, may, or
+   should be inserted now.  See 'enum ugll_insert_mode' for more
+   info.  */
 
 static void
-update_global_location_list (int should_insert)
+update_global_location_list (enum ugll_insert_mode insert_mode)
 {
   struct breakpoint *b;
   struct bp_location **locp, *loc;
@@ -12900,23 +12935,23 @@  update_global_location_list (int should_insert)
 			"a permanent breakpoint"));
     }
 
-  if (breakpoints_always_inserted_mode ()
-      && (have_live_inferiors ()
-	  || (gdbarch_has_global_breakpoints (target_gdbarch ()))))
+  if (insert_mode == UGLL_INSERT || breakpoints_should_be_inserted_now ())
     {
-      if (should_insert)
+      if (insert_mode != UGLL_DONT_INSERT)
 	insert_breakpoint_locations ();
       else
 	{
-	  /* Though should_insert is false, we may need to update conditions
-	     on the target's side if it is evaluating such conditions.  We
+	  /* Even though the caller told us to not insert new
+	     locations, we may still need to update conditions on the
+	     target's side of breakpoints that were already inserted
+	     if the target is evaluating breakpoint conditions.  We
 	     only update conditions for locations that are marked
 	     "needs_update".  */
 	  update_inserted_breakpoint_locations ();
 	}
     }
 
-  if (should_insert)
+  if (insert_mode != UGLL_DONT_INSERT)
     download_tracepoint_locations ();
 
   do_cleanups (cleanups);
@@ -12938,12 +12973,12 @@  breakpoint_retire_moribund (void)
 }
 
 static void
-update_global_location_list_nothrow (int inserting)
+update_global_location_list_nothrow (enum ugll_insert_mode insert_mode)
 {
   volatile struct gdb_exception e;
 
   TRY_CATCH (e, RETURN_MASK_ERROR)
-    update_global_location_list (inserting);
+    update_global_location_list (insert_mode);
 }
 
 /* Clear BKP from a BPS.  */
@@ -14096,7 +14131,7 @@  delete_breakpoint (struct breakpoint *bpt)
      itself, since remove_breakpoint looks at location's owner.  It
      might be better design to have location completely
      self-contained, but it's not the case now.  */
-  update_global_location_list (0);
+  update_global_location_list (UGLL_DONT_INSERT);
 
   bpt->ops->dtor (bpt);
   /* On the chance that someone will soon try again to delete this
@@ -14425,7 +14460,7 @@  update_breakpoint_locations (struct breakpoint *b,
       /* Ranged breakpoints have only one start location and one end
 	 location.  */
       b->enable_state = bp_disabled;
-      update_global_location_list (1);
+      update_global_location_list (UGLL_MAY_INSERT);
       printf_unfiltered (_("Could not reset ranged breakpoint %d: "
 			   "multiple locations found\n"),
 			 b->number);
@@ -14528,7 +14563,7 @@  update_breakpoint_locations (struct breakpoint *b,
   if (!locations_are_equal (existing_locations, b->loc))
     observer_notify_breakpoint_modified (b);
 
-  update_global_location_list (1);
+  update_global_location_list (UGLL_MAY_INSERT);
 }
 
 /* Find the SaL locations corresponding to the given ADDR_STRING.
@@ -14986,7 +15021,7 @@  disable_breakpoint (struct breakpoint *bpt)
 	target_disable_tracepoint (location);
     }
 
-  update_global_location_list (0);
+  update_global_location_list (UGLL_DONT_INSERT);
 
   observer_notify_breakpoint_modified (bpt);
 }
@@ -15041,7 +15076,7 @@  disable_command (char *args, int from_tty)
 		      && is_tracepoint (loc->owner))
 		    target_disable_tracepoint (loc);
 		}
-	      update_global_location_list (0);
+	      update_global_location_list (UGLL_DONT_INSERT);
 	    }
 	  else
 	    map_breakpoint_numbers (num, do_map_disable_breakpoint, NULL);
@@ -15111,7 +15146,7 @@  enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
 
   bpt->disposition = disposition;
   bpt->enable_count = count;
-  update_global_location_list (1);
+  update_global_location_list (UGLL_MAY_INSERT);
 
   observer_notify_breakpoint_modified (bpt);
 }
@@ -15175,7 +15210,7 @@  enable_command (char *args, int from_tty)
 		      && is_tracepoint (loc->owner))
 		    target_enable_tracepoint (loc);
 		}
-	      update_global_location_list (1);
+	      update_global_location_list (UGLL_MAY_INSERT);
 	    }
 	  else
 	    map_breakpoint_numbers (num, do_map_enable_breakpoint, NULL);
@@ -16987,18 +17022,15 @@  a warning will be emitted for such breakpoints."),
 			   &breakpoint_set_cmdlist,
 			   &breakpoint_show_cmdlist);
 
-  add_setshow_auto_boolean_cmd ("always-inserted", class_support,
-				&always_inserted_mode, _("\
+  add_setshow_boolean_cmd ("always-inserted", class_support,
+			   &always_inserted_mode, _("\
 Set mode for inserting breakpoints."), _("\
 Show mode for inserting breakpoints."), _("\
-When this mode is off, breakpoints are inserted in inferior when it is\n\
-resumed, and removed when execution stops.  When this mode is on,\n\
-breakpoints are inserted immediately and removed only when the user\n\
-deletes the breakpoint.  When this mode is auto (which is the default),\n\
-the behaviour depends on the non-stop setting (see help set non-stop).\n\
-In this case, if gdb is controlling the inferior in non-stop mode, gdb\n\
-behaves as if always-inserted mode is on; if gdb is controlling the\n\
-inferior in all-stop mode, gdb behaves as if always-inserted mode is off."),
+When this mode is on, breakpoints are inserted immediately as soon as\n\
+they're created, kept inserted even when execution stops, and removed\n\
+only when the user deletes them.  When this mode is off (the default),\n\
+breakpoints are inserted only when execution continues, and removed\n\
+when execution stops."),
 				NULL,
 				&show_always_inserted_mode,
 				&breakpoint_set_cmdlist,
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 00c8802..e191c10 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1491,7 +1491,7 @@  extern void breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
 				    const gdb_byte *writebuf_org,
 				    ULONGEST memaddr, LONGEST len);
 
-extern int breakpoints_always_inserted_mode (void);
+extern int breakpoints_should_be_inserted_now (void);
 
 /* Called each time new event from target is processed.
    Retires previously deleted breakpoint locations that
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1bb1c0c..537fae8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3850,22 +3850,13 @@  This behavior can be controlled with the following commands::
 @item set breakpoint always-inserted off
 All breakpoints, including newly added by the user, are inserted in
 the target only when the target is resumed.  All breakpoints are
-removed from the target when it stops.
+removed from the target when it stops.  This is the default mode.
 
 @item set breakpoint always-inserted on
 Causes all breakpoints to be inserted in the target at all times.  If
 the user adds a new breakpoint, or changes an existing breakpoint, the
 breakpoints in the target are updated immediately.  A breakpoint is
-removed from the target only when breakpoint itself is removed.
-
-@cindex non-stop mode, and @code{breakpoint always-inserted}
-@item set breakpoint always-inserted auto
-This is the default mode.  If @value{GDBN} is controlling the inferior
-in non-stop mode (@pxref{Non-Stop Mode}), gdb behaves as if
-@code{breakpoint always-inserted} mode is on.  If @value{GDBN} is
-controlling the inferior in all-stop mode, @value{GDBN} behaves as if
-@code{breakpoint always-inserted} mode is off.
-@end table
+removed from the target only when breakpoint itself is deleted.
 
 @value{GDBN} handles conditional breakpoints by evaluating these conditions
 when a breakpoint breaks.  If the condition is true, then the process being
diff --git a/gdb/infrun.c b/gdb/infrun.c
index c18267f..3725f2d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3448,8 +3448,7 @@  handle_inferior_event (struct execution_control_state *ecs)
 	{
 	  /* Loading of shared libraries might have changed breakpoint
 	     addresses.  Make sure new breakpoints are inserted.  */
-	  if (stop_soon == NO_STOP_QUIETLY
-	      && !breakpoints_always_inserted_mode ())
+	  if (stop_soon == NO_STOP_QUIETLY)
 	    insert_breakpoints ();
 	  resume (0, GDB_SIGNAL_0);
 	  prepare_to_wait (ecs);
@@ -6110,7 +6109,7 @@  normal_stop (void)
       printf_filtered (_("No unwaited-for children left.\n"));
     }
 
-  if (!breakpoints_always_inserted_mode () && target_has_execution)
+  if (!breakpoints_should_be_inserted_now () && target_has_execution)
     {
       if (remove_breakpoints ())
 	{
diff --git a/gdb/remote.c b/gdb/remote.c
index 357e9f2..41ea012 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3604,9 +3604,8 @@  remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
      up.  */
   rs->starting_up = 0;
 
-  /* If breakpoints are global, insert them now.  */
-  if (gdbarch_has_global_breakpoints (target_gdbarch ())
-      && breakpoints_always_inserted_mode ())
+  /* Maybe breakpoints are global and need to be inserted now.  */
+  if (breakpoints_should_be_inserted_now ())
     insert_breakpoints ();
 }