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>
/* 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);
+ {
+ /* The breakpoint is now associated with a completely different
+ program space. Discard all of the current locations and then
+ re-set the breakpoint in the new program space, this will
+ create the new locations. */
+ b->clear_locations ();
+ breakpoint_re_set_one (b, new_pspace);
+ }
- /* Let others know the breakpoint has changed. */
- notify_breakpoint_modified (b);
+ /* If the program space didn't change, or the breakpoint didn't
+ acquire any new locations after the clear_locations call, then we
+ need to notify of the breakpoint modification now. */
+ if (old_pspace == new_pspace || !b->has_locations ())
+ notify_breakpoint_modified (b);
}
}
}
if (old_pspace != new_pspace)
- breakpoint_re_set_one (b, new_pspace);
+ {
+ /* The breakpoint is now associated with a completely different
+ program space. Discard all of the current locations and then
+ re-set the breakpoint in the new program space, this will
+ create the new locations. */
+ b->clear_locations ();
+ breakpoint_re_set_one (b, new_pspace);
+ }
- notify_breakpoint_modified (b);
+ /* If the program space didn't change, or the breakpoint didn't
+ acquire any new locations after the clear_locations call, then we
+ need to notify of the breakpoint modification now. */
+ if (old_pspace == new_pspace || !b->has_locations ())
+ notify_breakpoint_modified (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.
-
- 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. */
+ individual locations. */
if (all_locations_are_pending (b, filter_pspace) && sals.empty ())
- {
- if (b->thread != -1)
- b->clear_locations ();
- return;
- }
+ 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;
return -1
}
+delete_breakpoints
+
+# Create a thread-specific b/p on main.
+gdb_breakpoint "main thread 1"
+set bpnum [get_integer_valueof "\$bpnum" "INVALID" \
+ "get number for thread specific b/p on main"]
+
+# Check the b/p has a location and is displayed correctly.
+gdb_test "info breakpoints" \
+ [multi_line \
+ "" \
+ "$bpnum\\s+breakpoint\\s+keep\\s+y\\s+$hex\\s+in main at \[^\r\n\]+/$srcfile:$decimal"\
+ "\\s+stop only in thread 1"] \
+ "check thread b/p on main has a location"
+
gdb_test "add-inferior -exec ${binfile}" "Added inferior 2.*" "add inferior 2"
gdb_test "inferior 2"
+# The breakpoint should still have a location, but should now display
+# information indicating this breakpoint is only in inferior 1.
+gdb_test "info breakpoints" \
+ [multi_line \
+ "" \
+ "$bpnum\\s+breakpoint\\s+keep\\s+y\\s+$hex\\s+in main at \[^\r\n\]+/$srcfile:$decimal inf 1"\
+ "\\s+stop only in thread 1\\.1"] \
+ "check thread b/p on main still has updated correctly"
+
if {![runto_main]} {
return -1
}
static void
bar (void)
{
- global_var = 0;
+ global_var = 0; /* First location of bar. */
foo ();
}
static int
bar (void)
{
- return baz ();
+ return baz (); /* Second location of bar. */
}
static int
# Clear out any other breakpoints.
delete_breakpoints
+# Create an inferior specific breakpoint and then change the inferior
+# using the Python API. Use 'info breakpoint' to check that the
+# breakpoint was updated as we expect.
+if { [allow_python_tests] } {
+ with_test_prefix "update breakpoint inferior" {
+ # Create the b/p and grab its number.
+ gdb_breakpoint "bar inferior 1"
+ set bpnum [get_integer_valueof "\$bpnum" "INVALID" \
+ "get b/p number for breakpoint on bar"]
+
+ # Get the line number for the two locations, the first in
+ # inferior 1, the second in inferior 2.
+ set bar_lineno_1 \
+ [gdb_get_line_number "First location of bar" $srcfile]
+ set bar_lineno_2 \
+ [gdb_get_line_number "Second location of bar" $srcfile2]
+
+ # Check the b/p was created with a single location where we
+ # expect it.
+ gdb_test "info breakpoint $bpnum" \
+ [multi_line \
+ "" \
+ "$bpnum\\s+breakpoint\\s+keep\\s+y\\s+$hex\\s+in bar at \[^\r\n\]+/$srcfile:$bar_lineno_1 inf 1" \
+ "\\s+stop only in inferior 1"] \
+ "check original details for breakpoint on bar"
+
+ # Use the Python API to update the b/p's inferior.
+ gdb_test_no_output "python bp = gdb.breakpoints()\[0\]"
+ gdb_test_no_output "python bp.inferior = 2"
+
+ # We should still only have a single location, but now in
+ # inferior 2.
+ gdb_test "info breakpoint $bpnum" \
+ [multi_line \
+ "" \
+ "$bpnum\\s+breakpoint\\s+keep\\s+y\\s+$hex\\s+in bar at \[^\r\n\]+/$srcfile2:$bar_lineno_2 inf 2" \
+ "\\s+stop only in inferior 2"] \
+ "check updated details for breakpoint on bar"
+
+ # Use the Python API to remove the inferior restriction on the
+ # breakpoint.
+ gdb_test_no_output "python bp.inferior = None"
+
+ # The breakpoint should now have multiple locations.
+ gdb_test "info breakpoint $bpnum" \
+ [multi_line \
+ "" \
+ "$bpnum\\s+breakpoint\\s+keep\\s+y\\s+<MULTIPLE>\\s*" \
+ "$bpnum.1\\s+y\\s+$hex\\s+in bar at\[^\r\n\]+$srcfile:$bar_lineno_1 inf 1" \
+ "$bpnum.2\\s+y\\s+$hex\\s+in bar at\[^\r\n\]+$srcfile2:$bar_lineno_2 inf 2"] \
+ "check breakpoint bar now inferior requirement is gone"
+
+ # Finally, add the inferior requirement back.
+ gdb_test_no_output "python bp.inferior = 1"
+
+ # Check the original location and restriction is restored.
+ gdb_test "info breakpoint $bpnum" \
+ [multi_line \
+ "" \
+ "$bpnum\\s+breakpoint\\s+keep\\s+y\\s+$hex\\s+in bar at \[^\r\n\]+/$srcfile:$bar_lineno_1 inf 1" \
+ "\\s+stop only in inferior 1"] \
+ "check original details for breakpoint on bar are back"
+
+ delete_breakpoints
+ }
+}
+
# Use 'info breakpoint' to check that the inferior specific breakpoint is
# present in the breakpoint list. TESTNAME is the name used for this test,
# BP_NUMBER is the number for the breakpoint, and EXPECTED_LOC_COUNT is the
projects / thirdparty / binutils-gdb.git / commitdiff
