[PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set

[PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set

[Aaron Merey]

During breakpoint re-setting, the source_filename of an
explicit_location_spec is used to lookup the symtabs associated with
the breakpoint being re-set.  This source_filename is compared with each
known symtab filename in order to retrieve the breakpoint's symtabs.

However the source_filename may have been originally copied from a
symtab's fullname (the path where GDB found the source file) when the
breakpoint was first created.  If a breakpoint symtab's filename and
fullname differ, this will cause a NOT_FOUND_ERROR to be thrown during
re-setting.

Fix this by using a symtab's filename to set the explicit_location_spec
source_filename instead of the symtab's fullname.
---
 gdb/linespec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 3db35998f7e..805c98ca201 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2281,13 +2281,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 	/* Make sure we have a filename for canonicalization.  */
 	if (ls->explicit_loc.source_filename == NULL)
 	  {
-	    const char *fullname = symtab_to_fullname (state->default_symtab);
+	    const char *filename = state->default_symtab->filename;
 
 	    /* It may be more appropriate to keep DEFAULT_SYMTAB in its symtab
 	       form so that displaying SOURCE_FILENAME can follow the current
 	       FILENAME_DISPLAY_STRING setting.  But as it is used only rarely
 	       it has been kept for code simplicity only in absolute form.  */
-	    ls->explicit_loc.source_filename = xstrdup (fullname);
+	    ls->explicit_loc.source_filename = xstrdup (filename);
 	  }
     }
   else
-- 
2.37.3

Re: [PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set

[Bruno Larsen]

On 08/10/2022 01:44, Aaron Merey via Gdb-patches wrote:
> During breakpoint re-setting, the source_filename of an
> explicit_location_spec is used to lookup the symtabs associated with
> the breakpoint being re-set.  This source_filename is compared with each
> known symtab filename in order to retrieve the breakpoint's symtabs.
>
> However the source_filename may have been originally copied from a
> symtab's fullname (the path where GDB found the source file) when the
> breakpoint was first created.  If a breakpoint symtab's filename and
> fullname differ, this will cause a NOT_FOUND_ERROR to be thrown during
> re-setting.
>
> Fix this by using a symtab's filename to set the explicit_location_spec
> source_filename instead of the symtab's fullname.

This seems like a good change, and introduces no regressions.

Reviewed-By: Bruno Larsen <blarsen@redhat.com>


Cheers,
Bruno

> ---
>   gdb/linespec.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 3db35998f7e..805c98ca201 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -2281,13 +2281,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
>   	/* Make sure we have a filename for canonicalization.  */
>   	if (ls->explicit_loc.source_filename == NULL)
>   	  {
> -	    const char *fullname = symtab_to_fullname (state->default_symtab);
> +	    const char *filename = state->default_symtab->filename;
>   
>   	    /* It may be more appropriate to keep DEFAULT_SYMTAB in its symtab
>   	       form so that displaying SOURCE_FILENAME can follow the current
>   	       FILENAME_DISPLAY_STRING setting.  But as it is used only rarely
>   	       it has been kept for code simplicity only in absolute form.  */
> -	    ls->explicit_loc.source_filename = xstrdup (fullname);
> +	    ls->explicit_loc.source_filename = xstrdup (filename);
>   	  }
>       }
>     else

Re: [PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set.

[Aaron Merey]

Hi,

I'm reposting this patch with a testcase added to
testsuite/gdb.debuginfod/fetch_src_and_symbols.exp.  I tried finding a
way to reproduce the error without debuginfod by using 'set
substitute-path' but the substitution rules always converted the symtab
fullname to the filename which avoided the error.  Since I don't know
of any other way to trigger the error other than with source files
downloaded from debuginfod, the gdb.debuginfod seems like the best (only?)
place for the test.

Aaron

---

During breakpoint re-setting, the source_filename of an
explicit_location_spec is used to lookup the symtabs associated with
the breakpoint being re-set.  This source_filename is compared with each
known symtab filename in order to retrieve the breakpoint's symtabs.

However the source_filename may have been originally copied from a
symtab's fullname (the path where GDB found the source file) when the
breakpoint was first created.  If a breakpoint symtab's filename and
fullname differ and there is no substitute-path setting that converts
the fullname to the filename, this will cause a NOT_FOUND_ERROR to be
thrown during re-setting.

Fix this by using a symtab's filename to set the explicit_location_spec
source_filename instead of the symtab's fullname.
---
 gdb/linespec.c                                         | 4 ++--
 gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp | 7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 3db35998f7e..805c98ca201 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2281,13 +2281,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 	/* Make sure we have a filename for canonicalization.  */
 	if (ls->explicit_loc.source_filename == NULL)
 	  {
-	    const char *fullname = symtab_to_fullname (state->default_symtab);
+	    const char *filename = state->default_symtab->filename;
 
 	    /* It may be more appropriate to keep DEFAULT_SYMTAB in its symtab
 	       form so that displaying SOURCE_FILENAME can follow the current
 	       FILENAME_DISPLAY_STRING setting.  But as it is used only rarely
 	       it has been kept for code simplicity only in absolute form.  */
-	    ls->explicit_loc.source_filename = xstrdup (fullname);
+	    ls->explicit_loc.source_filename = xstrdup (filename);
 	  }
     }
   else
diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index 8bb9203686d..9e7d4321913 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -213,8 +213,11 @@ proc_with_prefix local_url { } {
     gdb_test "file $binfile" "" "file [file tail $binfile]" "Enable debuginfod?.*" "y"
     gdb_test_no_output "set substitute-path $outputdir /dev/null" \
 	"set substitute-path"
-    gdb_test "br main" "Breakpoint 1 at.*file.*"
-    gdb_test "l" ".*This program is distributed in the hope.*"
+    gdb_test "set cwd $debugdir" "" "file [file tail $binfile] cwd"
+    gdb_test "list 1" ".*This program is distributed in the hope.*"
+    gdb_test "break 25" "Breakpoint 1 at.*file.*"
+    gdb_test "run" "Breakpoint 1,.*" \
+	"file [file tail $binfile] set breakpoint]"
 
     # GDB should now find the executable file.
     clean_restart
-- 
2.37.3

Re: [PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set.

[Bruno Larsen]

On 12/10/2022 22:49, Aaron Merey via Gdb-patches wrote:
> Hi,
>
> I'm reposting this patch with a testcase added to
> testsuite/gdb.debuginfod/fetch_src_and_symbols.exp.  I tried finding a
> way to reproduce the error without debuginfod by using 'set
> substitute-path' but the substitution rules always converted the symtab
> fullname to the filename which avoided the error.  Since I don't know
> of any other way to trigger the error other than with source files
> downloaded from debuginfod, the gdb.debuginfod seems like the best (only?)
> place for the test.

Another option would be making a unit test, so you can programmatically 
control the state from within GDB.

That said, your test does trigger the problem and shows that the patch 
fixes it, so I'm ok with this test. Just a minor nit.

> Aaron
>
> ---
>
> During breakpoint re-setting, the source_filename of an
> explicit_location_spec is used to lookup the symtabs associated with
> the breakpoint being re-set.  This source_filename is compared with each
> known symtab filename in order to retrieve the breakpoint's symtabs.
>
> However the source_filename may have been originally copied from a
> symtab's fullname (the path where GDB found the source file) when the
> breakpoint was first created.  If a breakpoint symtab's filename and
> fullname differ and there is no substitute-path setting that converts
> the fullname to the filename, this will cause a NOT_FOUND_ERROR to be
> thrown during re-setting.
>
> Fix this by using a symtab's filename to set the explicit_location_spec
> source_filename instead of the symtab's fullname.
> ---
>   gdb/linespec.c                                         | 4 ++--
>   gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp | 7 +++++--
>   2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 3db35998f7e..805c98ca201 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -2281,13 +2281,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
>   	/* Make sure we have a filename for canonicalization.  */
>   	if (ls->explicit_loc.source_filename == NULL)
>   	  {
> -	    const char *fullname = symtab_to_fullname (state->default_symtab);
> +	    const char *filename = state->default_symtab->filename;
>   
>   	    /* It may be more appropriate to keep DEFAULT_SYMTAB in its symtab
>   	       form so that displaying SOURCE_FILENAME can follow the current
>   	       FILENAME_DISPLAY_STRING setting.  But as it is used only rarely
>   	       it has been kept for code simplicity only in absolute form.  */
> -	    ls->explicit_loc.source_filename = xstrdup (fullname);
> +	    ls->explicit_loc.source_filename = xstrdup (filename);
>   	  }
>       }
>     else
> diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> index 8bb9203686d..9e7d4321913 100644
> --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> @@ -213,8 +213,11 @@ proc_with_prefix local_url { } {
>       gdb_test "file $binfile" "" "file [file tail $binfile]" "Enable debuginfod?.*" "y"
>       gdb_test_no_output "set substitute-path $outputdir /dev/null" \
>   	"set substitute-path"
> -    gdb_test "br main" "Breakpoint 1 at.*file.*"
> -    gdb_test "l" ".*This program is distributed in the hope.*"
> +    gdb_test "set cwd $debugdir" "" "file [file tail $binfile] cwd"
> +    gdb_test "list 1" ".*This program is distributed in the hope.*"
> +    gdb_test "break 25" "Breakpoint 1 at.*file.*"
> +    gdb_test "run" "Breakpoint 1,.*" \
> +	"file [file tail $binfile] set breakpoint]"

extra ']' at the end

Also, would be nice to have a small comment mentioning that there could 
be an error when resetting the breakpoint, so we have some context when 
looking at this test in the future.

With these changes, I'm ok with the patch going in, but I can't approve 
it for pushing. Don't forget to add the tag to the commit :)

Reviewed-By: Bruno Larsen <blarsen@redhat.com>

Cheers,
Bruno

>   
>       # GDB should now find the executable file.
>       clean_restart

Re: [PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set.

[Aaron Merey]

On Thu, Oct 13, 2022 at 3:59 AM Bruno Larsen <blarsen@redhat.com> wrote:
> extra ']' at the end
>
> Also, would be nice to have a small comment mentioning that there could
> be an error when resetting the breakpoint, so we have some context when
> looking at this test in the future.
>
> With these changes, I'm ok with the patch going in, but I can't approve
> it for pushing. Don't forget to add the tag to the commit :)
>
> Reviewed-By: Bruno Larsen <blarsen@redhat.com>

Thanks Bruno, fixed.

[PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set

[Aaron Merey]

Ping. Reposting this patch with a couple tweaks suggested by Bruno.

During breakpoint re-setting, the source_filename of an
explicit_location_spec is used to lookup the symtabs associated with
the breakpoint being re-set.  This source_filename is compared with each
known symtab filename in order to retrieve the breakpoint's symtabs.

However the source_filename may have been originally copied from a
symtab's fullname (the path where GDB found the source file) when the
breakpoint was first created.  If a breakpoint symtab's filename and
fullname differ and there is no substitute-path rule that converts the
fullname to the filename, this will cause a NOT_FOUND_ERROR to be thrown
during re-setting.

Fix this by using a symtab's filename to set the explicit_location_spec
source_filename instead of the symtab's fullname.
---
 gdb/linespec.c                                        |  4 ++--
 .../gdb.debuginfod/fetch_src_and_symbols.exp          | 11 +++++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index 3db35998f7e..805c98ca201 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2281,13 +2281,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 	/* Make sure we have a filename for canonicalization.  */
 	if (ls->explicit_loc.source_filename == NULL)
 	  {
-	    const char *fullname = symtab_to_fullname (state->default_symtab);
+	    const char *filename = state->default_symtab->filename;
 
 	    /* It may be more appropriate to keep DEFAULT_SYMTAB in its symtab
 	       form so that displaying SOURCE_FILENAME can follow the current
 	       FILENAME_DISPLAY_STRING setting.  But as it is used only rarely
 	       it has been kept for code simplicity only in absolute form.  */
-	    ls->explicit_loc.source_filename = xstrdup (fullname);
+	    ls->explicit_loc.source_filename = xstrdup (filename);
 	  }
     }
   else
diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index 9bffb3397ec..a6575f033e0 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -216,8 +216,15 @@ proc_with_prefix local_url { } {
 	$enable_debuginfod_question "y"
     gdb_test_no_output "set substitute-path $outputdir /dev/null" \
 	"set substitute-path"
-    gdb_test "br main" "Breakpoint 1 at.*file.*"
-    gdb_test "l" ".*This program is distributed in the hope.*"
+    gdb_test "set cwd $debugdir" "" "file [file tail $binfile] cwd"
+    gdb_test "list 1" ".*This program is distributed in the hope.*"
+
+    # Verify that breakpoints re-set correctly when the actual location
+    # of the source file in the debuginfod client cache differs from
+    # the contents of DW_AT_comp_dir and DW_AT_name.
+    gdb_test "break 25" "Breakpoint 1 at.*file.*"
+    gdb_test "run" "Breakpoint 1,.*" \
+	"file [file tail $binfile] set breakpoint"
 
     # GDB should now find the executable file.
     clean_restart
-- 
2.37.3

[PING*2][PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set

[Aaron Merey]

Ping. I tweaked the testcase so that it applies to the master branch.

Thanks,
Aaron

---
During breakpoint re-setting, the source_filename of an
explicit_location_spec is used to lookup the symtabs associated with
the breakpoint being re-set.  This source_filename is compared with each
known symtab filename in order to retrieve the breakpoint's symtabs.

However the source_filename may have been originally copied from a
symtab's fullname (the path where GDB found the source file) when the
breakpoint was first created.  If a breakpoint symtab's filename and
fullname differ and there is no substitute-path rule that converts the
fullname to the filename, this will cause a NOT_FOUND_ERROR to be thrown
during re-setting.

Fix this by using a symtab's filename to set the explicit_location_spec
source_filename instead of the symtab's fullname.
---
 gdb/linespec.c                                         | 4 ++--
 gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/gdb/linespec.c b/gdb/linespec.c
index e9339c3338c..6db0f02e318 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2283,13 +2283,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
 	/* Make sure we have a filename for canonicalization.  */
 	if (ls->explicit_loc.source_filename == NULL)
 	  {
-	    const char *fullname = symtab_to_fullname (state->default_symtab);
+	    const char *filename = state->default_symtab->filename;
 
 	    /* It may be more appropriate to keep DEFAULT_SYMTAB in its symtab
 	       form so that displaying SOURCE_FILENAME can follow the current
 	       FILENAME_DISPLAY_STRING setting.  But as it is used only rarely
 	       it has been kept for code simplicity only in absolute form.  */
-	    ls->explicit_loc.source_filename = xstrdup (fullname);
+	    ls->explicit_loc.source_filename = xstrdup (filename);
 	  }
     }
   else
diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index 6d5af1370b0..bfe6e639e4f 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -224,6 +224,15 @@ proc_with_prefix local_url { } {
     set lineno [gdb_get_line_number "Breakpoint here"]
     gdb_test "list $lineno" "return 0;\[^\r\n\]+Breakpoint here\\. .*"
 
+    # Verify that a breakpoint re-sets correctly when the actual location
+    # of the source file in the debuginfod client cache differs from
+    # the contents of DW_AT_comp_dir and DW_AT_name.
+    gdb_test "set cwd $debugdir" "" "file [file tail $binfile] cwd"
+    gdb_test_no_output "del breakpoint 1"
+    gdb_test "break $lineno" "Breakpoint 2 at.*file.*"
+    gdb_test "run" "Breakpoint 2.*" \
+	"file [file tail $binfile] set breakpoint"
+
     # GDB should now find the executable file.
     clean_restart
     gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \
-- 
2.39.0

Re: [PING*2][PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set

[Tom Tromey]

>>>>> "Aaron" == Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:

Aaron> Ping. I tweaked the testcase so that it applies to the master branch.

Oops, I reviewed the wrong version.
This one is ok.  Thank you.

Tom

Re: [PING*2][PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set

[Aaron Merey]

On Mon, Jan 9, 2023 at 2:28 PM Tom Tromey <tom@tromey.com> wrote:
> Oops, I reviewed the wrong version.
> This one is ok.  Thank you.

Thanks Tom, pushed as commit 7dd38e31d67.

On Mon, Jan 9, 2023 at 2:27 PM Tom Tromey <tom@tromey.com> wrote:
> It's pretty hard to review a patch like this, since it's hard to know
> what impact it might have.  However I am mostly fine with it (see the
> end), assuming you regression-tested it.

Regression tested on x86_64 F36 and F37.

Aaron

Re: [PING*2][PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set

[Andrew Burgess]

Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:

> Ping. I tweaked the testcase so that it applies to the master branch.
>
> Thanks,
> Aaron
>
> ---
> During breakpoint re-setting, the source_filename of an
> explicit_location_spec is used to lookup the symtabs associated with
> the breakpoint being re-set.  This source_filename is compared with each
> known symtab filename in order to retrieve the breakpoint's symtabs.
>
> However the source_filename may have been originally copied from a
> symtab's fullname (the path where GDB found the source file) when the
> breakpoint was first created.  If a breakpoint symtab's filename and
> fullname differ and there is no substitute-path rule that converts the
> fullname to the filename, this will cause a NOT_FOUND_ERROR to be thrown
> during re-setting.
>
> Fix this by using a symtab's filename to set the explicit_location_spec
> source_filename instead of the symtab's fullname.
> ---
>  gdb/linespec.c                                         | 4 ++--
>  gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp | 9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index e9339c3338c..6db0f02e318 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -2283,13 +2283,13 @@ convert_linespec_to_sals (struct linespec_state *state, linespec *ls)
>  	/* Make sure we have a filename for canonicalization.  */
>  	if (ls->explicit_loc.source_filename == NULL)
>  	  {
> -	    const char *fullname = symtab_to_fullname (state->default_symtab);
> +	    const char *filename = state->default_symtab->filename;
>  
>  	    /* It may be more appropriate to keep DEFAULT_SYMTAB in its symtab
>  	       form so that displaying SOURCE_FILENAME can follow the current
>  	       FILENAME_DISPLAY_STRING setting.  But as it is used only rarely
>  	       it has been kept for code simplicity only in absolute form.  */
> -	    ls->explicit_loc.source_filename = xstrdup (fullname);
> +	    ls->explicit_loc.source_filename = xstrdup (filename);
>  	  }
>      }
>    else
> diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> index 6d5af1370b0..bfe6e639e4f 100644
> --- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> +++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
> @@ -224,6 +224,15 @@ proc_with_prefix local_url { } {
>      set lineno [gdb_get_line_number "Breakpoint here"]
>      gdb_test "list $lineno" "return 0;\[^\r\n\]+Breakpoint here\\. .*"
>  
> +    # Verify that a breakpoint re-sets correctly when the actual location
> +    # of the source file in the debuginfod client cache differs from
> +    # the contents of DW_AT_comp_dir and DW_AT_name.
> +    gdb_test "set cwd $debugdir" "" "file [file tail $binfile] cwd"
> +    gdb_test_no_output "del breakpoint 1"
> +    gdb_test "break $lineno" "Breakpoint 2 at.*file.*"
> +    gdb_test "run" "Breakpoint 2.*" \
> +	"file [file tail $binfile] set breakpoint"

Calling "run" like this will not work when testing with the
native-gdbserver board, which can be done like:

  make check-gdb RUNTESTFLAGS="--target_board=native-gdbserver"

The other board that is worth testing with is native-extended-gdbserver.

I've pushed the patch below which fixes the test to work with the
native-gdbserver board.  I checked that the test still fails if I revert
the GDB fix in your above patch.

Thanks,
Andrew

---

commit cded17bfca35566fa4d36e9ec06fa071bd7dab17
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Feb 1 17:09:47 2023 +0000

    gdb/testsuite: fix fetch_src_and_symbols.exp with native-gdbserver board
    
    I noticed that the gdb.debuginfod/fetch_src_and_symbols.exp script
    doesn't work with the native-gdbserver board, I see this error:
    
      ERROR: tcl error sourcing /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp.
      ERROR: gdbserver does not support run without extended-remote
          while executing
      "error "gdbserver does not support $command without extended-remote""
          (procedure "gdb_test_multiple" line 51)
          invoked from within
    
    This was introduced with this commit:
    
      commit 7dd38e31d67c2548b52bea313ab18e40824c05da
      Date:   Fri Jan 6 18:45:27 2023 -0500
    
          gdb/linespec.c: Fix missing source file during breakpoint re-set
    
    The problem is that the above commit introduces a direct use of the
    "run" command, which doesn't work with 'target remote' targets, as
    exercised by the native-gdbserver board.
    
    To avoid this, in this commit I switch to using runto_main.  However,
    calling runto_main will, by default, delete all the currently set
    breakpoints.  As the point of the above commit was to check that a
    breakpoint set before stating an inferior would be correctly re-set,
    we need to avoid this breakpoint deleting behaviour.
    
    To do this I make use of with_override, and override the
    delete_breakpoints proc with a dummy proc which does nothing.
    
    By reverting the GDB changes in commit 7dd38e31d67c I have confirmed
    that even after my changes in this commit, the test still fails.  But
    with the fixes in commit 7dd38e31d67c, this test now passed using the
    unix, native-gdbserver, and native-extended-gdbserver boards.

diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index c9cd8a30a1c..8158c5c3cc6 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -195,6 +195,12 @@ proc test_urls {urls pattern_re test} {
 	$test
 }
 
+# Used as a replacement for delete_breakpoints while calling
+# runto_main in one case where we don't want to delete all the
+# breakpoints.
+proc disable_delete_breakpoints {} {
+}
+
 # Uses the global variables DEBUGDIR and DB which are setup elsewhere
 # in this script.
 #
@@ -214,13 +220,12 @@ proc_with_prefix local_url { } {
 
     # GDB should now find the symbol and source files.
     clean_restart
-    set enable_debuginfod_question \
-	"Enable debuginfod for this session. \\(y or \\\[n\\\]\\) "
-    gdb_test "file $binfile" "" "file [file tail $binfile]" \
-	$enable_debuginfod_question "y"
+    gdb_test_no_output "set debuginfod enabled on" \
+	"enabled debuginfod for initial test"
+    gdb_load $binfile
     gdb_test_no_output "set substitute-path $outputdir /dev/null" \
 	"set substitute-path"
-    gdb_test "br main" "Breakpoint 1 at.*file.*"
+
     set lineno [gdb_get_line_number "Breakpoint here"]
     gdb_test "list $lineno" "return 0;\[^\r\n\]+Breakpoint here\\. .*"
 
@@ -228,12 +233,18 @@ proc_with_prefix local_url { } {
     # of the source file in the debuginfod client cache differs from
     # the contents of DW_AT_comp_dir and DW_AT_name.
     gdb_test "set cwd $debugdir" "" "file [file tail $binfile] cwd"
-    gdb_test_no_output "del breakpoint 1"
-    gdb_test "break $lineno" "Breakpoint 2 at.*file.*"
-    gdb_test "run" "Breakpoint 2.*" \
-	"file [file tail $binfile] set breakpoint"
+    gdb_breakpoint $lineno
+    with_override delete_breakpoints disable_delete_breakpoints {
+	if {![runto_main]} {
+	    return
+	}
+	gdb_continue_to_breakpoint "runto breakpoint in main" \
+	    ".* Breakpoint here\\. .*"
+    }
 
     # GDB should now find the executable file.
+    set enable_debuginfod_question \
+	"Enable debuginfod for this session. \\(y or \\\[n\\\]\\) "
     clean_restart
     gdb_test "core $::corefile" ".*return 0.*" "file [file tail $::corefile]" \
 	$enable_debuginfod_question "y"

Re: [PATCH] gdb/linespec.c: Fix missing source file during breakpoint re-set.

[Tom Tromey]

>>>>> "Aaron" == Aaron Merey via Gdb-patches <gdb-patches@sourceware.org> writes:

Aaron> I'm reposting this patch with a testcase added to
Aaron> testsuite/gdb.debuginfod/fetch_src_and_symbols.exp.  I tried finding a
Aaron> way to reproduce the error without debuginfod by using 'set
Aaron> substitute-path' but the substitution rules always converted the symtab
Aaron> fullname to the filename which avoided the error.  Since I don't know
Aaron> of any other way to trigger the error other than with source files
Aaron> downloaded from debuginfod, the gdb.debuginfod seems like the best (only?)
Aaron> place for the test.

Sorry about the long delay on this.

It's pretty hard to review a patch like this, since it's hard to know
what impact it might have.  However I am mostly fine with it (see the
end), assuming you regression-tested it.

Aaron> +    gdb_test "run" "Breakpoint 1,.*" \
Aaron> +	"file [file tail $binfile] set breakpoint]"
 
I think there's an unmatched "]" here.

Tom