From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id WCzTHww1BGklthMAWB0awg (envelope-from ) for ; Fri, 31 Oct 2025 00:03:24 -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=qEVCHgAM; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 72D611E0BC; Fri, 31 Oct 2025 00:03:24 -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 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 146D31E057 for ; Fri, 31 Oct 2025 00:03:22 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 88931385840B for ; Fri, 31 Oct 2025 04:03:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 88931385840B 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=qEVCHgAM Received: from mail-pj1-x1033.google.com (mail-pj1-x1033.google.com [IPv6:2607:f8b0:4864:20::1033]) by sourceware.org (Postfix) with ESMTPS id 6E7BB3858D39 for ; Fri, 31 Oct 2025 04:02:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6E7BB3858D39 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 6E7BB3858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::1033 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1761883349; cv=none; b=j17fF140FCL7vvikeY/mekvz3rLUByfHDaYd7Su/Ph1Vp1TYdsyPfcVKuk6JDZWgoHpmARS6RYCsgs2o+FMkkm+QEAIwI3SMi8sH30lNJoxRji9XvrL90c0uuLRxVzSqdqk1yAm7v5wLJdIeapIrLe9d1zv97tU15eUVWUlQJxQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1761883349; c=relaxed/simple; bh=S2h+rpwJBzToaftWgvo2a9OZBx9uL3WWxQfofIDkrns=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=QbsDpF+y7cKga2eVKl6Xsyvgzcsw66sTwl3Puz6CHDHx2b4S2nSWP+gMZzinMzEh4cZwPv4asKHQvKKsLlzkWwRvp5jHFCUZOjXLCNK/E5YvJt0K0e/shJOMgztvKY5xwXzYqsjKc6+9sirVGf0j/uPyNuzucEO6ILgmeKwKQYQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6E7BB3858D39 Received: by mail-pj1-x1033.google.com with SMTP id 98e67ed59e1d1-3304dd2f119so1391316a91.2 for ; Thu, 30 Oct 2025 21:02:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1761883348; x=1762488148; darn=sourceware.org; h=content-transfer-encoding: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=ypXunPhQdpW14NvJcI83LKiN/xru3rqxLIjFjmEicJg=; b=qEVCHgAMOIUA1ETeMdIO4cWlmHQdAP7skdjKLlmbLnJbTfDhaHl+HiqAVFaiF/DxLJ sRu5ZJYDsJVm0dI9bWkeNPNn1If4yIfSQscQ1ab+AlFq1MetcnEsgKiyFsCcDnIXKt7B yPGGnjXgU0YETVVrdkmVUKAJZxphz1CpcWuiFfaPCWsXxjWLYF8yhg3bwhOjR8Ykj+jL +z+8yB7vMnSwSJcmOejDSxvX9r7uU6DECXNb8c+snFrudXI0JK3gvGS22diUvPpYEegY qqpig3Q2UHfrfFoKeuGhfROdCezgYDJOAyWoeiVBj0jlXQmmqDjzWkOz1ngxnHAIbzhh LfRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761883348; x=1762488148; h=content-transfer-encoding:mime-version:message-id:date:user-agent :references:in-reply-to:subject:cc:to:from:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=ypXunPhQdpW14NvJcI83LKiN/xru3rqxLIjFjmEicJg=; b=uC7Elj0Vmo7SvlBDrC1FB2FwlE/Z1fZ5Cj6Q3HCNbCA8kQFKG2IkLm/Sw2PULoQzmJ STY+4J3u2uz8x/XU2CTsO8O9yxr5bBIOhCt3d5+HtB0W6BTcruLnMdx1P2I1iLWF/5jT rzDUmEWNbhzbmNVvm5Q6k+Ogf9M6TjycAvInqlK/I3gkY6EZZU01SvMY3jhieBJiTTjH oq6NSibne/FBnzuPBkhBKAaHnKyio59V8v2I3EaTivc3GKI8qZyGZImRMfuobA1dyynw n/Gn7Xg0u6pEzzc32KZw174beYrUy1M4RDbmBSYyCkgt/0FimnAk459zzOVoTm24uPp/ F5Ag== X-Gm-Message-State: AOJu0YyEmh+VV5gXw7uoAEIPoQX6L0W8TXN5Q+1HfvhEcW6H6IWpSbkN rU8q7xqqlVN9mJQ6TNa5rxkMRTgdH9RqSEwMSl5FmEdpBfWq9phhJ7g8mMTFQCKnv9Y= X-Gm-Gg: ASbGncuEw4eOy5CRtjV/5+f/6tm4IA3utXFHazLBqs7AApBdXlvfbkzciwZ7PBNQ8bW nbN9iGXTYNouO7gTJLgPI8Bo9EKYCwsBNHcVuOhsjNQfILCvB54USkoGe4sNaOwYlEYhmVooFhY 5G4TXO5n6FzLO7wSdtgKX34S4erpS1xMlRhTYaMCIRjTBdnGuaPgQZWhxlD5mBnU8IEv3+hcLWm CFeFK/S0JCw84AdCmmgvRvjFGWz2RBVAADWd7hhnEuJlKkXZe1c1JnsfHwLW1gNzdrH9gdlZjjo cju15hZpAIdOD68lGPg1fMfeyWLfNmZlUot6JhXQEk8uN6sGqCUHkU+cFg6g/eZxJnsNyw/zYig +6DqUt+XPYiGx2LpO/fKqEEIFUanGX9IzRaVBy0zqVaLderkpr4WXxxeuMHvm9lBtTUBdgqsCJH ncrD/3dvNCDg== X-Google-Smtp-Source: AGHT+IGcCrPWV63HxvTJvvnxA7NFQtyrZMfee5cT+coW4BKhiN1zkXJpppcUjIkdXsnzAAulOkrOPQ== X-Received: by 2002:a17:90b:2b83:b0:32e:7c34:70cf with SMTP id 98e67ed59e1d1-3408309e96bmr2582148a91.36.1761883343678; Thu, 30 Oct 2025 21:02:23 -0700 (PDT) Received: from localhost ([2804:14d:7e39:88d6:dd80:7850:7b40:c343]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-34050ba1464sm4266186a91.16.2025.10.30.21.02.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Oct 2025 21:02:22 -0700 (PDT) From: Thiago Jung Bauermann To: Christina Schimpe Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 6/9] gdb: Implement 'bt shadow' to print the shadow stack backtrace. In-Reply-To: <20250923111842.4091694-7-christina.schimpe@intel.com> (Christina Schimpe's message of "Tue, 23 Sep 2025 11:18:39 +0000") References: <20250923111842.4091694-1-christina.schimpe@intel.com> <20250923111842.4091694-7-christina.schimpe@intel.com> User-Agent: mu4e 1.12.11; emacs 30.2 Date: Fri, 31 Oct 2025 01:02:19 -0300 Message-ID: <874irfsoqc.fsf@linaro.org> MIME-Version: 1.0 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 Christina Schimpe writes: > Add a subcommand 'bt shadow' for the ordinary backtrace command which > prints the shadow stack backtrace. > Similar to the ordinary backtrace command 'bt shadow' can be configured > using COUNT and the command line option -frame-info. However, we always > print the address and the command is not affected by the setting > "print address" as well as the setting "print frame-info location-and-add= ress". > Also we do not print the frame arguments. > > Usage: backtrace|bt shadow [OPTION]... [COUNT | -COUNT] > > Help output: > ~~ > (gdb) help bt shadow > Print backtrace of all shadow stack frames, or innermost COUNT frames. > Usage: backtrace shadow [OPTION]... [COUNT | -COUNT] > > Options: > -frame-info auto|source-line|location|source-and-location|location-and-= address|short-location > Set printing of shadow stack frame information. > > With a negative COUNT, print outermost -COUNT elements. > ~~ There's another thread discussion whether to use "bt -shadow" instead, and/or "shadow-stack backtrace" so I won't comment on this here. > Example for the output of 'bt shadow' on amd64 linux: > ~~ > (gdb) bt shadow > /#0 0x000000000040111f in call1 at amd64-shadow-stack.c:14 > /#1 0x000000000040112f in main at amd64-shadow-stack.c:21 > /#2 0x00007ffff7c3fe70 in __libc_start_call_main at ../sysdeps/nptl/libc= _start_call_main.h:58 > /#3 0x00007ffff7c3ff20 in __libc_start_main_impl at ../csu/libc-start.c:= 128 > /#4 0x0000000000401045 in _start > ~~ Here's an example output in my aarch64-linux test VM: (gdb) bt shadow #0 0x0000aaaaaaaa08ac in call2 at aarch64-gcs-return.c:65 #1 0x0000aaaaaaaa08c0 in call1 at aarch64-gcs-return.c:71 #2 0x0000aaaaaaaa09d4 in main at aarch64-gcs-return.c:110 #3 0x0000000000000000 in ?? And here is the regular backtrace at the same point: (gdb) bt #0 call3 () at aarch64-gcs-return.c:58 #1 0x0000aaaaaaaa08ac in call2 () at aarch64-gcs-return.c:64 #2 0x0000aaaaaaaa08c0 in call1 () at aarch64-gcs-return.c:70 #3 0x0000aaaaaaaa09d4 in main () at aarch64-gcs-return.c:107 I have a few comments on this output, at least as it is appears for GCS. I'm assuming x86 shadow stack output is similar: 1. The first thing that I notice is that the number of the frames don't match between the regular stack and the shadow stack: frame 0 in the regular stack doesn't exist in the shadow stack, regular frame 1 is shadow frame 0, and so on. I think this is confusing. It makes sense that frame 0 doesn't appear in the shadow stack backtrace because there really isn't an entry for it in the shadow stack, but I think the shadow entries should start with number 1, to be consistent. 2. The line numbers corresponding to the return addresses don't match between the regular backtrace and the shadow backtrace, even though the return addresses are the same. I would say this is a bug. Perhaps there can be a test making sure they match, if it's not too complicated? 3. One difference between my GCS test programs and your x86 test programs is the point at which the shadow stack is enabled. In your case, it appears to be done by glibc or the dynamic loader, very early in the process execution. For the AArch64 testcases I'm doing it explicitly in main itself, so that the test programs don't have to depend on a supported glibc or loader. All this to say that there is no GCS entry before main, yet one appears in the output above with a zeroed return address. I think this can be solved by stopping the backtrace loop if gdbarch_top_addr_empty_shadow_stack returns true in backtrace_shadow_command. I have a suggestion about this further down. > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index e8515883820..ebda4546b58 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -8840,6 +8840,53 @@ Display an absolute filename. > Show the current way to display filenames. > @end table >=20=20 > +@subsection Shadow stack backtrace > +@value{GDBN} provides a subcommand of the backtrace command to print the > +shadow stack backtrace. > + > +@anchor{shadowstack-backtrace-command} > +@kindex backtrace shadow > +@kindex bt shadow @r{(@code{backtrace shadow})} > +To print a backtrace of the entire shadow stack, use the > +@code{backtrace shadow} command, or its alias @code{bt shadow}. This > +command will print one line per element (so-called shadow stack frames) = on > +the shadow stack. > + > +A shadow stack is supported, for instance, with the Intel Control-Flow > +Enforcement Technology (@xref{CET}) on x86. Can you also add: ... and the Guarded Control Stack feature (@xref{GCS}) on AArch64. which would also require a @node entry here: @@ -27112,6 +27113,7 @@ information automatically from the core file, and w= ill show one of the above messages depending on whether the synchronous or asynchronous mode is sele= cted. @xref{Memory Tagging}. @xref{Memory}. =20 +@node GCS @subsubsection AArch64 Guarded Control Stack @cindex Guarded Control Stack, AArch64 @cindex GCS, AArch64 > +/* Return true, if PC is in the middle of a statement. Note that in the > + middle of a statement PC range includes sal.end (SAL.PC, SAL.END]. > + Return false, if > + - SAL.IS_STMT is false > + - there is no location information associated with this SAL, which > + could happen in case of inlined functions > + - PC is not in the range (SAL.PC, SAL.END]. > + This function is similar to stack.c:frame_show_address but is used > + to determine if we are in the middle of a statement only, not to deci= de > + if we should print a frame's address. */ > + > +static bool > +pc_in_middle_of_statement (CORE_ADDR pc, symtab_and_line sal) > +{ > + if (sal.is_stmt =3D=3D false) > + return false; > + > + /* If there is a line number, but no PC, then there is no location > + information associated with this sal. The only way that should > + happen is for the call sites of inlined functions (SAL comes from > + find_frame_sal). Otherwise, we would have some PC range if the In the case of stack.c:frame_show_address, SAL comes from find_frame_sal, but in this case it comes from find_pc_line, right? Does the comment still apply in this case? > + SAL came from a line table. However, as we don't have a frame for > + this function we cannot assert (in contrast to > + frame_show_address). */ > + if (sal.line !=3D 0 && sal.pc =3D=3D 0 && sal.end =3D=3D 0) > + return false; > + > + return pc > sal.pc && pc <=3D sal.end; > +} > + > +enum class ssp_unwind_stop_reason > +{ > + /* No particular reason; either we haven't tried unwinding yet, or we > + didn't fail. */ > + no_error =3D 0, > + > + /* We could not read the memory of the shadow stack element. */ > + memory_read_error > +}; > + > +/* Information of a shadow stack frame belonging to a shadow stack eleme= nt > + at shadow stack pointer SSP. */ > + > +class shadow_stack_frame_info > +{ > +public: > + /* If possible, unwind the previous shadow stack frame info. RANGE is > + the shadow stack memory range [start_address, end_address) belonging > + to this frame's shadow stack pointer. If we cannot unwind the > + previous frame info, set the unwind_stop_reason attribute. If we > + reached the bottom of the shadow stack just don't return a value. = */ > + std::optional unwind_prev_shadow_stack_frame_= info > + (gdbarch *gdbarch, std::pair range); > + > + /* The shadow stack pointer. */ > + CORE_ADDR ssp; Please add an empty line between each member variable and the next variable's documentation comment. > + /* The value of the shadow stack at SSP. */ > + CORE_ADDR value; > + /* The level of the element on the shadow stack. */ > + unsigned long level; > + /* If unwinding of the previous frame info fails assign this value to a > + matching condition ssp_unwind_stop_reason > + > ssp_unwind_stop_reason::no_error. */ > + ssp_unwind_stop_reason unwind_stop_reason > + =3D ssp_unwind_stop_reason::no_error; > +}; > + > +static > +gdb::unique_xmalloc_ptr find_pc_funname (CORE_ADDR pc) The return type should be together with the static. Also, this function needs a documentation comment. > +{ > + symbol *func =3D find_pc_function (pc); > + if (func) > + return find_symbol_funname (func); > + > + gdb::unique_xmalloc_ptr funname; > + bound_minimal_symbol msymbol =3D lookup_minimal_symbol_by_pc (pc); > + if (msymbol.minsym !=3D nullptr) > + funname.reset (xstrdup (msymbol.minsym->print_name ())); Just repeating Tom's comment from patch 4: This should use make_unique_xstrdup and plain assignment. > + > + return funname; > +} > + > +/* Print information of shadow stack frame info FRAME. The output is > + formatted according to PRINT_WHAT. For the meaning of PRINT_WHAT, see > + enum print_what comments in frame.h. Note that PRINT_WHAT is overrid= den, > + if PRINT_OPTIONS.print_frame_info !=3D print_frame_info_auto. */ > + > +static void > +do_print_shadow_stack_frame_info > + (ui_out *uiout, gdbarch *gdbarch, > + const shadow_stack_print_options &print_options, > + const shadow_stack_frame_info &frame, print_what print_what) > +{ > + if (print_options.print_frame_info !=3D print_frame_info_auto) > + { > + /* Use the specific frame information desired by the user. */ > + print_what > + =3D *print_frame_info_to_print_what (print_options.print_frame_info); > + } > + > + /* In contrast to find_frame_sal which is used for the ordinary backtr= ace > + command, we always want to print the line that is actually referred > + to by the address in the linetable. That's why we always pass 0 he= re > + as second argument. */ > + symtab_and_line sal =3D find_pc_line (frame.value, 0); > + > + if (should_print_location (print_what) || sal.symtab =3D=3D nullptr) > + { > + gdb::unique_xmalloc_ptr funname > + =3D find_pc_funname (frame.value); This line fits in 80 columns and doesn't need to be broken. > + { /* Extra scope to print frame tuple. */ > + ui_out_emit_tuple tuple_emitter (uiout, "shadow-stack-frame"); > + > + annotate_shadowstack_frame_begin (frame.level, gdbarch, > + frame.value); > + > + uiout->text ("#"); > + uiout->field_fmt_signed (2, ui_left, "level", frame.level); > + > + annotate_shadowstack_frame_address (); > + > + /* On x86 there can be a shadow stack token at bit 63. For x32, > + the address size is only 32 bit. Thus, we still must use > + gdbarch_shadow_stack_element_size_aligned (and not > + gdbarch_addr_bit) to determine the width of the address to be > + printed. */ > + const int element_size > + =3D gdbarch_shadow_stack_element_size_aligned (gdbarch); > + > + uiout->field_string > + ("addr", hex_string_custom (frame.value, element_size * 2), > + address_style.style ()); > + > + annotate_shadowstack_frame_address_end (); > + > + uiout->text (" in "); > + print_funname (uiout, funname, true); > + > + if (print_what !=3D SHORT_LOCATION && sal.symtab !=3D nullptr) > + print_filename (uiout, sal, true); > + > + if (print_what !=3D SHORT_LOCATION > + && (funname =3D=3D nullptr || sal.symtab =3D=3D nullptr) > + && sal.pspace !=3D nullptr) > + { > + const char *lib =3D solib_name_from_address (sal.pspace, > + frame.value); This line fits in 80 columns and doesn't need to be broken. > + if (lib !=3D nullptr) > + print_lib (uiout, lib, true); > + } > + } /* Extra scope to print frame tuple. */ > + > + uiout->text ("\n"); > + } > + > + if (print_what =3D=3D SRC_LINE || print_what =3D=3D SRC_AND_LOC) > + { > + int mid_statement =3D pc_in_middle_of_statement (frame.value, sal); mid_statement should be a bool. > + > + /* While for the ordinary backtrace printing of pc is based on > + MID_STATEMENT determined by stack.c:frame_show_address and the > + and the print configuration, for shadow stack backtrace we always > + print the pc/address on the shadow stack. */ > + bool print_address =3D true; > + print_source (uiout, gdbarch, frame.value, sal, print_address, > + mid_statement, ""); > + } > + > + annotate_shadowstack_frame_end (); > + gdb_flush (gdb_stdout); > +} > + > +/* Redirect output to a temporary buffer for the duration of > + do_print_shadow_stack_frame_info. */ > + > +static void > +print_shadow_stack_frame_info > + (gdbarch *gdbarch, const shadow_stack_print_options &print_options, > + const shadow_stack_frame_info &frame, print_what print_what) > +{ > + do_with_buffered_output > + (do_print_shadow_stack_frame_info, current_uiout, gdbarch, > + print_options, frame, print_what); > +} > + > + > +/* Extract a char array which can be used for printing a reasonable > + error message for REASON. Note that in case REASON has the value > + NO_ERROR the returned array is empty. */ > + > +static const char * > +ssp_unwind_stop_reason_to_err_string (ssp_unwind_stop_reason reason) > +{ > + switch (reason) > + { > + case ssp_unwind_stop_reason::no_error: > + return _(""); The empty string doesn't need to be translated. All callers make sure that reason isn't no_error, so perhaps just remove this case? > + case ssp_unwind_stop_reason::memory_read_error: > + return _("shadow stack memory read failure"); > + } > + > + gdb_assert_not_reached ("invalid reason"); > +} > + > + > +/* Read the memory at shadow stack pointer SSP and assign it to > + RETURN_VALUE. In case we cannot read the memory, set REASON to > + ssp_unwind_stop_reason::memory_read_error and return false. */ > + > +static bool > +read_shadow_stack_memory (gdbarch *gdbarch, CORE_ADDR ssp, > + CORE_ADDR *return_value, > + ssp_unwind_stop_reason *reason) If a function assumes that a pointer it gets as argument is non-null as this one does with return_value and reason, then IMO it's better to get it as a reference instead of a pointer. Then the compiler makes sure that it is indeed non-null. > +{ > + /* On x86 there can be a shadow stack token at bit 63. For x32, the > + address size is only 32 bit. Thus, we still must use > + gdbarch_shadow_stack_element_size_aligned (and not gdbarch_addr_bit) > + to read the full element for x32 as well. */ > + const int element_size > + =3D gdbarch_shadow_stack_element_size_aligned (gdbarch); > + > + const bfd_endian byte_order =3D gdbarch_byte_order (gdbarch); > + if (!safe_read_memory_unsigned_integer (ssp, element_size, byte_order, > + return_value)) > + { > + *reason =3D ssp_unwind_stop_reason::memory_read_error; > + return false; > + } > + > + return true; > +} > + > +/* If possible, return a shadow stack frame info which is COUNT elements > + above the bottom of the shadow stack. FRAME should point to the top > + of the shadow stack. RANGE is the shadow stack memory range > + [start_address, end_address) corresponding to FRAME's shadow stack > + pointer. If COUNT is bigger than the number of elements on the shad= ow > + stack, return FRAME. Here is another place where I find "above/bottom/top" confusing, since stacks can grow up or grow down (as pointed out in the next comment in this function), and these terms mean different things in each case. > + In case of failure, assign an appropriate ssp_unwind_stop_reason in > + FRAME->UNWIND_STOP_REASON. */ > + > +static std::optional > +get_trailing_outermost_shadow_stack_frame_info > + (gdbarch *gdbarch, const std::pair range, > + const ULONGEST count, shadow_stack_frame_info &frame) > +{ > + /* Compute the number of bytes on the shadow stack, starting at > + FRAME->SSP, which depends on the direction the shadow stack > + grows. */ > + const int element_size > + =3D gdbarch_shadow_stack_element_size_aligned (gdbarch); > + const unsigned long shadow_stack_bytes > + =3D (gdbarch_stack_grows_down (gdbarch)) The parentheses around the function call are superfluous. > + ? range.second - frame.ssp : frame.ssp - range.first + element_si= ze; > + > + gdb_assert ((shadow_stack_bytes % element_size) =3D=3D 0); > + const unsigned long shadow_stack_size > + =3D shadow_stack_bytes / element_size; This line fits 80 columns and doesn't need to be broken. > + const long level =3D shadow_stack_size - count; In a comment further below you mention that level is 0-based, but doesn't this expression mean that it's 1-based? Perhaps it's missing a "- 1" term? Also, this doesn't work for GCS because it assumes all elements in the stack are return addresses. In GCS, the oldest element is a 0 entry. The 0 entry is why this patch requires AArch64 to implement its own gdbarch top_addr_empty_shadow_stack hook. Finally, not a concern for userspace support (and thus this patch series) but for OS and hypervisor software there's another kind of GCS entry which is the exception return record that is put on the stack when an exception is taken. That entry is 32 bytes in size. I mention this just to illustrate that calculating the number of entries in the stack can be non-trivial. Not sure how to account for these variations in generic code. Maybe add a new gdbarch method for returning the number of entries in the shadow stack? > + > + /* COUNT exceeds the number of elements on the shadow stack. Return t= he > + starting shadow stack frame info FRAME. */ > + if (level <=3D 0) > + return std::optional (frame); > + > + CORE_ADDR new_ssp =3D update_shadow_stack_pointer > + (gdbarch, frame.ssp, count, ssp_update_direction::bottom); Shouldn't update_shadow_stack_pointer be called with 'level' rather than 'count' as argument? > + if (gdbarch_stack_grows_down (gdbarch)) > + gdb_assert (new_ssp < range.second); > + else > + gdb_assert (new_ssp >=3D range.first); > + > + CORE_ADDR new_value; > + if (!read_shadow_stack_memory (gdbarch, new_ssp, &new_value, > + &frame.unwind_stop_reason)) > + return {}; > + > + return std::optional > + ({new_ssp, new_value, (ULONGEST) level, > + ssp_unwind_stop_reason::no_error}); This line causes a compilation error in arm-linux-gnueabihf, and I suppose other 32-bit targets as well: /home/bauermann/src/binutils-gdb/gdb/shadow-stack.c:471:27: error: narrowin= g conversion of =E2=80=98(ULONGEST)((long int)level)=E2=80=99 from =E2=80= =98ULONGEST=E2=80=99 {aka =E2=80=98long long unsigned int=E2=80=99} to =E2= =80=98long unsigned int=E2=80=99 [-Werror=3Dnarrowing] 471 | ({new_ssp, new_value, (ULONGEST) level, ssp_unwind_stop_reason:= :no_error}); | ^~~~~~~~~~~~~~~~ Also, it fits 80 columns and doesn't need to be broken. > +} > + > +std::optional > +shadow_stack_frame_info::unwind_prev_shadow_stack_frame_info > + (gdbarch *gdbarch, std::pair range) > +{ > + /* If the user's backtrace limit has been exceeded, stop. We must > + add two to the current level; one of those accounts for > + backtrace_limit being 1-based and the level being 0-based, and the > + other accounts for the level of the new frame instead of the level > + of the current frame. */ > + if (this->level + 2 > user_set_backtrace_options.backtrace_limit) > + return {}; > + > + CORE_ADDR new_ssp > + =3D update_shadow_stack_pointer (gdbarch, this->ssp, 1, > + ssp_update_direction::bottom); > + > + if (gdbarch_stack_grows_down (gdbarch)) To make this work for GCS, I had to add another if statement before the one above, to handle the case where new_ssp is at the initial 0 value: if (gdbarch_top_addr_empty_shadow_stack_p (gdbarch) && gdbarch_top_addr_empty_shadow_stack (gdbarch, new_ssp, range)) return {}; else if (gdbarch_stack_grows_down (gdbarch)) =E2=8B=AE Otherwise "bt shadow" will print the 0 entry: (gdb) bt shadow #0 0x0000aaaaaaaa08ac in call2 at aarch64-gcs-return.c:65 #1 0x0000aaaaaaaa08c0 in call1 at aarch64-gcs-return.c:71 #2 0x0000aaaaaaaa09d4 in main at aarch64-gcs-return.c:110 #3 0x0000000000000000 in ?? > + { > + /* The shadow stack grows downwards. */ > + if (new_ssp >=3D range.second) > + { > + /* We reached the bottom of the shadow stack. */ In this case, we reached the top actually. This is why I find it confusing to use bottom/top as if it where agnostic to whether the stack grows up or down... > + return {}; > + } > + /* We updated new_ssp towards the bottom of the shadow stack befor= e, > + and new_ssp must be pointing to shadow stack memory. */ > + gdb_assert (new_ssp > range.first); > + } > + else > + { > + /* The shadow stack grows upwards. */ > + if (new_ssp < range.first) > + { > + /* We reached the bottom of the shadow stack. */ > + return {}; > + } > + /* We updated new_ssp towards the bottom of the shadow stack befor= e, > + and new_ssp must be pointing to shadow stack memory. */ > + gdb_assert (new_ssp <=3D range.second); > + } > + > + CORE_ADDR new_value; > + if (!read_shadow_stack_memory (gdbarch, new_ssp, &new_value, > + &this->unwind_stop_reason)) > + return {}; > + > + return std::optional > + ({new_ssp, new_value, this->level + 1, > + ssp_unwind_stop_reason::no_error}); This line fits 80 columns and doesn't need to be broken. > +} > + > +/* Print all elements on the shadow stack or just the innermost COUNT_EXP > + frames. */ > + > +static void > +backtrace_shadow_command (const shadow_stack_print_options &print_option= s, > + const char *count_exp, int from_tty) > +{ > + if (!target_has_stack ()) > + error (_("No shadow stack.")); > + > + gdbarch *gdbarch =3D get_current_arch (); > + if (!gdbarch_address_in_shadow_stack_memory_range_p (gdbarch)) > + error (_("Printing of the shadow stack backtrace is not supported fo= r" > + " the current target.")); > + > + regcache *regcache =3D get_thread_regcache (inferior_thread ()); > + bool shadow_stack_enabled =3D false; > + > + std::optional start_ssp > + =3D gdbarch_get_shadow_stack_pointer (gdbarch, regcache, > + shadow_stack_enabled); > + > + if (!start_ssp.has_value () || !shadow_stack_enabled) > + error (_("Shadow stack is not enabled for the current target.")); At least for AArch64, GCS can be enabled or disabled per-thread so I would say "not enabled for the current thread" here. > + /* Check if START_SSP points to a shadow stack memory range and use > + the returned range to determine when to stop unwinding. > + Note that a shadow stack memory range can change, due to shadow sta= ck > + switches for instance on x86 for an inter-privilege far call or when > + calling an interrupt/exception handler at a higher privilege level. > + Shadow stack for userspace is supported for amd64 linux starting wi= th > + Linux kernel v6.6. However, shadow stack switches are not supported > + due to missing kernel space support. We therefore implement this > + command without support for shadow stack switches for now. */ Hm, I'm not sure if aarch64-linux supports GCS switching. The kernel's gcs.rst documentation says: * The architecture provides instructions for switching between guarded control stacks with checks to ensure that the new stack is a valid target for switching. And a comment in AArch64's implementation of the map_shadow_stack syscall says: /* * Put a cap token at the end of the allocated region so it * can be switched to. */ But I don't see anything positively mention that it's supported. I assume so, but I'll have to confirm. If that is the case, later I will submit a patch with any necessary changes. > + std::pair range; > + if (!gdbarch_address_in_shadow_stack_memory_range (gdbarch, *start_ssp, > + &range)) Shouldn't this if condition also check gdbarch_top_addr_empty_shadow_stack? > + { > + /* If the current shadow stack pointer does not point to shadow > + stack memory, the shadow stack is empty. */ > + gdb_printf (_("The shadow stack is empty.\n")); > + return; > + } > + > + /* Extract the first shadow stack frame info (level 0). */ > + ssp_unwind_stop_reason reason =3D ssp_unwind_stop_reason::no_error; > + std::optional current; > + CORE_ADDR new_value; > + if (read_shadow_stack_memory (gdbarch, *start_ssp, &new_value, &reason= )) > + current =3D {*start_ssp, new_value, 0, > + ssp_unwind_stop_reason::no_error}; This line fits in 80 columns and doesn't need to be broken. > + > + std::optional trailing =3D current; > + > + LONGEST count =3D -1; > + if (current.has_value () && count_exp !=3D nullptr) > + { > + count =3D parse_and_eval_long (count_exp); > + /* If count is negative, update trailing with the shadow stack fra= me > + info from which we should start printing. */ Actually, trailing will be the frame above the one from which we should start printing. For example, if count is -3 get_trailing_outermost_shadow_stack_frame_info will return the frame "which is COUNT elements above the bottom of the shadow stack", which in this case is the 4th frame counting from the oldest frame. But we need to start printing from the 3rd oldest frame ... > + if (count < 0) > + { > + trailing =3D get_trailing_outermost_shadow_stack_frame_info > + (gdbarch, range, std::abs (count), *current); ... so this call should pass std::abs (count) - 1 as argument. > + if (!trailing.has_value ()) > + reason =3D current->unwind_stop_reason; > + } > + } > + > + if (!trailing.has_value ()) > + { > + if (reason > ssp_unwind_stop_reason::no_error) > + error (_("Cannot print shadow stack backtrace: %s.\n"), > + ssp_unwind_stop_reason_to_err_string (reason)); > + else > + gdb_assert_not_reached ("invalid reason"); > + } It's clearer to write this as: if (!trailing.has_value ()) { gdb_assert (reason !=3D ssp_unwind_stop_reason::no_error); error (_("Cannot print shadow stack backtrace: %s.\n"), ssp_unwind_stop_reason_to_err_string (reason)); } > + > + current =3D trailing; > + while (current.has_value () && count--) The GDB Coding Standard says that comparing a number to zero should be explicit, so "count-- !=3D 0" here. I see that backtrace_command_1 already has the implicit comparison, but it's probably a mistake there (or just old code). Also, another way of expressing the above would be: for (current =3D trailing; current.has_value () && count !=3D 0; count--) I personally think it's clearer (especially because the comparison and decrement are separate), but it's more of a personal preference so I don't really mind either way. > + { > + QUIT; > + > + print_shadow_stack_frame_info (gdbarch, print_options, *current, > + LOCATION); > + > + trailing =3D current; > + current =3D current->unwind_prev_shadow_stack_frame_info (gdbarch, > + range); This line fits in 80 columns and doesn't need to be broken. > + } > + > + /* If we've stopped before the end, mention that. */ > + if (current && from_tty) While it's correct to use an std::optional in this way to check whether it has a value, IMHO it's clearer to be more explicit and use the has_value method. It's also consistent with the rest of the code in this function. > + gdb_printf (_("(More shadow stack frames follow...)\n")); > + > + /* If we've run out of shadow stack frames, and the reason appears to > + be an error condition, print it. */ > + if (!current.has_value () && trailing.has_value () If I'm not mistaken, trailing.has_value () is always true at this point. > + && (trailing->unwind_stop_reason > ssp_unwind_stop_reason::no_erro= r)) > + gdb_printf (_("Shadow stack backtrace stopped at shadow stack " \ > + "pointer %s due to: %s.\n"), > + paddress (gdbarch, trailing->ssp), > + ssp_unwind_stop_reason_to_err_string > + (trailing->unwind_stop_reason)); > +} > diff --git a/gdb/stack.c b/gdb/stack.c > index 40756f74a00..30a7d8621be 100644 > --- a/gdb/stack.c > +++ b/gdb/stack.c > @@ -50,6 +50,7 @@ > #include "linespec.h" > #include "cli/cli-utils.h" > #include "objfiles.h" > +#include "shadow-stack.h" >=20=20 > #include "symfile.h" > #include "extension.h" > @@ -86,7 +87,7 @@ const char print_frame_info_source_and_location[] =3D "= source-and-location"; > const char print_frame_info_location_and_address[] =3D "location-and-add= ress"; > const char print_frame_info_short_location[] =3D "short-location"; >=20=20 > -static const char *const print_frame_info_choices[] =3D > +const char *const print_frame_info_choices[] =3D > { > print_frame_info_auto, > print_frame_info_source_line, > @@ -962,11 +963,9 @@ do_gdb_disassembly (struct gdbarch *gdbarch, > } > } >=20=20 > -/* Converts the PRINT_FRAME_INFO choice to an optional enum print_what. > - Value not present indicates to the caller to use default values > - specific to the command being executed. */ > +/* See stack.h. */ >=20=20 > -static std::optional > +std::optional > print_frame_info_to_print_what (const char *print_frame_info) > { > for (int i =3D 0; print_frame_info_choices[i] !=3D NULL; i++) > @@ -1016,7 +1015,7 @@ get_user_print_what_frame_info (std::optional *what) > /* Return true if PRINT_WHAT is configured to print the location of a > frame. */ Change comment here to /* See stack.h. */ > -static bool > +bool > should_print_location (print_what print_what) > { > return (print_what =3D=3D LOCATION > @@ -1025,14 +1024,9 @@ should_print_location (print_what print_what) > || print_what =3D=3D SHORT_LOCATION); > } >=20=20 > -/* Print the source information for PC and SAL to UIOUT. Based on the > - user-defined configuration disassemble-next-line, display disassembly > - of the next source line, in addition to displaying the source line > - itself. Print annotations describing source file and and line number > - based on MID_STATEMENT information. If SHOW_ADDRESS is true, print t= he > - program counter PC including, if non-empty, PC_ADDRESS_FLAGS. */ > +/* See stack.h. */ >=20=20 > -static void > +void > print_source (ui_out *uiout, gdbarch *gdbarch, CORE_ADDR pc, > symtab_and_line sal, bool show_address, int mid_statement, > const std::string &pc_address_flags) > @@ -1298,7 +1292,7 @@ get_last_displayed_sal () >=20=20 > /* Find the function name for the symbol SYM. */ Change comment here to /* See stack.h. */ >=20=20 > -static gdb::unique_xmalloc_ptr > +gdb::unique_xmalloc_ptr > find_symbol_funname (const symbol *sym) > { > gdb::unique_xmalloc_ptr funname; > +/* Completer for the "backtrace shadow" sub-command. */ > + > +static void > +backtrace_shadow_command_completer (struct cmd_list_element *ignore, > + completion_tracker &tracker, > + const char *text, const char */*word*/) > +{ > + const auto group > + =3D make_backtrace_shadow_options_def_group (nullptr); > + if (gdb::option::complete_options > + (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, = group)) > + return; > + > + const char *word =3D advance_to_expression_complete_word_point (tracke= r, text); > + expression_completer (ignore, tracker, text, word); > +} I would put this function in shadow-stack.c. > diff --git a/gdb/testsuite/gdb.arch/amd64-shadow-stack-cmds.exp b/gdb/tes= tsuite/gdb.arch/amd64-shadow-stack-cmds.exp > index 0ae172d7c41..4563e49d9e4 100644 > --- a/gdb/testsuite/gdb.arch/amd64-shadow-stack-cmds.exp > +++ b/gdb/testsuite/gdb.arch/amd64-shadow-stack-cmds.exp > @@ -109,6 +109,94 @@ save_vars { ::env(GLIBC_TUNABLES) } { > gdb_test "print /x \$pl3_ssp" "=3D $ssp_call2" "check pl3_ssp of frame = 0" > } >=20=20 > + set fill "\[^\r\n\]+" > + # Build shadow stack frames to test the 'backtrace shadow' command. > + set sspval_main [get_valueof "/z" "*(long long int*)$ssp_main" ""] > + set sspval_call1 [get_valueof "/z" "*(long long int*)$ssp_call1" ""] > + set sspval_call2 [get_valueof "/z" "*(long long int*)$ssp_call2" ""] I suggest adding a descriptive test name to the get_valueof calls above. Perhaps "read newest shadow stack entry at main/call1/call2"? > + set frame0 "#0\[ \t\]*$sspval_call2 in call1$fill" > + set frame1 "#1\[ \t\]*$sspval_call1 in main$fill" > + set frame2 "#2\[ \t\]*$sspval_main$fill" > + > + # We can only test that we print the first 3 frames correctly, as the > + # shadow stack enablement might depend on the underlying OS. > + gdb_test "bt shadow" \ > + [multi_line \ > + "$frame0" \ > + "$frame1" \ > + "$frame2" \ > + ".*" ] \ > + "test shadow stack backtrace until the bottom of the stack." > + > + gdb_test "bt shadow 2" \ > + [multi_line \ > + "$frame0" \ > + "$frame1" \ > + "\\(More shadow stack frames follow...\\)" ] \ > + "test shadow stack backtrace with a positive value for count" > + > + # We can only test that we print a single frame, as the shadow stack > + # enablement might depend on the underlying OS. > + gdb_test "bt shadow -1" "#$decimal\[ \t\]*$hex$fill" \ > + "test shadow stack backtrace with a negative value for count" In my GCS testcase this test is passing, even though the command is printing more than one frame: bt shadow -1 #3 0x0000aaaaaaaa08c0 in call1 at /home/bauermann/src/binutils-gdb/gdb/tes= tsuite/gdb.arch/aarch64-gcs-return.c:71 #4 0x0000aaaaaaaa09d4 in main at /home/bauermann/src/binutils-gdb/gdb/test= suite/gdb.arch/aarch64-gcs-return.c:110 #5 0x0000000000000000 in ?? (gdb) PASS: gdb.arch/aarch64-gcs-backtrace.exp: test shadow stack backtrace= with a negative value for count So there's one bug in GDB, and one in the testcase. To fix the testcase bug, I believe you just have to add '^' to the beginning of the pattern. The bug in GDB was fixed by changing the expression to calculate 'level' in get_trailing_outermost_shadow_stack_frame_info as I mentioned above. --=20 Thiago