From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id uZSqF9ZsDmpXZgsAWB0awg (envelope-from ) for ; Wed, 20 May 2026 22:24:22 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=InMq6/nT; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 4A1B91E098; Wed, 20 May 2026 22:24:22 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (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 F1ED71E062 for ; Wed, 20 May 2026 22:24:20 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1D2964B920B5 for ; Thu, 21 May 2026 02:24:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1D2964B920B5 Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=InMq6/nT Received: from mail-vs1-xe36.google.com (mail-vs1-xe36.google.com [IPv6:2607:f8b0:4864:20::e36]) by sourceware.org (Postfix) with ESMTPS id BBD214B9208B for ; Thu, 21 May 2026 02:23:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BBD214B9208B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BBD214B9208B Authentication-Results: sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::e36 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1779330228; cv=none; b=UWgHkl8aeeeM22LHlSQNMVRARW1qBSFkeSWTP67iJIfBh3aB1GZkWirAWYmqxZKEAES8w78iBIz7cNE3X8cnNQMf6cKC2csMfw0QyA/hy/psV0//MwLWKVUR7Puia2dHxXrmOwtj11SdW/944z1MvUDr8GGEDVgG6rp8jF0V0qk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1779330228; c=relaxed/simple; bh=HqTNNpGUT0Ez9O3EnJFX55VzTYcaj9vn7UgDcuMSVzA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Z1Pwcg+jfeyIexupIxq1QbAIN+BWtoQc/3zBg/H/Na0JhyiapF8fmVBkMtCXRjf5FajgrxrLQ+Pe+WOwI+oIt9jNPop34wxv/Bzg13x1HEyJdzhvIIg7bx8fJNyShNy1YCvH1SU6AvnPlWv5TJcAQ7Omi84PkPTOyuD+YNjiSyw= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=InMq6/nT DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BBD214B9208B Received: by mail-vs1-xe36.google.com with SMTP id ada2fe7eead31-63127c440ccso3977169137.3 for ; Wed, 20 May 2026 19:23:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1779330228; x=1779935028; darn=sourceware.org; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=kQH+CjNK/qOFvBb8SSrXTKpN7X2hUXLpuXZmtCCg4AM=; b=InMq6/nTgvClbVNh2KQbZ1J1iSqS/Ol/w26jKbKWqam/krsLPLeiLEsXcJAawWcZKo DTol6BoB5/VxxQUp7QS+YOLxh6hfqAp4vVuPPi3Fpz8RvWt+oFrwUAPry45a4qehjPKh MjkrAE9s4HnBnJzZX6s54p4PrnItbdLS+qMyPb9uQr9fu34SIB/AdyZKlQ2jWDuOIF09 00TRZD78zr9+v4DLhV9nWETSsBxuxASOyYRSbc8W1irQcccP/+tFMUZQrsJ0KjO6rQfg HRZt/bDZCqK8fFvbpLvBAZIvzvIZIgLvj63nH6MW/4+7+dz95lNiQOodgUdo/YVXAMN3 sSgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779330228; x=1779935028; h=mime-version:message-id:date:user-agent:references:in-reply-to :subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=kQH+CjNK/qOFvBb8SSrXTKpN7X2hUXLpuXZmtCCg4AM=; b=AtKl+4gnm6WtsYs9y5vgekIDYZliG+TunciGmdS9m4AD6KY+eQdgGGKnOJYlAMLRTN 7E42VSIpLDFWp2rqkJ4xc1IzTmv8RIe14TWMBbHxM4xru6UM78CezhgvkOQGN2X+ABn0 KxGk5uqGTaNSMrQBcE7UkExGriLOm5stOmfqwhj9VGYIQDjs5rqLp5A40eALik8JpvmG WqiLZ5A2lHqEvsnH5WObWCD4z5fzEPwAIffR6G+5RqSPM75wPx+IQHk8vz5hRSOW2sgH Pu+f9l6YbL9YK/5zDqxs5f2Yq2yYPP1IZzHOdsC06sdYm0E0kIpvprWlZ4OfWvCmU+fZ Wzyg== X-Forwarded-Encrypted: i=1; AFNElJ9Mt8hlanRvYB0x9AGrhshzX0eBkAZzHPp2mko2j30yB3FIXZSWiGNha+NsRxoyWPn8p07G+r+f5sh0sg==@sourceware.org X-Gm-Message-State: AOJu0YxvCB+v+kxZi3P8Ap91g1GkoDcEtUJkrIlx8KWyYI/62SF0mzjN G547GRKRhn61iuHYiSbON05mBK+r2NiO/q2vGrE3ZmOZ/x3iiGAEX7I+UmsVAi009wg= X-Gm-Gg: Acq92OH3/1hjp4gtkatQofOi60JIJIKsg5XN2wEWVrKtmukpZt2HctxY1vFCl8JRHeB 91Hl19fGNqxjDnMju5ikm2jeayADDX3sQIyVh9YGbRNdd854Xwdq/iQyFtsqFDAXly/GX7rXQ2T Z9C7bz08SD5jgBeFk+nno1lJasOK+XtFV/wgpXNexQJdEj6w5/CaGmfFV0Olt5HD++BbvwKogOb 06aXsTI1oM/T4S4c0xuYfKSJI46nFDxBRx5SUm6cGId+z+8F9D0Di1iDPn8qfvO6kkxfE/v17j3 XummD3+THJmyQz2TSA1beprmpVs53zGJOGvFyYcThSM+/J0tvmIAeif9nEOYEN3Mh+dVJGH3/TA xqo4HDJn9uF3VBNgLDJFJvOSB22pk1K55XOOPbVLie4QqoWnzdn9pbcxvddvoHt+DafGi0Ub7xd cco/uHRhO/1P8rLskt4AngCQP7UX5Uif75mbYDbrV2Dios X-Received: by 2002:a05:6102:3f94:b0:631:44bc:c119 with SMTP id ada2fe7eead31-6738e4d46d0mr722264137.7.1779330228057; Wed, 20 May 2026 19:23:48 -0700 (PDT) Received: from localhost ([2804:14d:7e39:8083:f04c:42e3:5943:38f6]) by smtp.gmail.com with ESMTPSA id ada2fe7eead31-63ccf18e114sm10859381137.1.2026.05.20.19.23.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 May 2026 19:23:47 -0700 (PDT) From: Thiago Jung Bauermann To: "Schimpe, Christina" Cc: Tom Tromey , "gdb-patches@sourceware.org" Subject: Re: [PATCH v2 0/9] Add new command to print the shadow stack backtrace In-Reply-To: (Christina Schimpe's message of "Mon, 18 May 2026 08:39:34 +0000") References: <20260123080532.878738-1-christina.schimpe@intel.com> User-Agent: mu4e 1.14.1; emacs 30.2 Date: Wed, 20 May 2026 23:23:44 -0300 Message-ID: <874ik11owf.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain 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 Hello Christina, "Schimpe, Christina" writes: > Hi Tom and Thiago, > > First, thanks a lot for your review efforts on this series so far! Thanks for moving this forward! > As indicated in the cover letter of v2, I would like to post v3, including the -past-main option. Great! > Enabling this made me realize that I need some refactoring of the commits: > - "gdb: Add command option 'bt -shadow' to print the shadow stack backtrace." > - "gdb: Provide gdbarch hook to distinguish shadow stack backtrace elements." > > I have mixed feelings about this. On the one hand, the patches have already been very well > reviewed (thanks again!) and the refactoring means that new review effort is required. But > on the other hand, I think these changes improve the series overall. Below, I briefly describe > the main changes to those commits. If this approach makes sense to you, I will post v3 > shortly after. I think it makes sense to do the refactoring. Changing approaches as the patch series evolve is part of the process. > To avoid increasing your review effort, I also considered adding these changes as new > patches on top. However, it is probably cleaner to merge them into the existing commits. > Another option is that I post the V3 including fixups on top (and merge in a v4) to make > the review easier for you. Or, we could also consider merging the series (once fully approved) > without -past-main and I could submit a follow-up series for that feature. If you have any > preference, I'm happy about your feedback. Thank you for your concern in this regard. Of course I can't speak for Tom but IMHO you can merge the changes into the existing commits. I use git range-diff between versions to see what changed within each patch, which provides similar output to the fixups approach. > My main changes to enable -past-main are: The changes below look good to me. > 1) A new commit (required before "gdb: Add command option 'bt -shadow' to print the shadow stack backtrace."): > > gdb: Add get_main_func_start_pc to refactor frame.c:inside_main_func. > > Refactor frame.c:inside_main_func to use the new function > get_main_func_start_pc, which will be used in a following commit. > > 2) Refactoring of "gdb: Add command option 'bt -shadow' to print the shadow stack backtrace." : > > To enable the -past-main option in the shadow stack backtrace the function > get_main_func_start_pc (gdbarch* gdbarch, const language lang) is called by a > new function shadow_stack_frame_info::inside_main_func. This probably requires > a frame specific gdbarch and language in the struct shadow_stack_frame_info. > I am not sure the gdbarch or language ever changes for IA, but it might be a good > idea to add it to be safe. I agree that it's a good idea to add them. > So, due to that we need some refactoring of the shadow > stack backtrace implementation: > a. The frame specific arch is extracted based on the symtab_and_line (SAL) > object. To avoid that we extract the SAL and gdbarch multiple times for > the same frame we store it in the shadow stack frame. > b. The gdbarch hook get_shadow_stack_size is obsolete since the function > get_trailing_outermost_shadow_stack_frame_info now unwinds each frame. This > helps to configure the previous frame attributes in case the gdbarch or SAL > information cannot be extracted for special shadow stack elements. We don't need > the COUNT parameter anymore in the function update_shadow_stack_pointer, too. > Now, this is also inline with the function trailing_outermost_frame which is used > for the normal backtrace. > c. Introduce function get_shadow_stack_frame_info to share some more logic: > ~~~ > /* If possible, get shadow stack frame info for the shadow stack pointer > SSP and its current frame LEVEL. Pass a fallback gdbarch FALLBACK_ARCH > which can be used as fallback in case the gdbarch cannot be extracted > from the SAL. Usually this is the gdbarch of the previous frame. */ > > static std::optional > get_shadow_stack_frame_info > (gdbarch *fallback_arch, const CORE_ADDR ssp, ULONGEST level) > ~~~ > > 3) Refactoring of "gdb: Provide gdbarch hook to distinguish shadow stack backtrace elements. > > a. Like normal frames used in the ordinary backtrace command, introduce an enum > ssp_frame_type (normal_frame, non_return_frame) which classifies each frame and is > an attribute of the shadow stack frame info. Calling of inside_main_func () to check > if we should stop unwinding is only done for normal frames. For the currently known > cases of non-return-address shadow stack elements, the value is not a PC of the program > and the past-main check is not applicable. > More frame types will be added in the following patches to enable sigtramp and inferior > call frames. > b. The gdbarch hook is_no_return_shadow_stack_address now returns the complete shadow stack > frame (classified as non_return_frame) in case the element on the shadow stack is not a return > address (instead of the optional return address description string used before). The target can > then assign the correct gdbarch and an optional SAL object. Due to that hook is no longer called > when printing the frame, but already when constructing it in get_shadow_stack_frame_info and > we don't pass the shadow stack frame as argument anymore, but the shadow stack pointer, its > value and level. > > The new interface looks as follows: > ~~~ > Method( > comment=""" > There can be elements on the shadow stack which are not return addresses. > This happens for example on x86 with CET in case of signals. > If an architecture implements the command options 'backtrace -shadow' and s/options/option/ > the shadow stack can contain elements which are not return addresses, this > function has to be provided. > Return a shadow stack frame info with frame type > ssp_frame_type::non_return_frame, if the shadow stack pointer SSP belongs > to a valid shadow stack frame while the element on the shadow stack > VALUE does not refer to a return address. In that case, also the frame's > attribute non_return_description must be set to a string which is displayed > instead of the element on the shadow stack in the shadow stack backtrace. > Otherwise, return an empty optional. > """, > type="std::optional", > name="is_no_return_shadow_stack_address", > params=[ > ("const CORE_ADDR", "ssp"), > ("const CORE_ADDR", "value"), > ("const unsigned long", "level") > ], > predicate=True, > ) > ~~~ > c. The shadow stack frame contains a new string attribute "non_return_description" > which is printed instead of the element on the shadow stack in case the frame is > classified as non-return address frame. It is empty in case the frame is not a > " ssp_frame_type::non_return_frame" frame. We could also use an optional here, > I am not sure what is preferred. I'm not sure either, but I have a slight preference for an optional. > This is now the class for a shadow stack frame: > > /* Information of a shadow stack frame belonging to a shadow stack element > at shadow stack pointer SSP. */ > > class shadow_stack_frame_info > { Looks good to me. -- Thiago (he/him)