From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from outbound-ss-820.bluehost.com (outbound-ss-820.bluehost.com [69.89.24.241]) by sourceware.org (Postfix) with ESMTPS id E38E13858D1E for ; Wed, 2 Aug 2023 20:33:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E38E13858D1E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com Received: from cmgw14.mail.unifiedlayer.com (unknown [10.0.90.129]) by progateway2.mail.pro1.eigbox.com (Postfix) with ESMTP id 3AAAB1004440F for ; Wed, 2 Aug 2023 20:33:48 +0000 (UTC) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTP id RIXUqDBAiYDyaRIXUqKXR1; Wed, 02 Aug 2023 20:33:48 +0000 X-Authority-Reason: nr=8 X-Authority-Analysis: v=2.4 cv=cZwXElPM c=1 sm=1 tr=0 ts=64cabdac a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=OWjo9vPv0XrRhIrVQ50Ab3nP57M=:19 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=UttIx32zK-AA:10:nop_rcvd_month_year a=Qbun_eYptAEA:10:endurance_base64_authed_username_1 a=CCpqsmhAAAAA:8 a=QyXUC8HyAAAA:8 a=rT93jsFu2kox0RwCjmkA:9 a=atKgkdIoenIA:10:uccc_2email_address a=ul9cdbp4aOFLsgKbc677:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date:References :Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=d3/+Ol/HoCpfEw+vRBAFwyerluO3SsFhs9TAnIky6Rg=; b=OobgUts86ibpujFf1Ec2Ijo6kj 6PcbqBBDdeJCxZkA8W0nUFmrjRs0y1TKs+rgMyXWBSv+ys9MMYbA61WN5OAUlArRdMf1qxSFuSYEj 3d04WBKNB+lI4kJ21QjJSeaWR; Received: from 75-166-148-59.hlrn.qwest.net ([75.166.148.59]:44342 helo=murgatroyd) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1qRIXT-0030DN-2R; Wed, 02 Aug 2023 14:33:47 -0600 From: Tom Tromey To: Abdul Basit Ijaz via Gdb-patches Cc: Abdul Basit Ijaz , JiniSusan.George@amd.com, tom@tromey.com, eliz@gnu.org, blarsen@redhat.com, Nils-Christian Kempke Subject: Re: [PATCH v4 3/4] gdb/infrun: handle stepping through functions with DW_AT_trampoline References: <20230801224744.24433-1-abdul.b.ijaz@intel.com> <20230801224744.24433-4-abdul.b.ijaz@intel.com> X-Attribution: Tom Date: Wed, 02 Aug 2023 14:33:46 -0600 In-Reply-To: <20230801224744.24433-4-abdul.b.ijaz@intel.com> (Abdul Basit Ijaz via Gdb-patches's message of "Wed, 2 Aug 2023 00:47:43 +0200") Message-ID: <87leetchg5.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 75.166.148.59 X-Source-L: No X-Exim-ID: 1qRIXT-0030DN-2R X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 75-166-148-59.hlrn.qwest.net (murgatroyd) [75.166.148.59]:44342 X-Source-Auth: tom+tromey.com X-Email-Count: 19 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-Spam-Status: No, score=-3018.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,JMQ_SPF_NEUTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: >>>>> "Abdul" == Abdul Basit Ijaz via Gdb-patches writes: Abdul> From: Nils-Christian Kempke Abdul> This patch makes infrun continue stepping into and through trampoline Abdul> functions marked via the DW_AT_trampoline in DWARF. The attribute can Abdul> be emitted by the compiler for certain subroutines/inlined subroutines Abdul> that are compiler generated and should be hidden from a user. Thanks for the patch. infrun isn't really my area but I do have some nits. Abdul> + bool in_trampoline = skip_trampoline_functions Abdul> + && in_trampoline_function (stop_pc); Abdul> + Abdul> + if (in_trampoline) The assignment is not formatted correctly, but it seems to me that there's no need for this variable at all, and the expression can be inlined into the 'if' -- which will also fix the formatting. Abdul> + real_stop_pc = find_function_trampoline_target (stop_pc); Abdul> + Abdul> + for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE Abdul> + && in_trampoline_function (real_stop_pc); ++i) Normally we'd split at the ';': for (int i = 0; i < MAX_TRAMPOLINE_CHAIN_SIZE && in_trampoline_function (real_stop_pc); ++i) ... with the middle expression being all on one line if it fits. Abdul> + { Abdul> + real_stop_pc = find_function_trampoline_target (real_stop_pc); Abdul> + /* Exit if find_function_trampoline_target failed to find the Abdul> + trampoline target. Do not try to resolve the trampolines Abdul> + in this case. */ Abdul> + if (real_stop_pc == 0x0) Just '0', not '0x0'. Abdul> + /* If we failed to find a target we will just single step in the Abdul> + hope of leaving the trampoline again soon. */ Abdul> + if (real_stop_pc == 0x0) Ditto. Abdul> + if (real_stop_pc == 0) Abdul> + real_stop_pc = skip_language_trampoline (frame, stop_pc); Abdul> if (real_stop_pc == 0) Abdul> real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc); This is pre-existing code, but I wonder how all these different pieces interact, or are supposed to interact. I guess the language stuff is there to work around a GCC deficiency, in that it does not emit DW_AT_trampoline. The gdbarch stuff... I don't know but I see a lot of arches implement this (though mostly deferring to the solib code). Anyway, is it possible to have a gdbarch or c++ trampoline then call a DW_AT_trampoline function? If so then wouldn't we end up with an erroneous stop? I'm just wondering if these ought to be unified in some way, like say look for any sort of trampoline target in the loop. Also LOL at the gnu-v3-abi.c code: if (thunk_name == NULL || strstr (thunk_name, " thunk to ") == NULL) return 0; It also seems a bit weird that skip_trampoline_functions only affects this one subset of trampoline functions. Tom