RE: [PATCHv9 14/14] gdb: only insert thread-specific breakpoints in the relevant inferior

["Willgerodt, Felix" <felix.willgerodt@intel.com>]

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Dienstag, 5. März 2024 16:22
> To: gdb-patches@sourceware.org
> Cc: Andrew Burgess <aburgess@redhat.com>; Eli Zaretskii <eliz@gnu.org>
> Subject: [PATCHv9 14/14] gdb: only insert thread-specific breakpoints in the
> relevant inferior
> 
> This commit updates GDB so that thread or inferior specific
> breakpoints are only inserted into the program space in which the
> specific thread or inferior is running.
> 
> In terms of implementation, getting this basically working is easy
> enough, now that a breakpoint's thread or inferior field is setup
> prior to GDB looking for locations, we can easily use this information
> to find a suitable program_space and pass this to as a filter when
> creating the sals.
> 
> Or we could if breakpoint_ops::create_sals_from_location_spec allowed
> us to pass in a filter program_space.
> 
> So, this commit extends breakpoint_ops::create_sals_from_location_spec
> to take a program_space argument, and uses this to filter the set of
> returned sals.  This accounts for about half the change in this patch.
> 
> The second set of changes starts from breakpoint_set_thread and
> breakpoint_set_inferior, this is called when the thread or inferior
> for a breakpoint changes, e.g. from the Python API.
> 
> Previously this call would never result in the locations of a
> breakpoint changing, after all, locations were inserted in every
> program space, and we just use the thread or inferior variable to
> decide when we should stop.  Now though, changing a breakpoint's
> thread or inferior can mean we need to figure out a new set of
> breakpoint locations.
> 
> To support this I've added a new breakpoint_re_set_one function, which
> is like breakpoint_re_set, but takes a single breakpoint, and just
> updates the locations for that one breakpoint.  We only need to call
> this function if the program_space in which a breakpoint's thread (or
> inferior) is running actually changes.  If the program_space does
> change then we call the new breakpoint_re_set_one function passing in
> the program_space which should be used to filter the new locations (or
> nullptr to indicate we should set locations in all program spaces).
> This filter program_space needs to propagate down to all the re_set
> methods, this accounts for the remaining half of the changes in this
> patch.
> 
> There were a couple of existing tests that created thread or inferior
> specific breakpoints and then checked the 'info breakpoints' output,
> these needed updating.  These were:
> 
>   gdb.mi/user-selected-context-sync.exp
>   gdb.multi/bp-thread-specific.exp
>   gdb.multi/multi-target-continue.exp
>   gdb.multi/multi-target-ping-pong-next.exp
>   gdb.multi/tids.exp
>   gdb.mi/new-ui-bp-deleted.exp
>   gdb.multi/inferior-specific-bp.exp
>   gdb.multi/pending-bp-del-inferior.exp
> 
> I've also added some additional tests to:
> 
>   gdb.multi/pending-bp.exp
> 
> I've updated the documentation and added a NEWS entry.
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> ---
>  gdb/NEWS                                      |   7 +
>  gdb/ada-lang.c                                |   6 +-
>  gdb/break-catch-throw.c                       |   6 +-
>  gdb/breakpoint.c                              | 280 ++++++++++++++----
>  gdb/breakpoint.h                              |  29 +-
>  gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp    |   8 +-
>  .../gdb.mi/user-selected-context-sync.exp     |   4 +-
>  .../gdb.multi/bp-thread-specific.exp          |   7 +-
>  .../gdb.multi/inferior-specific-bp.exp        |  12 +-
>  .../gdb.multi/multi-target-continue.exp       |   2 +-
>  .../gdb.multi/multi-target-ping-pong-next.exp |   4 +-
>  .../gdb.multi/pending-bp-del-inferior.exp     |   6 +-
>  gdb/testsuite/gdb.multi/pending-bp.exp        | 206 +++++++++++++
>  gdb/testsuite/gdb.multi/tids.exp              |   6 +-
>  14 files changed, 484 insertions(+), 99 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index c170385a50e..2a888d64e4d 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -17,6 +17,13 @@
>    'thread' or 'task' keywords are parsed at the time the breakpoint is
>    created, rather than at the time the breakpoint becomes non-pending.
> 
> +* Thread-specific breakpoints are only inserted into the program space
> +  in which the thread of interest is running.  In most cases program
> +  spaces are unique for each inferior, so this means that
> +  thread-specific breakpoints will usually only be inserted for the
> +  inferior containing the thread of interest.  The breakpoint will
> +  be hit no less than before.
> +
>  * Changed commands
> 
>  disassemble
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 1c26ebf7b30..c88acdf4035 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -12010,11 +12010,11 @@ struct ada_catchpoint : public
> code_breakpoint
>      enable_state = enabled ? bp_enabled : bp_disabled;
>      language = language_ada;
> 
> -    re_set ();
> +    re_set (pspace);
>    }
> 
>    struct bp_location *allocate_location () override;
> -  void re_set () override;
> +  void re_set (program_space *pspace) override;
>    void check_status (struct bpstat *bs) override;
>    enum print_stop_action print_it (const bpstat *bs) const override;
>    bool print_one (const bp_location **) const override;
> @@ -12059,7 +12059,7 @@ static struct symtab_and_line ada_exception_sal
>     catchpoint kinds.  */
> 
>  void
> -ada_catchpoint::re_set ()
> +ada_catchpoint::re_set (program_space *pspace)
>  {
>    std::vector<symtab_and_line> sals;
>    try
> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
> index d053bd5fbe0..7191d1b38fa 100644
> --- a/gdb/break-catch-throw.c
> +++ b/gdb/break-catch-throw.c
> @@ -82,10 +82,10 @@ struct exception_catchpoint : public code_breakpoint
>  				     _("invalid type-matching regexp")))
>    {
>      pspace = current_program_space;
> -    re_set ();
> +    re_set (pspace);
>    }
> 
> -  void re_set () override;
> +  void re_set (program_space *pspace) override;
>    enum print_stop_action print_it (const bpstat *bs) const override;
>    bool print_one (const bp_location **) const override;
>    void print_mention () const override;
> @@ -198,7 +198,7 @@ exception_catchpoint::check_status (struct bpstat
> *bs)
>  /* Implement the 're_set' method.  */
> 
>  void
> -exception_catchpoint::re_set ()
> +exception_catchpoint::re_set (program_space *pspace)
>  {
>    std::vector<symtab_and_line> sals;
>    struct program_space *filter_pspace = current_program_space;
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index e1efde66feb..a190f2a78bf 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -91,9 +91,12 @@
>  static void map_breakpoint_numbers (const char *,
>  				    gdb::function_view<void (breakpoint *)>);
> 
> -static void
> -  create_sals_from_location_spec_default (location_spec *locspec,
> -					  linespec_result *canonical);
> +static void parse_breakpoint_sals (location_spec *locspec,
> +				   linespec_result *canonical,
> +				   program_space *search_pspace);
> +
> +static void breakpoint_re_set_one (breakpoint *b,
> +				   program_space *filter_pspace);
> 
>  static void create_breakpoints_sal (struct gdbarch *,
>  				    struct linespec_result *,
> @@ -283,11 +286,12 @@ static bool strace_marker_p (struct breakpoint *b);
> 
>  static void bkpt_probe_create_sals_from_location_spec
>       (location_spec *locspec,
> -      struct linespec_result *canonical);
> +      struct linespec_result *canonical,
> +      struct program_space *search_pspace);
> 
>  const struct breakpoint_ops code_breakpoint_ops =
>  {
> -  create_sals_from_location_spec_default,
> +  parse_breakpoint_sals,
>    create_breakpoints_sal,
>  };
> 
> @@ -352,7 +356,7 @@ struct internal_breakpoint : public code_breakpoint
>      disposition = disp_donttouch;
>    }
> 
> -  void re_set () override;
> +  void re_set (program_space *pspace) override;
>    void check_status (struct bpstat *bs) override;
>    enum print_stop_action print_it (const bpstat *bs) const override;
>    void print_mention () const override;
> @@ -389,7 +393,7 @@ struct momentary_breakpoint : public
> code_breakpoint
>      gdb_assert (inferior == -1);
>    }
> 
> -  void re_set () override;
> +  void re_set (program_space *pspace) override;
>    void check_status (struct bpstat *bs) override;
>    enum print_stop_action print_it (const bpstat *bs) const override;
>    void print_mention () const override;
> @@ -400,7 +404,7 @@ struct dprintf_breakpoint : public
> ordinary_breakpoint
>  {
>    using ordinary_breakpoint::ordinary_breakpoint;
> 
> -  void re_set () override;
> +  void re_set (program_space *pspace) override;
>    int breakpoint_hit (const struct bp_location *bl,
>  		      const address_space *aspace,
>  		      CORE_ADDR bp_addr,
> @@ -1549,7 +1553,36 @@ breakpoint_set_thread (struct breakpoint *b, int
> thread)
>    int old_thread = b->thread;
>    b->thread = thread;
>    if (old_thread != thread)
> -    notify_breakpoint_modified (b);
> +    {
> +      /* If THREAD is in a different program_space than OLD_THREAD, or the
> +	 breakpoint has switched to or from being thread-specific, then we
> +	 need to re-set the locations of this breakpoint.  First, figure
> +	 out the program_space for the old and new threads, use a value of
> +	 nullptr to indicate the breakpoint is in all program spaces.  */
> +      program_space *old_pspace = nullptr;
> +      if (old_thread != -1)
> +	{
> +	  struct thread_info *thr = find_thread_global_id (old_thread);
> +	  gdb_assert (thr != nullptr);
> +	  old_pspace = thr->inf->pspace;
> +	}
> +
> +      program_space *new_pspace = nullptr;
> +      if (thread != -1)
> +	{
> +	  struct thread_info *thr = find_thread_global_id (thread);
> +	  gdb_assert (thr != nullptr);
> +	  new_pspace = thr->inf->pspace;
> +	}
> +
> +      /* If the program space has changed for this breakpoint, then
> +	 re-evaluate it's locations.  */
> +      if (old_pspace != new_pspace)
> +	breakpoint_re_set_one (b, new_pspace);
> +
> +      /* Let others know the breakpoint has changed.  */
> +      notify_breakpoint_modified (b);
> +    }
>  }
> 
>  /* See breakpoint.h.  */
> @@ -1568,7 +1601,34 @@ breakpoint_set_inferior (struct breakpoint *b, int
> inferior)
>    int old_inferior = b->inferior;
>    b->inferior = inferior;
>    if (old_inferior != inferior)
> -    notify_breakpoint_modified (b);
> +    {
> +      /* If INFERIOR is in a different program_space than OLD_INFERIOR, or
> +	 the breakpoint has switch to or from inferior-specific, then we
> +	 need to re-set the locations of this breakpoint.  First, figure
> +	 out the program_space for the old and new inferiors, use a value
> +	 of nullptr to indicate the breakpoint is in all program
> +	 spaces.  */
> +      program_space *old_pspace = nullptr;
> +      if (old_inferior != -1)
> +	{
> +	  struct inferior *inf = find_inferior_id (old_inferior);
> +	  gdb_assert (inf != nullptr);
> +	  old_pspace = inf->pspace;
> +	}
> +
> +      program_space *new_pspace = nullptr;
> +      if (inferior != -1)
> +	{
> +	  struct inferior *inf = find_inferior_id (inferior);
> +	  gdb_assert (inf != nullptr);
> +	  new_pspace = inf->pspace;
> +	}
> +
> +      if (old_pspace != new_pspace)
> +	breakpoint_re_set_one (b, new_pspace);
> +
> +      notify_breakpoint_modified (b);
> +    }
>  }
> 
>  /* See breakpoint.h.  */
> @@ -8799,7 +8859,8 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
> 
>  static void
>  parse_breakpoint_sals (location_spec *locspec,
> -		       struct linespec_result *canonical)
> +		       struct linespec_result *canonical,
> +		       struct program_space *search_pspace)
>  {
>    struct symtab_and_line cursal;
> 
> @@ -8864,7 +8925,7 @@ parse_breakpoint_sals (location_spec *locspec,
>  	      && strchr ("+-", spec[0]) != NULL
>  	      && spec[1] != '['))
>  	{
> -	  decode_line_full (locspec, DECODE_LINE_FUNFIRSTLINE, NULL,
> +	  decode_line_full (locspec, DECODE_LINE_FUNFIRSTLINE,
> search_pspace,
>  			    get_last_displayed_symtab (),
>  			    get_last_displayed_line (),
>  			    canonical, NULL, NULL);
> @@ -8872,7 +8933,7 @@ parse_breakpoint_sals (location_spec *locspec,
>  	}
>      }
> 
> -  decode_line_full (locspec, DECODE_LINE_FUNFIRSTLINE, NULL,
> +  decode_line_full (locspec, DECODE_LINE_FUNFIRSTLINE, search_pspace,
>  		    cursal.symtab, cursal.line, canonical, NULL, NULL);
>  }
> 
> @@ -8971,6 +9032,39 @@ breakpoint_ops_for_location_spec_type (enum
> location_spec_type locspec_type,
>      }
>  }
> 
> +/* Return the program space to use as a filter when searching for locations
> +   of a breakpoint specific to THREAD or INFERIOR.  If THREAD and INFERIOR
> +   are both -1, meaning all threads/inferiors, then this function returns
> +   nullptr, indicating no program space filtering should be performed.
> +   Otherwise, this function returns the program space for the inferior that
> +   contains THREAD (when THREAD is not -1), or the program space for
> +   INFERIOR (when INFERIOR is not -1).  */
> +
> +static struct program_space *
> +find_program_space_for_breakpoint (int thread, int inferior)
> +{
> +  if (thread != -1)
> +    {
> +      gdb_assert (inferior == -1);
> +
> +      struct thread_info *thr = find_thread_global_id (thread);
> +      gdb_assert (thr != nullptr);
> +      gdb_assert (thr->inf != nullptr);
> +      return thr->inf->pspace;
> +    }
> +  else if (inferior != -1)
> +    {
> +      gdb_assert (thread == -1);
> +
> +      struct inferior *inf = find_inferior_id (inferior);
> +      gdb_assert (inf != nullptr);
> +
> +      return inf->pspace;
> +    }
> +
> +  return nullptr;
> +}
> +
>  /* See breakpoint.h.  */
> 
>  const struct breakpoint_ops *
> @@ -9072,7 +9166,10 @@ create_breakpoint (struct gdbarch *gdbarch,
> 
>    try
>      {
> -      ops->create_sals_from_location_spec (locspec, &canonical);
> +      struct program_space *search_pspace
> +	= find_program_space_for_breakpoint (thread, inferior);
> +      ops->create_sals_from_location_spec (locspec, &canonical,
> +					   search_pspace);
>      }
>    catch (const gdb_exception_error &e)
>      {
> @@ -9545,7 +9642,7 @@ break_range_command (const char *arg, int
> from_tty)
>    arg_start = arg;
>    location_spec_up start_locspec
>      = string_to_location_spec (&arg, current_language);
> -  parse_breakpoint_sals (start_locspec.get (), &canonical_start);
> +  parse_breakpoint_sals (start_locspec.get (), &canonical_start, nullptr);
> 
>    if (arg[0] != ',')
>      error (_("Too few arguments."));
> @@ -9646,7 +9743,7 @@ watchpoint_exp_is_const (const struct expression
> *exp)
>  /* Implement the "re_set" method for watchpoints.  */
> 
>  void
> -watchpoint::re_set ()
> +watchpoint::re_set (struct program_space *pspace)
>  {
>    /* Watchpoint can be either on expression using entirely global
>       variables, or it can be on local variables.
> @@ -11757,7 +11854,7 @@ breakpoint::print_recreate (struct ui_file *fp)
> const
>  /* Default breakpoint_ops methods.  */
> 
>  void
> -code_breakpoint::re_set ()
> +code_breakpoint::re_set (struct program_space *pspace)
>  {
>    /* FIXME: is this still reachable?  */
>    if (breakpoint_location_spec_empty_p (this))
> @@ -11767,7 +11864,7 @@ code_breakpoint::re_set ()
>        return;
>      }
> 
> -  re_set_default ();
> +  re_set_default (pspace);
>  }
> 
>  int
> @@ -11973,7 +12070,7 @@ code_breakpoint::decode_location_spec
> (location_spec *locspec,
>  /* Virtual table for internal breakpoints.  */
> 
>  void
> -internal_breakpoint::re_set ()
> +internal_breakpoint::re_set (struct program_space *pspace)
>  {
>    switch (type)
>      {
> @@ -12066,7 +12163,7 @@ internal_breakpoint::print_mention () const
>  /* Virtual table for momentary breakpoints  */
> 
>  void
> -momentary_breakpoint::re_set ()
> +momentary_breakpoint::re_set (struct program_space *pspace)
>  {
>    /* Keep temporary breakpoints, which can be encountered when we step
>       over a dlopen call and solib_add is resetting the breakpoints.
> @@ -12107,12 +12204,13 @@ longjmp_breakpoint::~longjmp_breakpoint ()
> 
>  static void
>  bkpt_probe_create_sals_from_location_spec (location_spec *locspec,
> -					   struct linespec_result *canonical)
> +					   struct linespec_result *canonical,
> +					   struct program_space
> *search_pspace)
> 
>  {
>    struct linespec_sals lsal;
> 
> -  lsal.sals = parse_probes (locspec, NULL, canonical);
> +  lsal.sals = parse_probes (locspec, search_pspace, canonical);
>    lsal.canonical = xstrdup (canonical->locspec->to_string ());
>    canonical->lsals.push_back (std::move (lsal));
>  }
> @@ -12202,9 +12300,9 @@ tracepoint::print_recreate (struct ui_file *fp)
> const
>  }
> 
>  void
> -dprintf_breakpoint::re_set ()
> +dprintf_breakpoint::re_set (struct program_space *pspace)
>  {
> -  re_set_default ();
> +  re_set_default (pspace);
> 
>    /* 1 - connect to target 1, that can run breakpoint commands.
>       2 - create a dprintf, which resolves fine.
> @@ -12258,8 +12356,10 @@ dprintf_breakpoint::after_condition_true
> (struct bpstat *bs)
>     markers (`-m').  */
> 
>  static void
> -strace_marker_create_sals_from_location_spec (location_spec *locspec,
> -					      struct linespec_result *canonical)
> +strace_marker_create_sals_from_location_spec
> +	(location_spec *locspec,
> +	 struct linespec_result *canonical,
> +	 struct program_space *search_pspace)
>  {
>    struct linespec_sals lsal;
>    const char *arg_start, *arg;
> @@ -12776,12 +12876,32 @@ update_breakpoint_locations
> (code_breakpoint *b,
>       all locations are in the same shared library, that was unloaded.
>       We'd like to retain the location, so that when the library is
>       loaded again, we don't loose the enabled/disabled status of the
> -     individual locations.  */
> +     individual locations.
> +
> +     Thread specific breakpoints will also trigger this case if the thread
> +     is changed to a different program space, and all of the old locations
> +     go out of scope.  In this case we do (currently) discard the old
> +     locations -- we assume the change in thread is permanent and the old
> +     locations will never come back into scope.  */
>    if (all_locations_are_pending (b, filter_pspace) && sals.empty ())
> -    return;
> +    {
> +      if (b->thread != -1)
> +	b->clear_locations ();
> +      return;
> +    }
> 
>    bp_location_list existing_locations = b->steal_locations (filter_pspace);
> 
> +  /* If this is a thread-specific breakpoint then any locations left on the
> +     breakpoint are for a program space in which the thread of interest
> +     does not operate.  This can happen when the user changes the thread of
> +     a thread-specific breakpoint.
> +
> +     We assume that the change in thread is permanent, and that the old
> +     locations will never be used again, so discard them now.  */
> +  if (b->thread != -1)
> +    b->clear_locations ();
> +
>    for (const auto &sal : sals)
>      {
>        struct bp_location *new_loc;
> @@ -12947,40 +13067,45 @@ code_breakpoint::location_spec_to_sals
> (location_spec *locspec,
>     locations.  */
> 
>  void
> -code_breakpoint::re_set_default ()
> +code_breakpoint::re_set_default (struct program_space *filter_pspace)
>  {
> -  struct program_space *filter_pspace = current_program_space;
>    std::vector<symtab_and_line> expanded, expanded_end;
> 
> -  int found;
> -  std::vector<symtab_and_line> sals = location_spec_to_sals (locspec.get (),
> -							     filter_pspace,
> -							     &found);
> -  if (found)
> -    expanded = std::move (sals);
> -
> -  if (locspec_range_end != nullptr)
> -    {
> -      std::vector<symtab_and_line> sals_end
> -	= location_spec_to_sals (locspec_range_end.get (),
> -				 filter_pspace, &found);
> +  /* If this breakpoint is thread-specific then find the program space in
> +     which the specific thread exists.  Otherwise, for breakpoints that are
> +     not thread-specific THREAD_PSPACE will be nullptr.  */
> +  program_space *bp_pspace
> +    = find_program_space_for_breakpoint (this->thread, this->inferior);
> +
> +  /* If this is not a thread or inferior specific breakpoint, or it is a
> +     thread or inferior specific breakpoint but we are looking for new
> +     locations in the program space that the specific thread or inferior is
> +     running, then look for new locations for this breakpoint.  */
> +  if (bp_pspace == nullptr || filter_pspace == bp_pspace)
> +    {
> +      int found;
> +      std::vector<symtab_and_line> sals
> +	= location_spec_to_sals (locspec.get (), filter_pspace, &found);
>        if (found)
> -	expanded_end = std::move (sals_end);
> +	expanded = std::move (sals);
> +
> +      if (locspec_range_end != nullptr)
> +	{
> +	  std::vector<symtab_and_line> sals_end
> +	    = location_spec_to_sals (locspec_range_end.get (),
> +				     filter_pspace, &found);
> +	  if (found)
> +	    expanded_end = std::move (sals_end);
> +	}
>      }
> 
> +  /* Update the locations for this breakpoint.  For thread-specific
> +     breakpoints this will remove any old locations that are for the wrong
> +     program space -- this can happen if the user changes the thread of a
> +     thread-specific breakpoint.  */
>    update_breakpoint_locations (this, filter_pspace, expanded,
> expanded_end);
>  }
> 
> -/* Default method for creating SALs from an address string.  It basically
> -   calls parse_breakpoint_sals.  Return 1 for success, zero for failure.  */
> -
> -static void
> -create_sals_from_location_spec_default (location_spec *locspec,
> -					struct linespec_result *canonical)
> -{
> -  parse_breakpoint_sals (locspec, canonical);
> -}
> -
>  /* Re-set breakpoint locations for the current program space.
>     Locations bound to other program spaces are left untouched.  */
> 
> @@ -13015,7 +13140,7 @@ breakpoint_re_set (void)
>  	  {
>  	    input_radix = b.input_radix;
>  	    set_language (b.language);
> -	    b.re_set ();
> +	    b.re_set (current_program_space);
>  	  }
>  	catch (const gdb_exception &ex)
>  	  {
> @@ -13036,6 +13161,53 @@ breakpoint_re_set (void)
>    /* Now we can insert.  */
>    update_global_location_list (UGLL_MAY_INSERT);
>  }
> +
> +/* Re-set locations for breakpoint B in FILTER_PSPACE.  If FILTER_PSPACE is
> +   nullptr then re-set locations for B in all program spaces.  Locations
> +   bound to program spaces other than FILTER_PSPACE are left untouched.  */
> +
> +static void
> +breakpoint_re_set_one (breakpoint *b, program_space *filter_pspace)
> +{
> +  {
> +    scoped_restore_current_language save_language;
> +    scoped_restore save_input_radix = make_scoped_restore (&input_radix);
> +    scoped_restore_current_pspace_and_thread restore_pspace_thread;
> +
> +    /* To ::re_set each breakpoint we set the current_language to the
> +       language of the breakpoint before re-evaluating the breakpoint's
> +       location.  This change can unfortunately get undone by accident if
> +       the language_mode is set to auto, and we either switch frames, or
> +       more likely in this context, we select the current frame.
> +
> +       We prevent this by temporarily turning the language_mode to
> +       language_mode_manual.  We restore it once all breakpoints
> +       have been reset.  */
> +    scoped_restore save_language_mode = make_scoped_restore
> (&language_mode);
> +    language_mode = language_mode_manual;
> +
> +    /* Note: we must not try to insert locations until after all
> +       breakpoints have been re-set.  Otherwise, e.g., when re-setting
> +       breakpoint 1, we'd insert the locations of breakpoint 2, which
> +       hadn't been re-set yet, and thus may have stale locations.  */
> +
> +    try
> +      {
> +	input_radix = b->input_radix;
> +	set_language (b->language);
> +	b->re_set (filter_pspace);
> +      }
> +    catch (const gdb_exception &ex)
> +      {
> +	exception_fprintf (gdb_stderr, ex,
> +			   "Error in re-setting breakpoint %d: ",
> +			   b->number);
> +      }
> +  }
> +
> +  /* Now we can insert.  */
> +  update_global_location_list (UGLL_MAY_INSERT);
> +}
> 
> 
> 
>  /* Reset the thread number of this breakpoint:
> 
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index 42b49144e79..dea55deb314 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -570,15 +570,15 @@ enum print_stop_action
> 
>  struct breakpoint_ops
>  {
> -  /* Create SALs from location spec, storing the result in
> -     linespec_result.
> -
> -     For an explanation about the arguments, see the function
> -     `create_sals_from_location_spec_default'.
> +  /* Create SALs from LOCSPEC, storing the result in linespec_result
> +     CANONICAL.  If SEARCH_PSPACE is not nullptr then only results in the
> +     corresponding program space are returned.  If SEARCH_PSPACE is nullptr
> +     then results for all program spaces are returned.
> 
>       This function is called inside `create_breakpoint'.  */
>    void (*create_sals_from_location_spec) (location_spec *locspec,
> -					  struct linespec_result *canonical);
> +					  linespec_result *canonical,
> +					  program_space *search_pspace);
> 
>    /* This method will be responsible for creating a breakpoint given its SALs.
>       Usually, it just calls `create_breakpoints_sal' (for ordinary
> @@ -710,8 +710,15 @@ struct breakpoint : public
> intrusive_list_node<breakpoint>
> 
>    /* Reevaluate a breakpoint.  This is necessary after symbols change
>       (e.g., an executable or DSO was loaded, or the inferior just
> -     started).  */
> -  virtual void re_set ()
> +     started).
> +
> +     If not nullptr, then FILTER_PSPACE is the program space in which
> +     symbols may have changed, we only need to add new locations in
> +     FILTER_PSPACE.
> +
> +     If FILTER_PSPACE is nullptr then all program spaces may have changed,
> +     new locations need to be searched for in every program space.  */
> +  virtual void re_set (program_space *filter_pspace)
>    {
>      /* Nothing to re-set.  */
>    }
> @@ -955,7 +962,7 @@ struct code_breakpoint : public breakpoint
>    /* Add a location for SAL to this breakpoint.  */
>    bp_location *add_location (const symtab_and_line &sal);
> 
> -  void re_set () override;
> +  void re_set (program_space *pspace) override;
>    int insert_location (struct bp_location *) override;
>    int remove_location (struct bp_location *,
>  		       enum remove_bp_reason reason) override;
> @@ -977,7 +984,7 @@ struct code_breakpoint : public breakpoint
>       struct program_space *search_pspace);
> 
>    /* Helper method that does the basic work of re_set.  */
> -  void re_set_default ();
> +  void re_set_default (program_space *pspace);
> 
>    /* Find the SaL locations corresponding to the given LOCATION.
>       On return, FOUND will be 1 if any SaL was found, zero otherwise.  */
> @@ -999,7 +1006,7 @@ struct watchpoint : public breakpoint
>  {
>    using breakpoint::breakpoint;
> 
> -  void re_set () override;
> +  void re_set (program_space *pspace) override;
>    int insert_location (struct bp_location *) override;
>    int remove_location (struct bp_location *,
>  		       enum remove_bp_reason reason) override;
> diff --git a/gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp
> b/gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp
> index f736994d234..938e6deec05 100644
> --- a/gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp
> +++ b/gdb/testsuite/gdb.mi/new-ui-bp-deleted.exp
> @@ -76,8 +76,12 @@ foreach_mi_ui_mode mode {
>      set loc2 [make_bp_loc "$::decimal\\.2"]
> 
>      # Create the inferior-specific breakpoint.
> -    mi_create_breakpoint_multi "-g i2 foo" "create breakpoint in inferior 2" \
> -	-inferior "2" -locations "\\\[$loc1,$loc2\\\]"
> +    mi_create_breakpoint "-g i2 foo" "create breakpoint in inferior 2" \
> +	-number "$decimal" \
> +	-type "breakpoint" \
> +	-enabled "y" \
> +	-func "foo" \
> +	-inferior "2"
>      set bpnum [mi_get_valueof "/d" "\$bpnum" "INVALID"]
> 
>      if {$mode eq "separate"} {
> diff --git a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> index 93b91b42f92..ed331aff873 100644
> --- a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> +++ b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> @@ -370,7 +370,7 @@ proc test_continue_to_start { mode inf } {
>  			    "thread $inf.2 stops MI"
>  		    } else {
>  			mi_expect_stop "breakpoint-hit"
> "child_sub_function" \
> -			    "" "$srcfile" "$decimal" {"" "disp=\"del\""
> "locno=\"[0-9]+\""} \
> +			    "" "$srcfile" "$decimal" {"" "disp=\"del\""} \
>  			    "thread $inf.2 stops MI"
>  		    }

Hi Andrew,

For what it is worth: At some point I had backported your v8 on the
gdb 14 branch to play around with it and see if it fixes a downstream
problem I had. It didn't, but I tested your v8 a bit and didn't find any
issues with it on gdb 14.

While doing that I saw that we probably can just get rid of the
if else in this testcase here now. Sorry for not commenting this earlier.

Regards,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928