gdb: don't duplicate 'thread' field in MI breakpoint output

When creating a thread-specific breakpoint with a single location, the
'thread' field would be repeated in the MI output.  This can be seen
in two existing tests gdb.mi/mi-nsmoribund.exp and
gdb.mi/mi-pending.exp, e.g.:

  (gdb)
  -break-insert -p 1 bar
  ^done,bkpt={number="1",type="breakpoint",disp="keep",
	      enabled="y",
	      addr="0x000000000040110a",func="bar",
	      file="/tmp/mi-thread-specific-bp.c",
	      fullname="/tmp/mi-thread-specific-bp.c",
	      line="32",thread-groups=["i1"],
	      thread="1",thread="1",  <================ DUPLICATION!
	      times="0",original-location="bar"}

I know we need to be careful when adjusting MI output, but I'm hopeful
in this case, as the field is duplicated, and the field contents are
always identical, that we might get away with removing one of the
duplicates.

The change in GDB is a fairly trivial condition change.

We did have a couple of tests that contained the duplicate fields in
their expected output, but given there was no comment pointing out
this oddity either in the GDB code, or in the test, I suspect this was
more a case of copying whatever output GDB produced and using that as
the expected results.  I've updated these tests to remove the
duplication.

I've update lib/mi-support.exp to provide support for building
breakpoint patterns that contain the thread field, and I've made use
of this in a new test I've added that is just about creating
thread-specific breakpoints and checking the results.  The two tests I
mentioned above as being updated could also use the new
lib/mi-support.exp functionality, but I'm going to do that in a later
patch, this way it is clear what changes I'm actually proposing to
make to the expected output.

As I said, I hope that frontends will be able to handle this change,
but I still think its worth adding a NEWS entry, that way, if someone
runs into problems, there's a chance they can figure out what's going
on.

This should not impact CLI output at all.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Pedro Alves <pedro@palves.net>

diff --git a/gdb/NEWS b/gdb/NEWS
index 75cd11b204eb89a20667701fcc2fc13c7502c626..bea604d7e756b26a98e4cdf4b4ff92124d9d7939 100644 (file)
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -83,6 +83,14 @@ maintenance info frame-unwinders
 ** mi now reports 'no-history' as a stop reason when hitting the end of the
    reverse execution history.
 
+** When creating a thread-specific breakpoint using the '-p' option,
+   the -break-insert command would report the 'thread' field twice in
+   the reply.  The content of both fields was always identical.  This
+   has now been fixed; the 'thread' field will be reported just once
+   for thread-specific breakpoints, or not at all for breakpoints
+   without a thread restriction.  The same is also true for the 'task'
+   field of an Ada task-specific breakpoint.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7d11f7199fe4786cb91b44919bcb0a50facfc28e..36c53e5dae611b971825a8c7aa35ed947472f089 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6480,7 +6480,7 @@ print_one_breakpoint_location (struct breakpoint *b,
      For the CLI output, the thread/task information is printed on a
      separate line, see the 'stop only in thread' and 'stop only in task'
      output below.  */
-  if (!header_of_multiple && uiout->is_mi_like_p ())
+  if (part_of_multiple && uiout->is_mi_like_p ())
     {
       if (b->thread != -1)
        uiout->field_signed ("thread", b->thread);

diff --git a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
index 103aa45d7e1d40136674e2c00ace86aa02f38271..55450e4621ac7fb192d9e14c5b58d675ddce1b75 100644 (file)
--- a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
+++ b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
@@ -74,7 +74,7 @@ mi_delete_breakpoints
 
 # Recreate the same breakpoint, but this time, specific to thread 5.
 mi_gdb_test "234-break-insert -p 5 $srcfile:$bkpt_line" \
-    "234\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\".*\",func=\"thread_function\",file=\".*\",fullname=\".*\",line=\".*\",thread-groups=\\\[\".*\"\\\],thread=\"5\",thread=\"5\",times=\"0\",original-location=\".*\"\}" \
+    "234\\^done,bkpt=\{number=\"3\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\".*\",func=\"thread_function\",file=\".*\",fullname=\".*\",line=\".*\",thread-groups=\\\[\".*\"\\\],thread=\"5\",times=\"0\",original-location=\".*\"\}" \
     "thread specific breakpoint at thread_function" 
 
 # Resume all threads.  Only thread 5 should report a stop.

diff --git a/gdb/testsuite/gdb.mi/mi-pending.exp b/gdb/testsuite/gdb.mi/mi-pending.exp
index 153efdf45ceebe95c6cb2b0b4792d86780b5e9cf..cd1301c21e34976c8ed8ff8b3f3b057b6bfc3abb 100644 (file)
--- a/gdb/testsuite/gdb.mi/mi-pending.exp
+++ b/gdb/testsuite/gdb.mi/mi-pending.exp
@@ -98,7 +98,7 @@ mi_gdb_test "-break-delete 3" "\\^done" "delete breakpoint 3"
 
 # Set pending breakpoint with a thread via MI.
 mi_gdb_test "-break-insert -p 2 -f pendfunc3" \
-    ".*\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc3\",thread=\"2\",thread=\"2\",times=\"0\",original-location=\"pendfunc3\"\}"\
+    ".*\\^done,bkpt=\{number=\"4\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"<PENDING>\",pending=\"pendfunc3\",thread=\"2\",times=\"0\",original-location=\"pendfunc3\"\}"\
     "MI pending breakpoint on pendfunc3"
 
 mi_send_resuming_command "exec-continue" "continuing execution to thread condition"

diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.c b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.c
new file mode 100644 (file)
index 0000000..8c87f01
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.c
@@ -0,0 +1,44 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2022-2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+volatile int global_var = 0;
+
+__attribute__((__always_inline__)) static inline void
+foo (void)
+{
+  int i;
+
+  for (i = 0; i < 10; ++i)
+    global_var = i;
+}
+
+__attribute__((__noinline__)) static void
+bar (void)
+{
+  global_var = 0;
+  foo ();
+}
+
+int
+main (void)
+{
+  global_var = 0;
+  foo ();
+  bar ();
+  foo ();
+  return 0;
+}

diff --git a/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
new file mode 100644 (file)
index 0000000..4586fa4
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp
@@ -0,0 +1,49 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test is for creating thread-specific breakpoint using the MI,
+# and checking the results from GDB.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+if {[mi_clean_restart]} {
+    return
+}
+
+standard_testfile
+
+if [build_executable ${testfile}.exp ${binfile} ${srcfile}] {
+    return -1
+}
+
+if {[mi_clean_restart $binfile]} {
+    return -1
+}
+
+mi_create_breakpoint "-p 1 bar" "thread-specific b/p on bar" \
+    -thread "1"
+
+proc make_loc {num} {
+    return [mi_make_breakpoint_loc -thread "1" -number "$::decimal\\.$num"]
+}
+
+set loc1 [make_loc 1]
+set loc2 [make_loc 2]
+set loc3 [make_loc 3]
+
+mi_create_breakpoint_multi "-p 1 foo" "thread-specific b/p on foo" \
+    -thread "1" \
+    -locations "\\\[$loc1,$loc2,$loc3\\\]"

diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 767ea72ff7026b4e01ce420fd8daa8c334750a20..3e20afa70494512b211a235a5c4ce91e569941e5 100644 (file)
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2498,8 +2498,11 @@ proc mi_build_kv_pairs {attr_list {joiner ,}} {
 # locations.
 #
 # All arguments for the breakpoint location may be specified using the
-# options number, enabled, addr, func, file, fullname, line and
-# thread-groups.
+# options: number, enabled, addr, func, file, fullname, line,
+# thread-groups, and thread.
+#
+# For the option -thread the corresponding output field is only added
+# if the option is present and not set to the empty string.
 #
 # Example: mi_make_breakpoint_loc -number 2.1 -file ".*/myfile.c" -line 3
 # will return the breakpoint location:
@@ -2509,7 +2512,7 @@ proc mi_build_kv_pairs {attr_list {joiner ,}} {
 proc mi_make_breakpoint_loc {args} {
     parse_args {{number .*} {enabled .*} {addr .*}
        {func .*} {file .*} {fullname .*} {line .*}
-       {thread-groups \\\[.*\\\]}}
+       {thread-groups \\\[.*\\\]} {thread ""}}
 
     set attr_list {}
     foreach attr [list number enabled addr func file \
@@ -2517,18 +2520,31 @@ proc mi_make_breakpoint_loc {args} {
        lappend attr_list $attr [set $attr]
     }
 
-    return "{[mi_build_kv_pairs $attr_list]}"
+    set result [mi_build_kv_pairs $attr_list]
+
+    if {[string length $thread] > 0} {
+       append result ","
+       append result [mi_build_kv_pairs [list "thread" $thread]]
+    }
+
+    return "{$result}"
 }
 
 # Bits shared between mi_make_breakpoint and mi_make_breakpoint_multi.
 
-proc mi_make_breakpoint_1 {attr_list cond evaluated-by times \
+proc mi_make_breakpoint_1 {attr_list thread cond evaluated-by times \
                           ignore script original-location} {
     set result "bkpt=\\\{[mi_build_kv_pairs $attr_list]"
 
     # There are always exceptions.
 
-    # If COND is not preset, do not output it.
+    # If THREAD is not present, do not output it.
+    if {[string length $thread] > 0} {
+       append result ","
+       append result [mi_build_kv_pairs [list "thread" $thread]]
+    }
+
+    # If COND is not present, do not output it.
     if {[string length $cond] > 0} {
        append result ","
        append result [mi_build_kv_pairs [list "cond" $cond]]
@@ -2574,13 +2590,17 @@ proc mi_make_breakpoint_1 {attr_list cond evaluated-by times \
 # -dprintf-insert, or -break-info with breakpoints with multiple
 # locations.
 #
-# All arguments for the breakpoint may be specified using the options
-# number, type, disp, enabled, func, cond, evaluated-by, times,
-# ignore, script and locations.
+# All arguments for the breakpoint may be specified using the options:
+# number, type, disp, enabled, times, ignore, script,
+# original-location, cond, evaluated-by, locations, and thread.
 #
 # Only if -script and -ignore are given will they appear in the output.
 # Otherwise, this procedure will skip them using ".*".
 #
+# For the options -thread and -cond the corresponding output fields
+# are only added if the options are present and not set to the empty
+# string.
+#
 # Example: mi_make_breakpoint_multi -number 2 -locations "$loc"
 # will return the breakpoint:
 # bkpt={number="2",type=".*",disp=".*",enabled=".*",addr="<MULTIPLE>",
@@ -2592,7 +2612,7 @@ proc mi_make_breakpoint_multi {args} {
     parse_args {{number .*} {type .*} {disp .*} {enabled .*}
        {times .*} {ignore 0}
        {script ""} {original-location .*} {cond ""} {evaluated-by ""}
-       {locations .*}}
+       {locations .*} {thread ""}}
 
     set attr_list {}
     foreach attr [list number type disp enabled] {
@@ -2602,7 +2622,7 @@ proc mi_make_breakpoint_multi {args} {
     lappend attr_list "addr" "<MULTIPLE>"
 
     set result [mi_make_breakpoint_1 \
-                   $attr_list $cond ${evaluated-by} $times \
+                   $attr_list $thread $cond ${evaluated-by} $times \
                    $ignore $script ${original-location}]
 
     append result ","
@@ -2616,8 +2636,11 @@ proc mi_make_breakpoint_multi {args} {
 # be used to test the output of -break-insert, -dprintf-insert, or
 # -break-info for pending breakpoints.
 #
-# Arguments for the breakpoint may be specified using the options
-# number, type, disp, enabled, pending.
+# Arguments for the breakpoint may be specified using the options:
+# number, type, disp, enabled, pending, original-location, and thread.
+#
+# For the option -thread the corresponding output field is only
+# included if the option is present and not the empty string.
 #
 # Example: mi_make_breakpoint_pending -number 2 -pending func
 # will return the breakpoint:
@@ -2626,7 +2649,7 @@ proc mi_make_breakpoint_multi {args} {
 
 proc mi_make_breakpoint_pending {args} {
     parse_args {{number .*} {type .*} {disp .*} {enabled .*}
-       {pending .*} {original-location .*}}
+       {pending .*} {original-location .*} {thread ""}}
 
     set attr_list {}
     foreach attr [list number type disp enabled] {
@@ -2646,7 +2669,7 @@ proc mi_make_breakpoint_pending {args} {
     set evaluated-by ""
 
     set result [mi_make_breakpoint_1 \
-                   $attr_list $cond ${evaluated-by} $times \
+                   $attr_list $thread $cond ${evaluated-by} $times \
                    $ignore $script ${original-location}]
 
     append result "\\\}"
@@ -2674,7 +2697,8 @@ proc mi_make_breakpoint {args} {
     parse_args {{number .*} {type .*} {disp .*} {enabled .*} {addr .*}
        {func .*} {file .*} {fullname .*} {line .*}
        {thread-groups \\\[.*\\\]} {times .*} {ignore 0}
-       {script ""} {original-location .*} {cond ""} {evaluated-by ""}}
+       {script ""} {original-location .*} {cond ""} {evaluated-by ""}
+       {thread ""}}
 
     set attr_list {}
     foreach attr [list number type disp enabled addr func file \
@@ -2683,7 +2707,7 @@ proc mi_make_breakpoint {args} {
     }
 
     set result [mi_make_breakpoint_1 \
-                   $attr_list $cond ${evaluated-by} $times \
+                   $attr_list $thread $cond ${evaluated-by} $times \
                    $ignore $script ${original-location}]
 
     append result "\\\}"