Re: [PATCH v2 6/9] gdb: Add command option 'bt -shadow' to print the shadow stack backtrace.

Tom Tromey
2026-02-19T18:20:21Z

>>>>> Christina Schimpe <christina.schimpe@intel.com> writes:

> Add command option '-shadow" to the backtrace command to print the shadow
> stack backtrace instead of the normal backtrace.

Thanks.

> -annotate_frame_function_name (void)
> +annotate_frame_function_name (bool shadowstack_frame)
> {
> if (annotation_level == 2)
> - printf_unfiltered (("\n\032\032frame-function-name\n"));
> + {
> + if (!shadowstack_frame)
> + printf_unfiltered (("\n\032\032frame-function-name\n"));
> + else
> + printf_unfiltered (("\n\032\032shadow-stack-frame-function-name\n"));
> + }

I think it is fine to just drop all the annotation changes.
As far as I know, no client even uses annotation_level > 1.

Emacs, maybe the only existing user of annotations, passes --fullname
which uses:

case 'f':
annotation_level = 1;

> + if (should_print_location (print_what) || sal.symtab == nullptr)
> + {
> + gdb::unique_xmalloc_ptr<char> funname = find_pc_funname (frame.value);
> +
> + { /* Extra scope to print frame tuple. */
> + ui_out_emit_tuple tuple_emitter (uiout, "shadow-stack-frame");

The extra scope doesn't really look necessary here, since:

> + } /* Extra scope to print frame tuple. */
> +
> + uiout->text ("\n");

... it seems fine to emit this text before the tuple emitter is
destroyed? And there's already an extras scope from the "then" block.

> + gdb_flush (gdb_stdout);

Is flushing really needed? I feel like I saw this in a different patch
in the series as well, and forgot to ask about it there.

> +
> +/* 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. */

It seems odd to combine a bool return and an out parameter when the enum
has a "no_error" value.

Maybe just a bool return would be more appropriate and then the callers
could set their own out parameters. Especially since one caller doesn't
even need this.

> -static const char *const print_frame_info_choices[] =
> +const char *const print_frame_info_choices[] =

This is public now but I didn't see other uses of it.

Tom