From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by sourceware.org (Postfix) with ESMTPS id F16643858D28 for ; Mon, 17 Jul 2023 14:14:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F16643858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-3fbc1218262so46378615e9.3 for ; Mon, 17 Jul 2023 07:14:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689603295; x=1692195295; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=sIlGrFpkvZFqb9rrXDfOZRco9TdAuVjxD7v7YH/xxhM=; b=WvrNgKnQHSwMy3Kj41PFpu2dhLc5qKnZo2TGHyx8dxmR+tw8FEqUUlzOqg3sLw0kvj eExNjRl+Itfo5vtu+nQAOw+ouI9lBZ4GQb9v6epjEgzgviuQUuaACsCdD++9beoto+OF ln+SICJR3tzitEBulSFlhQWGEtvkDnN1DQHLWVt6KJvGVyvlmqpQnYQ3GnEKRvnlQPk4 bXVsiVRzUcC3SD/eDogETScUMPyiIVTByNNqojO1F1h1XrBWiaOhRPpBwks2pKu+Vx+8 HLzpYyCjrnN7w3T8FWx03V6s8kU8nwTPJRWDLXPY7SFDUL3X40t/vInaEZ9HdmG/5BFD LELg== X-Gm-Message-State: ABy/qLZ19Bk4BqIGDH/YOtxZv1HmxmDqy1rDdUA/9P8q8wVwWYmoY6H/ UeInJ8/1ByUkL6acq19fTylEk20sJig= X-Google-Smtp-Source: APBJJlEg6ACUxAV7AlxG4PQNBbY499jreuECKNrCEF5VQNqomkNv5A2QIigjWvHcgYsPfQDnO2lDGw== X-Received: by 2002:a1c:7407:0:b0:3fb:e573:4172 with SMTP id p7-20020a1c7407000000b003fbe5734172mr10081122wmc.31.1689603294936; Mon, 17 Jul 2023 07:14:54 -0700 (PDT) Received: from ?IPV6:2001:8a0:f91d:bc00:41d8:caa5:3b08:ee75? ([2001:8a0:f91d:bc00:41d8:caa5:3b08:ee75]) by smtp.gmail.com with ESMTPSA id o18-20020a05600c379200b003fa973e6612sm8035426wmr.44.2023.07.17.07.14.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 17 Jul 2023 07:14:54 -0700 (PDT) Message-ID: <30e63f84-1bc7-68ff-dfca-e3edd6f58755@palves.net> Date: Mon, 17 Jul 2023 15:14:52 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH v4 2/4] gdb/cli: add '.' as an argument for 'list' command Content-Language: en-US To: Bruno Larsen , gdb-patches@sourceware.org References: <20230713102411.2279542-1-blarsen@redhat.com> <20230713102411.2279542-3-blarsen@redhat.com> <835af40f-edae-ba48-f121-a1cdd1f1147e@palves.net> <430fb2d9-ae07-6551-3426-adb5334c127d@redhat.com> From: Pedro Alves In-Reply-To: <430fb2d9-ae07-6551-3426-adb5334c127d@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,BODY_8BITS,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,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: On 2023-07-17 09:21, Bruno Larsen wrote: > On 14/07/2023 19:50, Pedro Alves wrote: >> Hi, >> >> Sorry for coming late to the party here, but while trying to catch up on the >> list I spotted a few things that I think should be fixed.  See below. > I forgot to send the email, but I've already pushed this patch... No worries, I had seen that. That's what I meant by being late to the party. :-) > I'll fix up the things you pointed in a moment and send a new patch, I just have a few questions. Thanks. >> >> BTW, I think the feature is cool! >> >> On 2023-07-13 11:24, Bruno Larsen via Gdb-patches wrote: >>> diff --git a/gdb/NEWS b/gdb/NEWS >>> index b924834d3d7..eef440a5242 100644 >>> --- a/gdb/NEWS >>> +++ b/gdb/NEWS >>> @@ -84,6 +84,10 @@ >>>     is 64k.  To print longer strings you should increase >>>     'max-value-size'. >>>   +* The 'list' command now accepts '.' as an argument, which tells GDB to >>> +  print the location where the inferior is stopped.  If the inferior hasn't >>> +  started yet, the command will print around the main function. >> It would be more accurate to say that it's where the current thread is stopped. >> >> Say you run to breakpoint hit in thread 1, and then switch to thread 2, and then do >> "list .".  That will show you the current frame of thread 2, while "where the >> inferior is stopped" could very well be interpreted as "thread 1". > > The wording does need changing, I agree. I think using the wording that already exists in the documentation is the best way to go: "prints around the last solitary line printed as part of displaying a stack frame". The wording I originally used (and the improvement you suggested) could make it sound like I would always print the lowermost frame, when that is not the point I wanted. What I wanted was to re-do what the first usage of "list" does without needing to know the lines. Makes sense. So this is really just a shortcut for: (gdb) frame (gdb) list >>> +      else if (arg[0] == '.') >>> +    { >>> +      try >>> +        { >>> +          /* Find the current line by getting the PC of the currently >>> +         selected frame, and finding the line associated to it.  */ >>> +          frame_info_ptr frame = get_selected_frame (nullptr); >> So this is actually using the selected frame, not the current frame.  So even >> the "where the thread is stopped" description would be incorrect.  If you really >> wanted to use frame #0 (where the thread stopped), then this should use get_current_frame. > Yeah... that's the confusion I mentioned earlier... I want the selected frame, since its what "list" does when you first call it after entering a frame. >> >> >>> +          CORE_ADDR curr_pc = get_frame_pc (frame); >>> +          cursal = find_pc_line (curr_pc, 0); >>> +        } >>> +      catch (const gdb_exception &e) >>> +        { >>> +          /* If there was an exception above, it means the inferior >>> +             is not running, so reset the current source location to >>> +             the default.  */ >> Aww.  Please don't use a try/catch for this...  You can just check target_has_execution. > Oh, I thought target_has_execution would query if the target can execute the inferior, rather than looking if the inferior is being executed. Will change Actually, target_has_stack would be even more appropriate.