From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 2D9323858414 for ; Mon, 24 Apr 2023 14:11:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2D9323858414 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1682345459; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=jjtN3keDoZYWNI6Czwt1egaWyQcQeGaJAgcVWEYgj88=; b=fKKxVojdv/mT7CzCOoNd3BqHJAWAcf28IQT9SnXaxpGXPN0afw8xWtpoVlwjXEMFuMFruY 83UbwvgOOzTDma0TtdInjJXfNh3KNgWYdzYR6kz3Kj0u5YfJ7l0p6jyh5cpPCAkD2kmzHZ BGzI0Q87D0xU/Ty2EkOsdq4U061PK/E= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-421-jfHdnhhyM1-FV58Dd7UFVw-1; Mon, 24 Apr 2023 10:10:58 -0400 X-MC-Unique: jfHdnhhyM1-FV58Dd7UFVw-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-3f1912ed0daso12529415e9.1 for ; Mon, 24 Apr 2023 07:10:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682345457; x=1684937457; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=jjtN3keDoZYWNI6Czwt1egaWyQcQeGaJAgcVWEYgj88=; b=aQdzFLokPj2tEghNj+jJLQAdCAGLdu0IfS3xabbGJ5XaXN4M9funmpIzR1AJIEr2YA Civh88BFeAFX5GTHFOye0ahrf3CLcQcbHKPBS5s+e018JMbqNVO8eJB9yGZsUnk6wahI lQVPu9O91xGS+B2KiJNeIbXm25rwO2bYsTqYDQZd9udWblEpSmEVGxo6kikaxxthajKH zDU90X7m+lhzorHvbiQaks2mcB22mnN85bHnPvwjQ9IdApLZS7ltk4ZVd0/HRrvLAyug ModXF/EZFGgajOvdfj2s5DN5uY0kk36VfDNyvbZm9qSKftbUIDn6NH4r+necgFcCSsuJ yAGw== X-Gm-Message-State: AAQBX9fGbI2bs5zEr1DTDJuICI6vdqQ62rcBoXgRogPV6ax61KIXY1yp Mve9yXQ8p1n46AtH1+EQ+oiBc8RNjYdNbH2Kez32oQcuz9sngSSWBued37z0MYNfO9vLfXKA+yH D+up/eK26OEzIbzD5xF51Ug== X-Received: by 2002:a5d:4611:0:b0:2f6:987f:a0f5 with SMTP id t17-20020a5d4611000000b002f6987fa0f5mr9605699wrq.5.1682345457361; Mon, 24 Apr 2023 07:10:57 -0700 (PDT) X-Google-Smtp-Source: AKy350bv6kfMJttpQa/iu30GeZy4NJrF2hbgPcObqbKTlKNxByjZFDCS9wWk17f1YP5D59z9PjCOFg== X-Received: by 2002:a5d:4611:0:b0:2f6:987f:a0f5 with SMTP id t17-20020a5d4611000000b002f6987fa0f5mr9605678wrq.5.1682345456874; Mon, 24 Apr 2023 07:10:56 -0700 (PDT) Received: from localhost (11.72.115.87.dyn.plus.net. [87.115.72.11]) by smtp.gmail.com with ESMTPSA id j14-20020adfea4e000000b002fc3d8c134bsm10889636wrn.74.2023.04.24.07.10.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Apr 2023 07:10:56 -0700 (PDT) From: Andrew Burgess To: Matti Puputti , gdb-patches@sourceware.org Subject: Re: [PATCH v2 1/1] gdb, infcmd: Support jump command in multi-inferior case. In-Reply-To: <20230223095453.1878886-2-matti.puputti@intel.com> References: <20230223095453.1878886-1-matti.puputti@intel.com> <20230223095453.1878886-2-matti.puputti@intel.com> Date: Mon, 24 Apr 2023 15:10:54 +0100 Message-ID: <875y9lcqvl.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Matti Puputti 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 [] ()] > (gdb) info inferiors > Num Description Connection Executable > 1 process 6250 1 (native) /temp/simple > * 2 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. Thanks for the fixes. I have a couple of minor nits but I think this is pretty much OK. > --- > gdb/infcmd.c | 3 +- > gdb/linespec.c | 7 +- > gdb/linespec.h | 5 +- > gdb/testsuite/gdb.base/jump.exp | 193 ++++++++++++++++++-------------- > 4 files changed, 119 insertions(+), 89 deletions(-) > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index a851fe1f8c8..18a537847cf 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 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 36f2ef46a7c..536636851e7 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 > -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 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 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 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..ca214993025 100644 > --- a/gdb/testsuite/gdb.base/jump.exp > +++ b/gdb/testsuite/gdb.base/jump.exp > @@ -18,99 +18,126 @@ 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 bp_on_non_call 0 This line is now redundant as we unconditionally set a value below. > + 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"] This line is a bit long. Please wrap it somewhere. > + > + # 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 bp_on_call 0 Again this line can be removed. > + 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 be > + # there if do_test is called again. I think s/there/hit/ in this comment. When I read this I initially expected the code to be deleting the breakpoint rather than just disabling it. > + gdb_test_no_output "disable ${bp_on_non_call}" "disable bp_on_non_call" > + > + Can delete one of the empty lines added here. > + # Verify that GDB responds gracefully to the "jump" command without > + # an argument. > + # > + gdb_test "jump" "Argument required .starting address.*" \ > + "jump without argument disallowed" > + > + Again, only need a single blank line here. > + # 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" > + > + And here. > + # 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. > + # Drop this random trailing '#' line. > + > + 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] then { We don't use the 'then' keyword, this should be written like this: if {![runto_main]} { > + perror "Couldn't run inferior ${inf} to main" I don't think this perror call is needed here. runto_main will already emit a FAIL if something goes wrong. > + 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 > +return 0 This 'return 0' can be dropped. With these fixed: Approved-By: Andrew Burgess Thanks, Andrew > -- > 2.25.1 > > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, 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