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

Willgerodt, Felix
2024-03-05T15:50:40Z

> -----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