[PATCH 2/8] gdb: don't duplicate 'thread' field in MI breakpoint output

[Andrew Burgess <aburgess@redhat.com>]

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 was 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.
---
 gdb/NEWS                                      |  8 +++
 gdb/breakpoint.c                              |  2 +-
 gdb/testsuite/gdb.mi/mi-nsmoribund.exp        |  2 +-
 gdb/testsuite/gdb.mi/mi-pending.exp           |  2 +-
 gdb/testsuite/gdb.mi/mi-thread-specific-bp.c  | 44 +++++++++++++++++
 .../gdb.mi/mi-thread-specific-bp.exp          | 49 +++++++++++++++++++
 gdb/testsuite/lib/mi-support.exp              | 32 ++++++++----
 7 files changed, 127 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-thread-specific-bp.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-thread-specific-bp.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 75cd11b204e..bea604d7e75 100644
--- 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 902119c7d2c..f3121c8d6eb 100644
--- 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 103aa45d7e1..55450e4621a 100644
--- 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 153efdf45ce..cd1301c21e3 100644
--- 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
index 00000000000..8c87f01f14b
--- /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
index 00000000000..4586fa44bbc
--- /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 767ea72ff70..0963701140e 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2509,7 +2509,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,17 +2517,30 @@ 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 THREAD is not preset, do not output it.
+    if {[string length $thread] > 0} {
+	append result ","
+	append result [mi_build_kv_pairs [list "thread" $thread]]
+    }
+
     # If COND is not preset, do not output it.
     if {[string length $cond] > 0} {
 	append result ","
@@ -2592,7 +2605,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 +2615,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 ","
@@ -2626,7 +2639,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 +2659,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 +2687,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 +2697,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 "\\\}"
-- 
2.25.4