Re: [PATCHv2 3/6] gdb: fix display of thread condition for multi-location breakpoints

[Pedro Alves <pedro@palves.net>]

On 2023-02-06 2:48 p.m., Andrew Burgess wrote:
> Pedro Alves <pedro@palves.net> writes:
> 

>>    (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....

Thanks.  I still hope to get back to it at some point, but probably only after I manage to
be done with the step over thread clone/exit series, and the big ctrl-c rework.

>> 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.

Thank you.

> 
> 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?

Oh yes, I totally agree with merging your patch.  I only referenced the
multi-locations proposal to help with justifying why I think your patch
is good.  I did not mean to suggest that getting my proposal in would
remove the need for your patch.

In sum, in my view, code locations are not thread specific, so the "thread N" part
should not be displayed as if part of the location.

Approved-By: Pedro Alves <pedro@palves.net>

Pedro Alves