gdb/testsuite: gdb.base/args.exp: add KFAIL for native-extended-gdbserver
[binutils-gdb.git] / gdb / breakpoint.c
index f5f80794c0554974df8b56dc55b17782e64627df..fb011fc1e0fc6839e8467626b5e2a8fcebc099a9 100644 (file)
@@ -223,8 +223,6 @@ static void set_tracepoint_count (int num);
 
 static bool is_masked_watchpoint (const struct breakpoint *b);
 
-static struct bp_location **get_first_locp_gte_addr (CORE_ADDR address);
-
 /* Return 1 if B refers to a static tracepoint set by marker ("-m"), zero
    otherwise.  */
 
@@ -491,54 +489,21 @@ bool target_exact_watchpoints = false;
             B ? (TMP=B->next, 1): 0;   \
             B = TMP)
 
-/* Similar iterator for the low-level breakpoints.  SAFE variant is
-   not provided so update_global_location_list must not be called
-   while executing the block of ALL_BP_LOCATIONS.  */
-
-#define ALL_BP_LOCATIONS(B,BP_TMP)                                     \
-       for (BP_TMP = bp_locations.data ();                             \
-            BP_TMP < bp_locations.data () + bp_locations.size () && (B = *BP_TMP);\
-            BP_TMP++)
-
-/* Iterates through locations with address ADDRESS for the currently selected
-   program space.  BP_LOCP_TMP points to each object.  BP_LOCP_START points
-   to where the loop should start from.
-   If BP_LOCP_START is a NULL pointer, the macro automatically seeks the
-   appropriate location to start with.  */
-
-#define ALL_BP_LOCATIONS_AT_ADDR(BP_LOCP_TMP, BP_LOCP_START, ADDRESS)  \
-       for (BP_LOCP_START = BP_LOCP_START == NULL ? get_first_locp_gte_addr (ADDRESS) : BP_LOCP_START, \
-            BP_LOCP_TMP = BP_LOCP_START;                               \
-            BP_LOCP_START                                              \
-            && (BP_LOCP_TMP < bp_locations.data () + bp_locations.size () \
-            && (*BP_LOCP_TMP)->address == ADDRESS);                    \
-            BP_LOCP_TMP++)
-
 /* Chains of all breakpoints defined.  */
 
 static struct breakpoint *breakpoint_chain;
 
-/* Breakpoint linked list range.  */
-
-using breakpoint_range = next_adapter<breakpoint, breakpoint_iterator>;
-
-/* Return a range to iterate over all breakpoints.  */
+/* See breakpoint.h.  */
 
-static breakpoint_range
+breakpoint_range
 all_breakpoints ()
 {
   return breakpoint_range (breakpoint_chain);
 }
 
-/* Breakpoint linked list range, safe against deletion of the current
-   breakpoint while iterating.  */
-
-using breakpoint_safe_range = basic_safe_range<breakpoint_range>;
-
-/* Return a range to iterate over all breakpoints.  This range is safe against
-   deletion of the current breakpoint while iterating.  */
+/* See breakpoint.h.  */
 
-static breakpoint_safe_range
+breakpoint_safe_range
 all_breakpoints_safe ()
 {
   return breakpoint_safe_range (all_breakpoints ());
@@ -556,6 +521,73 @@ all_tracepoints ()
 
 static std::vector<bp_location *> bp_locations;
 
+/* See breakpoint.h.  */
+
+const std::vector<bp_location *> &
+all_bp_locations ()
+{
+  return bp_locations;
+}
+
+/* Range to iterate over breakpoint locations at a given address.  */
+
+struct bp_locations_at_addr_range
+{
+  using iterator = std::vector<bp_location *>::iterator;
+
+  bp_locations_at_addr_range (CORE_ADDR addr)
+  {
+    struct compare
+    {
+      bool operator() (const bp_location *loc, CORE_ADDR addr_) const
+      { return loc->address < addr_; }
+
+      bool operator() (CORE_ADDR addr_, const bp_location *loc) const
+      { return addr_ < loc->address; }
+    };
+
+    auto it_pair = std::equal_range (bp_locations.begin (), bp_locations.end (),
+                                    addr, compare ());
+
+    m_begin = it_pair.first;
+    m_end = it_pair.second;
+  }
+
+  iterator begin () const
+  { return m_begin; }
+
+  iterator end () const
+  { return m_end; }
+
+private:
+  iterator m_begin;
+  iterator m_end;
+};
+
+/* Return a range to iterate over all breakpoint locations exactly at address
+   ADDR.
+
+   If it's needed to iterate multiple times on the same range, it's possible
+   to save the range in a local variable and use it multiple times:
+
+     auto range = all_bp_locations_at_addr (addr);
+
+     for (bp_location *loc : range)
+       // use loc
+
+     for (bp_location *loc : range)
+       // use loc
+
+   This saves a bit of time, as it avoids re-doing the binary searches to find
+   the range's boundaries.  Just remember not to change the bp_locations vector
+   in the mean time, as it could make the range's iterators stale.  */
+
+static bp_locations_at_addr_range
+all_bp_locations_at_addr (CORE_ADDR addr)
+{
+  return bp_locations_at_addr_range (addr);
+}
+
 /* Maximum alignment offset between bp_target_info.PLACED_ADDRESS and
    ADDRESS for the current elements of BP_LOCATIONS which get a valid
    result from bp_location_has_shadow.  You can use it for roughly
@@ -740,7 +772,6 @@ set_condition_evaluation_mode (const char *args, int from_tty,
   /* Only update the mode if the user picked a different one.  */
   if (new_mode != old_mode)
     {
-      struct bp_location *loc, **loc_tmp;
       /* If the user switched to a different evaluation mode, we
         need to synch the changes with the target as follows:
 
@@ -752,15 +783,15 @@ set_condition_evaluation_mode (const char *args, int from_tty,
        {
          /* Mark everything modified and synch conditions with the
             target.  */
-         ALL_BP_LOCATIONS (loc, loc_tmp)
+         for (bp_location *loc : all_bp_locations ())
            mark_breakpoint_location_modified (loc);
-       }
+       }
       else
        {
          /* Manually mark non-duplicate locations to synch conditions
             with the target.  We do this to remove all the conditions the
             target knows about.  */
-         ALL_BP_LOCATIONS (loc, loc_tmp)
+         for (bp_location *loc : all_bp_locations ())
            if (is_breakpoint (loc->owner) && loc->inserted)
              loc->needs_update = 1;
        }
@@ -790,56 +821,6 @@ show_condition_evaluation_mode (struct ui_file *file, int from_tty,
                      value);
 }
 
-/* A comparison function for bp_location AP and BP that is used by
-   bsearch.  This comparison function only cares about addresses, unlike
-   the more general bp_location_is_less_than function.  */
-
-static int
-bp_locations_compare_addrs (const void *ap, const void *bp)
-{
-  const struct bp_location *a = *(const struct bp_location **) ap;
-  const struct bp_location *b = *(const struct bp_location **) bp;
-
-  if (a->address == b->address)
-    return 0;
-  else
-    return ((a->address > b->address) - (a->address < b->address));
-}
-
-/* Helper function to skip all bp_locations with addresses
-   less than ADDRESS.  It returns the first bp_location that
-   is greater than or equal to ADDRESS.  If none is found, just
-   return NULL.  */
-
-static struct bp_location **
-get_first_locp_gte_addr (CORE_ADDR address)
-{
-  struct bp_location dummy_loc;
-  struct bp_location *dummy_locp = &dummy_loc;
-  struct bp_location **locp_found = NULL;
-
-  /* Initialize the dummy location's address field.  */
-  dummy_loc.address = address;
-
-  /* Find a close match to the first location at ADDRESS.  */
-  locp_found = ((struct bp_location **)
-               bsearch (&dummy_locp, bp_locations.data (), bp_locations.size (),
-                        sizeof (struct bp_location **),
-                        bp_locations_compare_addrs));
-
-  /* Nothing was found, nothing left to do.  */
-  if (locp_found == NULL)
-    return NULL;
-
-  /* We may have found a location that is at ADDRESS but is not the first in the
-     location's list.  Go backwards (if possible) and locate the first one.  */
-  while ((locp_found - 1) >= bp_locations.data ()
-        && (*(locp_found - 1))->address == address)
-    locp_found--;
-
-  return locp_found;
-}
-
 /* Parse COND_STRING in the context of LOC and set as the condition
    expression of LOC.  BP_NUM is the number of LOC's owner, LOC_NUM is
    the number of LOC within its owner.  In case of parsing error, mark
@@ -1624,28 +1605,29 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
   /* Now do full processing of the found relevant range of elements.  */
 
   for (bc = bc_l; bc < bp_locations.size (); bc++)
-  {
-    struct bp_location *bl = bp_locations[bc];
+    {
+      struct bp_location *bl = bp_locations[bc];
 
-    /* bp_location array has BL->OWNER always non-NULL.  */
-    if (bl->owner->type == bp_none)
-      warning (_("reading through apparently deleted breakpoint #%d?"),
-              bl->owner->number);
+      /* bp_location array has BL->OWNER always non-NULL.  */
+      if (bl->owner->type == bp_none)
+       warning (_("reading through apparently deleted breakpoint #%d?"),
+                bl->owner->number);
 
-    /* Performance optimization: any further element can no longer affect BUF
-       content.  */
+      /* Performance optimization: any further element can no longer affect BUF
+        content.  */
 
-    if (bl->address >= bp_locations_placed_address_before_address_max
-       && memaddr + len <= (bl->address
-                            - bp_locations_placed_address_before_address_max))
-      break;
+      if (bl->address >= bp_locations_placed_address_before_address_max
+         && (memaddr + len
+             <= (bl->address
+                 - bp_locations_placed_address_before_address_max)))
+       break;
 
-    if (!bp_location_has_shadow (bl))
-      continue;
+      if (!bp_location_has_shadow (bl))
+       continue;
 
-    one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org,
-                               memaddr, len, &bl->target_info, bl->gdbarch);
-  }
+      one_breakpoint_xfer_memory (readbuf, writebuf, writebuf_org,
+                                 memaddr, len, &bl->target_info, bl->gdbarch);
+    }
 }
 
 /* See breakpoint.h.  */
@@ -2252,10 +2234,8 @@ parse_cond_to_aexpr (CORE_ADDR scope, struct expression *cond)
 static void
 build_target_condition_list (struct bp_location *bl)
 {
-  struct bp_location **locp = NULL, **loc2p;
   int null_condition_or_parse_error = 0;
   int modified = bl->needs_update;
-  struct bp_location *loc;
 
   /* Release conditions left over from a previous insert.  */
   bl->target_info.conditions.clear ();
@@ -2268,6 +2248,8 @@ build_target_condition_list (struct bp_location *bl)
       || !target_supports_evaluation_of_breakpoint_conditions ())
     return;
 
+  auto loc_range = all_bp_locations_at_addr (bl->address);
+
   /* Do a first pass to check for locations with no assigned
      conditions or conditions that fail to parse to a valid agent
      expression bytecode.  If any of these happen, then it's no use to
@@ -2277,9 +2259,8 @@ build_target_condition_list (struct bp_location *bl)
      even if the locations aren't considered duplicates (e.g.,
      software breakpoint and hardware breakpoint at the same
      address).  */
-  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
+  for (bp_location *loc : loc_range)
     {
-      loc = (*loc2p);
       if (is_breakpoint (loc->owner) && loc->pspace->num == bl->pspace->num)
        {
          if (modified)
@@ -2310,9 +2291,8 @@ build_target_condition_list (struct bp_location *bl)
      being evaluated by GDB or the remote stub.  */
   if (null_condition_or_parse_error)
     {
-      ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
+      for (bp_location *loc : loc_range)
        {
-         loc = (*loc2p);
          if (is_breakpoint (loc->owner) && loc->pspace->num == bl->pspace->num)
            {
              /* Only go as far as the first NULL bytecode is
@@ -2331,21 +2311,18 @@ build_target_condition_list (struct bp_location *bl)
      considered duplicates, but we still marge all the conditions
      anyway, as it's simpler, and doesn't really make a practical
      difference.  */
-  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
-    {
-      loc = (*loc2p);
-      if (loc->cond
-         && is_breakpoint (loc->owner)
-         && loc->pspace->num == bl->pspace->num
-         && loc->owner->enable_state == bp_enabled
-         && loc->enabled
-         && !loc->disabled_by_cond)
-       {
-         /* Add the condition to the vector.  This will be used later
-            to send the conditions to the target.  */
-         bl->target_info.conditions.push_back (loc->cond_bytecode.get ());
-       }
-    }
+  for (bp_location *loc : loc_range)
+    if (loc->cond
+       && is_breakpoint (loc->owner)
+       && loc->pspace->num == bl->pspace->num
+       && loc->owner->enable_state == bp_enabled
+       && loc->enabled
+       && !loc->disabled_by_cond)
+      {
+       /* Add the condition to the vector.  This will be used later
+          to send the conditions to the target.  */
+       bl->target_info.conditions.push_back (loc->cond_bytecode.get ());
+      }
 
   return;
 }
@@ -2434,10 +2411,8 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
 static void
 build_target_command_list (struct bp_location *bl)
 {
-  struct bp_location **locp = NULL, **loc2p;
   int null_command_or_parse_error = 0;
   int modified = bl->needs_update;
-  struct bp_location *loc;
 
   /* Clear commands left over from a previous insert.  */
   bl->target_info.tcommands.clear ();
@@ -2449,27 +2424,25 @@ build_target_command_list (struct bp_location *bl)
   if (dprintf_style != dprintf_style_agent)
     return;
 
+  auto loc_range = all_bp_locations_at_addr (bl->address);
+
   /* For now, if we have any location at the same address that isn't a
      dprintf, don't install the target-side commands, as that would
      make the breakpoint not be reported to the core, and we'd lose
      control.  */
-  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
-    {
-      loc = (*loc2p);
-      if (is_breakpoint (loc->owner)
-         && loc->pspace->num == bl->pspace->num
-         && loc->owner->type != bp_dprintf)
-       return;
-    }
+  for (bp_location *loc : loc_range)
+    if (is_breakpoint (loc->owner)
+       && loc->pspace->num == bl->pspace->num
+       && loc->owner->type != bp_dprintf)
+      return;
 
   /* Do a first pass to check for locations with no assigned
      conditions or conditions that fail to parse to a valid agent expression
      bytecode.  If any of these happen, then it's no use to send conditions
      to the target since this location will always trigger and generate a
      response back to GDB.  */
-  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
+  for (bp_location *loc : loc_range)
     {
-      loc = (*loc2p);
       if (is_breakpoint (loc->owner) && loc->pspace->num == bl->pspace->num)
        {
          if (modified)
@@ -2497,20 +2470,17 @@ build_target_command_list (struct bp_location *bl)
      and so clean up.  */
   if (null_command_or_parse_error)
     {
-      ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
-       {
-         loc = (*loc2p);
-         if (is_breakpoint (loc->owner)
-             && loc->pspace->num == bl->pspace->num)
-           {
-             /* Only go as far as the first NULL bytecode is
-                located.  */
-             if (loc->cmd_bytecode == NULL)
-               return;
+      for (bp_location *loc : loc_range)
+       if (is_breakpoint (loc->owner)
+           && loc->pspace->num == bl->pspace->num)
+         {
+           /* Only go as far as the first NULL bytecode is
+              located.  */
+           if (loc->cmd_bytecode == NULL)
+             return;
 
-             loc->cmd_bytecode.reset ();
-           }
-       }
+           loc->cmd_bytecode.reset ();
+         }
     }
 
   /* No NULL commands or failed bytecode generation.  Build a command
@@ -2521,22 +2491,19 @@ build_target_command_list (struct bp_location *bl)
      could end up running the commands twice.  For the moment, we only
      support targets-side commands with dprintf, but it doesn't hurt
      to be pedantically correct in case that changes.  */
-  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
-    {
-      loc = (*loc2p);
-      if (breakpoint_locations_match (bl, loc)
-         && loc->owner->extra_string
-         && is_breakpoint (loc->owner)
-         && loc->pspace->num == bl->pspace->num
-         && loc->owner->enable_state == bp_enabled
-         && loc->enabled
-         && !loc->disabled_by_cond)
-       {
-         /* Add the command to the vector.  This will be used later
-            to send the commands to the target.  */
-         bl->target_info.tcommands.push_back (loc->cmd_bytecode.get ());
-       }
-    }
+  for (bp_location *loc : loc_range)
+    if (breakpoint_locations_match (bl, loc)
+       && loc->owner->extra_string
+       && is_breakpoint (loc->owner)
+       && loc->pspace->num == bl->pspace->num
+       && loc->owner->enable_state == bp_enabled
+       && loc->enabled
+       && !loc->disabled_by_cond)
+      {
+       /* Add the command to the vector.  This will be used later
+          to send the commands to the target.  */
+       bl->target_info.tcommands.push_back (loc->cmd_bytecode.get ());
+      }
 
   bl->target_info.persist = 0;
   /* Maybe flag this location as persistent.  */
@@ -2825,12 +2792,10 @@ insert_bp_location (struct bp_location *bl,
         supported, try emulating one with an access watchpoint.  */
       if (val == 1 && bl->watchpoint_type == hw_read)
        {
-         struct bp_location *loc, **loc_temp;
-
          /* But don't try to insert it, if there's already another
             hw_access location that would be considered a duplicate
             of this one.  */
-         ALL_BP_LOCATIONS (loc, loc_temp)
+         for (bp_location *loc : all_bp_locations ())
            if (loc != bl
                && loc->watchpoint_type == hw_access
                && watchpoint_locations_match (bl, loc))
@@ -2895,8 +2860,6 @@ of catchpoint."), bl->owner->number);
 void
 breakpoint_program_space_exit (struct program_space *pspace)
 {
-  struct bp_location *loc, **loc_temp;
-
   /* Remove any breakpoint that was set through this program space.  */
   for (breakpoint *b : all_breakpoints_safe ())
     if (b->pspace == pspace)
@@ -2904,7 +2867,7 @@ breakpoint_program_space_exit (struct program_space *pspace)
 
   /* Breakpoints set through other program spaces could have locations
      bound to PSPACE as well.  Remove those.  */
-  ALL_BP_LOCATIONS (loc, loc_temp)
+  for (bp_location *loc : all_bp_locations ())
     {
       struct bp_location *tmp;
 
@@ -2953,19 +2916,6 @@ insert_breakpoints (void)
   update_global_location_list (UGLL_INSERT);
 }
 
-/* Invoke CALLBACK for each of bp_location.  */
-
-void
-iterate_over_bp_locations (gdb::function_view<void (bp_location *)> callback)
-{
-  struct bp_location *loc, **loc_tmp;
-
-  ALL_BP_LOCATIONS (loc, loc_tmp)
-    {
-      callback (loc);
-    }
-}
-
 /* This is used when we need to synch breakpoint conditions between GDB and the
    target.  It is the case with deleting and disabling of breakpoints when using
    always-inserted mode.  */
@@ -2973,7 +2923,6 @@ iterate_over_bp_locations (gdb::function_view<void (bp_location *)> callback)
 static void
 update_inserted_breakpoint_locations (void)
 {
-  struct bp_location *bl, **blp_tmp;
   int error_flag = 0;
   int val = 0;
   int disabled_breaks = 0;
@@ -2988,7 +2937,7 @@ update_inserted_breakpoint_locations (void)
 
   scoped_restore_current_pspace_and_thread restore_pspace_thread;
 
-  ALL_BP_LOCATIONS (bl, blp_tmp)
+  for (bp_location *bl : all_bp_locations ())
     {
       /* We only want to update software breakpoints and hardware
         breakpoints.  */
@@ -3029,7 +2978,6 @@ update_inserted_breakpoint_locations (void)
 static void
 insert_breakpoint_locations (void)
 {
-  struct bp_location *bl, **blp_tmp;
   int error_flag = 0;
   int val = 0;
   int disabled_breaks = 0;
@@ -3044,7 +2992,7 @@ insert_breakpoint_locations (void)
 
   scoped_restore_current_pspace_and_thread restore_pspace_thread;
 
-  ALL_BP_LOCATIONS (bl, blp_tmp)
+  for (bp_location *bl : all_bp_locations ())
     {
       if (!should_be_inserted (bl) || (bl->inserted && !bl->needs_update))
        continue;
@@ -3129,14 +3077,12 @@ You may have requested too many hardware breakpoints/watchpoints.\n");
 int
 remove_breakpoints (void)
 {
-  struct bp_location *bl, **blp_tmp;
   int val = 0;
 
-  ALL_BP_LOCATIONS (bl, blp_tmp)
-  {
+  for (bp_location *bl : all_bp_locations ())
     if (bl->inserted && !is_tracepoint (bl->owner))
       val |= remove_breakpoint (bl);
-  }
+
   return val;
 }
 
@@ -3167,21 +3113,20 @@ Thread-specific breakpoint %d deleted - thread %s no longer in the thread list.\
 void
 remove_breakpoints_inf (inferior *inf)
 {
-  struct bp_location *bl, **blp_tmp;
   int val;
 
-  ALL_BP_LOCATIONS (bl, blp_tmp)
-  {
-    if (bl->pspace != inf->pspace)
-      continue;
+  for (bp_location *bl : all_bp_locations ())
+    {
+      if (bl->pspace != inf->pspace)
+       continue;
 
-    if (bl->inserted && !bl->target_info.persist)
-      {
-       val = remove_breakpoint (bl);
-       if (val != 0)
-         return;
-      }
-  }
+      if (bl->inserted && !bl->target_info.persist)
+       {
+         val = remove_breakpoint (bl);
+         if (val != 0)
+           return;
+       }
+    }
 }
 
 static int internal_breakpoint_number = -1;
@@ -3654,8 +3599,6 @@ breakpoint_event_location_empty_p (const struct breakpoint *b)
 void
 update_breakpoints_after_exec (void)
 {
-  struct bp_location *bploc, **bplocp_tmp;
-
   /* We're about to delete breakpoints from GDB's lists.  If the
      INSERTED flag is true, GDB will try to lift the breakpoints by
      writing the breakpoints' "shadow contents" back into memory.  The
@@ -3664,118 +3607,117 @@ update_breakpoints_after_exec (void)
      breakpoints out as soon as it detects an exec.  We don't do that
      here instead, because there may be other attempts to delete
      breakpoints after detecting an exec and before reaching here.  */
-  ALL_BP_LOCATIONS (bploc, bplocp_tmp)
+  for (bp_location *bploc : all_bp_locations ())
     if (bploc->pspace == current_program_space)
       gdb_assert (!bploc->inserted);
 
   for (breakpoint *b : all_breakpoints_safe ())
-  {
-    if (b->pspace != current_program_space)
-      continue;
-
-    /* Solib breakpoints must be explicitly reset after an exec().  */
-    if (b->type == bp_shlib_event)
-      {
-       delete_breakpoint (b);
+    {
+      if (b->pspace != current_program_space)
        continue;
-      }
 
-    /* JIT breakpoints must be explicitly reset after an exec().  */
-    if (b->type == bp_jit_event)
-      {
-       delete_breakpoint (b);
-       continue;
-      }
+      /* Solib breakpoints must be explicitly reset after an exec().  */
+      if (b->type == bp_shlib_event)
+       {
+         delete_breakpoint (b);
+         continue;
+       }
 
-    /* Thread event breakpoints must be set anew after an exec(),
-       as must overlay event and longjmp master breakpoints.  */
-    if (b->type == bp_thread_event || b->type == bp_overlay_event
-       || b->type == bp_longjmp_master || b->type == bp_std_terminate_master
-       || b->type == bp_exception_master)
-      {
-       delete_breakpoint (b);
-       continue;
-      }
+      /* JIT breakpoints must be explicitly reset after an exec().  */
+      if (b->type == bp_jit_event)
+       {
+         delete_breakpoint (b);
+         continue;
+       }
 
-    /* Step-resume breakpoints are meaningless after an exec().  */
-    if (b->type == bp_step_resume || b->type == bp_hp_step_resume)
-      {
-       delete_breakpoint (b);
-       continue;
-      }
+      /* Thread event breakpoints must be set anew after an exec(),
+        as must overlay event and longjmp master breakpoints.  */
+      if (b->type == bp_thread_event || b->type == bp_overlay_event
+         || b->type == bp_longjmp_master || b->type == bp_std_terminate_master
+         || b->type == bp_exception_master)
+       {
+         delete_breakpoint (b);
+         continue;
+       }
 
-    /* Just like single-step breakpoints.  */
-    if (b->type == bp_single_step)
-      {
-       delete_breakpoint (b);
-       continue;
-      }
+      /* Step-resume breakpoints are meaningless after an exec().  */
+      if (b->type == bp_step_resume || b->type == bp_hp_step_resume)
+       {
+         delete_breakpoint (b);
+         continue;
+       }
 
-    /* Longjmp and longjmp-resume breakpoints are also meaningless
-       after an exec.  */
-    if (b->type == bp_longjmp || b->type == bp_longjmp_resume
-       || b->type == bp_longjmp_call_dummy
-       || b->type == bp_exception || b->type == bp_exception_resume)
-      {
-       delete_breakpoint (b);
-       continue;
-      }
+      /* Just like single-step breakpoints.  */
+      if (b->type == bp_single_step)
+       {
+         delete_breakpoint (b);
+         continue;
+       }
 
-    if (b->type == bp_catchpoint)
-      {
-       /* For now, none of the bp_catchpoint breakpoints need to
-          do anything at this point.  In the future, if some of
-          the catchpoints need to something, we will need to add
-          a new method, and call this method from here.  */
-       continue;
-      }
+      /* Longjmp and longjmp-resume breakpoints are also meaningless
+        after an exec.  */
+      if (b->type == bp_longjmp || b->type == bp_longjmp_resume
+         || b->type == bp_longjmp_call_dummy
+         || b->type == bp_exception || b->type == bp_exception_resume)
+       {
+         delete_breakpoint (b);
+         continue;
+       }
 
-    /* bp_finish is a special case.  The only way we ought to be able
-       to see one of these when an exec() has happened, is if the user
-       caught a vfork, and then said "finish".  Ordinarily a finish just
-       carries them to the call-site of the current callee, by setting
-       a temporary bp there and resuming.  But in this case, the finish
-       will carry them entirely through the vfork & exec.
-
-       We don't want to allow a bp_finish to remain inserted now.  But
-       we can't safely delete it, 'cause finish_command has a handle to
-       the bp on a bpstat, and will later want to delete it.  There's a
-       chance (and I've seen it happen) that if we delete the bp_finish
-       here, that its storage will get reused by the time finish_command
-       gets 'round to deleting the "use to be a bp_finish" breakpoint.
-       We really must allow finish_command to delete a bp_finish.
-
-       In the absence of a general solution for the "how do we know
-       it's safe to delete something others may have handles to?"
-       problem, what we'll do here is just uninsert the bp_finish, and
-       let finish_command delete it.
-
-       (We know the bp_finish is "doomed" in the sense that it's
-       momentary, and will be deleted as soon as finish_command sees
-       the inferior stopped.  So it doesn't matter that the bp's
-       address is probably bogus in the new a.out, unlike e.g., the
-       solib breakpoints.)  */
-
-    if (b->type == bp_finish)
-      {
-       continue;
-      }
+      if (b->type == bp_catchpoint)
+       {
+         /* For now, none of the bp_catchpoint breakpoints need to
+            do anything at this point.  In the future, if some of
+            the catchpoints need to something, we will need to add
+            a new method, and call this method from here.  */
+         continue;
+       }
 
-    /* Without a symbolic address, we have little hope of the
-       pre-exec() address meaning the same thing in the post-exec()
-       a.out.  */
-    if (breakpoint_event_location_empty_p (b))
-      {
-       delete_breakpoint (b);
-       continue;
-      }
-  }
+      /* bp_finish is a special case.  The only way we ought to be able
+        to see one of these when an exec() has happened, is if the user
+        caught a vfork, and then said "finish".  Ordinarily a finish just
+        carries them to the call-site of the current callee, by setting
+        a temporary bp there and resuming.  But in this case, the finish
+        will carry them entirely through the vfork & exec.
+
+        We don't want to allow a bp_finish to remain inserted now.  But
+        we can't safely delete it, 'cause finish_command has a handle to
+        the bp on a bpstat, and will later want to delete it.  There's a
+        chance (and I've seen it happen) that if we delete the bp_finish
+        here, that its storage will get reused by the time finish_command
+        gets 'round to deleting the "use to be a bp_finish" breakpoint.
+        We really must allow finish_command to delete a bp_finish.
+
+        In the absence of a general solution for the "how do we know
+        it's safe to delete something others may have handles to?"
+        problem, what we'll do here is just uninsert the bp_finish, and
+        let finish_command delete it.
+
+        (We know the bp_finish is "doomed" in the sense that it's
+        momentary, and will be deleted as soon as finish_command sees
+        the inferior stopped.  So it doesn't matter that the bp's
+        address is probably bogus in the new a.out, unlike e.g., the
+        solib breakpoints.)  */
+
+      if (b->type == bp_finish)
+       {
+         continue;
+       }
+
+      /* Without a symbolic address, we have little hope of the
+        pre-exec() address meaning the same thing in the post-exec()
+        a.out.  */
+      if (breakpoint_event_location_empty_p (b))
+       {
+         delete_breakpoint (b);
+         continue;
+       }
+    }
 }
 
 int
 detach_breakpoints (ptid_t ptid)
 {
-  struct bp_location *bl, **blp_tmp;
   int val = 0;
   scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
   struct inferior *inf = current_inferior ();
@@ -3785,23 +3727,23 @@ detach_breakpoints (ptid_t ptid)
 
   /* Set inferior_ptid; remove_breakpoint_1 uses this global.  */
   inferior_ptid = ptid;
-  ALL_BP_LOCATIONS (bl, blp_tmp)
-  {
-    if (bl->pspace != inf->pspace)
-      continue;
-
-    /* This function must physically remove breakpoints locations
-       from the specified ptid, without modifying the breakpoint
-       package's state.  Locations of type bp_loc_other are only
-       maintained at GDB side.  So, there is no need to remove
-       these bp_loc_other locations.  Moreover, removing these
-       would modify the breakpoint package's state.  */
-    if (bl->loc_type == bp_loc_other)
-      continue;
-
-    if (bl->inserted)
-      val |= remove_breakpoint_1 (bl, DETACH_BREAKPOINT);
-  }
+  for (bp_location *bl : all_bp_locations ())
+    {
+      if (bl->pspace != inf->pspace)
+       continue;
+
+      /* This function must physically remove breakpoints locations
+        from the specified ptid, without modifying the breakpoint
+        package's state.  Locations of type bp_loc_other are only
+        maintained at GDB side.  So, there is no need to remove
+        these bp_loc_other locations.  Moreover, removing these
+        would modify the breakpoint package's state.  */
+      if (bl->loc_type == bp_loc_other)
+       continue;
+
+      if (bl->inserted)
+       val |= remove_breakpoint_1 (bl, DETACH_BREAKPOINT);
+    }
 
   return val;
 }
@@ -3980,9 +3922,7 @@ remove_breakpoint (struct bp_location *bl)
 void
 mark_breakpoints_out (void)
 {
-  struct bp_location *bl, **blp_tmp;
-
-  ALL_BP_LOCATIONS (bl, blp_tmp)
+  for (bp_location *bl : all_bp_locations ())
     if (bl->pspace == current_program_space)
       bl->inserted = 0;
 }
@@ -4012,83 +3952,83 @@ breakpoint_init_inferior (enum inf_context context)
   mark_breakpoints_out ();
 
   for (breakpoint *b : all_breakpoints_safe ())
-  {
-    if (b->loc && b->loc->pspace != pspace)
-      continue;
+    {
+      if (b->loc && b->loc->pspace != pspace)
+       continue;
 
-    switch (b->type)
-      {
-      case bp_call_dummy:
-      case bp_longjmp_call_dummy:
+      switch (b->type)
+       {
+       case bp_call_dummy:
+       case bp_longjmp_call_dummy:
 
-       /* If the call dummy breakpoint is at the entry point it will
-          cause problems when the inferior is rerun, so we better get
-          rid of it.  */
+         /* If the call dummy breakpoint is at the entry point it will
+            cause problems when the inferior is rerun, so we better get
+            rid of it.  */
 
-      case bp_watchpoint_scope:
+       case bp_watchpoint_scope:
 
-       /* Also get rid of scope breakpoints.  */
+         /* Also get rid of scope breakpoints.  */
 
-      case bp_shlib_event:
+       case bp_shlib_event:
 
-       /* Also remove solib event breakpoints.  Their addresses may
-          have changed since the last time we ran the program.
-          Actually we may now be debugging against different target;
-          and so the solib backend that installed this breakpoint may
-          not be used in by the target.  E.g.,
+         /* Also remove solib event breakpoints.  Their addresses may
+            have changed since the last time we ran the program.
+            Actually we may now be debugging against different target;
+            and so the solib backend that installed this breakpoint may
+            not be used in by the target.  E.g.,
 
-          (gdb) file prog-linux
-          (gdb) run               # native linux target
-          ...
-          (gdb) kill
-          (gdb) file prog-win.exe
-          (gdb) tar rem :9999     # remote Windows gdbserver.
-       */
+            (gdb) file prog-linux
+            (gdb) run               # native linux target
+            ...
+            (gdb) kill
+            (gdb) file prog-win.exe
+            (gdb) tar rem :9999     # remote Windows gdbserver.
+         */
 
-      case bp_step_resume:
+       case bp_step_resume:
 
-       /* Also remove step-resume breakpoints.  */
+         /* Also remove step-resume breakpoints.  */
 
-      case bp_single_step:
+       case bp_single_step:
 
-       /* Also remove single-step breakpoints.  */
+         /* Also remove single-step breakpoints.  */
 
-       delete_breakpoint (b);
-       break;
+         delete_breakpoint (b);
+         break;
 
-      case bp_watchpoint:
-      case bp_hardware_watchpoint:
-      case bp_read_watchpoint:
-      case bp_access_watchpoint:
-       {
-         struct watchpoint *w = (struct watchpoint *) b;
+       case bp_watchpoint:
+       case bp_hardware_watchpoint:
+       case bp_read_watchpoint:
+       case bp_access_watchpoint:
+         {
+           struct watchpoint *w = (struct watchpoint *) b;
 
-         /* Likewise for watchpoints on local expressions.  */
-         if (w->exp_valid_block != NULL)
-           delete_breakpoint (b);
-         else
-           {
-             /* Get rid of existing locations, which are no longer
-                valid.  New ones will be created in
-                update_watchpoint, when the inferior is restarted.
-                The next update_global_location_list call will
-                garbage collect them.  */
-             b->loc = NULL;
-
-             if (context == inf_starting)
-               {
-                 /* Reset val field to force reread of starting value in
-                    insert_breakpoints.  */
-                 w->val.reset (nullptr);
-                 w->val_valid = false;
-               }
-           }
+           /* Likewise for watchpoints on local expressions.  */
+           if (w->exp_valid_block != NULL)
+             delete_breakpoint (b);
+           else
+             {
+               /* Get rid of existing locations, which are no longer
+                  valid.  New ones will be created in
+                  update_watchpoint, when the inferior is restarted.
+                  The next update_global_location_list call will
+                  garbage collect them.  */
+               b->loc = NULL;
+
+               if (context == inf_starting)
+                 {
+                   /* Reset val field to force reread of starting value in
+                      insert_breakpoints.  */
+                   w->val.reset (nullptr);
+                   w->val_valid = false;
+                 }
+             }
+         }
+         break;
+       default:
+         break;
        }
-       break;
-      default:
-       break;
-      }
-  }
+    }
 
   /* Get rid of the moribund locations.  */
   for (bp_location *bl : moribund_locations)
@@ -4114,10 +4054,9 @@ breakpoint_init_inferior (enum inf_context context)
 enum breakpoint_here
 breakpoint_here_p (const address_space *aspace, CORE_ADDR pc)
 {
-  struct bp_location *bl, **blp_tmp;
   int any_breakpoint_here = 0;
 
-  ALL_BP_LOCATIONS (bl, blp_tmp)
+  for (bp_location *bl : all_bp_locations ())
     {
       if (bl->loc_type != bp_loc_software_breakpoint
          && bl->loc_type != bp_loc_hardware_breakpoint)
@@ -4148,9 +4087,7 @@ int
 breakpoint_in_range_p (const address_space *aspace,
                       CORE_ADDR addr, ULONGEST len)
 {
-  struct bp_location *bl, **blp_tmp;
-
-  ALL_BP_LOCATIONS (bl, blp_tmp)
+  for (bp_location *bl : all_bp_locations ())
     {
       if (bl->loc_type != bp_loc_software_breakpoint
          && bl->loc_type != bp_loc_hardware_breakpoint)
@@ -4214,12 +4151,8 @@ bp_location_inserted_here_p (struct bp_location *bl,
 int
 breakpoint_inserted_here_p (const address_space *aspace, CORE_ADDR pc)
 {
-  struct bp_location **blp, **blp_tmp = NULL;
-
-  ALL_BP_LOCATIONS_AT_ADDR (blp, blp_tmp, pc)
+  for (bp_location *bl : all_bp_locations_at_addr (pc))
     {
-      struct bp_location *bl = *blp;
-
       if (bl->loc_type != bp_loc_software_breakpoint
          && bl->loc_type != bp_loc_hardware_breakpoint)
        continue;
@@ -4237,12 +4170,8 @@ int
 software_breakpoint_inserted_here_p (const address_space *aspace,
                                     CORE_ADDR pc)
 {
-  struct bp_location **blp, **blp_tmp = NULL;
-
-  ALL_BP_LOCATIONS_AT_ADDR (blp, blp_tmp, pc)
+  for (bp_location *bl : all_bp_locations_at_addr (pc))
     {
-      struct bp_location *bl = *blp;
-
       if (bl->loc_type != bp_loc_software_breakpoint)
        continue;
 
@@ -4259,12 +4188,8 @@ int
 hardware_breakpoint_inserted_here_p (const address_space *aspace,
                                     CORE_ADDR pc)
 {
-  struct bp_location **blp, **blp_tmp = NULL;
-
-  ALL_BP_LOCATIONS_AT_ADDR (blp, blp_tmp, pc)
+  for (bp_location *bl : all_bp_locations_at_addr (pc))
     {
-      struct bp_location *bl = *blp;
-
       if (bl->loc_type != bp_loc_hardware_breakpoint)
        continue;
 
@@ -7432,9 +7357,10 @@ set_longjmp_breakpoint_for_call_dummy (void)
    TP.  Remove those which can no longer be found in the current frame
    stack.
 
-   You should call this function only at places where it is safe to currently
-   unwind the whole stack.  Failed stack unwind would discard live dummy
-   frames.  */
+   If the unwind fails then there is not sufficient information to discard
+   dummy frames.  In this case, elide the clean up and the dummy frames will
+   be cleaned up next time this function is called from a location where
+   unwinding is possible.  */
 
 void
 check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
@@ -7446,12 +7372,55 @@ check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
       {
        struct breakpoint *dummy_b = b->related_breakpoint;
 
+       /* Find the bp_call_dummy breakpoint in the list of breakpoints
+          chained off b->related_breakpoint.  */
        while (dummy_b != b && dummy_b->type != bp_call_dummy)
          dummy_b = dummy_b->related_breakpoint;
+
+       /* If there was no bp_call_dummy breakpoint then there's nothing
+          more to do.  Or, if the dummy frame associated with the
+          bp_call_dummy is still on the stack then we need to leave this
+          bp_call_dummy in place.  */
        if (dummy_b->type != bp_call_dummy
            || frame_find_by_id (dummy_b->frame_id) != NULL)
          continue;
-       
+
+       /* We didn't find the dummy frame on the stack, this could be
+          because we have longjmp'd to a stack frame that is previous to
+          the dummy frame, or it could be because the stack unwind is
+          broken at some point between the longjmp frame and the dummy
+          frame.
+
+          Next we figure out why the stack unwind stopped.  If it looks
+          like the unwind is complete then we assume the dummy frame has
+          been jumped over, however, if the unwind stopped for an
+          unexpected reason then we assume the stack unwind is currently
+          broken, and that we will (eventually) return to the dummy
+          frame.
+
+          It might be tempting to consider using frame_id_inner here, but
+          that is not safe.   There is no guarantee that the stack frames
+          we are looking at here are even on the same stack as the
+          original dummy frame, hence frame_id_inner can't be used.  See
+          the comments on frame_id_inner for more details.  */
+       bool unwind_finished_unexpectedly = false;
+       for (struct frame_info *fi = get_current_frame (); fi != nullptr; )
+         {
+           struct frame_info *prev = get_prev_frame (fi);
+           if (prev == nullptr)
+             {
+               /* FI is the last stack frame.  Why did this frame not
+                  unwind further?  */
+               auto stop_reason = get_frame_unwind_stop_reason (fi);
+               if (stop_reason != UNWIND_NO_REASON
+                   && stop_reason != UNWIND_OUTERMOST)
+                 unwind_finished_unexpectedly = true;
+             }
+           fi = prev;
+         }
+       if (unwind_finished_unexpectedly)
+         continue;
+
        dummy_frame_discard (dummy_b->frame_id, tp);
 
        while (b->related_breakpoint != b)
@@ -7469,11 +7438,11 @@ enable_overlay_breakpoints (void)
 {
   for (breakpoint *b : all_breakpoints ())
     if (b->type == bp_overlay_event)
-    {
-      b->enable_state = bp_enabled;
-      update_global_location_list (UGLL_MAY_INSERT);
-      overlay_events_enabled = 1;
-    }
+      {
+       b->enable_state = bp_enabled;
+       update_global_location_list (UGLL_MAY_INSERT);
+       overlay_events_enabled = 1;
+      }
 }
 
 void
@@ -7481,11 +7450,11 @@ disable_overlay_breakpoints (void)
 {
   for (breakpoint *b : all_breakpoints ())
     if (b->type == bp_overlay_event)
-    {
-      b->enable_state = bp_disabled;
-      update_global_location_list (UGLL_DONT_INSERT);
-      overlay_events_enabled = 0;
-    }
+      {
+       b->enable_state = bp_disabled;
+       update_global_location_list (UGLL_DONT_INSERT);
+       overlay_events_enabled = 0;
+      }
 }
 
 /* Set an active std::terminate breakpoint for each std::terminate
@@ -7620,30 +7589,28 @@ create_and_insert_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR add
 void
 disable_breakpoints_in_shlibs (void)
 {
-  struct bp_location *loc, **locp_tmp;
+  for (bp_location *loc : all_bp_locations ())
+    {
+      /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL.  */
+      struct breakpoint *b = loc->owner;
 
-  ALL_BP_LOCATIONS (loc, locp_tmp)
-  {
-    /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL.  */
-    struct breakpoint *b = loc->owner;
-
-    /* We apply the check to all breakpoints, including disabled for
-       those with loc->duplicate set.  This is so that when breakpoint
-       becomes enabled, or the duplicate is removed, gdb will try to
-       insert all breakpoints.  If we don't set shlib_disabled here,
-       we'll try to insert those breakpoints and fail.  */
-    if (((b->type == bp_breakpoint)
-        || (b->type == bp_jit_event)
-        || (b->type == bp_hardware_breakpoint)
-        || (is_tracepoint (b)))
-       && loc->pspace == current_program_space
-       && !loc->shlib_disabled
-       && solib_name_from_address (loc->pspace, loc->address)
-       )
-      {
-       loc->shlib_disabled = 1;
-      }
-  }
+      /* We apply the check to all breakpoints, including disabled for
+        those with loc->duplicate set.  This is so that when breakpoint
+        becomes enabled, or the duplicate is removed, gdb will try to
+        insert all breakpoints.  If we don't set shlib_disabled here,
+        we'll try to insert those breakpoints and fail.  */
+      if (((b->type == bp_breakpoint)
+          || (b->type == bp_jit_event)
+          || (b->type == bp_hardware_breakpoint)
+          || (is_tracepoint (b)))
+         && loc->pspace == current_program_space
+         && !loc->shlib_disabled
+         && solib_name_from_address (loc->pspace, loc->address)
+         )
+       {
+         loc->shlib_disabled = 1;
+       }
+    }
 }
 
 /* Disable any breakpoints and tracepoints that are in SOLIB upon
@@ -7653,43 +7620,42 @@ disable_breakpoints_in_shlibs (void)
 static void
 disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
 {
-  struct bp_location *loc, **locp_tmp;
   int disabled_shlib_breaks = 0;
 
-  ALL_BP_LOCATIONS (loc, locp_tmp)
-  {
-    /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL.  */
-    struct breakpoint *b = loc->owner;
+  for (bp_location *loc : all_bp_locations ())
+    {
+      /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL.  */
+      struct breakpoint *b = loc->owner;
 
-    if (solib->pspace == loc->pspace
-       && !loc->shlib_disabled
-       && (((b->type == bp_breakpoint
-             || b->type == bp_jit_event
-             || b->type == bp_hardware_breakpoint)
-            && (loc->loc_type == bp_loc_hardware_breakpoint
-                || loc->loc_type == bp_loc_software_breakpoint))
-           || is_tracepoint (b))
-       && solib_contains_address_p (solib, loc->address))
-      {
-       loc->shlib_disabled = 1;
-       /* At this point, we cannot rely on remove_breakpoint
-          succeeding so we must mark the breakpoint as not inserted
-          to prevent future errors occurring in remove_breakpoints.  */
-       loc->inserted = 0;
+      if (solib->pspace == loc->pspace
+         && !loc->shlib_disabled
+         && (((b->type == bp_breakpoint
+               || b->type == bp_jit_event
+               || b->type == bp_hardware_breakpoint)
+              && (loc->loc_type == bp_loc_hardware_breakpoint
+                  || loc->loc_type == bp_loc_software_breakpoint))
+             || is_tracepoint (b))
+         && solib_contains_address_p (solib, loc->address))
+       {
+         loc->shlib_disabled = 1;
+         /* At this point, we cannot rely on remove_breakpoint
+            succeeding so we must mark the breakpoint as not inserted
+            to prevent future errors occurring in remove_breakpoints.  */
+         loc->inserted = 0;
 
-       /* This may cause duplicate notifications for the same breakpoint.  */
-       gdb::observers::breakpoint_modified.notify (b);
+         /* This may cause duplicate notifications for the same breakpoint.  */
+         gdb::observers::breakpoint_modified.notify (b);
 
-       if (!disabled_shlib_breaks)
-         {
-           target_terminal::ours_for_output ();
-           warning (_("Temporarily disabling breakpoints "
-                      "for unloaded shared library \"%s\""),
-                    solib->so_name);
-         }
-       disabled_shlib_breaks = 1;
-      }
-  }
+         if (!disabled_shlib_breaks)
+           {
+             target_terminal::ours_for_output ();
+             warning (_("Temporarily disabling breakpoints "
+                        "for unloaded shared library \"%s\""),
+                      solib->so_name);
+           }
+         disabled_shlib_breaks = 1;
+       }
+    }
 }
 
 /* Disable any breakpoints and tracepoints in OBJFILE upon
@@ -8053,22 +8019,22 @@ breakpoint_hit_catch_solib (const struct bp_location *bl,
     return 1;
 
   for (breakpoint *other : all_breakpoints ())
-  {
-    if (other == bl->owner)
-      continue;
+    {
+      if (other == bl->owner)
+       continue;
 
-    if (other->type != bp_shlib_event)
-      continue;
+      if (other->type != bp_shlib_event)
+       continue;
 
-    if (self->pspace != NULL && other->pspace != self->pspace)
-      continue;
+      if (self->pspace != NULL && other->pspace != self->pspace)
+       continue;
 
-    for (bp_location *other_bl : other->locations ())
-      {
-       if (other->ops->breakpoint_hit (other_bl, aspace, bp_addr, ws))
-         return 1;
-      }
-  }
+      for (bp_location *other_bl : other->locations ())
+       {
+         if (other->ops->breakpoint_hit (other_bl, aspace, bp_addr, ws))
+           return 1;
+       }
+    }
 
   return 0;
 }
@@ -10790,7 +10756,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
          gdbarch *caller_arch = frame_unwind_caller_arch (wp_frame);
          CORE_ADDR caller_pc = frame_unwind_caller_pc (wp_frame);
 
-         scope_breakpoint
+         scope_breakpoint
            = create_internal_breakpoint (caller_arch, caller_pc,
                                          bp_watchpoint_scope,
                                          &momentary_breakpoint_ops);
@@ -11660,12 +11626,10 @@ bp_location_is_less_than (const bp_location *a, const bp_location *b)
 static void
 bp_locations_target_extensions_update (void)
 {
-  struct bp_location *bl, **blp_tmp;
-
   bp_locations_placed_address_before_address_max = 0;
   bp_locations_shadow_len_after_address_max = 0;
 
-  ALL_BP_LOCATIONS (bl, blp_tmp)
+  for (bp_location *bl : all_bp_locations ())
     {
       CORE_ADDR start, end, addr;
 
@@ -11775,8 +11739,6 @@ swap_insertion (struct bp_location *left, struct bp_location *right)
 static void
 force_breakpoint_reinsertion (struct bp_location *bl)
 {
-  struct bp_location **locp = NULL, **loc2p;
-  struct bp_location *loc;
   CORE_ADDR address = 0;
   int pspace_num;
 
@@ -11795,10 +11757,8 @@ force_breakpoint_reinsertion (struct bp_location *bl)
      the same program space as the location
      as "its condition has changed".  We need to
      update the conditions on the target's side.  */
-  ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, address)
+  for (bp_location *loc : all_bp_locations_at_addr (address))
     {
-      loc = *loc2p;
-
       if (!is_breakpoint (loc->owner)
          || pspace_num != loc->pspace->num)
        continue;
@@ -12095,8 +12055,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
   awp_loc_first = NULL;
   rwp_loc_first = NULL;
 
-  bp_location *loc, **locp;
-  ALL_BP_LOCATIONS (loc, locp)
+  for (bp_location *loc : all_bp_locations ())
     {
       /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always
         non-NULL.  */
@@ -13259,10 +13218,10 @@ delete_breakpoint (struct breakpoint *bpt)
 
   for (breakpoint *b : all_breakpoints ())
     if (b->next == bpt)
-    {
-      b->next = bpt->next;
-      break;
-    }
+      {
+       b->next = bpt->next;
+       break;
+      }
 
   /* Be sure no bpstat's are pointing at the breakpoint after it's
      been freed.  */
@@ -13998,33 +13957,33 @@ set_ignore_count (int bptnum, int count, int from_tty)
 
   for (breakpoint *b : all_breakpoints ())
     if (b->number == bptnum)
-    {
-      if (is_tracepoint (b))
-       {
-         if (from_tty && count != 0)
-           printf_filtered (_("Ignore count ignored for tracepoint %d."),
-                            bptnum);
-         return;
-       }
-      
-      b->ignore_count = count;
-      if (from_tty)
-       {
-         if (count == 0)
-           printf_filtered (_("Will stop next time "
-                              "breakpoint %d is reached."),
-                            bptnum);
-         else if (count == 1)
-           printf_filtered (_("Will ignore next crossing of breakpoint %d."),
-                            bptnum);
-         else
-           printf_filtered (_("Will ignore next %d "
-                              "crossings of breakpoint %d."),
-                            count, bptnum);
-       }
-      gdb::observers::breakpoint_modified.notify (b);
-      return;
-    }
+      {
+       if (is_tracepoint (b))
+         {
+           if (from_tty && count != 0)
+             printf_filtered (_("Ignore count ignored for tracepoint %d."),
+                              bptnum);
+           return;
+         }
+
+       b->ignore_count = count;
+       if (from_tty)
+         {
+           if (count == 0)
+             printf_filtered (_("Will stop next time "
+                                "breakpoint %d is reached."),
+                              bptnum);
+           else if (count == 1)
+             printf_filtered (_("Will ignore next crossing of breakpoint %d."),
+                              bptnum);
+           else
+             printf_filtered (_("Will ignore next %d "
+                                "crossings of breakpoint %d."),
+                              count, bptnum);
+         }
+       gdb::observers::breakpoint_modified.notify (b);
+       return;
+      }
 
   error (_("No breakpoint number %d."), bptnum);
 }
@@ -14931,10 +14890,10 @@ trace_pass_command (const char *args, int from_tty)
        error (_("Junk at end of arguments."));
 
       for (breakpoint *b : all_tracepoints ())
-      {
-       t1 = (struct tracepoint *) b;
-       trace_pass_set_count (t1, count, from_tty);
-      }
+       {
+         t1 = (struct tracepoint *) b;
+         trace_pass_set_count (t1, count, from_tty);
+       }
     }
   else if (*args == '\0')
     {
@@ -15016,9 +14975,7 @@ get_tracepoint_by_number (const char **arg,
 
   for (breakpoint *t : all_tracepoints ())
     if (t->number == tpnum)
-    {
       return (struct tracepoint *) t;
-    }
 
   printf_unfiltered ("No tracepoint number %d.\n", tpnum);
   return NULL;
@@ -15053,25 +15010,25 @@ save_breakpoints (const char *filename, int from_tty,
 
   /* See if we have anything to save.  */
   for (breakpoint *tp : all_breakpoints ())
-  {
-    /* Skip internal and momentary breakpoints.  */
-    if (!user_breakpoint_p (tp))
-      continue;
+    {
+      /* Skip internal and momentary breakpoints.  */
+      if (!user_breakpoint_p (tp))
+       continue;
 
-    /* If we have a filter, only save the breakpoints it accepts.  */
-    if (filter && !filter (tp))
-      continue;
+      /* If we have a filter, only save the breakpoints it accepts.  */
+      if (filter && !filter (tp))
+       continue;
 
-    any = 1;
+      any = 1;
 
-    if (is_tracepoint (tp))
-      {
-       extra_trace_bits = 1;
+      if (is_tracepoint (tp))
+       {
+         extra_trace_bits = 1;
 
-       /* We can stop searching.  */
-       break;
-      }
-  }
+         /* We can stop searching.  */
+         break;
+       }
+    }
 
   if (!any)
     {
@@ -15091,65 +15048,65 @@ save_breakpoints (const char *filename, int from_tty,
     save_trace_state_variables (&fp);
 
   for (breakpoint *tp : all_breakpoints ())
-  {
-    /* Skip internal and momentary breakpoints.  */
-    if (!user_breakpoint_p (tp))
-      continue;
+    {
+      /* Skip internal and momentary breakpoints.  */
+      if (!user_breakpoint_p (tp))
+       continue;
 
-    /* If we have a filter, only save the breakpoints it accepts.  */
-    if (filter && !filter (tp))
-      continue;
+      /* If we have a filter, only save the breakpoints it accepts.  */
+      if (filter && !filter (tp))
+       continue;
 
-    tp->ops->print_recreate (tp, &fp);
+      tp->ops->print_recreate (tp, &fp);
 
-    /* Note, we can't rely on tp->number for anything, as we can't
-       assume the recreated breakpoint numbers will match.  Use $bpnum
-       instead.  */
+      /* Note, we can't rely on tp->number for anything, as we can't
+        assume the recreated breakpoint numbers will match.  Use $bpnum
+        instead.  */
 
-    if (tp->cond_string)
-      fp.printf ("  condition $bpnum %s\n", tp->cond_string);
+      if (tp->cond_string)
+       fp.printf ("  condition $bpnum %s\n", tp->cond_string);
 
-    if (tp->ignore_count)
-      fp.printf ("  ignore $bpnum %d\n", tp->ignore_count);
+      if (tp->ignore_count)
+       fp.printf ("  ignore $bpnum %d\n", tp->ignore_count);
 
-    if (tp->type != bp_dprintf && tp->commands)
-      {
-       fp.puts ("  commands\n");
-       
-       current_uiout->redirect (&fp);
-       try
-         {
-           print_command_lines (current_uiout, tp->commands.get (), 2);
-         }
-       catch (const gdb_exception &ex)
-         {
-         current_uiout->redirect (NULL);
-           throw;
-         }
+      if (tp->type != bp_dprintf && tp->commands)
+       {
+         fp.puts ("  commands\n");
 
-       current_uiout->redirect (NULL);
-       fp.puts ("  end\n");
-      }
+         current_uiout->redirect (&fp);
+         try
+           {
+             print_command_lines (current_uiout, tp->commands.get (), 2);
+           }
+         catch (const gdb_exception &ex)
+           {
+           current_uiout->redirect (NULL);
+             throw;
+           }
 
-    if (tp->enable_state == bp_disabled)
-      fp.puts ("disable $bpnum\n");
+         current_uiout->redirect (NULL);
+         fp.puts ("  end\n");
+       }
 
-    /* If this is a multi-location breakpoint, check if the locations
-       should be individually disabled.  Watchpoint locations are
-       special, and not user visible.  */
-    if (!is_watchpoint (tp) && tp->loc && tp->loc->next)
-      {
-       int n = 1;
+      if (tp->enable_state == bp_disabled)
+       fp.puts ("disable $bpnum\n");
 
-       for (bp_location *loc : tp->locations ())
-         {
-           if (!loc->enabled)
-             fp.printf ("disable $bpnum.%d\n", n);
+      /* If this is a multi-location breakpoint, check if the locations
+        should be individually disabled.  Watchpoint locations are
+        special, and not user visible.  */
+      if (!is_watchpoint (tp) && tp->loc && tp->loc->next)
+       {
+         int n = 1;
 
-           n++;
-         }
-      }
-  }
+         for (bp_location *loc : tp->locations ())
+           {
+             if (!loc->enabled)
+               fp.printf ("disable $bpnum.%d\n", n);
+
+             n++;
+           }
+       }
+    }
 
   if (extra_trace_bits && *default_collect)
     fp.printf ("set default-collect %s\n", default_collect);
@@ -15259,16 +15216,6 @@ add_catch_command (const char *name, const char *docstring,
   set_cmd_completer (command, completer);
 }
 
-struct breakpoint *
-iterate_over_breakpoints (gdb::function_view<bool (breakpoint *)> callback)
-{
-  for (breakpoint *b : all_breakpoints_safe ())
-    if (callback (b))
-      return b;
-
-  return NULL;
-}
-
 /* Zero if any of the breakpoint's locations could be a location where
    functions have been inlined, nonzero otherwise.  */
 
@@ -15311,9 +15258,7 @@ pc_at_non_inline_function (const address_space *aspace, CORE_ADDR pc,
 void
 breakpoint_free_objfile (struct objfile *objfile)
 {
-  struct bp_location **locp, *loc;
-
-  ALL_BP_LOCATIONS (loc, locp)
+  for (bp_location *loc : all_bp_locations ())
     if (loc->symtab != NULL && SYMTAB_OBJFILE (loc->symtab) == objfile)
       loc->symtab = NULL;
 }