Re: [PATCHv2 1/2] gdb: fix 'list' for multiple source file results

[Kevin Buettner <kevinb@redhat.com>]

On Wed, 19 Nov 2025 10:32:37 +0000
Andrew Burgess <aburgess@redhat.com> wrote:

> From: Andrew Burgess <aburgess@redhat.com>
> To: gdb-patches@sourceware.org
> Cc: Andrew Burgess <aburgess@redhat.com>
> Subject: [PATCHv2 1/2] gdb: fix 'list' for multiple source file results
> Date: Wed, 19 Nov 2025 10:32:37 +0000
> 
> This commit:
> 
>   commit c7a45b98a61451f05ff654c4fb72a9c9cb2fba36
>   Date:   Thu Jun 12 15:37:50 2025 +0000
> 
>       gdb, linespec: avoid multiple locations with same PC
> 
> broke GDB's ability to list multiple source files using a 'list'
> command.  In GDB 16 and earlier something like 'list foo.c:10' could
> print multiple results if there were multiple 'foo.c' files compiled
> into the executable.
> 
> The above commit added a filter to add_sal_to_sals (linespec.c) such
> that multiple sals in the same program space, but with the same pc
> value, could not be added, only the first sal would actually be
> recorded.  The problem with this is that add_sal_to_sals is used from
> decode_digits_list_mode (also linespec.c) where the pc value is forced
> to zero.  This force to zero makes sense I think as there might not be
> any compiled code for the requested line (this is for 'list' after
> all), so there might not be a valid pc to use.
> 
> I'm not a fan of using '0' as a special pc value, there are embedded
> targets where 0 is a valid pc value, but given we're already using 0
> here, I propose to just roll with it.
> 
> So, my proposal is that, if the pc is 0, add_sal_to_sals should always
> add the sal.  This fixes the decode_digits_list_mode, but should keep
> the fix that c7a45b98a614 introduced.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33647
> ---
>  gdb/linespec.c                               |  15 ++-
>  gdb/testsuite/gdb.base/list-multi-source.c   |  60 +++++++++
>  gdb/testsuite/gdb.base/list-multi-source.exp | 129 +++++++++++++++++++
>  3 files changed, 199 insertions(+), 5 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/list-multi-source.c
>  create mode 100644 gdb/testsuite/gdb.base/list-multi-source.exp

LGTM, also.  I, too, think it should go into 17.

Reviewed-by: Kevin Buettner <kevinb@redhat.com>