public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/1] gdb, infcmd: Support jump command in multi-inferior case.
@ 2023-02-21 15:13 Matti Puputti
  2023-02-21 16:56 ` Andrew Burgess
  0 siblings, 1 reply; 3+ messages in thread
From: Matti Puputti @ 2023-02-21 15:13 UTC (permalink / raw)
  To: gdb-patches

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                  |   2 +-
 gdb/testsuite/gdb.base/jump.exp | 186 +++++++++++++++++++-------------
 4 files changed, 116 insertions(+), 82 deletions(-)
 mode change 100644 => 100755 gdb/testsuite/gdb.base/jump.exp

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index fd88b8ca328..2f981cc895f 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1078,7 +1078,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 d3def7ae070..5969c46e685 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..70e04cbbb21 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -142,7 +142,7 @@ extern std::vector<symtab_and_line> decode_line_with_current_source
    codepoint's values as defaults, or nothing if they aren't valid.  */
 
 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
old mode 100644
new mode 100755
index 032c4a6279d..1040f6355a5
--- a/gdb/testsuite/gdb.base/jump.exp
+++ b/gdb/testsuite/gdb.base/jump.exp
@@ -25,92 +25,124 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
     }
 
 
-# Start with a fresh gdb
-
-clean_restart ${binfile}
-
-if {![runto_main]} {
-  return -1
-}
-
-# 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"
+proc do_tests {} {
+    global decimal hex srcfile gdb_prompt
+    # 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}: .*${srcfile}.*$non_call_line.*$gdb_prompt $" {
+	    set bp_on_non_call $expect_out(1,string)
+	    pass "break before jump to non-call"
+	}
     }
-}
 
-# 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"
+    # Can we jump to the statement?  Do we stop there?
+    #
+    gdb_test "jump $non_call_line" "Breakpoint ${decimal}(\.${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}: .*${srcfile}.*$call_line.*$gdb_prompt $" {
+	    set bp_on_call $expect_out(1,string)
+	    pass "break before jump to call"
+	}
     }
-}
 
-# 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"
+    # Can we jump to the statement?  Do we stop there?
+    #
+    gdb_test "jump $call_line" \
+	"Breakpoint ${decimal}(\.${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}(\.${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"
+}
 
-# Verify that GDB responds gracefully to the "jump" command without
-# an argument.
-#
-gdb_test "jump" "Argument required .starting address.*" \
-    "jump without argument disallowed"
 
+# Start with a fresh gdb.
 
-# 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"
+clean_restart ${binfile}
 
+set inferiors 1
+if {![use_gdb_stub]} {
+    set inferiors 2
+}
 
-# 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.
-#
+# 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] then {
+	perror "Couldn't run inferior ${inf} to main"
+	return -1
+    }
+}
 
-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"
+# 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
+    }
+}
 
 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] gdb, infcmd: Support jump command in multi-inferior case.
  2023-02-21 15:13 [PATCH 1/1] gdb, infcmd: Support jump command in multi-inferior case Matti Puputti
@ 2023-02-21 16:56 ` Andrew Burgess
  2023-02-22 17:43   ` Puputti, Matti
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Burgess @ 2023-02-21 16:56 UTC (permalink / raw)
  To: Matti Puputti, gdb-patches

Matti Puputti via Gdb-patches <gdb-patches@sourceware.org> 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                  |   2 +-
>  gdb/testsuite/gdb.base/jump.exp | 186 +++++++++++++++++++-------------
>  4 files changed, 116 insertions(+), 82 deletions(-)
>  mode change 100644 => 100755 gdb/testsuite/gdb.base/jump.exp
>
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index fd88b8ca328..2f981cc895f 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1078,7 +1078,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 d3def7ae070..5969c46e685 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..70e04cbbb21 100644
> --- a/gdb/linespec.h
> +++ b/gdb/linespec.h
> @@ -142,7 +142,7 @@ extern std::vector<symtab_and_line> decode_line_with_current_source
>     codepoint's values as defaults, or nothing if they aren't valid.  */
>  
>  extern std::vector<symtab_and_line> decode_line_with_last_displayed
> -    (const char *, int);
> +    (const char *, int, program_space *search_pspace = nullptr);

The comment above this declaration should be updated to mention the new
parameter and explain how to use it.

I notice there's only two uses of decode_line_with_last_displayed, the
other being in the 'info line' function.  I don't think it's something
to do in this patch; but I wonder if it would make sense to update that
call to restrict to the current program space too?  If I say:

  (gdb) info line 3

I'm probably asking about the current inferior..

Have you thought about this at all?

>  
>  /* 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
> old mode 100644
> new mode 100755
> index 032c4a6279d..1040f6355a5
> --- a/gdb/testsuite/gdb.base/jump.exp
> +++ b/gdb/testsuite/gdb.base/jump.exp
> @@ -25,92 +25,124 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
>      }
>  
>  
> -# Start with a fresh gdb
> -
> -clean_restart ${binfile}

I know this is not your fault; the test is clearly pretty old, but it
would be great, if instead of moving this clean_restart around we
instead updated the test to use prepare_for_testing.

> -
> -if {![runto_main]} {
> -  return -1
> -}
> -
> -# 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"
> +proc do_tests {} {
> +    global decimal hex srcfile gdb_prompt
> +    # 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}: .*${srcfile}.*$non_call_line.*$gdb_prompt $" {
> +	    set bp_on_non_call $expect_out(1,string)
> +	    pass "break before jump to non-call"

Similarly, here, instead of tweaking the pattern, this can all be
replaced with:

    # 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"]

which I think is much clearer.


> +	}
>      }
> -}
>  
> -# 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"
> +    # Can we jump to the statement?  Do we stop there?
> +    #
> +    gdb_test "jump $non_call_line" "Breakpoint ${decimal}(\.${decimal})?, .*${srcfile}:$non_call_line.*" \
> +	    "jump to non-call"

We collected bp_on_non_call above, but we never actually use it.  Again,
this is not your fault, this was just not a great test to begin with,
but we could change this pattern here to replace the first $decimal with
$bp_on_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}: .*${srcfile}.*$call_line.*$gdb_prompt $" {
> +	    set bp_on_call $expect_out(1,string)
> +	    pass "break before jump to call"

This can be cleaned up like the earlier one.

> +	}
>      }
> -}
>  
> -# 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"
> +    # Can we jump to the statement?  Do we stop there?
> +    #
> +    gdb_test "jump $call_line" \
> +	"Breakpoint ${decimal}(\.${decimal})?, .*${srcfile}:$call_line.*" \
> +	    "jump to call"

And we can use $bp_on_call to replace the first $decimal here.

Thanks,
Andrew

> +
> +    # 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}(\.${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"
> +}
>  
> -# Verify that GDB responds gracefully to the "jump" command without
> -# an argument.
> -#
> -gdb_test "jump" "Argument required .starting address.*" \
> -    "jump without argument disallowed"
>  
> +# Start with a fresh gdb.
>  
> -# 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"
> +clean_restart ${binfile}
>  
> +set inferiors 1
> +if {![use_gdb_stub]} {
> +    set inferiors 2
> +}
>  
> -# 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.
> -#
> +# 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] then {
> +	perror "Couldn't run inferior ${inf} to main"
> +	return -1
> +    }
> +}
>  
> -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"
> +# 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
> +    }
> +}
>  
>  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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH 1/1] gdb, infcmd: Support jump command in multi-inferior case.
  2023-02-21 16:56 ` Andrew Burgess
@ 2023-02-22 17:43   ` Puputti, Matti
  0 siblings, 0 replies; 3+ messages in thread
From: Puputti, Matti @ 2023-02-22 17:43 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

Hi Andrew,

Thank four for the review and comments.

> > --- a/gdb/linespec.h
> > +++ b/gdb/linespec.h
> > @@ -142,7 +142,7 @@ extern std::vector<symtab_and_line>
> decode_line_with_current_source
> >     codepoint's values as defaults, or nothing if they aren't valid.  */
> >
> >  extern std::vector<symtab_and_line> decode_line_with_last_displayed
> > -    (const char *, int);
> > +    (const char *, int, program_space *search_pspace = nullptr);
> 
> The comment above this declaration should be updated to mention the new
> parameter and explain how to use it.
> 

I will update the comment to explain the new parameter.

> I notice there's only two uses of decode_line_with_last_displayed, the
> other being in the 'info line' function.  I don't think it's something
> to do in this patch; but I wonder if it would make sense to update that
> call to restrict to the current program space too?  If I say:
> 
>   (gdb) info line 3
> 
> I'm probably asking about the current inferior..
> 
> Have you thought about this at all?
> 

In this commit, I focused on case where requested command cannot be executed.
If more than one locations are found, jump_command prints an error.
But info_line_command prints data on each found location.
I would propose to keep this commit focused on jump_command only.

> > --- a/gdb/testsuite/gdb.base/jump.exp
> > +++ b/gdb/testsuite/gdb.base/jump.exp
> > @@ -25,92 +25,124 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}"
> "${binfile}" executable {deb
> >      }
> >
> >
> > -# Start with a fresh gdb
> > -
> > -clean_restart ${binfile}
> 
> I know this is not your fault; the test is clearly pretty old, but it
> would be great, if instead of moving this clean_restart around we
> instead updated the test to use prepare_for_testing.
> 

I will update my commit to address this.

> > +    # 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}:
> .*${srcfile}.*$non_call_line.*$gdb_prompt $" {
> > +	    set bp_on_non_call $expect_out(1,string)
> > +	    pass "break before jump to non-call"
> 
> Similarly, here, instead of tweaking the pattern, this can all be
> replaced with:
> 
>     # 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"]
> 
> which I think is much clearer.
> 
> 

I will update my commit to address this.

> > +    # Can we jump to the statement?  Do we stop there?
> > +    #
> > +    gdb_test "jump $non_call_line" "Breakpoint ${decimal}(\.${decimal})?,
> .*${srcfile}:$non_call_line.*" \
> > +	    "jump to non-call"
> 
> We collected bp_on_non_call above, but we never actually use it.  Again,
> this is not your fault, this was just not a great test to begin with,
> but we could change this pattern here to replace the first $decimal with
> $bp_on_non_call.
> 

I will update my commit to address this.

> > +    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}:
> .*${srcfile}.*$call_line.*$gdb_prompt $" {
> > +	    set bp_on_call $expect_out(1,string)
> > +	    pass "break before jump to call"
> 
> This can be cleaned up like the earlier one.
> 

Will do.

> > +    # Can we jump to the statement?  Do we stop there?
> > +    #
> > +    gdb_test "jump $call_line" \
> > +	"Breakpoint ${decimal}(\.${decimal})?, .*${srcfile}:$call_line.*" \
> > +	    "jump to call"
> 
> And we can use $bp_on_call to replace the first $decimal here.

Will do.

> 
> Thanks,
> Andrew
> 

I will work on a v2.

Br,
Matti Puputti

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-02-22 17:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-21 15:13 [PATCH 1/1] gdb, infcmd: Support jump command in multi-inferior case Matti Puputti
2023-02-21 16:56 ` Andrew Burgess
2023-02-22 17:43   ` Puputti, Matti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).