Re: [PATCH v2 1/9] gdb: Generalize handling of the shadow stack pointer.

[Tom Tromey <tom@tromey.com>]

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

> Until now, handling of the shadow stack pointer has been done in the
> target dependent implementations of the gdbarch hook
> 'gdbarch_shadow_stack_push'.  Also amd64 and aarch64 linux specific
> unwinders for the shadow stack pointer are implemented.
> In a following patch a command line option "-shadow" will be added to
> the backtrace command to print the shadow stack backtrace.  This requires
> more target-independent logic to handle the shadow stack pointer.  To
> avoid that we duplicate the logic, add new source and header files
> "shadow-stack" for the implementation of shadow_stack_push and shadow
> stack pointer unwinding in a target-independent way.

This looks reasonable to me, but I think someone else ought to examine
the arch-specific bits.

Like I mentioned in another note, be sure to run check-gdbarch.py.

I had a nit or two, plus a question, but nothing really serious.

> +static void
> +amd64_init_reg (gdbarch *gdbarch, int regnum, dwarf2_frame_state_reg *reg,
> +		const frame_info_ptr &this_frame)
> +{
> +  if (regnum == gdbarch_pc_regnum (gdbarch))
> +    reg->how = DWARF2_FRAME_REG_RA;
> +  else if (regnum == gdbarch_sp_regnum (gdbarch))
> +    reg->how = DWARF2_FRAME_REG_CFA;
> +  else if (regnum == AMD64_PL3_SSP_REGNUM)
> +    {
> +      reg->how = DWARF2_FRAME_REG_FN;
> +      reg->loc.fn = dwarf2_prev_ssp;
> +    }
> +}
> +
>  void
>  amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>  		const target_desc *default_tdesc)
> @@ -3650,6 +3667,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>    set_gdbarch_in_indirect_branch_thunk (gdbarch,
>  					amd64_in_indirect_branch_thunk);
 
> +  set_gdbarch_ssp_regnum (gdbarch, tdep->ssp_regnum);
> +  dwarf2_frame_set_init_reg (gdbarch, amd64_init_reg);
> +

I was curious why this code refers to tdep->ssp_regnum but then the
regnum seems to be hard-coded in amd64_init_reg.

> +enum class ssp_update_direction
> +{
> +  /* Update ssp towards the oldest (outermost) element of the shadow
> +     stack.  */
> +  outer = 0,
> +
> +  /* Update ssp towards the most recent (innermost) element of the
> +     shadow stack.  */
> +  inner

Mildly prefer a trailing "," on enums.  That way if a new value is
needed the patch doesn't have to touch this line.

> +/* See shadow-stack.h.  */
> +
> +void shadow_stack_push (regcache *regcache, const CORE_ADDR new_addr)

Newline after 'void'

Reviewed-By: Tom Tromey <tom@tromey.com>

Tom