[PATCH 0/2] Fix regression in 'list' command

[PATCH 0/2] Fix regression in 'list' command

[Andrew Burgess]

The first patch fixes a regression in the 'list' command which has
been introduced since GDB 16, so should probably be fixed before GDB
17 is released.

The second patch is an improvement, it's not part of the regression
fix.

If approved then I plan to backport the first patch only to the GDB 17
branch.  Both patches would be applied to the master branch.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb: fix 'list' for multiple source file results
  gdb: display a symbol more often in multi-file list output

 gdb/cli/cli-cmds.c                           |  8 ++
 gdb/linespec.c                               | 15 ++--
 gdb/testsuite/gdb.base/list-multi-source.c   | 60 +++++++++++++
 gdb/testsuite/gdb.base/list-multi-source.exp | 95 ++++++++++++++++++++
 4 files changed, 173 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


base-commit: ccd56e22703c094e5b6d0025e28059277ed8d8c5
-- 
2.47.1

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

[Andrew Burgess]

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 | 95 ++++++++++++++++++++
 3 files changed, 165 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

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 277b45b4b7f..2cbfe2fcc20 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1071,11 +1071,16 @@ add_sal_to_sals (struct linespec_state *self,
 		 const symtab_and_line &sal,
 		 const char *symname, bool literal_canonical)
 {
-  /* We don't want two SALs with the same PC from the
-     same program space.  */
-  for (const auto &s : sals)
-    if (sal.pc == s.pc && sal.pspace == s.pspace)
-     return;
+  /* We don't want two SALs with the same PC from the same program space.
+     However, for the 'list' command we force the pc value to be 0, and in
+     this case we do want to see all SALs.  See decode_digits_list_mode
+     for where the 0 originates from.  */
+  if (sal.pc != 0)
+    {
+      for (const auto &s : sals)
+	if (sal.pc == s.pc && sal.pspace == s.pspace)
+	  return;
+    }
 
   sals.push_back (sal);
 
diff --git a/gdb/testsuite/gdb.base/list-multi-source.c b/gdb/testsuite/gdb.base/list-multi-source.c
new file mode 100644
index 00000000000..ab2b6a01b36
--- /dev/null
+++ b/gdb/testsuite/gdb.base/list-multi-source.c
@@ -0,0 +1,60 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+extern int function_a (void);
+extern int function_b (void);
+
+#ifdef MAIN
+int
+main (void)
+{
+  int res;
+
+  res = function_a ();
+
+  res += function_b ();
+
+  return res;
+}
+#endif
+
+#if defined FILE_A || defined FILE_B
+static int
+get_value_common (void)
+{
+  /* NOTE: When reading this file in the source tree, the variable used in
+     the return statement below will be replaced by a constant value when
+     the file is copied into the source tree.  */
+  return value_to_return;	/* List this line.  */
+}
+#endif
+
+#ifdef FILE_A
+int
+function_a (void)
+{
+  return get_value_common ();
+}
+#endif
+
+#ifdef FILE_B
+int
+function_b (void)
+{
+  return get_value_common ();
+}
+#endif
diff --git a/gdb/testsuite/gdb.base/list-multi-source.exp b/gdb/testsuite/gdb.base/list-multi-source.exp
new file mode 100644
index 00000000000..7809167bd0f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/list-multi-source.exp
@@ -0,0 +1,95 @@
+# Copyright 2025 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/>.
+
+# Test that 'list FILE:LINE' can print multiple results if FILE
+# matches multiple files from the source tree.
+#
+# Then test that we can use 'list DIR/FILE:LINE' to restrict the
+# results to a single source file.
+
+# This test uses a single source file that is copied into the build
+# tree 3 times.  The three copies are then copied with different
+# defines set so that we see different functions in each copy.
+standard_testfile .c
+
+# Create the source tree within the build directory.
+set src_root [standard_output_file "src"]
+set src_a "$src_root/a"
+set src_b "$src_root/b"
+set file_a "$src_a/foo.c"
+set file_b "$src_b/foo.c"
+set file_main "$src_root/main.c"
+remote_exec build "mkdir -p \"$src_root\""
+remote_exec build "mkdir -p \"$src_a\""
+remote_exec build "mkdir -p \"$src_b\""
+
+# Make three copies of the single source file in the build directory
+# based source tree.  Two of the source files are modified slighly to
+# make the output of 'list' unique for each copy.
+remote_exec build "cp \"$srcdir/$subdir/$srcfile\" \"$file_main\""
+remote_exec build "cp \"$srcdir/$subdir/$srcfile\" \"$file_a\""
+remote_exec build "cp \"$srcdir/$subdir/$srcfile\" \"$file_b\""
+remote_exec build "sed -i -e \"s/value_to_return/3/\" \"$file_a\""
+remote_exec build "sed -i -e \"s/value_to_return/-3/\" \"$file_b\""
+
+# Build the executable.  Use defines to make the source files
+# different.
+if { [prepare_for_testing_full "failed to prepare" \
+	  [list $testfile debug \
+	       $file_main [list debug additional_flags=-DMAIN] \
+	       $file_a [list debug additional_flags=-DFILE_A] \
+	       $file_b [list debug additional_flags=-DFILE_B]]]} {
+    return
+}
+
+# The LINENUM we should list, and the first and last lines that should
+# appear in the list output.
+set linenum [gdb_get_line_number "List this line"]
+set first_linenum [expr {$linenum - 5}]
+set last_linenum [expr {$linenum + 4}]
+
+# List using FILE:LINE for a filename that is ambiguous.
+gdb_test "list foo.c:$linenum" \
+    [multi_line \
+	 "file: \"\[^\r\n\]+/a/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return 3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+" \
+	 "file: \"\[^\r\n\]+/b/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return -3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]
+
+# Now list using a more acurate filename, we should only get a single
+# result.
+gdb_test "list a/foo.c:$linenum" \
+    [multi_line \
+	 "^$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return 3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]
+
+gdb_test "list b/foo.c:$linenum" \
+    [multi_line \
+	 "^$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return -3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]
-- 
2.47.1

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

[Andreas Schwab]

On Nov 18 2025, Andrew Burgess wrote:

> +remote_exec build "cp \"$srcdir/$subdir/$srcfile\" \"$file_a\""
> +remote_exec build "cp \"$srcdir/$subdir/$srcfile\" \"$file_b\""
> +remote_exec build "sed -i -e \"s/value_to_return/3/\" \"$file_a\""
> +remote_exec build "sed -i -e \"s/value_to_return/-3/\" \"$file_b\""

If you combine the cp and sed commands then you don't need the
nonportable -i option.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

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

[Andrew Burgess]

Andreas Schwab <schwab@suse.de> writes:

> On Nov 18 2025, Andrew Burgess wrote:
>
>> +remote_exec build "cp \"$srcdir/$subdir/$srcfile\" \"$file_a\""
>> +remote_exec build "cp \"$srcdir/$subdir/$srcfile\" \"$file_b\""
>> +remote_exec build "sed -i -e \"s/value_to_return/3/\" \"$file_a\""
>> +remote_exec build "sed -i -e \"s/value_to_return/-3/\" \"$file_b\""
>
> If you combine the cp and sed commands then you don't need the
> nonportable -i option.

Thank you for your feedback.

An updated patch is below that avoids '-i'.

Thanks,
Andrew

---

commit bb6621f9b7b57737d8c3f8238a38713d98b1205b
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Nov 17 11:43:36 2025 +0000

    gdb: fix 'list' for multiple source file results
    
    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

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 277b45b4b7f..2cbfe2fcc20 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1071,11 +1071,16 @@ add_sal_to_sals (struct linespec_state *self,
 		 const symtab_and_line &sal,
 		 const char *symname, bool literal_canonical)
 {
-  /* We don't want two SALs with the same PC from the
-     same program space.  */
-  for (const auto &s : sals)
-    if (sal.pc == s.pc && sal.pspace == s.pspace)
-     return;
+  /* We don't want two SALs with the same PC from the same program space.
+     However, for the 'list' command we force the pc value to be 0, and in
+     this case we do want to see all SALs.  See decode_digits_list_mode
+     for where the 0 originates from.  */
+  if (sal.pc != 0)
+    {
+      for (const auto &s : sals)
+	if (sal.pc == s.pc && sal.pspace == s.pspace)
+	  return;
+    }
 
   sals.push_back (sal);
 
diff --git a/gdb/testsuite/gdb.base/list-multi-source.c b/gdb/testsuite/gdb.base/list-multi-source.c
new file mode 100644
index 00000000000..ab2b6a01b36
--- /dev/null
+++ b/gdb/testsuite/gdb.base/list-multi-source.c
@@ -0,0 +1,60 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+extern int function_a (void);
+extern int function_b (void);
+
+#ifdef MAIN
+int
+main (void)
+{
+  int res;
+
+  res = function_a ();
+
+  res += function_b ();
+
+  return res;
+}
+#endif
+
+#if defined FILE_A || defined FILE_B
+static int
+get_value_common (void)
+{
+  /* NOTE: When reading this file in the source tree, the variable used in
+     the return statement below will be replaced by a constant value when
+     the file is copied into the source tree.  */
+  return value_to_return;	/* List this line.  */
+}
+#endif
+
+#ifdef FILE_A
+int
+function_a (void)
+{
+  return get_value_common ();
+}
+#endif
+
+#ifdef FILE_B
+int
+function_b (void)
+{
+  return get_value_common ();
+}
+#endif
diff --git a/gdb/testsuite/gdb.base/list-multi-source.exp b/gdb/testsuite/gdb.base/list-multi-source.exp
new file mode 100644
index 00000000000..76c6990adb1
--- /dev/null
+++ b/gdb/testsuite/gdb.base/list-multi-source.exp
@@ -0,0 +1,121 @@
+# Copyright 2025 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/>.
+
+# Test that 'list FILE:LINE' can print multiple results if FILE
+# matches multiple files from the source tree.
+#
+# Then test that we can use 'list DIR/FILE:LINE' to restrict the
+# results to a single source file.
+
+# This test uses a single source file that is copied into the build
+# tree 3 times.  The three copies are then copied with different
+# defines set so that we see different functions in each copy.
+standard_testfile .c
+
+# Create the source tree within the build directory.
+set src_root [standard_output_file "src"]
+set src_a "$src_root/a"
+set src_b "$src_root/b"
+set file_a "$src_a/foo.c"
+set file_b "$src_b/foo.c"
+set file_main "$src_root/main.c"
+file mkdir "$src_root"
+file mkdir "$src_a"
+file mkdir "$src_b"
+
+# Helper proc.  Copy global SRCFILE to DEST, replacing
+# 'value_to_return' with VALUE during the copy.
+proc copy_and_update_source_file { dest value } {
+    # Open the source file for reading.
+    set in [open "$::srcdir/$::subdir/$::srcfile" r]
+
+    # Read in the entire contents of the file.  This should be fine as
+    # the input file is not large.
+    set file_content [read $in]
+
+    # Close the input file.
+    close $in
+
+    # Perform the replacement over the entire file contents.
+    set updated_content [string map \
+			     [list "value_to_return" $value] \
+			     $file_content]
+
+    # Open the destination file for writing.
+    set out [open $dest w]
+
+    # Write the modified content to the destination file.
+    puts -nonewline $out $updated_content
+
+    # Close the output file.
+    close $out
+}
+
+# Make three copies of the single source file in the build directory
+# based source tree.  Two of the source files are modified slighly to
+# make the output of 'list' unique for each copy.
+file copy "$srcdir/$subdir/$srcfile" "$file_main"
+copy_and_update_source_file $file_a "3"
+copy_and_update_source_file $file_b "-3"
+
+# Build the executable.  Use defines to make the source files
+# different.
+if { [prepare_for_testing_full "failed to prepare" \
+	  [list $testfile debug \
+	       $file_main [list debug additional_flags=-DMAIN] \
+	       $file_a [list debug additional_flags=-DFILE_A] \
+	       $file_b [list debug additional_flags=-DFILE_B]]]} {
+    return
+}
+
+# The LINENUM we should list, and the first and last lines that should
+# appear in the list output.
+set linenum [gdb_get_line_number "List this line"]
+set first_linenum [expr {$linenum - 5}]
+set last_linenum [expr {$linenum + 4}]
+
+# List using FILE:LINE for a filename that is ambiguous.
+gdb_test "list foo.c:$linenum" \
+    [multi_line \
+	 "file: \"\[^\r\n\]+/a/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return 3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+" \
+	 "file: \"\[^\r\n\]+/b/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return -3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]
+
+# Now list using a more acurate filename, we should only get a single
+# result.
+gdb_test "list a/foo.c:$linenum" \
+    [multi_line \
+	 "^$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return 3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]
+
+gdb_test "list b/foo.c:$linenum" \
+    [multi_line \
+	 "^$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return -3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]

[PATCH 2/2] gdb: display a symbol more often in multi-file list output

[Andrew Burgess]

I noticed that when a command line 'list foo.c:10' displays multiple
files, the symbol would always be shown as "???", e.g.:

  file: "/tmp/foo.c", line number: 10, symbol: "???"

this is because, when the symtab_and_line is created for the
'foo.c:10', the pc and symbol are never filled in.

In this commit, I propose that, when we decide that the above header
line needs to be printed, we should attempt to lookup a symbol for the
relevant line, and if one is found, we can use that.

The symbol lookup is done by first calling find_pc_for_line, and then
using find_symbol_for_pc to find a suitable symbol.
---
 gdb/cli/cli-cmds.c                           | 8 ++++++++
 gdb/testsuite/gdb.base/list-multi-source.exp | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 300b77dbafe..4534c670730 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -2174,6 +2174,14 @@ print_sal_location (const symtab_and_line &sal)
   const char *sym_name = NULL;
   if (sal.symbol != NULL)
     sym_name = sal.symbol->print_name ();
+  else if (CORE_ADDR line_pc;
+	   find_pc_for_line (sal.symtab, sal.line, &line_pc))
+    {
+      struct symbol *sym = find_symbol_for_pc (line_pc);
+      if (sym != nullptr)
+	sym_name = sym->print_name ();
+    }
+
   gdb_printf (_("file: \"%s\", line number: %ps, symbol: \"%s\"\n"),
 	      symtab_to_filename_for_display (sal.symtab),
 	      styled_string (line_number_style.style (),
diff --git a/gdb/testsuite/gdb.base/list-multi-source.exp b/gdb/testsuite/gdb.base/list-multi-source.exp
index 7809167bd0f..33bd49ad14b 100644
--- a/gdb/testsuite/gdb.base/list-multi-source.exp
+++ b/gdb/testsuite/gdb.base/list-multi-source.exp
@@ -63,13 +63,13 @@ set last_linenum [expr {$linenum + 4}]
 # List using FILE:LINE for a filename that is ambiguous.
 gdb_test "list foo.c:$linenum" \
     [multi_line \
-	 "file: \"\[^\r\n\]+/a/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "file: \"\[^\r\n\]+/a/foo.c\", line number: $linenum, symbol: \"get_value_common\"" \
 	 "$first_linenum\\s+\[^\r\n\]+" \
 	 ".*" \
 	 "$linenum\\s+[string_to_regexp {return 3;	/* List this line.  */}]" \
 	 ".*" \
 	 "$last_linenum\\s+\[^\r\n\]+" \
-	 "file: \"\[^\r\n\]+/b/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "file: \"\[^\r\n\]+/b/foo.c\", line number: $linenum, symbol: \"get_value_common\"" \
 	 "$first_linenum\\s+\[^\r\n\]+" \
 	 ".*" \
 	 "$linenum\\s+[string_to_regexp {return -3;	/* List this line.  */}]" \
-- 
2.47.1

[PATCHv2 0/2] Fix regression in 'list' command

[Andrew Burgess]

In v2:

  - Removed use of 'sed -i' as suggested by Andreas.

  - Updated the test in patch (1) to avoid failures on remote host
    boards.

  - Rebased to avoid merge conflicts with recently added styling in
    cli/cli-cmds.c.

---

The first patch fixes a regression in the 'list' command which has
been introduced since GDB 16, so should probably be fixed before GDB
17 is released.

The second patch is an improvement, it's not part of the regression
fix.

If approved then I plan to backport the first patch only to the GDB 17
branch.  Both patches would be applied to the master branch.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb: fix 'list' for multiple source file results
  gdb: display a symbol more often in multi-file list output

 gdb/cli/cli-cmds.c                           |   8 ++
 gdb/linespec.c                               |  15 ++-
 gdb/testsuite/gdb.base/list-multi-source.c   |  60 +++++++++
 gdb/testsuite/gdb.base/list-multi-source.exp | 129 +++++++++++++++++++
 4 files changed, 207 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


base-commit: d07e866442112d19511d7722014cff5e16e43413
-- 
2.47.1

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

[Andrew Burgess]

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

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 277b45b4b7f..2cbfe2fcc20 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1071,11 +1071,16 @@ add_sal_to_sals (struct linespec_state *self,
 		 const symtab_and_line &sal,
 		 const char *symname, bool literal_canonical)
 {
-  /* We don't want two SALs with the same PC from the
-     same program space.  */
-  for (const auto &s : sals)
-    if (sal.pc == s.pc && sal.pspace == s.pspace)
-     return;
+  /* We don't want two SALs with the same PC from the same program space.
+     However, for the 'list' command we force the pc value to be 0, and in
+     this case we do want to see all SALs.  See decode_digits_list_mode
+     for where the 0 originates from.  */
+  if (sal.pc != 0)
+    {
+      for (const auto &s : sals)
+	if (sal.pc == s.pc && sal.pspace == s.pspace)
+	  return;
+    }
 
   sals.push_back (sal);
 
diff --git a/gdb/testsuite/gdb.base/list-multi-source.c b/gdb/testsuite/gdb.base/list-multi-source.c
new file mode 100644
index 00000000000..ab2b6a01b36
--- /dev/null
+++ b/gdb/testsuite/gdb.base/list-multi-source.c
@@ -0,0 +1,60 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2025 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/>.  */
+
+extern int function_a (void);
+extern int function_b (void);
+
+#ifdef MAIN
+int
+main (void)
+{
+  int res;
+
+  res = function_a ();
+
+  res += function_b ();
+
+  return res;
+}
+#endif
+
+#if defined FILE_A || defined FILE_B
+static int
+get_value_common (void)
+{
+  /* NOTE: When reading this file in the source tree, the variable used in
+     the return statement below will be replaced by a constant value when
+     the file is copied into the source tree.  */
+  return value_to_return;	/* List this line.  */
+}
+#endif
+
+#ifdef FILE_A
+int
+function_a (void)
+{
+  return get_value_common ();
+}
+#endif
+
+#ifdef FILE_B
+int
+function_b (void)
+{
+  return get_value_common ();
+}
+#endif
diff --git a/gdb/testsuite/gdb.base/list-multi-source.exp b/gdb/testsuite/gdb.base/list-multi-source.exp
new file mode 100644
index 00000000000..66f6582fdff
--- /dev/null
+++ b/gdb/testsuite/gdb.base/list-multi-source.exp
@@ -0,0 +1,129 @@
+# Copyright 2025 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/>.
+
+# Test that 'list FILE:LINE' can print multiple results if FILE
+# matches multiple files from the source tree.
+#
+# Then test that we can use 'list DIR/FILE:LINE' to restrict the
+# results to a single source file.
+
+# With a remote host, source files are automatically copied to the
+# host by dejagnu, and this drops the directory structure that is
+# needed for this test to work, i.e. we need a/foo.c and b/foo.c, but
+# dejagnu's automatic copying just gives us a single foo.c.  Instead
+# of trying to fix this, for now at least, just skip remote host
+# testing.
+require {!is_remote host}
+
+# This test uses a single source file that is copied into the build
+# tree 3 times.  The three copies are then copied with different
+# defines set so that we see different functions in each copy.
+standard_testfile .c
+
+# Create the source tree within the build directory.
+set src_root [standard_output_file "src"]
+set src_a "$src_root/a"
+set src_b "$src_root/b"
+set file_a "$src_a/foo.c"
+set file_b "$src_b/foo.c"
+set file_main "$src_root/main.c"
+file mkdir "$src_root"
+file mkdir "$src_a"
+file mkdir "$src_b"
+
+# Helper proc.  Copy global SRCFILE to DEST, replacing
+# 'value_to_return' with VALUE during the copy.
+proc copy_and_update_source_file { dest value } {
+    # Open the source file for reading.
+    set in [open "$::srcdir/$::subdir/$::srcfile" r]
+
+    # Read in the entire contents of the file.  This should be fine as
+    # the input file is not large.
+    set file_content [read $in]
+
+    # Close the input file.
+    close $in
+
+    # Perform the replacement over the entire file contents.
+    set updated_content [string map \
+			     [list "value_to_return" $value] \
+			     $file_content]
+
+    # Open the destination file for writing.
+    set out [open $dest w]
+
+    # Write the modified content to the destination file.
+    puts -nonewline $out $updated_content
+
+    # Close the output file.
+    close $out
+}
+
+# Make three copies of the single source file in the build directory
+# based source tree.  Two of the source files are modified slighly to
+# make the output of 'list' unique for each copy.
+file copy "$srcdir/$subdir/$srcfile" "$file_main"
+copy_and_update_source_file $file_a "3"
+copy_and_update_source_file $file_b "-3"
+
+# Build the executable.  Use defines to make the source files
+# different.
+if { [prepare_for_testing_full "failed to prepare" \
+	  [list $testfile debug \
+	       $file_main [list debug additional_flags=-DMAIN] \
+	       $file_a [list debug additional_flags=-DFILE_A] \
+	       $file_b [list debug additional_flags=-DFILE_B]]]} {
+    return
+}
+
+# The LINENUM we should list, and the first and last lines that should
+# appear in the list output.
+set linenum [gdb_get_line_number "List this line"]
+set first_linenum [expr {$linenum - 5}]
+set last_linenum [expr {$linenum + 4}]
+
+# List using FILE:LINE for a filename that is ambiguous.
+gdb_test "list foo.c:$linenum" \
+    [multi_line \
+	 "file: \"\[^\r\n\]+/a/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return 3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+" \
+	 "file: \"\[^\r\n\]+/b/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return -3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]
+
+# Now list using a more acurate filename, we should only get a single
+# result.
+gdb_test "list a/foo.c:$linenum" \
+    [multi_line \
+	 "^$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return 3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]
+
+gdb_test "list b/foo.c:$linenum" \
+    [multi_line \
+	 "^$first_linenum\\s+\[^\r\n\]+" \
+	 ".*" \
+	 "$linenum\\s+[string_to_regexp {return -3;	/* List this line.  */}]" \
+	 ".*" \
+	 "$last_linenum\\s+\[^\r\n\]+"]
-- 
2.47.1

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

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> I'm not a fan of using '0' as a special pc value, there are embedded
Andrew> targets where 0 is a valid pc value, but given we're already using 0
Andrew> here, I propose to just roll with it.

A lot to dislike here -- SALs, gdb's code-at-0 handling, and the fact
that some targets even allow this :-)

Andrew> So, my proposal is that, if the pc is 0, add_sal_to_sals should always
Andrew> add the sal.  This fixes the decode_digits_list_mode, but should keep
Andrew> the fix that c7a45b98a614 introduced.

Seems reasonable to me.  And of course for 17 as well.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

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

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> I'm not a fan of using '0' as a special pc value, there are embedded
> Andrew> targets where 0 is a valid pc value, but given we're already using 0
> Andrew> here, I propose to just roll with it.
>
> A lot to dislike here -- SALs, gdb's code-at-0 handling, and the fact
> that some targets even allow this :-)
>
> Andrew> So, my proposal is that, if the pc is 0, add_sal_to_sals should always
> Andrew> add the sal.  This fixes the decode_digits_list_mode, but should keep
> Andrew> the fix that c7a45b98a614 introduced.
>
> Seems reasonable to me.  And of course for 17 as well.
> Approved-By: Tom Tromey <tom@tromey.com>

Thanks everyone for the reviews.

Patch #1 is pushed to master and gdb-17-branch.

Patch #2 pushed only to master (not needed for gdb-17-branch).

Thanks,
Andrew

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

[Kevin Buettner]

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>

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

[Eli Zaretskii]

> Date: Thu, 4 Dec 2025 13:18:35 -0700
> From: Kevin Buettner <kevinb@redhat.com>
> Cc: gdb-patches@sourceware.org
> 
> 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.

I agree that the fix should go into GDB 17, because otherwise it will
be a bad regression.

[PATCHv2 2/2] gdb: display a symbol more often in multi-file list output

[Andrew Burgess]

I noticed that when a command line 'list foo.c:10' displays multiple
files, the symbol would always be shown as "???", e.g.:

  file: "/tmp/foo.c", line number: 10, symbol: "???"

this is because, when the symtab_and_line is created for the
'foo.c:10', the pc and symbol are never filled in.

In this commit, I propose that, when we decide that the above header
line needs to be printed, we should attempt to lookup a symbol for the
relevant line, and if one is found, we can use that.

The symbol lookup is done by first calling find_pc_for_line, and then
using find_symbol_for_pc to find a suitable symbol.
---
 gdb/cli/cli-cmds.c                           | 8 ++++++++
 gdb/testsuite/gdb.base/list-multi-source.exp | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 43c1bc7a96f..cf5571c5419 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -2182,6 +2182,14 @@ print_sal_location (const symtab_and_line &sal)
   const char *sym_name = NULL;
   if (sal.symbol != NULL)
     sym_name = sal.symbol->print_name ();
+  else if (CORE_ADDR line_pc;
+	   find_pc_for_line (sal.symtab, sal.line, &line_pc))
+    {
+      struct symbol *sym = find_symbol_for_pc (line_pc);
+      if (sym != nullptr)
+	sym_name = sym->print_name ();
+    }
+
   gdb_printf (_("file: \"%ps\", line number: %ps, symbol: \"%s\"\n"),
 	      styled_string (file_name_style.style (),
 			     symtab_to_filename_for_display (sal.symtab)),
diff --git a/gdb/testsuite/gdb.base/list-multi-source.exp b/gdb/testsuite/gdb.base/list-multi-source.exp
index 66f6582fdff..887ff961933 100644
--- a/gdb/testsuite/gdb.base/list-multi-source.exp
+++ b/gdb/testsuite/gdb.base/list-multi-source.exp
@@ -97,13 +97,13 @@ set last_linenum [expr {$linenum + 4}]
 # List using FILE:LINE for a filename that is ambiguous.
 gdb_test "list foo.c:$linenum" \
     [multi_line \
-	 "file: \"\[^\r\n\]+/a/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "file: \"\[^\r\n\]+/a/foo.c\", line number: $linenum, symbol: \"get_value_common\"" \
 	 "$first_linenum\\s+\[^\r\n\]+" \
 	 ".*" \
 	 "$linenum\\s+[string_to_regexp {return 3;	/* List this line.  */}]" \
 	 ".*" \
 	 "$last_linenum\\s+\[^\r\n\]+" \
-	 "file: \"\[^\r\n\]+/b/foo.c\", line number: $linenum, symbol: \"\[^\r\n\]+\"" \
+	 "file: \"\[^\r\n\]+/b/foo.c\", line number: $linenum, symbol: \"get_value_common\"" \
 	 "$first_linenum\\s+\[^\r\n\]+" \
 	 ".*" \
 	 "$linenum\\s+[string_to_regexp {return -3;	/* List this line.  */}]" \
-- 
2.47.1

Re: [PATCHv2 2/2] gdb: display a symbol more often in multi-file list output

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> In this commit, I propose that, when we decide that the above header
Andrew> line needs to be printed, we should attempt to lookup a symbol for the
Andrew> relevant line, and if one is found, we can use that.

Ok.
Approved-By: Tom Tromey <tom@tromey.com>

I didn't look into the reason but it does seem unfortunate that the SAL
isn't fully filled in.

Tom