public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: Pedro Alves <pedro@palves.net>, gdb-patches@sourceware.org
Subject: Re: [PATCHv2 3/6] gdb: fix display of thread condition for multi-location breakpoints
Date: Mon, 06 Feb 2023 14:48:44 +0000	[thread overview]
Message-ID: <87h6vyx31v.fsf@redhat.com> (raw)
In-Reply-To: <c4b87868-a50c-a5bd-5096-987097b8d8eb@palves.net>

Pedro Alves <pedro@palves.net> writes:

> On 2023-01-20 9:46 a.m., Andrew Burgess via Gdb-patches wrote:
>> If a breakpoint with multiple locations has a thread condition, then
>> the 'info breakpoints' output is a little messed up, here's an example
>> of the current output:
>> 
>>   (gdb) break foo thread 1
>>   Breakpoint 2 at 0x401114: foo. (3 locations)
>>   (gdb) break bar thread 1
>>   Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
>>   (gdb) info breakpoints
>>   Num     Type           Disp Enb Address            What
>>   2       breakpoint     keep y   <MULTIPLE>          thread 1
>>           stop only in thread 1
>>   2.1                         y   0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>>   2.2                         y   0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>>   2.3                         y   0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>>   3       breakpoint     keep y   0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32 thread 1
>>           stop only in thread 1
>> 
>> Notice that, at the end of the location for breakpoint 3, the 'thread
>> 1' condition is printed, but this is then repeated on the next line
>> with 'stop only in thread 1'.
>> 
>> In contrast, for breakpoint 2, the 'thread 1' appears randomly, in the
>> "What" column, though slightly offset, non of the separate locations
>> have the 'thread 1' information.  Additionally for breakpoint 2 we
>> also get a 'stop only in thread 1' line.
>> 
>> There's two things going on here.  First the randomly placed 'thread
>> 1' for breakpoint 2 is due to a bug in print_one_breakpoint_location,
>> where we check the variable part_of_multiple instead of
>> header_of_multiple.
>> 
>> If I fix this oversight, then the output is now:
>> 
>>   (gdb) break foo thread 1
>>   Breakpoint 2 at 0x401114: foo. (3 locations)
>>   (gdb) break bar thread 1
>>   Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
>>   (gdb) info breakpoints
>>   Num     Type           Disp Enb Address            What
>>   2       breakpoint     keep y   <MULTIPLE>
>>           stop only in thread 1
>>   2.1                         y   0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
>>   2.2                         y   0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
>>   2.3                         y   0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
>>   3       breakpoint     keep y   0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32 thread 1
>>           stop only in thread 1
>> 
>> The 'thread 1' condition is now displayed at the end of each location,
>> which makes the output the same for single location breakpoints and
>> multi-location breakpoints.
>> 
>> However, there's still some duplication here.  Both breakpoints 2 and
>> 3 include a 'stop only in thread 1' line, and it feels like the
>> additional 'thread 1' is redundant.  In fact, there's a comment to
>> this very effect in the code:
>> 
>>   /* FIXME: This seems to be redundant and lost here; see the
>>      "stop only in" line a little further down.  */
>> 
>> So, lets fix this FIXME.  The new plan is to remove all the trailing
>> 'thread 1' markers from the CLI output, we now get this:
>> 
>>   (gdb) break foo thread 1
>>   Breakpoint 2 at 0x401114: foo. (3 locations)
>>   (gdb) break bar thread 1
>>   Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
>>   (gdb) info breakpoints
>>   Num     Type           Disp Enb Address            What
>>   2       breakpoint     keep y   <MULTIPLE>
>>           stop only in thread 1
>>   2.1                         y   0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>>   2.2                         y   0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>>   2.3                         y   0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>>   3       breakpoint     keep y   0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32
>>           stop only in thread 1
>> 
>
> This is the right direction, IMO.  The "thread 1" string is at best part of the breakpoint's
> location spec, not of the code locations that spec resolved to.
>
> If we went forward with my proposal to always show breakpoints using the multi-locations mode,
> and, to include the location spec in the "What" column of the breakpoint header, we could show:
>
>    (gdb) info breakpoints
>    Num     Type           Disp Enb Address            What
>    2       breakpoint     keep y                      foo thread 1
>            stop only in thread 1   
>     2.1                        y   0x0000000000401114  in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>     2.2                        y   0x0000000000401146  in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>     2.3                        y   0x0000000000401168  in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
>    3       breakpoint     keep y                      bar thread 1
>            stop only in thread 1
>     3.1    breakpoint     keep y   0x000000000040110a  in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32
>
> ... for instance.  Or not include the "thread 1".  But my point is that if we were to show it, that's where we would
> show it, not in the breapoint locations.

I dug out your multi-location proposal, and it looks good.  It's a shame
it got bogged down as it did....

>
>> All of the above points are also true for the Ada 'task' breakpoint
>> condition, and the changes I've made also update how the task
>> information is printed, though in the case of the Ada task there was
>> no 'stop only in task XXX' line printed, so I've added one of those.
>> 
>> Obviously it can't be quite that easy.  For MI backwards compatibility
>> I've retained the existing code (but now only for MI like outputs),
>> which ensures we should generate backwards compatible output.
>> 
>> I've extended an Ada test to cover the new task related output, and
>> updated all the tests I could find that checked for the old output.
>> ---
>>  gdb/breakpoint.c                              | 30 ++++----
>>  gdb/testsuite/gdb.ada/tasks.exp               | 15 ++--
>>  gdb/testsuite/gdb.base/save-bp.exp            |  2 +-
>>  gdb/testsuite/gdb.base/thread-bp-multi-loc.c  | 44 ++++++++++++
>>  .../gdb.base/thread-bp-multi-loc.exp          | 72 +++++++++++++++++++
>>  5 files changed, 141 insertions(+), 22 deletions(-)
>>  create mode 100644 gdb/testsuite/gdb.base/thread-bp-multi-loc.c
>>  create mode 100644 gdb/testsuite/gdb.base/thread-bp-multi-loc.exp
>> 
>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>> index 00cc2ab401c..b2cd89511fb 100644
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -6468,20 +6468,19 @@ print_one_breakpoint_location (struct breakpoint *b,
>>        output_thread_groups (uiout, "thread-groups", inf_nums, mi_only);
>>      }
>>  
>> -  if (!part_of_multiple)
>> +  /* In the MI output, each location of a thread or task specific
>> +     breakpoint is with the relevant thread or task ID.  This is done for
>> +     backwards compatibility reasons.
>
> Is "is with" idiomatic English meaning "includes"?  I don't think I ever heard that.
> Or is some word missing somewhere?  I'd suggest using "includes" or some
> such instead.

Fixed.

>
>> +
>> +     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 (b->thread != -1)
>> -	{
>> -	  /* FIXME: This seems to be redundant and lost here; see the
>> -	     "stop only in" line a little further down.  */
>> -	  uiout->text (" thread ");
>> -	  uiout->field_signed ("thread", b->thread);
>> -	}
>> +	uiout->field_signed ("thread", b->thread);
>>        else if (b->task != 0)
>> -	{
>> -	  uiout->text (" task ");
>> -	  uiout->field_signed ("task", b->task);
>> -	}
>> +	uiout->field_signed ("task", b->task);
>>      }
>>  
>>    uiout->text ("\n");
>> @@ -6536,7 +6535,14 @@ print_one_breakpoint_location (struct breakpoint *b,
>>  	}
>>        uiout->text ("\n");
>>      }
>> -  
>> +
>> +  if (!part_of_multiple && b->task != 0)
>> +    {
>> +      uiout->text ("\tstop only in task ");
>> +      uiout->field_signed ("task", b->task);
>> +      uiout->text ("\n");
>> +    }
>> +
>>    if (!part_of_multiple)
>>      {
>>        if (b->hit_count)
>> diff --git a/gdb/testsuite/gdb.ada/tasks.exp b/gdb/testsuite/gdb.ada/tasks.exp
>> index a9b58f20cf6..23bf3937a20 100644
>> --- a/gdb/testsuite/gdb.ada/tasks.exp
>> +++ b/gdb/testsuite/gdb.ada/tasks.exp
>> @@ -46,21 +46,18 @@ gdb_test "info tasks" \
>>  # breakpoint in the list that matched the triggered-breakpoint's
>>  # address, no matter which task it was specific to.
>>  gdb_test "break break_me task 1" "Breakpoint .* at .*"
>> +gdb_test "info breakpoints" "foo.adb:${decimal}\r\n\\s+stop only in task 1" \
>> +    "check info breakpoints for task 1 breakpoint"
>>  
>>  # Now, insert a breakpoint that should stop only if task 3 stops, and
>>  # extract its number.
>> -set bp_number -1
>> -set test "break break_me task 3"
>> -gdb_test_multiple $test $test {
>> -    -re "Breakpoint (.*) at .*$gdb_prompt $" {
>> -	set bp_number $expect_out(1,string)
>> -	pass $test
>> -    }
>> -}
>> -
>> +gdb_breakpoint "break_me task 3"
>> +set bp_number [get_integer_valueof "\$bpnum" -1]
>>  if {$bp_number < 0} {
>>      return
>>  }
>> +gdb_test "info breakpoints" "foo.adb:${decimal}\r\n\\s+stop only in task 3" \
>> +    "check info breakpoints for task 3 breakpoint"
>>  
>>  # Continue to that breakpoint.  Task 2 should hit it first, and GDB
>>  # is expected to ignore that hit and resume the execution.  Only then
>> diff --git a/gdb/testsuite/gdb.base/save-bp.exp b/gdb/testsuite/gdb.base/save-bp.exp
>> index a39712c7f5c..41d71837fb6 100644
>> --- a/gdb/testsuite/gdb.base/save-bp.exp
>> +++ b/gdb/testsuite/gdb.base/save-bp.exp
>> @@ -79,7 +79,7 @@ gdb_test_sequence "info break" "info break" [list				\
>>    "\[\r\n\]+Num +Type +Disp +Enb +Address +What"				\
>>    "\[\r\n\]+$bp_row_start break_me at \[^\r\n\]*$srcfile:\[0-9\]+"		\
>>    "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp2"			\
>> -  "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp3 +thread 1"	\
>> +  "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp3"			\
>>    "\[\r\n\]+\[ \t]+stop only in thread 1"					\
>>    "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp4"			\
>>    "\[\r\n\]+\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?"		\
>> diff --git a/gdb/testsuite/gdb.base/thread-bp-multi-loc.c b/gdb/testsuite/gdb.base/thread-bp-multi-loc.c
>> new file mode 100644
>> index 00000000000..cab009c39ec
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/thread-bp-multi-loc.c
>> @@ -0,0 +1,44 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2022 Free Software Foundation, Inc.
>
> Needs to include 2023 in range now.

Fixed.

>
>> +
>> +   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;
>> +}
>> +
>> +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.base/thread-bp-multi-loc.exp b/gdb/testsuite/gdb.base/thread-bp-multi-loc.exp
>> new file mode 100644
>> index 00000000000..9665fe9e21f
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/thread-bp-multi-loc.exp
>> @@ -0,0 +1,72 @@
>> +# Copyright 2022 Free Software Foundation, Inc.
>
> Ditto.
>
>> +
>> +# 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/>.
>> +
>> +# Create a multi-location breakpoint with a thread condition, then check the
>> +# output of 'info breakpoints' to ensure that the thread condition is
>> +# displayed correctly.
>> +
>> +standard_testfile
>> +
>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
>> +    return -1
>> +}
>> +
>> +if {![runto_main]} {
>> +    return -1
>> +}
>> +
>> +delete_breakpoints
>> +
>> +gdb_breakpoint "foo thread 1"
>> +set bp_number [get_integer_valueof "\$bpnum" 0]
>> +if { $bp_number == 0 } {
>> +    unresolved "breakpoint not placed correctly"
>> +    return -1
>> +}
>> +
>> +set saw_header false
>> +set saw_cond false
>> +set loc_count 0
>> +gdb_test_multiple "info breakpoints" "" {
>> +    -re "\r\nNum\\s+\[^\r\n\]+\r\n" {
>> +	exp_continue
>> +    }
>> +
>> +    -re "^$bp_number\\s+breakpoint\\s+keep\\s+y\\s+<MULTIPLE>\\s*\r\n" {
>> +	set saw_header true
>> +	exp_continue
>> +    }
>> +
>> +    -re "^\\s+stop only in thread 1\r\n" {
>> +	set saw_cond true
>> +	exp_continue
>> +    }
>> +
>> +    -re "^$bp_number\\.\[123\]\\s+\[^\r\n\]+:${decimal}\r\n" {
>> +	incr loc_count
>> +	exp_continue
>> +    }
>> +
>> +    -re "^$gdb_prompt $" {
>> +	with_test_prefix $gdb_test_name {
>> +	    gdb_assert { $saw_header } \
>> +		"saw header line"
>> +	    gdb_assert { $saw_cond } \
>> +		"saw b/p condition line"
>> +	    gdb_assert { $loc_count == 3 } \
>> +		"saw all three locations"
>
> It's better if the tests are written such that FAIL/PASSes match.  In this case,
> if gdb_test_multiple hits an internal pattern, then we'll have a single
>
>  FAIL: info breakpoints
>
> To fix that, we should merge those three gdb_asserts into a single gdb_assert,
> and we should $gdb_test_name for its test name, along with passing a meaningful
> test name to gdb_test_multiple, so internal FAILs get that meaningful name
> as well.  If we want to show the individual conditions, that can
> still be done by outputting them to the log.

Done.

I agree that, if/when your multi-location work is merged we might want
to once again adjust how this information is displayed, inline with your
suggestion above.

However... how would you feel if this patch (as shown below) was merged
now?  I think this fixes the stray "thread 1" text immediately, and the
multi-location display patch should be easily updated on top of this, if
that was something that you plan to continue developing?

Thanks,
Andrew

---

commit fb13c47255f0738644ac85f705f1dae4ec3890e7
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Nov 7 17:18:55 2022 +0000

    gdb: fix display of thread condition for multi-location breakpoints
    
    If a breakpoint with multiple locations has a thread condition, then
    the 'info breakpoints' output is a little messed up, here's an example
    of the current output:
    
      (gdb) break foo thread 1
      Breakpoint 2 at 0x401114: foo. (3 locations)
      (gdb) break bar thread 1
      Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
      (gdb) info breakpoints
      Num     Type           Disp Enb Address            What
      2       breakpoint     keep y   <MULTIPLE>          thread 1
              stop only in thread 1
      2.1                         y   0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
      2.2                         y   0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
      2.3                         y   0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
      3       breakpoint     keep y   0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32 thread 1
              stop only in thread 1
    
    Notice that, at the end of the location for breakpoint 3, the 'thread
    1' condition is printed, but this is then repeated on the next line
    with 'stop only in thread 1'.
    
    In contrast, for breakpoint 2, the 'thread 1' appears randomly, in the
    "What" column, though slightly offset, non of the separate locations
    have the 'thread 1' information.  Additionally for breakpoint 2 we
    also get a 'stop only in thread 1' line.
    
    There's two things going on here.  First the randomly placed 'thread
    1' for breakpoint 2 is due to a bug in print_one_breakpoint_location,
    where we check the variable part_of_multiple instead of
    header_of_multiple.
    
    If I fix this oversight, then the output is now:
    
      (gdb) break foo thread 1
      Breakpoint 2 at 0x401114: foo. (3 locations)
      (gdb) break bar thread 1
      Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
      (gdb) info breakpoints
      Num     Type           Disp Enb Address            What
      2       breakpoint     keep y   <MULTIPLE>
              stop only in thread 1
      2.1                         y   0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
      2.2                         y   0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
      2.3                         y   0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25 thread 1
      3       breakpoint     keep y   0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32 thread 1
              stop only in thread 1
    
    The 'thread 1' condition is now displayed at the end of each location,
    which makes the output the same for single location breakpoints and
    multi-location breakpoints.
    
    However, there's still some duplication here.  Both breakpoints 2 and
    3 include a 'stop only in thread 1' line, and it feels like the
    additional 'thread 1' is redundant.  In fact, there's a comment to
    this very effect in the code:
    
      /* FIXME: This seems to be redundant and lost here; see the
         "stop only in" line a little further down.  */
    
    So, lets fix this FIXME.  The new plan is to remove all the trailing
    'thread 1' markers from the CLI output, we now get this:
    
      (gdb) break foo thread 1
      Breakpoint 2 at 0x401114: foo. (3 locations)
      (gdb) break bar thread 1
      Breakpoint 3 at 0x40110a: file /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c, line 32.
      (gdb) info breakpoints
      Num     Type           Disp Enb Address            What
      2       breakpoint     keep y   <MULTIPLE>
              stop only in thread 1
      2.1                         y   0x0000000000401114 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
      2.2                         y   0x0000000000401146 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
      2.3                         y   0x0000000000401168 in foo at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:25
      3       breakpoint     keep y   0x000000000040110a in bar at /tmp/src/gdb/testsuite/gdb.base/thread-bp-multi-loc.c:32
              stop only in thread 1
    
    All of the above points are also true for the Ada 'task' breakpoint
    condition, and the changes I've made also update how the task
    information is printed, though in the case of the Ada task there was
    no 'stop only in task XXX' line printed, so I've added one of those.
    
    Obviously it can't be quite that easy.  For MI backwards compatibility
    I've retained the existing code (but now only for MI like outputs),
    which ensures we should generate backwards compatible output.
    
    I've extended an Ada test to cover the new task related output, and
    updated all the tests I could find that checked for the old output.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8bc62743bb5..adf38e7d722 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6468,20 +6468,19 @@ print_one_breakpoint_location (struct breakpoint *b,
       output_thread_groups (uiout, "thread-groups", inf_nums, mi_only);
     }
 
-  if (!part_of_multiple)
+  /* In the MI output, each location of a thread or task specific
+     breakpoint includes the relevant thread or task ID.  This is done for
+     backwards compatibility reasons.
+
+     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 (b->thread != -1)
-	{
-	  /* FIXME: This seems to be redundant and lost here; see the
-	     "stop only in" line a little further down.  */
-	  uiout->text (" thread ");
-	  uiout->field_signed ("thread", b->thread);
-	}
+	uiout->field_signed ("thread", b->thread);
       else if (b->task != 0)
-	{
-	  uiout->text (" task ");
-	  uiout->field_signed ("task", b->task);
-	}
+	uiout->field_signed ("task", b->task);
     }
 
   uiout->text ("\n");
@@ -6536,7 +6535,14 @@ print_one_breakpoint_location (struct breakpoint *b,
 	}
       uiout->text ("\n");
     }
-  
+
+  if (!part_of_multiple && b->task != 0)
+    {
+      uiout->text ("\tstop only in task ");
+      uiout->field_signed ("task", b->task);
+      uiout->text ("\n");
+    }
+
   if (!part_of_multiple)
     {
       if (b->hit_count)
diff --git a/gdb/testsuite/gdb.ada/tasks.exp b/gdb/testsuite/gdb.ada/tasks.exp
index 3eea04b3911..4441d92719c 100644
--- a/gdb/testsuite/gdb.ada/tasks.exp
+++ b/gdb/testsuite/gdb.ada/tasks.exp
@@ -50,21 +50,18 @@ gdb_test "watch j task 1 task 3" "You can specify only one task\\."
 # breakpoint in the list that matched the triggered-breakpoint's
 # address, no matter which task it was specific to.
 gdb_test "break break_me task 1" "Breakpoint .* at .*"
+gdb_test "info breakpoints" "foo.adb:${decimal}\r\n\\s+stop only in task 1" \
+    "check info breakpoints for task 1 breakpoint"
 
 # Now, insert a breakpoint that should stop only if task 3 stops, and
 # extract its number.
-set bp_number -1
-set test "break break_me task 3"
-gdb_test_multiple $test $test {
-    -re "Breakpoint (.*) at .*$gdb_prompt $" {
-	set bp_number $expect_out(1,string)
-	pass $test
-    }
-}
-
+gdb_breakpoint "break_me task 3"
+set bp_number [get_integer_valueof "\$bpnum" -1]
 if {$bp_number < 0} {
     return
 }
+gdb_test "info breakpoints" "foo.adb:${decimal}\r\n\\s+stop only in task 3" \
+    "check info breakpoints for task 3 breakpoint"
 
 # Continue to that breakpoint.  Task 2 should hit it first, and GDB
 # is expected to ignore that hit and resume the execution.  Only then
diff --git a/gdb/testsuite/gdb.base/save-bp.exp b/gdb/testsuite/gdb.base/save-bp.exp
index a39712c7f5c..41d71837fb6 100644
--- a/gdb/testsuite/gdb.base/save-bp.exp
+++ b/gdb/testsuite/gdb.base/save-bp.exp
@@ -79,7 +79,7 @@ gdb_test_sequence "info break" "info break" [list				\
   "\[\r\n\]+Num +Type +Disp +Enb +Address +What"				\
   "\[\r\n\]+$bp_row_start break_me at \[^\r\n\]*$srcfile:\[0-9\]+"		\
   "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp2"			\
-  "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp3 +thread 1"	\
+  "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp3"			\
   "\[\r\n\]+\[ \t]+stop only in thread 1"					\
   "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp4"			\
   "\[\r\n\]+\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?"		\
diff --git a/gdb/testsuite/gdb.base/thread-bp-multi-loc.c b/gdb/testsuite/gdb.base/thread-bp-multi-loc.c
new file mode 100644
index 00000000000..2adb179d93c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/thread-bp-multi-loc.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;
+}
+
+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.base/thread-bp-multi-loc.exp b/gdb/testsuite/gdb.base/thread-bp-multi-loc.exp
new file mode 100644
index 00000000000..6e1121e867a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/thread-bp-multi-loc.exp
@@ -0,0 +1,67 @@
+# 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/>.
+
+# Create a multi-location breakpoint with a thread condition, then check the
+# output of 'info breakpoints' to ensure that the thread condition is
+# displayed correctly.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+delete_breakpoints
+
+gdb_breakpoint "foo thread 1"
+set bp_number [get_integer_valueof "\$bpnum" 0]
+if { $bp_number == 0 } {
+    unresolved "breakpoint not placed correctly"
+    return -1
+}
+
+set saw_header false
+set saw_cond false
+set loc_count 0
+gdb_test_multiple "info breakpoints" \
+    "check thread condition is displayed correctly" {
+    -re "\r\nNum\\s+\[^\r\n\]+\r\n" {
+	exp_continue
+    }
+
+    -re "^$bp_number\\s+breakpoint\\s+keep\\s+y\\s+<MULTIPLE>\\s*\r\n" {
+	set saw_header true
+	exp_continue
+    }
+
+    -re "^\\s+stop only in thread 1\r\n" {
+	set saw_cond true
+	exp_continue
+    }
+
+    -re "^$bp_number\\.\[123\]\\s+\[^\r\n\]+:${decimal}\r\n" {
+	incr loc_count
+	exp_continue
+    }
+
+    -re "^$gdb_prompt $" {
+	gdb_assert { $saw_header && $saw_cond && $loc_count == 3} \
+	    $gdb_test_name
+    }
+}


  reply	other threads:[~2023-02-06 14:48 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28 11:25 [PATCH 0/6] Inferior specific breakpoints Andrew Burgess
2022-11-28 11:25 ` [PATCH 1/6] gdb/remote: announce thread exit events for remote targets Andrew Burgess
2022-11-28 11:25 ` [PATCH 2/6] gdb/testsuite: don't try to set non-stop mode on a running target Andrew Burgess
2022-11-28 11:25 ` [PATCH 3/6] gdb: fix display of thread condition for multi-location breakpoints Andrew Burgess
2022-12-23  8:37   ` Aktemur, Tankut Baris
2022-11-28 11:25 ` [PATCH 4/6] gdb: error if 'thread' or 'task' keywords are overused Andrew Burgess
2022-11-28 13:10   ` Eli Zaretskii
2022-11-28 11:25 ` [PATCH 5/6] gdb: add inferior-specific breakpoints and watchpoints Andrew Burgess
2022-11-28 13:18   ` Eli Zaretskii
2022-12-23 10:05   ` Aktemur, Tankut Baris
2023-01-19 19:13     ` Andrew Burgess
2023-01-20 13:12       ` Aktemur, Tankut Baris
2022-11-28 11:25 ` [PATCH 6/6] gdb: convert the 'start' breakpoint to use inferior keyword Andrew Burgess
2022-12-23 10:17   ` Aktemur, Tankut Baris
2022-12-23 10:55 ` [PATCH 0/6] Inferior specific breakpoints Aktemur, Tankut Baris
2023-01-20  9:46 ` [PATCHv2 " Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 1/6] gdb/remote: announce thread exit events for remote targets Andrew Burgess
2023-02-02 17:50     ` Pedro Alves
2023-02-04 15:46       ` Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 2/6] gdb/testsuite: don't try to set non-stop mode on a running target Andrew Burgess
2023-02-04 16:22     ` Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 3/6] gdb: fix display of thread condition for multi-location breakpoints Andrew Burgess
2023-02-02 18:13     ` Pedro Alves
2023-02-06 14:48       ` Andrew Burgess [this message]
2023-02-06 17:01         ` Pedro Alves
2023-02-07 14:42           ` Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 4/6] gdb: error if 'thread' or 'task' keywords are overused Andrew Burgess
2023-01-20 13:22     ` Eli Zaretskii
2023-02-02 14:08       ` Andrew Burgess
2023-02-02 14:31         ` Eli Zaretskii
2023-02-02 18:21     ` Pedro Alves
2023-02-03 16:41       ` Andrew Burgess
2023-02-04  5:52         ` Joel Brobecker
2023-02-04 15:40           ` Andrew Burgess
2023-02-06 11:06       ` Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 5/6] gdb: add inferior-specific breakpoints and watchpoints Andrew Burgess
2023-01-20 13:25     ` Eli Zaretskii
2023-02-02 19:17     ` Pedro Alves
2023-02-03 16:55       ` Andrew Burgess
2023-02-06 17:24         ` Pedro Alves
2023-02-16 12:56     ` Aktemur, Tankut Baris
2023-01-20  9:46   ` [PATCHv2 6/6] gdb: convert the 'start' breakpoint to use inferior keyword Andrew Burgess
2023-02-16 12:59     ` Aktemur, Tankut Baris
2023-03-16 17:03   ` [PATCHv3 0/2] Inferior specific breakpoints Andrew Burgess
2023-03-16 17:03     ` [PATCHv3 1/2] gdb: cleanup around some set_momentary_breakpoint_at_pc calls Andrew Burgess
2023-04-03 14:12       ` Andrew Burgess
2023-03-16 17:03     ` [PATCHv3 2/2] gdb: add inferior-specific breakpoints Andrew Burgess
2023-04-03 14:14     ` [PATCHv4] " Andrew Burgess
2023-05-15 19:15       ` [PATCHv5] " Andrew Burgess
2023-05-30 20:41         ` [PATCHv6] " Andrew Burgess
2023-07-07 10:23           ` [PATCHv7] " Andrew Burgess
2023-08-17 15:53             ` [PUSHEDv8] " Andrew Burgess
2023-08-23  8:06               ` [PUSHED] gdb: add missing notify_breakpoint_modified call Andrew Burgess
2023-08-23  8:19               ` [PUSHED] gdb/testsuite: improve MI support for inferior specific breakpoints Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h6vyx31v.fsf@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@palves.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).