From 442716d400655e252f3faf93ae17ec410e73869d Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Wed, 8 Feb 2023 13:56:22 +0000 Subject: [PATCH] gdb: don't use the global thread-id in the saved breakpoints file I noticed that breakpoint::print_recreate_thread was printing the global thread-id. This function is used to implement the 'save breakpoints' command, and should be writing out suitable CLI commands for recreating the current breakpoints. The CLI does not use global thread-ids, but instead uses the inferior specific thread-ids, e.g. "2.1". After some discussion on the mailing list it was suggested that the most consistent solution would be for the saved breakpoints file to always contain the inferior-qualified thread-id, so the file would include "thread 1.1" instead of just "thread 1", even when there is only a single inferior. So, this commit adds print_full_thread_id, which is just like the existing print_thread_id, only it always prints the inferior-qualified thread-id. I then update the existing print_thread_id to make use of this new function, and finally, I update breakpoint::print_recreate_thread to also use this new function. There's a multi-inferior test that confirms the saved breakpoints file correctly includes the fully-qualified thread-id, and I've also updated the single inferior test gdb.base/save-bp.exp to have it validate that the saved breakpoints file includes the inferior-qualified thread-id, even for this single inferior case. --- gdb/breakpoint.c | 5 +- gdb/gdbthread.h | 4 ++ gdb/testsuite/gdb.base/save-bp.exp | 16 +++++++ .../gdb.multi/bp-thread-specific.exp | 46 +++++++++++++++++++ gdb/thread.c | 19 ++++++-- 5 files changed, 85 insertions(+), 5 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 51bea46256f..7228acfd8fe 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -14142,7 +14142,10 @@ void breakpoint::print_recreate_thread (struct ui_file *fp) const { if (thread != -1) - gdb_printf (fp, " thread %d", thread); + { + struct thread_info *thr = find_thread_global_id (thread); + gdb_printf (fp, " thread %s", print_full_thread_id (thr)); + } if (task != -1) gdb_printf (fp, " task %d", task); diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h index c0f27a8a66e..848daa94410 100644 --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -661,6 +661,10 @@ extern int show_inferior_qualified_tids (void); circular static buffer, NUMCELLS deep. */ const char *print_thread_id (struct thread_info *thr); +/* Like print_thread_id, but always prints the inferior-qualified form, + even when there is only a single inferior. */ +const char *print_full_thread_id (struct thread_info *thr); + /* Boolean test for an already-known ptid. */ extern bool in_thread_list (process_stratum_target *targ, ptid_t ptid); diff --git a/gdb/testsuite/gdb.base/save-bp.exp b/gdb/testsuite/gdb.base/save-bp.exp index 41d71837fb6..68933d36427 100644 --- a/gdb/testsuite/gdb.base/save-bp.exp +++ b/gdb/testsuite/gdb.base/save-bp.exp @@ -89,3 +89,19 @@ gdb_test_sequence "info break" "info break" [list \ "\[\r\n\]+\[ \t\]+printf" \ "\[\r\n\]+$disabled_row_start main at \[^\r\n\]*$srcfile:$loc_bp8" \ ] + +# Copy the saved breakpoints file to the local machine (if necessary), +# and then check its contents. +if {[is_remote host]} { + set bps [remote_upload host bps [standard_output_file bps]] +} +set fh [open $bps] +set lines [split [read $fh] "\n"] +close $fh + +with_test_prefix "in bps file" { + gdb_assert {[lsearch -regexp $lines "break ${srcfile}:${loc_bp2}$"] != -1} \ + "check for general breakpoint" + gdb_assert {[lsearch -regexp $lines "break ${srcfile}:${loc_bp3} thread 1\\.1"] != -1} \ + "check for thread-specific breakpoint" +} diff --git a/gdb/testsuite/gdb.multi/bp-thread-specific.exp b/gdb/testsuite/gdb.multi/bp-thread-specific.exp index 777fcf85ab0..85c08f44a2c 100644 --- a/gdb/testsuite/gdb.multi/bp-thread-specific.exp +++ b/gdb/testsuite/gdb.multi/bp-thread-specific.exp @@ -15,6 +15,9 @@ # Check that GDB uses the correct thread-id when describing multiple # thread specific breakpoints at the same location. +# +# Also check that the correct thread-ids are used in the saved +# breakpoints file. # The plain remote target can't do multiple inferiors. require !use_gdb_stub @@ -59,3 +62,46 @@ gdb_test "break foo thread 1.1" \ "Note: breakpoint $bpnum \\(thread 2.1\\) also set at pc $hex\\." \ "Note: breakpoint $bpnum \\(thread 2.1\\) also set at pc $hex\\." \ "Breakpoint $decimal at $hex: foo\\. \\(2 locations\\)"] + +# Save the breakpoints into a file. +if {[is_remote host]} { + set bps bps +} else { + set bps [standard_output_file bps] +} + +remote_file host delete "$bps" +gdb_test "save breakpoints $bps" "" "save breakpoint to bps" + +if {[is_remote host]} { + set bps [remote_upload host bps [standard_output_file bps]] +} + +# Now dig through the saved breakpoints file and check that the +# thread-ids were written out correctly. First open the saved +# breakpoints and read them into a list. +set fh [open $bps] +set lines [split [read $fh] "\n"] +close $fh + +# Except the list created from the saved breakpoints file will have a +# blank line entry at the end, so remove it now. +gdb_assert {[string equal [lindex $lines end] ""]} \ + "check last item was an empty line" +set lines [lrange $lines 0 end-1] + +# These are the lines we expect in the saved breakpoints file, in the +# order that we expect them. These are strings, not regexps. +set expected_results \ + [list \ + "break -qualified main" \ + "break foo thread 2.1" \ + "break foo thread 1.1"] + +# Now check that the files contents (in LINES) matches the +# EXPECTED_RESULTS. +gdb_assert {[llength $lines] == [llength $expected_results]} \ + "correct number of lines in saved breakpoints file" +foreach a $lines b $expected_results { + gdb_assert {[string equal $a $b]} "line '$b'" +} diff --git a/gdb/thread.c b/gdb/thread.c index e4d98ce966a..3d4cba32303 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -1431,14 +1431,25 @@ show_inferior_qualified_tids (void) const char * print_thread_id (struct thread_info *thr) { + if (show_inferior_qualified_tids ()) + return print_full_thread_id (thr); + + char *s = get_print_cell (); + gdb_assert (thr != nullptr); + xsnprintf (s, PRINT_CELL_SIZE, "%d", thr->per_inf_num); + return s; +} +/* See gdbthread.h. */ + +const char * +print_full_thread_id (struct thread_info *thr) +{ char *s = get_print_cell (); - if (show_inferior_qualified_tids ()) - xsnprintf (s, PRINT_CELL_SIZE, "%d.%d", thr->inf->num, thr->per_inf_num); - else - xsnprintf (s, PRINT_CELL_SIZE, "%d", thr->per_inf_num); + gdb_assert (thr != nullptr); + xsnprintf (s, PRINT_CELL_SIZE, "%d.%d", thr->inf->num, thr->per_inf_num); return s; } -- 2.30.2