gdb: avoid breakpoint::clear_locations calls in update_breakpoint_locations

The commit:

  commit 6cce025114ccd0f53cc552fde12b6329596c6c65
  Date:   Fri Mar 3 19:03:15 2023 +0000

      gdb: only insert thread-specific breakpoints in the relevant inferior

added a couple of calls to breakpoint::clear_locations() inside
update_breakpoint_locations().

The intention of these calls was to avoid leaving redundant locations
around when a thread- or inferior-specific breakpoint was switched
from one thread or inferior to another.

Without the clear_locations() calls the tests gdb.multi/tids.exp and
gdb.multi/pending-bp.exp have some failures.  A b/p is changed such
that the program space it is associated with changes.  This triggers a
call to breakpoint_re_set_one() but the FILTER_PSPACE argument will be
the new program space.  As a result GDB correctly calculates the new
locations and adds these to the breakpoint, but the old locations, in
the old program space, are incorrectly retained.  The call to
clear_locations() solves this by deleting the old locations.

However, while working on another patch I realised that the approach
taken here is not correct.  The FILTER_PSPACE argument passed to
breakpoint_re_set_one() and then on to update_breakpoint_locations()
might not be the program space to which the breakpoint is associated.
Consider this example:

  (gdb) file /tmp/hello.x
  Reading symbols from /tmp/hello.x...
  (gdb) start
  Temporary breakpoint 1 at 0x401198: file hello.c, line 18.
  Starting program: /tmp/hello.x

  Temporary breakpoint 1, main () at hello.c:18
  18	  printf ("Hello World\n");
  (gdb) break main thread 1
  Breakpoint 2 at 0x401198: file hello.c, line 18.
  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  2       breakpoint     keep y   0x0000000000401198 in main at hello.c:18
  	stop only in thread 1
  (gdb) add-inferior -exec /tmp/hello.x
  [New inferior 2]
  Added inferior 2 on connection 1 (native)
  Reading symbols from /tmp/hello.x...
  (gdb) info breakpoints
  Num     Type           Disp Enb Address    What
  2       breakpoint     keep y   <PENDING>  main
  	stop only in thread 1.1

Notice that after creating the second inferior and loading a file the
thread-specific breakpoint was incorrectly made pending.  Loading the
exec file in the second inferior triggered a call to
breakpoint_re_set() with the new, second, program space as the
current_program_space.

This program space ends up being passed to
update_breakpoint_locations().

In update_breakpoint_locations this condition is true:

  if (all_locations_are_pending (b, filter_pspace) && sals.empty ())

and so we end up discarding all of the locations for this breakpoint,
making the breakpoint pending.

What we really want to do in update_breakpoint_locations() is, for
thread- or inferior- specific breakpoints, delete any locations which
are associated with a program space that this breakpoint is NOT
associated with.

But then I realised the answer was easier than that.

The ONLY time that a b/p can have locations associated with the
"wrong" program space like this is at the moment we change the thread
or inferior the b/p is associated with by calling
breakpoint_set_thread() or breakpoint_set_inferior().

And so, I think the correct solution is to hoist the call to
clear_locations() out of update_breakpoint_locations() and place a
call in each of the breakpoint_set_{thread,inferior} functions.

I've done this, and added a couple of new tests.  All of which are
now passing.

Approved-By: Tom Tromey <tom@tromey.com>

0 files changed, 0 insertions, 0 deletions