From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id CEB953858C20 for ; Mon, 31 Jan 2022 14:06:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CEB953858C20 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 20VE5ATw028902 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 31 Jan 2022 09:05:15 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 20VE5ATw028902 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 2BE371EA69; Mon, 31 Jan 2022 09:05:10 -0500 (EST) Message-ID: <310c1cc5-29c7-bfe5-0421-850df50dd8bf@polymtl.ca> Date: Mon, 31 Jan 2022 09:05:09 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH v2 2/2] gdb: Respect the DW_CC_nocall attribute Content-Language: en-US To: Bruno Larsen , Lancelot SIX , gdb-patches@sourceware.org Cc: lsix@lancelotsix.com References: <20220128142931.39750-1-lancelot.six@amd.com> <20220128142931.39750-3-lancelot.six@amd.com> <49c17cc9-fd06-31ea-b4f1-174c26b7eb9b@redhat.com> From: Simon Marchi In-Reply-To: <49c17cc9-fd06-31ea-b4f1-174c26b7eb9b@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Mon, 31 Jan 2022 14:05:10 +0000 X-Spam-Status: No, score=-3039.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Jan 2022 14:06:24 -0000 On 2022-01-31 08:19, Bruno Larsen via Gdb-patches wrote: > Hi Lancelot, > > Thanks for looking at this. The general direction looks good, just a few minor nits. Some more comments. Git gave me this warning when applying, maybe something to fix: Applying: gdb: Respect the DW_CC_nocall attribute .git/rebase-apply/patch:148: new blank line at EOF. + warning: 1 line adds whitespace errors. >> diff --git a/gdb/infcmd.c b/gdb/infcmd.c >> index 994dd5b32a3..186b1a2024a 100644 >> --- a/gdb/infcmd.c >> +++ b/gdb/infcmd.c >> @@ -1419,6 +1419,25 @@ get_return_value (struct value *function, struct type *value_type) >> value_type = check_typedef (value_type); >> gdb_assert (value_type->code () != TYPE_CODE_VOID); >> + if (is_nocall_function (check_typedef (::value_type (function)))) >> + { >> + CORE_ADDR funaddr = value_address (function); >> + const char *fname = nullptr; >> + char buf[RAW_FUNCTION_ADDRESS_SIZE]; >> + if (funaddr != 0) >> + fname = get_function_name (funaddr, buf, sizeof (buf)); >> + >> + if (fname != nullptr) My understand is that fname will be nullptr if funaddr is 0. Can you explain when funaddr is 0, and what that means? Just trying to understand how we can get there. My other comments about this would be: - One caller of this command, finish_command_fsm::should_stop, starts from a struct symbol. Getting the name from that is easy, you can simply call 'print_name' on it (as shown by your change in return_command). - The other caller of get_return_value is bpfinishpy_pre_stop_hook. bpfinishpy_pre_stop_hook does not have access to a struct symbol directly, but maybe it could, if we saved the symbol used to set finish_breakpoint_object::function_value, bpfinishpy_init. So if we can always have access to the symbol somehow, maybe we can avoid using get_function_name in get_return_value. >> + warning (_("The function '%s' does not follow the target calling " I think that when naming the thing, you should omit the "The" in front. For example, you'd say "President Biden", or "The president", but not "The president Biden". Maybe I'm wrong, I'm not a native english speaker. Simon