From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id rlpHIe6gLGccoygAWB0awg (envelope-from ) for ; Thu, 07 Nov 2024 06:13:50 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=T+wLyLo+; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 6BD081E603; Thu, 7 Nov 2024 06:13:50 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id AD4D71E601 for ; Thu, 7 Nov 2024 06:13:47 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3245E3858CDA for ; Thu, 7 Nov 2024 11:13:47 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 89BBB3858D20 for ; Thu, 7 Nov 2024 11:13:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 89BBB3858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 89BBB3858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730978002; cv=none; b=ut4msbVd07+VfM60Pp7e8stJDVZmclMOfurY6AAA3qm7VGhDObHjkmmmNYMbHwTXPkXIrWAuDRR5DSaYzORHRnC78ou5yFPGDajDh7Ok10YN4hNz/Z2NENRUwwPZ/BSVLYXLorZ88LwnszZpyZaZpj+UKaAvPCJ62qDXvvRzQxo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730978002; c=relaxed/simple; bh=4DEnZt18ZAsy4eEu0B4ycPKZdlMtRlHbfp1Ed5XSTpA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=ELBRCx0xdSadA94g1EbhBJH8UB184U2mC3bLeHNnPKruhERneHYRr9kPVd0cOm3q/S3R9Dt3Puon+vpPCWUV0e1EXrahGOS/TquonlMlvy3uw6CTGJ3rIzciZiR7sjIwmD5UCJ251G1RlBuUcSn1aQ578c6En5R/SsFkZGSgVrA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730977998; 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=qezUQthC7GYOLX7cDPz+SbOHuZi5i/oacAuxo+1IF28=; b=T+wLyLo+mBXKdwPGGqcSIsMrjZamkdal2RcNJodNHSes6tRNyK6wVLAGmpIcNtujwbItDl N1W34tPmn+0ih32cqgigHl0eK6ijDfLRzqp2JOxBB2Ahs9p2WiyLV4QcqrY39U3+AqDtpN UTJZU2w9b7JsJbqhZEPlAe61LDJbY+c= 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-329-ffPsAWdpPqG86GG7cZEy7g-1; Thu, 07 Nov 2024 06:13:17 -0500 X-MC-Unique: ffPsAWdpPqG86GG7cZEy7g-1 X-Mimecast-MFC-AGG-ID: ffPsAWdpPqG86GG7cZEy7g Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-43159c07193so7915305e9.0 for ; Thu, 07 Nov 2024 03:13:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730977995; x=1731582795; h=content-transfer-encoding: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=k39SYbJbFu9JUB04o787aAqJjmOkTvi91CwzxkCuyoU=; b=Sy5MenInCO5j8e0rx6k7N8ZrB+Ev0psHDqVfVB+HxRVJVFd7G+ZTB/obYk3ss7dLTd Mp2o7cj0KsZmgC8A+O0WO/gV2WKY4t586IfE3AkS5CZzoX5jUjxxoLUiNLg8u7KgMMTG LWqCr8BTywK2VmXI5OHrqQk1AAA9D3iGOgwnK1B+QMk5k98TqBJ4FHudTLiOD+Mv1J7l 1+MCvw2HVrxdMSsvIxQ/pnAYRA4PYsB8ExTjuzmV82PePjpBBBX4jeCfyF+8zSVjzz4m aB9iCBkO2fCpP5LXhC7Hg7uqgzIxs6xUrM+FVNIvjOBntwhmBWpI/TFxuo0r+yx8F6q1 P2kQ== X-Forwarded-Encrypted: i=1; AJvYcCWUCWfMppMrTT8g8wqNHgjdJMBB0K1BpeHJGzoO45vhDTEEopWfiVS2pw+vIiIbrh8mUIm7zTZm4YhRcQ==@sourceware.org X-Gm-Message-State: AOJu0Yze1xZP6uNLJ/VTlMbPonktPVrG4o4v1WbCjmFCL1UMyn7NZbh8 6iYfC4wKxGMYGQazdfQxJJB/smzd5v641hfwmnRVZh/uFGIcB8jFfLLWYh7K3jzc1utzZHlntz+ yXfHWcRUslpj1MarLz5GEOrHa5NP4+VKRc9VyXdjgquWP1icOnEHDTAn0dTBk0HX1VuA= X-Received: by 2002:a05:600c:5250:b0:431:5465:8072 with SMTP id 5b1f17b1804b1-43283294a9emr272710905e9.31.1730977995190; Thu, 07 Nov 2024 03:13:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IF5IfADiHuXXXzIeNim2xkAXn+xxRFV1rMa57vwJazWQMFiSYUSykWhmVQhcooGY2RCtMWbYA== X-Received: by 2002:a05:600c:5250:b0:431:5465:8072 with SMTP id 5b1f17b1804b1-43283294a9emr272710635e9.31.1730977994758; Thu, 07 Nov 2024 03:13:14 -0800 (PST) Received: from localhost ([62.31.95.162]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-432b0530694sm20678205e9.7.2024.11.07.03.13.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Nov 2024 03:13:14 -0800 (PST) From: Andrew Burgess To: =?utf-8?Q?Llu=C3=ADs?= Batlle i Rossell , gdb-patches@sourceware.org Subject: Re: [PATCH] Fix 'edit' command to go to right line In-Reply-To: References: Date: Thu, 07 Nov 2024 11:13:12 +0000 Message-ID: <87pln7qotz.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: ae71bN-b2W9RO3LP97ZGfGYMXHK3cGEC3gbWj31blag_1730977996 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Llu=C3=ADs Batlle i Rossell writes: > I attach this patch I used more than 20 years ago. > > I thought I had sent it in these years but apparently not. > From fc0f3f1200f18c86a6d90185f02b1f5ce0c0235b Mon Sep 17 00:00:00 2001 > From: =3D?UTF-8?q?Llu=3DC3=3DADs=3D20Batlle=3D20i=3D20Rossell?=3D > Date: Tue, 5 Nov 2024 23:18:30 +0100 > Subject: [PATCH] Fix 'edit' command to go to right line > > The cursor was placed at a line off by a bunch of lines. Thanks for reporting this issue. I took a look at this patch and started writing some tests for the 'edit' command. It would seem that this was mostly untested until now! Anyway, though your patch does fix some cases, I found a bunch more problems, so I'd like to propose the patch below as an alternative. I hope this should fix the case that I think you care about, though I'm guessing really as you didn't describe exactly which case you believe this fixes. If you are willing to give this alternative patch a try, and report if this fixes your use case or not that would be really helpful. Thanks, Andrew --- commit 3c340112d9c71ece32f98a59ec67ceb9fa28d393 Author: Andrew Burgess Date: Wed Nov 6 22:18:55 2024 +0000 gdb: fixes and tests for the 'edit' command =20 This commit was inspired by this mailing list post: =20 https://inbox.sourceware.org/gdb-patches/osmtfvf5xe3yx4n7oirukidym4ci= k7lehhy4re5mxpset2qgwt@6qlboxhqiwgm =20 When reviewing that patch, the first thing I wanted to do was add some tests for the 'edit' command because, as far as I can tell, there are no real tests right now. =20 The approach I've taken for testing is to override the EDITOR environment variable, setting this to just 'echo'. Now when the 'edit' command is run, instead of entering an interactive editor, the shell instead echos back the arguments that GDB is trying to pass to the editor. The output might look like this: =20 (gdb) edit +22 /tmp/gdb/testsuite/gdb.base/edit-cmd.c (gdb) =20 We can then test this like any other normal command. I then wrote some basic tests covering a few situations like, using 'edit' before the inferior is started. Using 'edit' without any arguments, and using 'edit' with a line number argument. =20 There are plenty of cases that are still not tested, for example, the test program only has a single source file for example. But we can always add more tests later. =20 I then used these tests to validate the fix proposed in the above patch. =20 The patch above does indeed fix some cases, specifically, when GDB stops at a location (e.g. a breakpoint location) and then the 'edit' command without any arguments is fixed. But using the 'list' command to show some other location, and then 'edit' to edit the just listed location broken before and after the above patch. =20 I am instead proposing this alternative patch which I think fixes more cases. When GDB stops at a location then 'edit' with no arguments should correctly edit the current line. And using 'list XX' to list a specific location, followed by 'edit' should also now edit the just listed location. =20 Co-Authored-By: Llu=C3=ADs Batlle i Rossell diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 65ac7d6e7fb..225615f0210 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -973,7 +973,8 @@ edit_command (const char *arg, int from_tty) { if (sal.symtab =3D=3D 0) =09error (_("No default source file yet.")); - sal.line +=3D get_lines_to_list () / 2; + if (get_first_line_listed () !=3D 0) +=09sal.line =3D get_first_line_listed () + get_lines_to_list () / 2; } else { diff --git a/gdb/testsuite/gdb.base/basic-edit-cmd.c b/gdb/testsuite/gdb.ba= se/basic-edit-cmd.c new file mode 100644 index 00000000000..fb0f70db51c --- /dev/null +++ b/gdb/testsuite/gdb.base/basic-edit-cmd.c @@ -0,0 +1,55 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2024 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 . = */ + +/* Used so we have some work to do. */ +volatile int global_var =3D 0; + +int +main (void) +{=09=09=09/* prologue location */ + ++global_var;=09=09/* first location */ + + /* + * + * + * This comment is here as filler. + * + * + */ + + ++global_var;=09=09/* second location */ + + /* + * + * + * This comment is also here as filler. + * + * + */ + + ++global_var;=09=09/* third location */ + + /* + * + * + * This is yet another filler comment. + * + * + */ + + return 0;=09=09/* fourth location */ +} diff --git a/gdb/testsuite/gdb.base/basic-edit-cmd.exp b/gdb/testsuite/gdb.= base/basic-edit-cmd.exp new file mode 100644 index 00000000000..dd8fe7022ab --- /dev/null +++ b/gdb/testsuite/gdb.base/basic-edit-cmd.exp @@ -0,0 +1,136 @@ +# Copyright 2024 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 . + +# Test the 'edit' command. + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} { + return +} + +# Check that 'echo' is available in the shell. +gdb_test_multiple "shell echo test 1234 xyz" "check echo is available" { + -re -wrap "^test 1234 xyz" { + } + + -re -wrap "" { +=09unsupported "shell cannot use echo command" +=09return + } +} + +# Find line numbers for use in tests. +set line_0 [gdb_get_line_number "prologue location"] +set line_1 [gdb_get_line_number "first location"] +set line_2 [gdb_get_line_number "second location"] +set line_3 [gdb_get_line_number "third location"] +set line_4 [gdb_get_line_number "fourth location"] + +# Regexp to match SRCFILE. +set srcfile_re "\[^\r\n\]+/[string_to_regexp $srcfile]" + +# Setup the EDITOR environment variable to run our helper script, and +# then run the tests. + +save_vars { env(EDITOR) } { + set env(EDITOR) "echo" + + # Start with no test binary loaded. + clean_restart + gdb_test "edit" \ +=09"^No symbol table is loaded. Use the \"file\" command\\." \ +=09"try edit when no symbol file is loaded" + + # Now start with a test binary. + clean_restart $binfile + + with_test_prefix "before starting inferior" { +=09gdb_test "edit" \ +=09 "\r\n\\+$line_0 $srcfile_re" \ +=09 "check edit of default location" + +=09gdb_test "list $line_4" \ +=09 "\r\n$line_4\\s+\[^\r\n\]+/\\* fourth location \\*/\r\n.*" \ +=09 "list lines around the fourth location" + +=09gdb_test "edit" \ +=09 "\r\n\\+$line_4 $srcfile_re" \ +=09 "check edit of fourth location after listing" + +=09gdb_test "edit $line_2" \ +=09 "\r\n\\+$line_2 $srcfile_re" \ +=09 "check edit of second location" + +=09gdb_test "edit xxx" \ +=09 "^Function \"xxx\" not defined\\." \ +=09 "try to edit an unknown function" + } + + if {![runto_main]} { +=09return + } + + set first_loc_pc [get_hexadecimal_valueof "\$pc" "*UNKNOWN*" \ +=09=09=09 "get \$pc at first location"] + + with_test_prefix "stopped at first location" { +=09gdb_test "edit" \ +=09 "\r\n\\+$line_1 $srcfile_re" \ +=09 "check edit of current location" + } + + gdb_breakpoint $line_2 + gdb_continue_to_breakpoint "stop at second location" + + with_test_prefix "at second location" { +=09gdb_test "edit" \ +=09 "\r\n\\+$line_2 $srcfile_re" \ +=09 "check edit current location results" + +=09gdb_test "edit $line_3" \ +=09 "\r\n\\+$line_3 $srcfile_re" \ +=09 "check edit third location results" + } + + with_test_prefix "list first location" { +=09gdb_test "list $line_1" \ +=09 "\r\n$line_1\\s+\[^\r\n\]+/\\* first location \\*/\r\n.*" \ +=09 "list lines around the first location" + +=09gdb_test "edit" \ +=09 "\r\n\\+$line_1 $srcfile_re" \ +=09 "check edit current location results" + } + + gdb_breakpoint $line_4 + gdb_continue_to_breakpoint "stop at fourth location" + + with_test_prefix "at fourth location" { +=09gdb_test "edit" \ +=09 "\r\n\\+$line_4 $srcfile_re" \ +=09 "check edit current location results" + +=09gdb_test "edit $line_1" \ +=09 "\r\n\\+$line_1 $srcfile_re" \ +=09 "check edit first location results" + +=09gdb_test "edit *$first_loc_pc" \ +=09 [multi_line \ +=09=09 "[string_to_regexp $first_loc_pc] is in main \\($srcfile_re:$line_1= \\)\\." \ +=09=09 "\\+$line_1 $srcfile_re"] \ +=09 "check edit first location by address results" + } +}