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

Pedro Alves
2019-08-07T19:24:50Z

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