[v3,1/1] gdb, infcmd: Support jump command in multi-inferior case.

  Fixes the issue where jump failed if multiple inferiors run the same source.

See the below example

    $ gdb -q ./simple
    Reading symbols from ./simple...
    (gdb) break 2
    Breakpoint 1 at 0x114e: file simple.c, line 2.
    (gdb) run
    Starting program: /temp/simple

    Breakpoint 1, main () at simple.c:2
    2         int a = 42;
    (gdb) add-inferior
    [New inferior 2]
    Added inferior 2 on connection 1 (native)
    (gdb) inferior 2
    [Switching to inferior 2 [<null>] (<noexec>)]
    (gdb) info inferiors
      Num  Description       Connection           Executable
      1    process 6250      1 (native)           /temp/simple
    * 2    <null>            1 (native)
    (gdb) file ./simple
    Reading symbols from ./simple...
    (gdb) run
    Starting program: /temp/simple

    Thread 2.1 "simple" hit Breakpoint 1, main () at simple.c:2
    2         int a = 42;
    (gdb) info inferiors
      Num  Description       Connection           Executable
      1    process 6250      1 (native)           /temp/simple
    * 2    process 6705      1 (native)           /temp/simple
    (gdb) jump 3
    Unreasonable jump request
    (gdb)

In this example, jump fails because the debugger finds two different
locations, one for each inferior.
Solution is to limit the search to the current program space.
---
 gdb/infcmd.c                    |   3 +-
 gdb/linespec.c                  |   7 +-
 gdb/linespec.h                  |   5 +-
 gdb/testsuite/gdb.base/jump.exp | 177 +++++++++++++++++---------------
 4 files changed, 103 insertions(+), 89 deletions(-)
  

Kindly asking for review, approval and help to push.


Patch 1:	https://sourceware.org/pipermail/gdb-patches/2023-February/197269.html
Andrew’s comments on patch 1:	https://sourceware.org/pipermail/gdb-patches/2023-February/197271.html
Patch 2:	https://sourceware.org/pipermail/gdb-patches/2023-February/197302.html
Andres’s comments on patch 2:	https://sourceware.org/pipermail/gdb-patches/2023-April/199055.html	
My msg to Andrew:	https://sourceware.org/pipermail/gdb-patches/2023-April/199096.html	
Patch 3:	https://sourceware.org/pipermail/gdb-patches/2023-April/199095.html	

Br,
Matti Puputti

> -----Original Message-----
> From: Gdb-patches <gdb-patches-
> bounces+matti.puputti=intel.com@sourceware.org> On Behalf Of Matti
> Puputti via Gdb-patches
> Sent: Tuesday, April 25, 2023 3:38 PM
> To: gdb-patches@sourceware.org
> Cc: aburgess@redhat.com
> Subject: [PATCH v3 1/1] gdb, infcmd: Support jump command in multi-
> inferior case.
> 
> Fixes the issue where jump failed if multiple inferiors run the same source.
> 
> See the below example
> 
>     $ gdb -q ./simple
>     Reading symbols from ./simple...
>     (gdb) break 2
>     Breakpoint 1 at 0x114e: file simple.c, line 2.
>     (gdb) run
>     Starting program: /temp/simple
> 
>     Breakpoint 1, main () at simple.c:2
>     2         int a = 42;
>     (gdb) add-inferior
>     [New inferior 2]
>     Added inferior 2 on connection 1 (native)
>     (gdb) inferior 2
>     [Switching to inferior 2 [<null>] (<noexec>)]
>     (gdb) info inferiors
>       Num  Description       Connection           Executable
>       1    process 6250      1 (native)           /temp/simple
>     * 2    <null>            1 (native)
>     (gdb) file ./simple
>     Reading symbols from ./simple...
>     (gdb) run
>     Starting program: /temp/simple
> 
>     Thread 2.1 "simple" hit Breakpoint 1, main () at simple.c:2
>     2         int a = 42;
>     (gdb) info inferiors
>       Num  Description       Connection           Executable
>       1    process 6250      1 (native)           /temp/simple
>     * 2    process 6705      1 (native)           /temp/simple
>     (gdb) jump 3
>     Unreasonable jump request
>     (gdb)
> 
> In this example, jump fails because the debugger finds two different
> locations, one for each inferior.
> Solution is to limit the search to the current program space.
> ---
>  gdb/infcmd.c                    |   3 +-
>  gdb/linespec.c                  |   7 +-
>  gdb/linespec.h                  |   5 +-
>  gdb/testsuite/gdb.base/jump.exp | 177 +++++++++++++++++---------------
>  4 files changed, 103 insertions(+), 89 deletions(-)
> 
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 103899432f7..a79b5d3d4d6 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1077,7 +1077,8 @@ jump_command (const char *arg, int from_tty)
>      error_no_arg (_("starting address"));
> 
>    std::vector<symtab_and_line> sals
> -    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE);
> +    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE,
> +				       current_program_space);
>    if (sals.size () != 1)
>      error (_("Unreasonable jump request"));
> 
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 7d969f37fbf..920fd3d37b7 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -3231,7 +3231,8 @@ decode_line_with_current_source (const char
> *string, int flags)
>  /* See linespec.h.  */
> 
>  std::vector<symtab_and_line>
> -decode_line_with_last_displayed (const char *string, int flags)
> +decode_line_with_last_displayed (const char *string, int flags,
> +				 program_space *search_pspace)
>  {
>    if (string == 0)
>      error (_("Empty line specification."));
> @@ -3240,10 +3241,10 @@ decode_line_with_last_displayed (const char
> *string, int flags)
>  						      current_language);
>    std::vector<symtab_and_line> sals
>      = (last_displayed_sal_is_valid ()
> -       ? decode_line_1 (locspec.get (), flags, NULL,
> +       ? decode_line_1 (locspec.get (), flags, search_pspace,
>  			get_last_displayed_symtab (),
>  			get_last_displayed_line ())
> -       : decode_line_1 (locspec.get (), flags, NULL, NULL, 0));
> +       : decode_line_1 (locspec.get (), flags, search_pspace, NULL, 0));
> 
>    if (*string)
>      error (_("Junk at end of line specification: %s"), string);
> diff --git a/gdb/linespec.h b/gdb/linespec.h
> index d5e7334fe2d..0eb9cb5d9f2 100644
> --- a/gdb/linespec.h
> +++ b/gdb/linespec.h
> @@ -139,10 +139,11 @@ extern std::vector<symtab_and_line>
> decode_line_with_current_source
>      (const char *, int);
> 
>  /* Given a string, return the line specified by it, using the last displayed
> -   codepoint's values as defaults, or nothing if they aren't valid.  */
> +   codepoint's values as defaults, or nothing if they aren't valid.
> +   Limit the search to given program space, if specified.  */
> 
>  extern std::vector<symtab_and_line> decode_line_with_last_displayed
> -    (const char *, int);
> +    (const char *, int, program_space *search_pspace = nullptr);
> 
>  /* Does P represent one of the keywords?  If so, return
>     the keyword.  If not, return NULL.  */
> diff --git a/gdb/testsuite/gdb.base/jump.exp
> b/gdb/testsuite/gdb.base/jump.exp
> index 032c4a6279d..4c0d4ed2dbb 100644
> --- a/gdb/testsuite/gdb.base/jump.exp
> +++ b/gdb/testsuite/gdb.base/jump.exp
> @@ -18,99 +18,110 @@ clear_xfail "*-*-*"
> 
>  standard_testfile .c
> 
> -# Build the test case
> -if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
> {debug nowarnings}] != "" } {
> -     untested "failed to compile"
> -     return -1
> -    }
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> +    return -1
> +}
> 
> 
> -# Start with a fresh gdb
> +proc do_tests {} {
> +    global decimal srcfile
> +
> +    # Set a breakpoint on the statement that we're about to jump to.
> +    # The statement doesn't contain a function call.
> +    set non_call_line [gdb_get_line_number "bp-on-non-call"]
> +    gdb_breakpoint "$non_call_line"
> +    set bp_on_non_call \
> +	[get_integer_valueof "\$bpnum" "INVALID" "bp_on_non_call"]
> +
> +    # Can we jump to the statement?  Do we stop there?
> +    gdb_test "jump $non_call_line" \
> +	"Breakpoint ${bp_on_non_call}(\.${decimal})?,
> .*${srcfile}:$non_call_line.*" \
> +	"jump to non-call"
> +
> +    # Set a breakpoint on the statement that we're about to jump to.
> +    # The statement does contain a function call.
> +    set call_line [gdb_get_line_number "bp-on-call"]
> +    gdb_breakpoint "$call_line"
> +    set bp_on_call [get_integer_valueof "\$bpnum" "INVALID" "bp_on_call"]
> +
> +    # Can we jump to the statement?  Do we stop there?
> +    gdb_test "jump $call_line" \
> +	"Breakpoint ${bp_on_call}(\.${decimal})?, .*${srcfile}:$call_line.*" \
> +	"jump to call"
> +
> +    # If we disable the breakpoint at the function call, and then
> +    # if we jump to that statement, do we not stop there, but at
> +    # the following breakpoint?
> +    gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
> +
> +    gdb_test "jump $call_line" \
> +	"Breakpoint ${bp_on_non_call}(\.${decimal})?,
> .*${srcfile}:$non_call_line.*" \
> +	"jump to call with disabled breakpoint"
> +
> +    # Disable the breakpoint at the non-function call, so it won't hit
> +    # if do_test is called again.
> +    gdb_test_no_output "disable ${bp_on_non_call}" "disable
> bp_on_non_call"
> +
> +    # Verify that GDB responds gracefully to the "jump" command without
> +    # an argument.
> +    gdb_test "jump" "Argument required .starting address.*" \
> +	"jump without argument disallowed"
> +
> +    # Verify that GDB responds gracefully to the "jump" command with
> +    # trailing junk.
> +    gdb_test "jump $call_line 100" \
> +	"malformed linespec error: unexpected number, \"100\"" \
> +	"jump with trailing argument junk"
> +
> +    # Verify that GDB responds gracefully to a request to jump out of
> +    # the current function.  (Note that this will very likely cause the
> +    # inferior to die.  Be prepared to rerun the inferior, if further
> +    # testing is desired.)
> +    # Try it both ways: confirming and not confirming the jump.
> +    set out_line [gdb_get_line_number "out-of-func"]
> +    gdb_test "jump $out_line" \
> +	"Not confirmed.*" \
> +	"aborted jump out of current function" \
> +	"Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> +	"n"
> +
> +    gdb_test "jump $out_line" \
> +	"Continuing at.*" \
> +	"jump out of current function" \
> +	"Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> +	"y"
> +}
> 
> -clean_restart ${binfile}
> 
> -if {![runto_main]} {
> -  return -1
> +set inferiors 1
> +if {![use_gdb_stub]} {
> +    set inferiors 2
>  }
> 
> -# Set a breakpoint on the statement that we're about to jump to.
> -# The statement doesn't contain a function call.
> -#
> -set bp_on_non_call 0
> -set non_call_line [gdb_get_line_number "bp-on-non-call"]
> -gdb_test_multiple "break $non_call_line" "break before jump to non-call" {
> -    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line
> $non_call_line.*$gdb_prompt $" {
> -	set bp_on_non_call $expect_out(1,string)
> -	pass "break before jump to non-call"
> +# Run to main, add inferiors if needed.
> +for {set inf 1} {$inf <= $inferiors} {incr inf} {
> +    if {$inf != 1} {
> +	# Start a new inferior, and run it with the same executable.
> +	gdb_test "add-inferior -exec ${binfile}" \
> +	    "Added inferior ${inf}.*" \
> +	    "add inferior ${inf} with -exec "
> +	gdb_test "inferior ${inf}" \
> +	    "Switching to inferior ${inf} .*" \
> +	    "switch to inferior ${inf}"
> +    }
> +    if {![runto_main]} {
> +	return -1
>      }
>  }
> 
> -# Can we jump to the statement?  Do we stop there?
> -#
> -gdb_test "jump $non_call_line" "Breakpoint ${decimal},
> .*${srcfile}:$non_call_line.*" \
> -    "jump to non-call"
> -
> -# Set a breakpoint on the statement that we're about to jump to.
> -# The statement does contain a function call.
> -#
> -set bp_on_call 0
> -set call_line [gdb_get_line_number "bp-on-call"]
> -gdb_test_multiple "break $call_line" "break before jump to call" {
> -    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line
> $call_line.*$gdb_prompt $" {
> -	set bp_on_call $expect_out(1,string)
> -	pass "break before jump to call"
> +# Run tests on all inferiors.
> +for {set inf 1} {$inf <= $inferiors} {incr inf} {
> +    with_test_prefix "inferior $inf" {
> +	# Switch to the target inferior.
> +	gdb_test "inferior $inf" ".*Switching to inferior $inf .*"
> +	# Run the tests.
> +	do_tests
>      }
>  }
> 
> -# Can we jump to the statement?  Do we stop there?
> -#
> -gdb_test "jump $call_line" \
> -    "Breakpoint ${decimal}, .*${srcfile}:$call_line.*" \
> -    "jump to call"
> -
> -# If we disable the breakpoint at the function call, and then
> -# if we jump to that statement, do we not stop there, but at
> -# the following breakpoint?
> -#
> -gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
> -
> -gdb_test "jump $call_line" "Breakpoint ${decimal},
> .*${srcfile}:$non_call_line.*" \
> -    "jump to call with disabled breakpoint"
> -
> -# Verify that GDB responds gracefully to the "jump" command without
> -# an argument.
> -#
> -gdb_test "jump" "Argument required .starting address.*" \
> -    "jump without argument disallowed"
> -
> -
> -# Verify that GDB responds gracefully to the "jump" command with
> -# trailing junk.
> -#
> -gdb_test "jump $call_line 100" \
> -    "malformed linespec error: unexpected number, \"100\"" \
> -    "jump with trailing argument junk"
> -
> -
> -# Verify that GDB responds gracefully to a request to jump out of
> -# the current function.  (Note that this will very likely cause the
> -# inferior to die.  Be prepared to rerun the inferior, if further
> -# testing is desired.)
> -#
> -# Try it both ways: confirming and not confirming the jump.
> -#
> -
> -set out_line [gdb_get_line_number "out-of-func"]
> -gdb_test "jump $out_line" \
> -    "Not confirmed.*" \
> -    "aborted jump out of current function" \
> -    "Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> -    "n"
> -
> -gdb_test "jump $out_line" \
> -    "Continuing at.*" \
> -    "jump out of current function" \
> -    "Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> -    "y"
> -
>  gdb_exit
> --
> 2.25.1
> 
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Matti Puputti <matti.puputti@intel.com> writes:

> Fixes the issue where jump failed if multiple inferiors run the same source.
>
> See the below example
>
>     $ gdb -q ./simple
>     Reading symbols from ./simple...
>     (gdb) break 2
>     Breakpoint 1 at 0x114e: file simple.c, line 2.
>     (gdb) run
>     Starting program: /temp/simple
>
>     Breakpoint 1, main () at simple.c:2
>     2         int a = 42;
>     (gdb) add-inferior
>     [New inferior 2]
>     Added inferior 2 on connection 1 (native)
>     (gdb) inferior 2
>     [Switching to inferior 2 [<null>] (<noexec>)]
>     (gdb) info inferiors
>       Num  Description       Connection           Executable
>       1    process 6250      1 (native)           /temp/simple
>     * 2    <null>            1 (native)
>     (gdb) file ./simple
>     Reading symbols from ./simple...
>     (gdb) run
>     Starting program: /temp/simple
>
>     Thread 2.1 "simple" hit Breakpoint 1, main () at simple.c:2
>     2         int a = 42;
>     (gdb) info inferiors
>       Num  Description       Connection           Executable
>       1    process 6250      1 (native)           /temp/simple
>     * 2    process 6705      1 (native)           /temp/simple
>     (gdb) jump 3
>     Unreasonable jump request
>     (gdb)
>
> In this example, jump fails because the debugger finds two different
> locations, one for each inferior.
> Solution is to limit the search to the current program space.
> ---
>  gdb/infcmd.c                    |   3 +-
>  gdb/linespec.c                  |   7 +-
>  gdb/linespec.h                  |   5 +-
>  gdb/testsuite/gdb.base/jump.exp | 177 +++++++++++++++++---------------
>  4 files changed, 103 insertions(+), 89 deletions(-)
>
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 103899432f7..a79b5d3d4d6 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1077,7 +1077,8 @@ jump_command (const char *arg, int from_tty)
>      error_no_arg (_("starting address"));
>  
>    std::vector<symtab_and_line> sals
> -    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE);
> +    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE,
> +				       current_program_space);

I wonder if we should stop using decode_line_with_last_displayed here,
and instead use decode_line_with_current_source?  It feels like a jump
should always be relative to the current location, rather than what we
happen to have listed.

>    if (sals.size () != 1)
>      error (_("Unreasonable jump request"));
>  
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 7d969f37fbf..920fd3d37b7 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -3231,7 +3231,8 @@ decode_line_with_current_source (const char *string, int flags)
>  /* See linespec.h.  */
>  
>  std::vector<symtab_and_line>
> -decode_line_with_last_displayed (const char *string, int flags)
> +decode_line_with_last_displayed (const char *string, int flags,
> +				 program_space *search_pspace)
>  {
>    if (string == 0)
>      error (_("Empty line specification."));
> @@ -3240,10 +3241,10 @@ decode_line_with_last_displayed (const char *string, int flags)
>  						      current_language);
>    std::vector<symtab_and_line> sals
>      = (last_displayed_sal_is_valid ()
> -       ? decode_line_1 (locspec.get (), flags, NULL,
> +       ? decode_line_1 (locspec.get (), flags, search_pspace,
>  			get_last_displayed_symtab (),
>  			get_last_displayed_line ())

This change is why I started looking at the details of this patch again.
I'm not sure this change makes sense, I would have expected us to use
get_last_displayed_pspace () here instead of search_pspace, but then it
becomes harder to describe what search_pspace does as it would only be
used below.

It we switch jump to use decode_line_with_current_source then I think
this problem goes away, the changes to decode_line_with_last_displayed
can be dropped.

At the end of this email you'll find a patch that applies on top of your
V3, which implements the change I'm suggesting.  This passes all the
'jump' tests I could find in the testsuite, so I think it's fine, but
it would be great to hear your thoughts.

Thanks,
Andrew

> -       : decode_line_1 (locspec.get (), flags, NULL, NULL, 0));
> +       : decode_line_1 (locspec.get (), flags, search_pspace, NULL, 0));
>  
>    if (*string)
>      error (_("Junk at end of line specification: %s"), string);
> diff --git a/gdb/linespec.h b/gdb/linespec.h
> index d5e7334fe2d..0eb9cb5d9f2 100644
> --- a/gdb/linespec.h
> +++ b/gdb/linespec.h
> @@ -139,10 +139,11 @@ extern std::vector<symtab_and_line> decode_line_with_current_source
>      (const char *, int);
>  
>  /* Given a string, return the line specified by it, using the last displayed
> -   codepoint's values as defaults, or nothing if they aren't valid.  */
> +   codepoint's values as defaults, or nothing if they aren't valid.
> +   Limit the search to given program space, if specified.  */
>  
>  extern std::vector<symtab_and_line> decode_line_with_last_displayed
> -    (const char *, int);
> +    (const char *, int, program_space *search_pspace = nullptr);
>  
>  /* Does P represent one of the keywords?  If so, return
>     the keyword.  If not, return NULL.  */
> diff --git a/gdb/testsuite/gdb.base/jump.exp b/gdb/testsuite/gdb.base/jump.exp
> index 032c4a6279d..4c0d4ed2dbb 100644
> --- a/gdb/testsuite/gdb.base/jump.exp
> +++ b/gdb/testsuite/gdb.base/jump.exp
> @@ -18,99 +18,110 @@ clear_xfail "*-*-*"
>  
>  standard_testfile .c
>  
> -# Build the test case
> -if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug nowarnings}] != "" } {
> -     untested "failed to compile"
> -     return -1
> -    }
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> +    return -1
> +}
>  
>  
> -# Start with a fresh gdb
> +proc do_tests {} {
> +    global decimal srcfile
> +
> +    # Set a breakpoint on the statement that we're about to jump to.
> +    # The statement doesn't contain a function call.
> +    set non_call_line [gdb_get_line_number "bp-on-non-call"]
> +    gdb_breakpoint "$non_call_line"
> +    set bp_on_non_call \
> +	[get_integer_valueof "\$bpnum" "INVALID" "bp_on_non_call"]
> +
> +    # Can we jump to the statement?  Do we stop there?
> +    gdb_test "jump $non_call_line" \
> +	"Breakpoint ${bp_on_non_call}(\.${decimal})?, .*${srcfile}:$non_call_line.*" \
> +	"jump to non-call"
> +
> +    # Set a breakpoint on the statement that we're about to jump to.
> +    # The statement does contain a function call.
> +    set call_line [gdb_get_line_number "bp-on-call"]
> +    gdb_breakpoint "$call_line"
> +    set bp_on_call [get_integer_valueof "\$bpnum" "INVALID" "bp_on_call"]
> +
> +    # Can we jump to the statement?  Do we stop there?
> +    gdb_test "jump $call_line" \
> +	"Breakpoint ${bp_on_call}(\.${decimal})?, .*${srcfile}:$call_line.*" \
> +	"jump to call"
> +
> +    # If we disable the breakpoint at the function call, and then
> +    # if we jump to that statement, do we not stop there, but at
> +    # the following breakpoint?
> +    gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
> +
> +    gdb_test "jump $call_line" \
> +	"Breakpoint ${bp_on_non_call}(\.${decimal})?, .*${srcfile}:$non_call_line.*" \
> +	"jump to call with disabled breakpoint"
> +
> +    # Disable the breakpoint at the non-function call, so it won't hit
> +    # if do_test is called again.
> +    gdb_test_no_output "disable ${bp_on_non_call}" "disable bp_on_non_call"
> +
> +    # Verify that GDB responds gracefully to the "jump" command without
> +    # an argument.
> +    gdb_test "jump" "Argument required .starting address.*" \
> +	"jump without argument disallowed"
> +
> +    # Verify that GDB responds gracefully to the "jump" command with
> +    # trailing junk.
> +    gdb_test "jump $call_line 100" \
> +	"malformed linespec error: unexpected number, \"100\"" \
> +	"jump with trailing argument junk"
> +
> +    # Verify that GDB responds gracefully to a request to jump out of
> +    # the current function.  (Note that this will very likely cause the
> +    # inferior to die.  Be prepared to rerun the inferior, if further
> +    # testing is desired.)
> +    # Try it both ways: confirming and not confirming the jump.
> +    set out_line [gdb_get_line_number "out-of-func"]
> +    gdb_test "jump $out_line" \
> +	"Not confirmed.*" \
> +	"aborted jump out of current function" \
> +	"Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> +	"n"
> +
> +    gdb_test "jump $out_line" \
> +	"Continuing at.*" \
> +	"jump out of current function" \
> +	"Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> +	"y"
> +}
>  
> -clean_restart ${binfile}
>  
> -if {![runto_main]} {
> -  return -1
> +set inferiors 1
> +if {![use_gdb_stub]} {
> +    set inferiors 2
>  }
>  
> -# Set a breakpoint on the statement that we're about to jump to.
> -# The statement doesn't contain a function call.
> -#
> -set bp_on_non_call 0
> -set non_call_line [gdb_get_line_number "bp-on-non-call"]
> -gdb_test_multiple "break $non_call_line" "break before jump to non-call" {
> -    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line $non_call_line.*$gdb_prompt $" {
> -	set bp_on_non_call $expect_out(1,string)
> -	pass "break before jump to non-call"
> +# Run to main, add inferiors if needed.
> +for {set inf 1} {$inf <= $inferiors} {incr inf} {
> +    if {$inf != 1} {
> +	# Start a new inferior, and run it with the same executable.
> +	gdb_test "add-inferior -exec ${binfile}" \
> +	    "Added inferior ${inf}.*" \
> +	    "add inferior ${inf} with -exec "
> +	gdb_test "inferior ${inf}" \
> +	    "Switching to inferior ${inf} .*" \
> +	    "switch to inferior ${inf}"
> +    }
> +    if {![runto_main]} {
> +	return -1
>      }
>  }
>  
> -# Can we jump to the statement?  Do we stop there?
> -#
> -gdb_test "jump $non_call_line" "Breakpoint ${decimal}, .*${srcfile}:$non_call_line.*" \
> -    "jump to non-call"
> -
> -# Set a breakpoint on the statement that we're about to jump to.
> -# The statement does contain a function call.
> -#
> -set bp_on_call 0
> -set call_line [gdb_get_line_number "bp-on-call"]
> -gdb_test_multiple "break $call_line" "break before jump to call" {
> -    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line $call_line.*$gdb_prompt $" {
> -	set bp_on_call $expect_out(1,string)
> -	pass "break before jump to call"
> +# Run tests on all inferiors.
> +for {set inf 1} {$inf <= $inferiors} {incr inf} {
> +    with_test_prefix "inferior $inf" {
> +	# Switch to the target inferior.
> +	gdb_test "inferior $inf" ".*Switching to inferior $inf .*"
> +	# Run the tests.
> +	do_tests
>      }
>  }
>  
> -# Can we jump to the statement?  Do we stop there?
> -#
> -gdb_test "jump $call_line" \
> -    "Breakpoint ${decimal}, .*${srcfile}:$call_line.*" \
> -    "jump to call"
> -
> -# If we disable the breakpoint at the function call, and then
> -# if we jump to that statement, do we not stop there, but at
> -# the following breakpoint?
> -#
> -gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
> -
> -gdb_test "jump $call_line" "Breakpoint ${decimal}, .*${srcfile}:$non_call_line.*" \
> -    "jump to call with disabled breakpoint"
> -
> -# Verify that GDB responds gracefully to the "jump" command without
> -# an argument.
> -#
> -gdb_test "jump" "Argument required .starting address.*" \
> -    "jump without argument disallowed"
> -
> -
> -# Verify that GDB responds gracefully to the "jump" command with
> -# trailing junk.
> -#
> -gdb_test "jump $call_line 100" \
> -    "malformed linespec error: unexpected number, \"100\"" \
> -    "jump with trailing argument junk"
> -
> -
> -# Verify that GDB responds gracefully to a request to jump out of
> -# the current function.  (Note that this will very likely cause the
> -# inferior to die.  Be prepared to rerun the inferior, if further
> -# testing is desired.)
> -#
> -# Try it both ways: confirming and not confirming the jump.
> -#
> -
> -set out_line [gdb_get_line_number "out-of-func"]
> -gdb_test "jump $out_line" \
> -    "Not confirmed.*" \
> -    "aborted jump out of current function" \
> -    "Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> -    "n"
> -
> -gdb_test "jump $out_line" \
> -    "Continuing at.*" \
> -    "jump out of current function" \
> -    "Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> -    "y"
> -
>  gdb_exit
> -- 
> 2.25.1
>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928

---

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 1be72ed09f0..1d10e2908ca 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1068,8 +1068,7 @@ jump_command (const char *arg, int from_tty)
     error_no_arg (_("starting address"));
 
   std::vector<symtab_and_line> sals
-    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE,
-				       current_program_space);
+    = decode_line_with_current_source (arg, DECODE_LINE_FUNFIRSTLINE);
   if (sals.size () != 1)
     error (_("Unreasonable jump request"));
 
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 920fd3d37b7..9f03a4b3604 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3220,7 +3220,8 @@ decode_line_with_current_source (const char *string, int flags)
   location_spec_up locspec = string_to_location_spec (&string,
 						      current_language);
   std::vector<symtab_and_line> sals
-    = decode_line_1 (locspec.get (), flags, NULL, cursal.symtab, cursal.line);
+    = decode_line_1 (locspec.get (), flags, cursal.pspace, cursal.symtab,
+		     cursal.line);
 
   if (*string)
     error (_("Junk at end of line specification: %s"), string);
@@ -3231,8 +3232,7 @@ decode_line_with_current_source (const char *string, int flags)
 /* See linespec.h.  */
 
 std::vector<symtab_and_line>
-decode_line_with_last_displayed (const char *string, int flags,
-				 program_space *search_pspace)
+decode_line_with_last_displayed (const char *string, int flags)
 {
   if (string == 0)
     error (_("Empty line specification."));
@@ -3241,10 +3241,10 @@ decode_line_with_last_displayed (const char *string, int flags,
 						      current_language);
   std::vector<symtab_and_line> sals
     = (last_displayed_sal_is_valid ()
-       ? decode_line_1 (locspec.get (), flags, search_pspace,
+       ? decode_line_1 (locspec.get (), flags, NULL,
 			get_last_displayed_symtab (),
 			get_last_displayed_line ())
-       : decode_line_1 (locspec.get (), flags, search_pspace, NULL, 0));
+       : decode_line_1 (locspec.get (), flags, NULL, NULL, 0));
 
   if (*string)
     error (_("Junk at end of line specification: %s"), string);
diff --git a/gdb/linespec.h b/gdb/linespec.h
index 0eb9cb5d9f2..c0eef7894d9 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -143,7 +143,7 @@ extern std::vector<symtab_and_line> decode_line_with_current_source
    Limit the search to given program space, if specified.  */
 
 extern std::vector<symtab_and_line> decode_line_with_last_displayed
-    (const char *, int, program_space *search_pspace = nullptr);
+    (const char *, int);
 
 /* Does P represent one of the keywords?  If so, return
    the keyword.  If not, return NULL.  */
  

Hi Andrew,

Thank you for these comments.
I'll be addressing them as soon as I will be back in office, in few weeks

Br,
Matti Puputti

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Wednesday, May 24, 2023 7:33 PM
> To: Puputti, Matti <matti.puputti@intel.com>; gdb-patches@sourceware.org
> Subject: Re: [PATCH v3 1/1] gdb, infcmd: Support jump command in multi-
> inferior case.
> 
> Matti Puputti <matti.puputti@intel.com> writes:
> 
> > Fixes the issue where jump failed if multiple inferiors run the same source.
> >
> > See the below example
> >
> >     $ gdb -q ./simple
> >     Reading symbols from ./simple...
> >     (gdb) break 2
> >     Breakpoint 1 at 0x114e: file simple.c, line 2.
> >     (gdb) run
> >     Starting program: /temp/simple
> >
> >     Breakpoint 1, main () at simple.c:2
> >     2         int a = 42;
> >     (gdb) add-inferior
> >     [New inferior 2]
> >     Added inferior 2 on connection 1 (native)
> >     (gdb) inferior 2
> >     [Switching to inferior 2 [<null>] (<noexec>)]
> >     (gdb) info inferiors
> >       Num  Description       Connection           Executable
> >       1    process 6250      1 (native)           /temp/simple
> >     * 2    <null>            1 (native)
> >     (gdb) file ./simple
> >     Reading symbols from ./simple...
> >     (gdb) run
> >     Starting program: /temp/simple
> >
> >     Thread 2.1 "simple" hit Breakpoint 1, main () at simple.c:2
> >     2         int a = 42;
> >     (gdb) info inferiors
> >       Num  Description       Connection           Executable
> >       1    process 6250      1 (native)           /temp/simple
> >     * 2    process 6705      1 (native)           /temp/simple
> >     (gdb) jump 3
> >     Unreasonable jump request
> >     (gdb)
> >
> > In this example, jump fails because the debugger finds two different
> > locations, one for each inferior.
> > Solution is to limit the search to the current program space.
> > ---
> >  gdb/infcmd.c                    |   3 +-
> >  gdb/linespec.c                  |   7 +-
> >  gdb/linespec.h                  |   5 +-
> >  gdb/testsuite/gdb.base/jump.exp | 177 +++++++++++++++++--------------
> -
> >  4 files changed, 103 insertions(+), 89 deletions(-)
> >
> > diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> > index 103899432f7..a79b5d3d4d6 100644
> > --- a/gdb/infcmd.c
> > +++ b/gdb/infcmd.c
> > @@ -1077,7 +1077,8 @@ jump_command (const char *arg, int from_tty)
> >      error_no_arg (_("starting address"));
> >
> >    std::vector<symtab_and_line> sals
> > -    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE);
> > +    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE,
> > +				       current_program_space);
> 
> I wonder if we should stop using decode_line_with_last_displayed here,
> and instead use decode_line_with_current_source?  It feels like a jump
> should always be relative to the current location, rather than what we
> happen to have listed.
> 
> >    if (sals.size () != 1)
> >      error (_("Unreasonable jump request"));
> >
> > diff --git a/gdb/linespec.c b/gdb/linespec.c
> > index 7d969f37fbf..920fd3d37b7 100644
> > --- a/gdb/linespec.c
> > +++ b/gdb/linespec.c
> > @@ -3231,7 +3231,8 @@ decode_line_with_current_source (const char
> *string, int flags)
> >  /* See linespec.h.  */
> >
> >  std::vector<symtab_and_line>
> > -decode_line_with_last_displayed (const char *string, int flags)
> > +decode_line_with_last_displayed (const char *string, int flags,
> > +				 program_space *search_pspace)
> >  {
> >    if (string == 0)
> >      error (_("Empty line specification."));
> > @@ -3240,10 +3241,10 @@ decode_line_with_last_displayed (const char
> *string, int flags)
> >  						      current_language);
> >    std::vector<symtab_and_line> sals
> >      = (last_displayed_sal_is_valid ()
> > -       ? decode_line_1 (locspec.get (), flags, NULL,
> > +       ? decode_line_1 (locspec.get (), flags, search_pspace,
> >  			get_last_displayed_symtab (),
> >  			get_last_displayed_line ())
> 
> This change is why I started looking at the details of this patch again.
> I'm not sure this change makes sense, I would have expected us to use
> get_last_displayed_pspace () here instead of search_pspace, but then it
> becomes harder to describe what search_pspace does as it would only be
> used below.
> 
> It we switch jump to use decode_line_with_current_source then I think
> this problem goes away, the changes to decode_line_with_last_displayed
> can be dropped.
> 
> At the end of this email you'll find a patch that applies on top of your
> V3, which implements the change I'm suggesting.  This passes all the
> 'jump' tests I could find in the testsuite, so I think it's fine, but
> it would be great to hear your thoughts.
> 
> Thanks,
> Andrew
> 
> > -       : decode_line_1 (locspec.get (), flags, NULL, NULL, 0));
> > +       : decode_line_1 (locspec.get (), flags, search_pspace, NULL, 0));
> >
> >    if (*string)
> >      error (_("Junk at end of line specification: %s"), string);
> > diff --git a/gdb/linespec.h b/gdb/linespec.h
> > index d5e7334fe2d..0eb9cb5d9f2 100644
> > --- a/gdb/linespec.h
> > +++ b/gdb/linespec.h
> > @@ -139,10 +139,11 @@ extern std::vector<symtab_and_line>
> decode_line_with_current_source
> >      (const char *, int);
> >
> >  /* Given a string, return the line specified by it, using the last displayed
> > -   codepoint's values as defaults, or nothing if they aren't valid.  */
> > +   codepoint's values as defaults, or nothing if they aren't valid.
> > +   Limit the search to given program space, if specified.  */
> >
> >  extern std::vector<symtab_and_line> decode_line_with_last_displayed
> > -    (const char *, int);
> > +    (const char *, int, program_space *search_pspace = nullptr);
> >
> >  /* Does P represent one of the keywords?  If so, return
> >     the keyword.  If not, return NULL.  */
> > diff --git a/gdb/testsuite/gdb.base/jump.exp
> b/gdb/testsuite/gdb.base/jump.exp
> > index 032c4a6279d..4c0d4ed2dbb 100644
> > --- a/gdb/testsuite/gdb.base/jump.exp
> > +++ b/gdb/testsuite/gdb.base/jump.exp
> > @@ -18,99 +18,110 @@ clear_xfail "*-*-*"
> >
> >  standard_testfile .c
> >
> > -# Build the test case
> > -if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
> {debug nowarnings}] != "" } {
> > -     untested "failed to compile"
> > -     return -1
> > -    }
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> > +    return -1
> > +}
> >
> >
> > -# Start with a fresh gdb
> > +proc do_tests {} {
> > +    global decimal srcfile
> > +
> > +    # Set a breakpoint on the statement that we're about to jump to.
> > +    # The statement doesn't contain a function call.
> > +    set non_call_line [gdb_get_line_number "bp-on-non-call"]
> > +    gdb_breakpoint "$non_call_line"
> > +    set bp_on_non_call \
> > +	[get_integer_valueof "\$bpnum" "INVALID" "bp_on_non_call"]
> > +
> > +    # Can we jump to the statement?  Do we stop there?
> > +    gdb_test "jump $non_call_line" \
> > +	"Breakpoint ${bp_on_non_call}(\.${decimal})?,
> .*${srcfile}:$non_call_line.*" \
> > +	"jump to non-call"
> > +
> > +    # Set a breakpoint on the statement that we're about to jump to.
> > +    # The statement does contain a function call.
> > +    set call_line [gdb_get_line_number "bp-on-call"]
> > +    gdb_breakpoint "$call_line"
> > +    set bp_on_call [get_integer_valueof "\$bpnum" "INVALID"
> "bp_on_call"]
> > +
> > +    # Can we jump to the statement?  Do we stop there?
> > +    gdb_test "jump $call_line" \
> > +	"Breakpoint ${bp_on_call}(\.${decimal})?, .*${srcfile}:$call_line.*" \
> > +	"jump to call"
> > +
> > +    # If we disable the breakpoint at the function call, and then
> > +    # if we jump to that statement, do we not stop there, but at
> > +    # the following breakpoint?
> > +    gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
> > +
> > +    gdb_test "jump $call_line" \
> > +	"Breakpoint ${bp_on_non_call}(\.${decimal})?,
> .*${srcfile}:$non_call_line.*" \
> > +	"jump to call with disabled breakpoint"
> > +
> > +    # Disable the breakpoint at the non-function call, so it won't hit
> > +    # if do_test is called again.
> > +    gdb_test_no_output "disable ${bp_on_non_call}" "disable
> bp_on_non_call"
> > +
> > +    # Verify that GDB responds gracefully to the "jump" command without
> > +    # an argument.
> > +    gdb_test "jump" "Argument required .starting address.*" \
> > +	"jump without argument disallowed"
> > +
> > +    # Verify that GDB responds gracefully to the "jump" command with
> > +    # trailing junk.
> > +    gdb_test "jump $call_line 100" \
> > +	"malformed linespec error: unexpected number, \"100\"" \
> > +	"jump with trailing argument junk"
> > +
> > +    # Verify that GDB responds gracefully to a request to jump out of
> > +    # the current function.  (Note that this will very likely cause the
> > +    # inferior to die.  Be prepared to rerun the inferior, if further
> > +    # testing is desired.)
> > +    # Try it both ways: confirming and not confirming the jump.
> > +    set out_line [gdb_get_line_number "out-of-func"]
> > +    gdb_test "jump $out_line" \
> > +	"Not confirmed.*" \
> > +	"aborted jump out of current function" \
> > +	"Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> > +	"n"
> > +
> > +    gdb_test "jump $out_line" \
> > +	"Continuing at.*" \
> > +	"jump out of current function" \
> > +	"Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> > +	"y"
> > +}
> >
> > -clean_restart ${binfile}
> >
> > -if {![runto_main]} {
> > -  return -1
> > +set inferiors 1
> > +if {![use_gdb_stub]} {
> > +    set inferiors 2
> >  }
> >
> > -# Set a breakpoint on the statement that we're about to jump to.
> > -# The statement doesn't contain a function call.
> > -#
> > -set bp_on_non_call 0
> > -set non_call_line [gdb_get_line_number "bp-on-non-call"]
> > -gdb_test_multiple "break $non_call_line" "break before jump to non-call"
> {
> > -    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line
> $non_call_line.*$gdb_prompt $" {
> > -	set bp_on_non_call $expect_out(1,string)
> > -	pass "break before jump to non-call"
> > +# Run to main, add inferiors if needed.
> > +for {set inf 1} {$inf <= $inferiors} {incr inf} {
> > +    if {$inf != 1} {
> > +	# Start a new inferior, and run it with the same executable.
> > +	gdb_test "add-inferior -exec ${binfile}" \
> > +	    "Added inferior ${inf}.*" \
> > +	    "add inferior ${inf} with -exec "
> > +	gdb_test "inferior ${inf}" \
> > +	    "Switching to inferior ${inf} .*" \
> > +	    "switch to inferior ${inf}"
> > +    }
> > +    if {![runto_main]} {
> > +	return -1
> >      }
> >  }
> >
> > -# Can we jump to the statement?  Do we stop there?
> > -#
> > -gdb_test "jump $non_call_line" "Breakpoint ${decimal},
> .*${srcfile}:$non_call_line.*" \
> > -    "jump to non-call"
> > -
> > -# Set a breakpoint on the statement that we're about to jump to.
> > -# The statement does contain a function call.
> > -#
> > -set bp_on_call 0
> > -set call_line [gdb_get_line_number "bp-on-call"]
> > -gdb_test_multiple "break $call_line" "break before jump to call" {
> > -    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line
> $call_line.*$gdb_prompt $" {
> > -	set bp_on_call $expect_out(1,string)
> > -	pass "break before jump to call"
> > +# Run tests on all inferiors.
> > +for {set inf 1} {$inf <= $inferiors} {incr inf} {
> > +    with_test_prefix "inferior $inf" {
> > +	# Switch to the target inferior.
> > +	gdb_test "inferior $inf" ".*Switching to inferior $inf .*"
> > +	# Run the tests.
> > +	do_tests
> >      }
> >  }
> >
> > -# Can we jump to the statement?  Do we stop there?
> > -#
> > -gdb_test "jump $call_line" \
> > -    "Breakpoint ${decimal}, .*${srcfile}:$call_line.*" \
> > -    "jump to call"
> > -
> > -# If we disable the breakpoint at the function call, and then
> > -# if we jump to that statement, do we not stop there, but at
> > -# the following breakpoint?
> > -#
> > -gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
> > -
> > -gdb_test "jump $call_line" "Breakpoint ${decimal},
> .*${srcfile}:$non_call_line.*" \
> > -    "jump to call with disabled breakpoint"
> > -
> > -# Verify that GDB responds gracefully to the "jump" command without
> > -# an argument.
> > -#
> > -gdb_test "jump" "Argument required .starting address.*" \
> > -    "jump without argument disallowed"
> > -
> > -
> > -# Verify that GDB responds gracefully to the "jump" command with
> > -# trailing junk.
> > -#
> > -gdb_test "jump $call_line 100" \
> > -    "malformed linespec error: unexpected number, \"100\"" \
> > -    "jump with trailing argument junk"
> > -
> > -
> > -# Verify that GDB responds gracefully to a request to jump out of
> > -# the current function.  (Note that this will very likely cause the
> > -# inferior to die.  Be prepared to rerun the inferior, if further
> > -# testing is desired.)
> > -#
> > -# Try it both ways: confirming and not confirming the jump.
> > -#
> > -
> > -set out_line [gdb_get_line_number "out-of-func"]
> > -gdb_test "jump $out_line" \
> > -    "Not confirmed.*" \
> > -    "aborted jump out of current function" \
> > -    "Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> > -    "n"
> > -
> > -gdb_test "jump $out_line" \
> > -    "Continuing at.*" \
> > -    "jump out of current function" \
> > -    "Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> > -    "y"
> > -
> >  gdb_exit
> > --
> > 2.25.1
> >
> > Intel Deutschland GmbH
> > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> > Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
> > Chairperson of the Supervisory Board: Nicole Lau
> > Registered Office: Munich
> > Commercial Register: Amtsgericht Muenchen HRB 186928
> 
> ---
> 
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 1be72ed09f0..1d10e2908ca 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1068,8 +1068,7 @@ jump_command (const char *arg, int from_tty)
>      error_no_arg (_("starting address"));
> 
>    std::vector<symtab_and_line> sals
> -    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE,
> -				       current_program_space);
> +    = decode_line_with_current_source (arg, DECODE_LINE_FUNFIRSTLINE);
>    if (sals.size () != 1)
>      error (_("Unreasonable jump request"));
> 
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 920fd3d37b7..9f03a4b3604 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -3220,7 +3220,8 @@ decode_line_with_current_source (const char
> *string, int flags)
>    location_spec_up locspec = string_to_location_spec (&string,
>  						      current_language);
>    std::vector<symtab_and_line> sals
> -    = decode_line_1 (locspec.get (), flags, NULL, cursal.symtab, cursal.line);
> +    = decode_line_1 (locspec.get (), flags, cursal.pspace, cursal.symtab,
> +		     cursal.line);
> 
>    if (*string)
>      error (_("Junk at end of line specification: %s"), string);
> @@ -3231,8 +3232,7 @@ decode_line_with_current_source (const char
> *string, int flags)
>  /* See linespec.h.  */
> 
>  std::vector<symtab_and_line>
> -decode_line_with_last_displayed (const char *string, int flags,
> -				 program_space *search_pspace)
> +decode_line_with_last_displayed (const char *string, int flags)
>  {
>    if (string == 0)
>      error (_("Empty line specification."));
> @@ -3241,10 +3241,10 @@ decode_line_with_last_displayed (const char
> *string, int flags,
>  						      current_language);
>    std::vector<symtab_and_line> sals
>      = (last_displayed_sal_is_valid ()
> -       ? decode_line_1 (locspec.get (), flags, search_pspace,
> +       ? decode_line_1 (locspec.get (), flags, NULL,
>  			get_last_displayed_symtab (),
>  			get_last_displayed_line ())
> -       : decode_line_1 (locspec.get (), flags, search_pspace, NULL, 0));
> +       : decode_line_1 (locspec.get (), flags, NULL, NULL, 0));
> 
>    if (*string)
>      error (_("Junk at end of line specification: %s"), string);
> diff --git a/gdb/linespec.h b/gdb/linespec.h
> index 0eb9cb5d9f2..c0eef7894d9 100644
> --- a/gdb/linespec.h
> +++ b/gdb/linespec.h
> @@ -143,7 +143,7 @@ extern std::vector<symtab_and_line>
> decode_line_with_current_source
>     Limit the search to given program space, if specified.  */
> 
>  extern std::vector<symtab_and_line> decode_line_with_last_displayed
> -    (const char *, int, program_space *search_pspace = nullptr);
> +    (const char *, int);
> 
>  /* Does P represent one of the keywords?  If so, return
>     the keyword.  If not, return NULL.  */

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Hi Andrew,

Finally back from my vocation, and coming back to this patch.
I just uploaded patch 4, which adopts all the changes you proposed.

You proposed to call decode_line_with_current_source() instead of decode_line_with_last_displayed().
As you seem to say, it may be matter of taste, which one should be called.
Such change may impact some use-cases - corner cases, one could say.
But there doesn't seem to be any testcase relaying on jump to be relative to what have been listed.
And in the end, it makes sense that jump is relative to the current location.

Patch 1: https://sourceware.org/pipermail/gdb-patches/2023-February/197269.html
Comments on patch 1: https://sourceware.org/pipermail/gdb-patches/2023-February/197271.html
Patch 2: https://sourceware.org/pipermail/gdb-patches/2023-February/197302.html
Comments on patch 2: https://sourceware.org/pipermail/gdb-patches/2023-April/199055.html
Patch 3: https://sourceware.org/pipermail/gdb-patches/2023-April/199095.html
Comments on patch 3: https://sourceware.org/pipermail/gdb-patches/2023-May/199853.html
Patch 4: https://sourceware.org/pipermail/gdb-patches/2023-June/200524.html

Br,
Matti Puputti

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Wednesday, May 24, 2023 7:33 PM
> To: Puputti, Matti <matti.puputti@intel.com>; gdb-patches@sourceware.org
> Subject: Re: [PATCH v3 1/1] gdb, infcmd: Support jump command in multi-
> inferior case.
> 
> Matti Puputti <matti.puputti@intel.com> writes:
> 
> > Fixes the issue where jump failed if multiple inferiors run the same source.
> >
> > See the below example
> >
> >     $ gdb -q ./simple
> >     Reading symbols from ./simple...
> >     (gdb) break 2
> >     Breakpoint 1 at 0x114e: file simple.c, line 2.
> >     (gdb) run
> >     Starting program: /temp/simple
> >
> >     Breakpoint 1, main () at simple.c:2
> >     2         int a = 42;
> >     (gdb) add-inferior
> >     [New inferior 2]
> >     Added inferior 2 on connection 1 (native)
> >     (gdb) inferior 2
> >     [Switching to inferior 2 [<null>] (<noexec>)]
> >     (gdb) info inferiors
> >       Num  Description       Connection           Executable
> >       1    process 6250      1 (native)           /temp/simple
> >     * 2    <null>            1 (native)
> >     (gdb) file ./simple
> >     Reading symbols from ./simple...
> >     (gdb) run
> >     Starting program: /temp/simple
> >
> >     Thread 2.1 "simple" hit Breakpoint 1, main () at simple.c:2
> >     2         int a = 42;
> >     (gdb) info inferiors
> >       Num  Description       Connection           Executable
> >       1    process 6250      1 (native)           /temp/simple
> >     * 2    process 6705      1 (native)           /temp/simple
> >     (gdb) jump 3
> >     Unreasonable jump request
> >     (gdb)
> >
> > In this example, jump fails because the debugger finds two different
> > locations, one for each inferior.
> > Solution is to limit the search to the current program space.
> > ---
> >  gdb/infcmd.c                    |   3 +-
> >  gdb/linespec.c                  |   7 +-
> >  gdb/linespec.h                  |   5 +-
> >  gdb/testsuite/gdb.base/jump.exp | 177 +++++++++++++++++---------------
> >  4 files changed, 103 insertions(+), 89 deletions(-)
> >
> > diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> > index 103899432f7..a79b5d3d4d6 100644
> > --- a/gdb/infcmd.c
> > +++ b/gdb/infcmd.c
> > @@ -1077,7 +1077,8 @@ jump_command (const char *arg, int from_tty)
> >      error_no_arg (_("starting address"));
> >
> >    std::vector<symtab_and_line> sals
> > -    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE);
> > +    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE,
> > +				       current_program_space);
> 
> I wonder if we should stop using decode_line_with_last_displayed here,
> and instead use decode_line_with_current_source?  It feels like a jump
> should always be relative to the current location, rather than what we
> happen to have listed.
> 
> >    if (sals.size () != 1)
> >      error (_("Unreasonable jump request"));
> >
> > diff --git a/gdb/linespec.c b/gdb/linespec.c
> > index 7d969f37fbf..920fd3d37b7 100644
> > --- a/gdb/linespec.c
> > +++ b/gdb/linespec.c
> > @@ -3231,7 +3231,8 @@ decode_line_with_current_source (const char
> *string, int flags)
> >  /* See linespec.h.  */
> >
> >  std::vector<symtab_and_line>
> > -decode_line_with_last_displayed (const char *string, int flags)
> > +decode_line_with_last_displayed (const char *string, int flags,
> > +				 program_space *search_pspace)
> >  {
> >    if (string == 0)
> >      error (_("Empty line specification."));
> > @@ -3240,10 +3241,10 @@ decode_line_with_last_displayed (const char
> *string, int flags)
> >  						      current_language);
> >    std::vector<symtab_and_line> sals
> >      = (last_displayed_sal_is_valid ()
> > -       ? decode_line_1 (locspec.get (), flags, NULL,
> > +       ? decode_line_1 (locspec.get (), flags, search_pspace,
> >  			get_last_displayed_symtab (),
> >  			get_last_displayed_line ())
> 
> This change is why I started looking at the details of this patch again.
> I'm not sure this change makes sense, I would have expected us to use
> get_last_displayed_pspace () here instead of search_pspace, but then it
> becomes harder to describe what search_pspace does as it would only be
> used below.
> 
> It we switch jump to use decode_line_with_current_source then I think
> this problem goes away, the changes to decode_line_with_last_displayed
> can be dropped.
> 
> At the end of this email you'll find a patch that applies on top of your
> V3, which implements the change I'm suggesting.  This passes all the
> 'jump' tests I could find in the testsuite, so I think it's fine, but
> it would be great to hear your thoughts.
> 
> Thanks,
> Andrew
> 
> > -       : decode_line_1 (locspec.get (), flags, NULL, NULL, 0));
> > +       : decode_line_1 (locspec.get (), flags, search_pspace, NULL, 0));
> >
> >    if (*string)
> >      error (_("Junk at end of line specification: %s"), string);
> > diff --git a/gdb/linespec.h b/gdb/linespec.h
> > index d5e7334fe2d..0eb9cb5d9f2 100644
> > --- a/gdb/linespec.h
> > +++ b/gdb/linespec.h
> > @@ -139,10 +139,11 @@ extern std::vector<symtab_and_line>
> decode_line_with_current_source
> >      (const char *, int);
> >
> >  /* Given a string, return the line specified by it, using the last displayed
> > -   codepoint's values as defaults, or nothing if they aren't valid.  */
> > +   codepoint's values as defaults, or nothing if they aren't valid.
> > +   Limit the search to given program space, if specified.  */
> >
> >  extern std::vector<symtab_and_line> decode_line_with_last_displayed
> > -    (const char *, int);
> > +    (const char *, int, program_space *search_pspace = nullptr);
> >
> >  /* Does P represent one of the keywords?  If so, return
> >     the keyword.  If not, return NULL.  */
> > diff --git a/gdb/testsuite/gdb.base/jump.exp
> b/gdb/testsuite/gdb.base/jump.exp
> > index 032c4a6279d..4c0d4ed2dbb 100644
> > --- a/gdb/testsuite/gdb.base/jump.exp
> > +++ b/gdb/testsuite/gdb.base/jump.exp
> > @@ -18,99 +18,110 @@ clear_xfail "*-*-*"
> >
> >  standard_testfile .c
> >
> > -# Build the test case
> > -if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
> {debug nowarnings}] != "" } {
> > -     untested "failed to compile"
> > -     return -1
> > -    }
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> > +    return -1
> > +}
> >
> >
> > -# Start with a fresh gdb
> > +proc do_tests {} {
> > +    global decimal srcfile
> > +
> > +    # Set a breakpoint on the statement that we're about to jump to.
> > +    # The statement doesn't contain a function call.
> > +    set non_call_line [gdb_get_line_number "bp-on-non-call"]
> > +    gdb_breakpoint "$non_call_line"
> > +    set bp_on_non_call \
> > +	[get_integer_valueof "\$bpnum" "INVALID" "bp_on_non_call"]
> > +
> > +    # Can we jump to the statement?  Do we stop there?
> > +    gdb_test "jump $non_call_line" \
> > +	"Breakpoint ${bp_on_non_call}(\.${decimal})?,
> .*${srcfile}:$non_call_line.*" \
> > +	"jump to non-call"
> > +
> > +    # Set a breakpoint on the statement that we're about to jump to.
> > +    # The statement does contain a function call.
> > +    set call_line [gdb_get_line_number "bp-on-call"]
> > +    gdb_breakpoint "$call_line"
> > +    set bp_on_call [get_integer_valueof "\$bpnum" "INVALID" "bp_on_call"]
> > +
> > +    # Can we jump to the statement?  Do we stop there?
> > +    gdb_test "jump $call_line" \
> > +	"Breakpoint ${bp_on_call}(\.${decimal})?, .*${srcfile}:$call_line.*" \
> > +	"jump to call"
> > +
> > +    # If we disable the breakpoint at the function call, and then
> > +    # if we jump to that statement, do we not stop there, but at
> > +    # the following breakpoint?
> > +    gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
> > +
> > +    gdb_test "jump $call_line" \
> > +	"Breakpoint ${bp_on_non_call}(\.${decimal})?,
> .*${srcfile}:$non_call_line.*" \
> > +	"jump to call with disabled breakpoint"
> > +
> > +    # Disable the breakpoint at the non-function call, so it won't hit
> > +    # if do_test is called again.
> > +    gdb_test_no_output "disable ${bp_on_non_call}" "disable
> bp_on_non_call"
> > +
> > +    # Verify that GDB responds gracefully to the "jump" command without
> > +    # an argument.
> > +    gdb_test "jump" "Argument required .starting address.*" \
> > +	"jump without argument disallowed"
> > +
> > +    # Verify that GDB responds gracefully to the "jump" command with
> > +    # trailing junk.
> > +    gdb_test "jump $call_line 100" \
> > +	"malformed linespec error: unexpected number, \"100\"" \
> > +	"jump with trailing argument junk"
> > +
> > +    # Verify that GDB responds gracefully to a request to jump out of
> > +    # the current function.  (Note that this will very likely cause the
> > +    # inferior to die.  Be prepared to rerun the inferior, if further
> > +    # testing is desired.)
> > +    # Try it both ways: confirming and not confirming the jump.
> > +    set out_line [gdb_get_line_number "out-of-func"]
> > +    gdb_test "jump $out_line" \
> > +	"Not confirmed.*" \
> > +	"aborted jump out of current function" \
> > +	"Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> > +	"n"
> > +
> > +    gdb_test "jump $out_line" \
> > +	"Continuing at.*" \
> > +	"jump out of current function" \
> > +	"Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> > +	"y"
> > +}
> >
> > -clean_restart ${binfile}
> >
> > -if {![runto_main]} {
> > -  return -1
> > +set inferiors 1
> > +if {![use_gdb_stub]} {
> > +    set inferiors 2
> >  }
> >
> > -# Set a breakpoint on the statement that we're about to jump to.
> > -# The statement doesn't contain a function call.
> > -#
> > -set bp_on_non_call 0
> > -set non_call_line [gdb_get_line_number "bp-on-non-call"]
> > -gdb_test_multiple "break $non_call_line" "break before jump to non-call" {
> > -    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line
> $non_call_line.*$gdb_prompt $" {
> > -	set bp_on_non_call $expect_out(1,string)
> > -	pass "break before jump to non-call"
> > +# Run to main, add inferiors if needed.
> > +for {set inf 1} {$inf <= $inferiors} {incr inf} {
> > +    if {$inf != 1} {
> > +	# Start a new inferior, and run it with the same executable.
> > +	gdb_test "add-inferior -exec ${binfile}" \
> > +	    "Added inferior ${inf}.*" \
> > +	    "add inferior ${inf} with -exec "
> > +	gdb_test "inferior ${inf}" \
> > +	    "Switching to inferior ${inf} .*" \
> > +	    "switch to inferior ${inf}"
> > +    }
> > +    if {![runto_main]} {
> > +	return -1
> >      }
> >  }
> >
> > -# Can we jump to the statement?  Do we stop there?
> > -#
> > -gdb_test "jump $non_call_line" "Breakpoint ${decimal},
> .*${srcfile}:$non_call_line.*" \
> > -    "jump to non-call"
> > -
> > -# Set a breakpoint on the statement that we're about to jump to.
> > -# The statement does contain a function call.
> > -#
> > -set bp_on_call 0
> > -set call_line [gdb_get_line_number "bp-on-call"]
> > -gdb_test_multiple "break $call_line" "break before jump to call" {
> > -    -re "\[Bb\]reakpoint (${decimal}) at ${hex}: file .*${srcfile}, line
> $call_line.*$gdb_prompt $" {
> > -	set bp_on_call $expect_out(1,string)
> > -	pass "break before jump to call"
> > +# Run tests on all inferiors.
> > +for {set inf 1} {$inf <= $inferiors} {incr inf} {
> > +    with_test_prefix "inferior $inf" {
> > +	# Switch to the target inferior.
> > +	gdb_test "inferior $inf" ".*Switching to inferior $inf .*"
> > +	# Run the tests.
> > +	do_tests
> >      }
> >  }
> >
> > -# Can we jump to the statement?  Do we stop there?
> > -#
> > -gdb_test "jump $call_line" \
> > -    "Breakpoint ${decimal}, .*${srcfile}:$call_line.*" \
> > -    "jump to call"
> > -
> > -# If we disable the breakpoint at the function call, and then
> > -# if we jump to that statement, do we not stop there, but at
> > -# the following breakpoint?
> > -#
> > -gdb_test_no_output "disable $bp_on_call" "disable breakpoint on call"
> > -
> > -gdb_test "jump $call_line" "Breakpoint ${decimal},
> .*${srcfile}:$non_call_line.*" \
> > -    "jump to call with disabled breakpoint"
> > -
> > -# Verify that GDB responds gracefully to the "jump" command without
> > -# an argument.
> > -#
> > -gdb_test "jump" "Argument required .starting address.*" \
> > -    "jump without argument disallowed"
> > -
> > -
> > -# Verify that GDB responds gracefully to the "jump" command with
> > -# trailing junk.
> > -#
> > -gdb_test "jump $call_line 100" \
> > -    "malformed linespec error: unexpected number, \"100\"" \
> > -    "jump with trailing argument junk"
> > -
> > -
> > -# Verify that GDB responds gracefully to a request to jump out of
> > -# the current function.  (Note that this will very likely cause the
> > -# inferior to die.  Be prepared to rerun the inferior, if further
> > -# testing is desired.)
> > -#
> > -# Try it both ways: confirming and not confirming the jump.
> > -#
> > -
> > -set out_line [gdb_get_line_number "out-of-func"]
> > -gdb_test "jump $out_line" \
> > -    "Not confirmed.*" \
> > -    "aborted jump out of current function" \
> > -    "Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> > -    "n"
> > -
> > -gdb_test "jump $out_line" \
> > -    "Continuing at.*" \
> > -    "jump out of current function" \
> > -    "Line $out_line is not in `main'.  Jump anyway.*y or n. $" \
> > -    "y"
> > -
> >  gdb_exit
> > --
> > 2.25.1
> >
> > Intel Deutschland GmbH
> > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> > Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
> > Chairperson of the Supervisory Board: Nicole Lau
> > Registered Office: Munich
> > Commercial Register: Amtsgericht Muenchen HRB 186928
> 
> ---
> 
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 1be72ed09f0..1d10e2908ca 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1068,8 +1068,7 @@ jump_command (const char *arg, int from_tty)
>      error_no_arg (_("starting address"));
> 
>    std::vector<symtab_and_line> sals
> -    = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE,
> -				       current_program_space);
> +    = decode_line_with_current_source (arg, DECODE_LINE_FUNFIRSTLINE);
>    if (sals.size () != 1)
>      error (_("Unreasonable jump request"));
> 
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 920fd3d37b7..9f03a4b3604 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -3220,7 +3220,8 @@ decode_line_with_current_source (const char
> *string, int flags)
>    location_spec_up locspec = string_to_location_spec (&string,
>  						      current_language);
>    std::vector<symtab_and_line> sals
> -    = decode_line_1 (locspec.get (), flags, NULL, cursal.symtab, cursal.line);
> +    = decode_line_1 (locspec.get (), flags, cursal.pspace, cursal.symtab,
> +		     cursal.line);
> 
>    if (*string)
>      error (_("Junk at end of line specification: %s"), string);
> @@ -3231,8 +3232,7 @@ decode_line_with_current_source (const char
> *string, int flags)
>  /* See linespec.h.  */
> 
>  std::vector<symtab_and_line>
> -decode_line_with_last_displayed (const char *string, int flags,
> -				 program_space *search_pspace)
> +decode_line_with_last_displayed (const char *string, int flags)
>  {
>    if (string == 0)
>      error (_("Empty line specification."));
> @@ -3241,10 +3241,10 @@ decode_line_with_last_displayed (const char
> *string, int flags,
>  						      current_language);
>    std::vector<symtab_and_line> sals
>      = (last_displayed_sal_is_valid ()
> -       ? decode_line_1 (locspec.get (), flags, search_pspace,
> +       ? decode_line_1 (locspec.get (), flags, NULL,
>  			get_last_displayed_symtab (),
>  			get_last_displayed_line ())
> -       : decode_line_1 (locspec.get (), flags, search_pspace, NULL, 0));
> +       : decode_line_1 (locspec.get (), flags, NULL, NULL, 0));
> 
>    if (*string)
>      error (_("Junk at end of line specification: %s"), string);
> diff --git a/gdb/linespec.h b/gdb/linespec.h
> index 0eb9cb5d9f2..c0eef7894d9 100644
> --- a/gdb/linespec.h
> +++ b/gdb/linespec.h
> @@ -143,7 +143,7 @@ extern std::vector<symtab_and_line>
> decode_line_with_current_source
>     Limit the search to given program space, if specified.  */
> 
>  extern std::vector<symtab_and_line> decode_line_with_last_displayed
> -    (const char *, int, program_space *search_pspace = nullptr);
> +    (const char *, int);
> 
>  /* Does P represent one of the keywords?  If so, return
>     the keyword.  If not, return NULL.  */

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928