Re: [PATCH] Move [PAC] into a new MI field addr_flags

[Eli Zaretskii <eliz at gnu dot org>]

> From: Alan Hayward <Alan.Hayward@arm.com>
> CC: nd <nd@arm.com>, Alan Hayward <Alan.Hayward@arm.com>
> Date: Mon, 12 Aug 2019 15:13:52 +0000
> 
> Add a new print_pc which prints both the PC and a new field addr_flags.
> Call this wherever the PC is printed in stack.c.
> 
> Add a new gdbarch method get_pc_address_flags to obtain the addr_flag
> contents. By default returns an empty string, on AArch64 this returns
> PAC if the address has been masked in the frame.
> 
> Document this in the manual and NEWS file.
> 
> gdb/ChangeLog:
> 
> 2019-08-12  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* NEWS (Other MI changes): New subsection.
> 	* aarch64-tdep.c (aarch64_get_pc_address_flags): New function.
> 	(aarch64_gdbarch_init): Add aarch64_get_pc_address_flags.
> 	* arch-utils.c (default_get_pc_address_flags): New function.
> 	* arch-utils.h (default_get_pc_address_flags): New declaration.
> 	* gdbarch.sh: Add get_pc_address_flags.
> 	* gdbarch.c: Regenerate.
> 	* gdbarch.h: Likewise.
> 	* stack.c (print_pc): New function.
> 	(print_frame_info) (print_frame): Call print_pc.
> 
> gdb/doc/ChangeLog:
> 
> 2019-08-12  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* gdb.texinfo (AArch64 Pointer Authentication)
> 	(GDB/MI Breakpoint Information) (Frame Information): Document
> 	addr_field.
> ---
>  gdb/NEWS            |  6 ++++++
>  gdb/aarch64-tdep.c  | 13 +++++++++++++
>  gdb/arch-utils.c    |  8 ++++++++
>  gdb/arch-utils.h    |  4 ++++
>  gdb/doc/gdb.texinfo | 18 +++++++++++++++++-
>  gdb/gdbarch.c       | 23 +++++++++++++++++++++++
>  gdb/gdbarch.h       |  6 ++++++
>  gdb/gdbarch.sh      |  3 +++
>  gdb/stack.c         | 29 ++++++++++++++++++++---------
>  9 files changed, 100 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index fa01adf6e8..42b2ba3d2b 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -287,6 +287,12 @@ maint show test-options-completion-result
>    These can be used to catch C++ exceptions in a similar fashion to
>    the CLI commands 'catch throw', 'catch rethrow', and 'catch catch'.
>  
> +* Other MI changes
> +
> + ** Backtraces and frames include a new optional field addr_flags which is
> +    given after the addr field.  Currently this is only used by AArch64
> +    for indicating PAC encyrpted addresses.

I think your original description at the beginning of your message
describes the purpose of this field more clearly.

Also, "encyrpted" is a typo.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -24397,7 +24397,8 @@ When @value{GDBN} is debugging the AArch64 architecture, and the program is
>  using the v8.3-A feature Pointer Authentication (PAC), then whenever the link
>  register @code{$lr} is pointing to an PAC function its value will be masked.
>  When GDB prints a backtrace, any addresses that required unmasking will be
> -postfixed with the marker [PAC].
> +postfixed with the marker [PAC].  When using the MI, this is printed as part
> +of the @code{addr_flags}. field
                           ^
That period should be moved to after "field".

> +@item addr_flags
> +Optional field containing any flags related to the address.  If there
> +are any flags defined for the current target then they are documented in
> +the @xref{Architectures} section.

I suggest to reword:

  These flags are architecture-dependent; see @ref{Architectures} for
  their meaning for a particular CPU.

Thanks.