Re: [PATCH V2] AArch64 pauth: Indicate unmasked addresses in backtrace

[Pedro Alves <palves@redhat.com>]

On 7/30/19 3:41 PM, Alan Hayward wrote:

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 0fcd131f71..b7dba2f918 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -24380,6 +24380,14 @@ but the lengths of the @code{z} and @code{p} registers will not change.  This
>  is a known limitation of @value{GDBN} and does not affect the execution of the
>  target process.
>  
> +@subsubsection AArch64 Pointer Authentication.
> +@cindex AArch64 Pointer Authentication.
> +
> +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 it's value will be masked.

s/it's value/its value/

> +When GDB prints a backtrace, any addresses that required unmasking will be
> +postfixed with the marker [PAC].
>  

> diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
> index a2a96ac0d3..d805ec68f2 100644
> --- a/gdb/python/py-framefilter.c
> +++ b/gdb/python/py-framefilter.c
> @@ -901,6 +901,8 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
>  	    {
>  	      annotate_frame_address ();
>  	      out->field_core_addr ("addr", gdbarch, address);
> +	      if (get_frame_pc_masked (frame))
> +		out->field_string ("pac", " [PAC]");
>  	      annotate_frame_address_end ();
>  	      out->text (" in ");
>  	    }
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 7833ca4aeb..9d49809895 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -1298,7 +1298,11 @@ print_frame (const frame_print_options &fp_opts,
>  	{
>  	  annotate_frame_address ();
>  	  if (pc_p)
> -	    uiout->field_core_addr ("addr", gdbarch, pc);
> +	    {
> +	      uiout->field_core_addr ("addr", gdbarch, pc);
> +	      if (get_frame_pc_masked (frame))
> +		uiout->field_string ("pac", " [PAC]");

Hmm, I had suggested considering MI in the previous iteration, but
I was just thinking of including the "[PAC]" text in the
"addr" field.  If we're adding a new field, then a few extra
things need to be considered:

 #1 - documentation, both manual and NEWS should mention this new MI field.

 #2 - calling the attribute "pac" makes it architecture specific. 
      I.e., to make use of it, a frontend will have to have Aarch64 awareness?
      Not sure that is a good thing.

 #3 - The MI attribute is called "pac", and its content is
      literally " [PAC]".  I'd find that odd if I were a frontend author:
      the content is right aligned with a space, making doing anything with
      it other than appending it to the address text probably look odd,
      unless you bake in awareness of the attribute's text...  If I saw
      an attribute named "pac", I'd expect it to be a boolean?  At the
      least, the left space should not be part of the field, I think?
      Maybe we should rename the field to something else, like "addr_attr"
     for "address attributes" or something.

Thanks,
Pedro Alves