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.133.124]) by sourceware.org (Postfix) with ESMTPS id 5CFB23858D20 for ; Thu, 13 Apr 2023 14:32:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5CFB23858D20 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=1681396365; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3Z9+km0AM70zcbXH9Jfu1AcjMF3vLiKgI2H9H29m14M=; b=f75LSPWa3Vcq0EhjunDX863FNZQoIysPkhDMUhBEktAceJlnHezIrAZruCU9PLafbryL4X zn2K9/E0qwilTZurhovRny5raicN9q1ZPI0JwgxjuQDVOt+6E9E81KPozz6SglCIKMYeZo c4UOA3uyA8b5YAlzbNAMWIm+Q69aLdI= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-484-ZeChJzuhMIqRzogILARipw-1; Thu, 13 Apr 2023 10:32:44 -0400 X-MC-Unique: ZeChJzuhMIqRzogILARipw-1 Received: by mail-qk1-f198.google.com with SMTP id r127-20020a374485000000b0074a6b927a49so11468125qka.3 for ; Thu, 13 Apr 2023 07:32:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681396363; x=1683988363; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=3Z9+km0AM70zcbXH9Jfu1AcjMF3vLiKgI2H9H29m14M=; b=gFoIRtq3R8rJhkzUoqSXi/7pT/Fd4pntN6Nibx5yhSqe1nttXrwtxexsfPXfwERDVY /Fe1dKawFd5L2svab2qdsB89U4txz68xAW8ghmKCEsI4+mLG8l/6MBNm967JmUirQCnq ngXPUSQQibs0gdmEEjRDy3xIeR1fXDlxgoK2K+QEq0gGMB3GViqi4QVlDy7LZ8ysGvVs ZV7urCjgYR8KKEx7IynJo0754YA19kCsj+1s2pWal0+TTd5ltIvy0qh/9xe0QueOVcMh OMQVq7uzIps8393acdvndCiXuhwUAt2AzCPd6r88+IJPtXXmEKTRM6e/hiRq/rlHo28Z GG+Q== X-Gm-Message-State: AAQBX9dtyoMZPqN+i8xoMCxumFkqcreyt8NWpEF1YZcW20dFKnF7ZkGK lhVopqHu39u4FcTtFigbokHeQCyyjQv4T323yV+9qPN2CdMwW6P0ECvZgd2OKnmy4D0Oah2A9b/ sIYUf5Pi8iw0r4RFYWhHJM+pn1nQucQ== X-Received: by 2002:a05:6214:5284:b0:568:c5e3:a0ce with SMTP id kj4-20020a056214528400b00568c5e3a0cemr2712061qvb.20.1681396363531; Thu, 13 Apr 2023 07:32:43 -0700 (PDT) X-Google-Smtp-Source: AKy350Z3LL6O1YzjExXWpoCfRv8zHkHGqVmLIOON9DYWb/ycEwm/uFm70rJ5nFGCKDk+cNHdRUaxew== X-Received: by 2002:a05:6214:5284:b0:568:c5e3:a0ce with SMTP id kj4-20020a056214528400b00568c5e3a0cemr2712024qvb.20.1681396363168; Thu, 13 Apr 2023 07:32:43 -0700 (PDT) Received: from [192.168.0.45] (ip-94-112-225-44.bb.vodafone.cz. [94.112.225.44]) by smtp.gmail.com with ESMTPSA id kr24-20020a0562142b9800b005eee320b5d7sm456781qvb.63.2023.04.13.07.32.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 Apr 2023 07:32:42 -0700 (PDT) Message-ID: <196ef911-3e32-cd48-094f-65fc5123a27e@redhat.com> Date: Thu, 13 Apr 2023 16:32:39 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH 1/1] gdb, infcmd: Support jump command with same line in multiple object files. To: Matti Puputti , gdb-patches@sourceware.org References: <20230308132607.1674441-1-matti.puputti@intel.com> <20230308132607.1674441-2-matti.puputti@intel.com> From: Bruno Larsen In-Reply-To: <20230308132607.1674441-2-matti.puputti@intel.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP 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: On 08/03/2023 14:26, Matti Puputti via Gdb-patches wrote: > If the jump target is found in multiple object files, select the one in > the current object file. Hi, thanks for working on this. I think this is a reasonable change, but I have some comments. First, I think your cover letter description makes for a better commit message than the one you provided here. Also, you say "object file" but use symtab in the code. Each symtab is related to a compilation unit, and an objfile can have multiple compilation units. I think bit more precise in the language would be nice :) > --- > gdb/infcmd.c | 14 ++++++- > gdb/testsuite/gdb.base/jump2.c | 29 +++++++++++++++ > gdb/testsuite/gdb.base/jump2.exp | 59 ++++++++++++++++++++++++++++++ > gdb/testsuite/gdb.base/jump2.h | 30 +++++++++++++++ > gdb/testsuite/gdb.base/jump2_foo.c | 24 ++++++++++++ > 5 files changed, 155 insertions(+), 1 deletion(-) > create mode 100755 gdb/testsuite/gdb.base/jump2.c > create mode 100755 gdb/testsuite/gdb.base/jump2.exp > create mode 100755 gdb/testsuite/gdb.base/jump2.h > create mode 100755 gdb/testsuite/gdb.base/jump2_foo.c > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index c369b795757..1b91562f137 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -1080,7 +1080,19 @@ jump_command (const char *arg, int from_tty) > std::vector sals > = decode_line_with_last_displayed (arg, DECODE_LINE_FUNFIRSTLINE); > if (sals.size () != 1) > - error (_("Unreasonable jump request")); > + { > + /* If multiple sal-objects were found, try dropping those that aren't > + from the current objectfile. */ > + sals.erase (std::remove_if (sals.begin (), sals.end (), > + [] (symtab_and_line &sal) > + { > + struct symtab_and_line cursal > + = get_current_source_symtab_and_line (); > + return sal.symtab != cursal.symtab; > + }), sals.end ()); > + if (sals.size () != 1) > + error (_("Unreasonable jump request")); > + } > > symtab_and_line &sal = sals[0]; > > diff --git a/gdb/testsuite/gdb.base/jump2.c b/gdb/testsuite/gdb.base/jump2.c > new file mode 100755 > index 00000000000..468838a9d1a > --- /dev/null > +++ b/gdb/testsuite/gdb.base/jump2.c I think that a more descriptive name, like jump_multiple_objfiles, would be better. > @@ -0,0 +1,29 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2021-2023 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#include "jump2.h" > + > +extern int foo (int n); > + > + > +int main () the coding guidelines ask that this be: int main () > +{ > + int n = foo (1); > + bar (n); > + > + return 0; > +} > diff --git a/gdb/testsuite/gdb.base/jump2.exp b/gdb/testsuite/gdb.base/jump2.exp > new file mode 100755 > index 00000000000..f6bc29dfe1c > --- /dev/null > +++ b/gdb/testsuite/gdb.base/jump2.exp > @@ -0,0 +1,59 @@ > +# Copyright 2021-2023 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . */ > +# > +# Tests GDBs support for jump, when the source line is in multiple > +# object files. > + > + > +standard_testfile .c > +set srcfile2 jump2_foo.c > +set srcfile3 jump2.h > + > + > +if { [prepare_for_testing "failed to prepare" $testfile \ > + [list ${srcfile} ${srcfile2}]] } { > + return -1 > +} > + > +if { ![runto_main] } { > + perror "couldn't run to breakpoint" > + return -1 > +} > + > + > +set bar_first_line [gdb_get_line_number "bar-first-line" ${srcfile3}] > +set bar_middle_line [gdb_get_line_number "bar-middle-line" ${srcfile3}] > +set bar_last_line [gdb_get_line_number "bar-last-line" ${srcfile3}] > + > + > +# Set breakpoints in the function bar. Executable has two object files, > +# and both have a copy of the same source lines. Therefore breakpoints > +# will have two locations. > +gdb_test "break ${srcfile3}:${bar_first_line}" \ > + "Breakpoint.* at .*${srcfile3}:${bar_first_line}\\\. \\\(2 locations\\\)" > +gdb_test "break ${srcfile3}:${bar_last_line}" \ > + "Breakpoint.* at .*${srcfile3}:${bar_last_line}\\\. \\\(2 locations\\\)" You can simplify this by using gdb_breakpoint, which is also more resilient that manually setting the breakpoint message. I also tested locally and seen no regressions, but I'd like to see my comments changed before giving my tag -- Cheers, Bruno > + > +# Run to the breakpoint in bar. > +gdb_continue_to_breakpoint "bar_first_line" \ > + ".*${srcfile3}:${bar_first_line}.*" > + > +# Jump within the function. Debugger shall be able to jump, even if the > +# target line is in two different object files. After jump, we will hit > +# the breakpoint at the last line of bar. > +gdb_test "jump ${bar_middle_line}" [multi_line \ > + "Continuing at ($hex).*" \ > + "Breakpoint ${decimal}.* at .*${srcfile3}:${bar_last_line}.*"] \ > + "Jump within the objectfile" > diff --git a/gdb/testsuite/gdb.base/jump2.h b/gdb/testsuite/gdb.base/jump2.h > new file mode 100755 > index 00000000000..5e3849cb3cb > --- /dev/null > +++ b/gdb/testsuite/gdb.base/jump2.h > @@ -0,0 +1,30 @@ > +/* Copyright (C) 2021-2023 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#ifndef JUMP2_H > +#define JUMP2_H > + > +static int > +bar (int n) > +{ > + int retval = n; > + retval += 1; /* bar-first-line */ > + retval *= -1; /* bar-middle-line */ > + return retval; /* bar-last-line */ > +} > + > +#endif /* JUMP2_H */ > diff --git a/gdb/testsuite/gdb.base/jump2_foo.c b/gdb/testsuite/gdb.base/jump2_foo.c > new file mode 100755 > index 00000000000..667f2398551 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/jump2_foo.c > @@ -0,0 +1,24 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2021-2023 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#include "jump2.h" > + > +int > +foo (int n) > +{ > + return bar (n); > +}