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

Alan Hayward
2019-08-07T12:35:52Z

> On 6 Aug 2019, at 17:15, Tom Tromey <tom@tromey.com> wrote:
>
>>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:
>
> Alan> Armv8.3-a Pointer Authentication causes the function return address to be
> Alan> obfuscated on entry to some functions. GDB must unmask the link register in
> Alan> order to produce a backtrace.
>
> Alan> The following patch adds markers of [PAC] to the bracktrace, to indicate
> Alan> which addresses needed unmasking. This includes the backtrace when using MI.
>
> Thanks for the patch. I don't think this was re-reviewed?

It hadn’t been - so thanks!

>
> Alan> --- a/gdb/frame.c
> Alan> +++ b/gdb/frame.c
> Alan> @@ -124,6 +124,8 @@ struct frame_info
> Alan> struct {
> Alan> enum cached_copy_status status;
> Alan> CORE_ADDR value;
> Alan> + /* Did VALUE require unmasking when being read. */
> Alan> + bool masked;
> Alan> } prev_pc;
>
> I think putting the new field between "status" and "value" will pack the
> structure a bit better.

It doesn’t look as nice when switched, but agreed and done.

>
> Alan> +bool
> Alan> +get_frame_pc_masked (struct frame_info *frame)
>
> Maybe make the parameter "const"? I see there isn't much use of const
> in frame.h, but on the other hand, that's probably just historical and
> not required.
>

Done.

> Otherwise the patch looks fine to me. It is ok. I'd prefer those
> changes but if you'd prefer not, a note to that effect is fine.
>
> thanks,
> Tom

Patch pushed and posted below for reference.

Alan.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 82e0819c13..bacf5d7636 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2019-08-07 Alan Hayward <alan.hayward@arm.com>
+
+ * NEWS: Expand the Pointer Authentication entry.
+ * aarch64-tdep.c (aarch64_frame_unmask_address): Rename from this.
+ (aarch64_frame_unmask_lr): ... to this.
+ (aarch64_prologue_prev_register, aarch64_dwarf2_prev_register):
+ Call aarch64_frame_unmask_lr.
+ * frame.c (struct frame_info): Add "masked" variable.
+ (frame_set_previous_pc_masked) (frame_get_pc_masked): New functions.
+ (fprint_frame): Check for masked pc.
+ * frame.h (frame_set_previous_pc_masked) (frame_get_pc_masked): New
+ declarations.
+ * python/py-framefilter.c (py_print_frame): Check for masked pc.
+ * stack.c (print_frame): Check for masked pc.
+
2019-08-06 Tom Tromey <tom@tromey.com>

* stabsread.c (patch_block_stabs, read_one_struct_field)
diff --git a/gdb/NEWS b/gdb/NEWS
index b4c59e4410..fa01adf6e8 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -16,7 +16,9 @@
architectures require kernel changes. TLS is not yet supported for
amd64 and i386 process core dumps.

-* Support for Pointer Authentication on AArch64 Linux.
+* Support for Pointer Authentication (PAC) on AArch64 Linux. Return
+ addresses that required unmasking are shown in the backtrace with the
+ postfix [PAC].

* Two new convenience functions $_cimag and $_creal that extract the
imaginary and real parts respectively from complex numbers.
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index e23cc3f956..9b6324f0fc 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -250,13 +250,13 @@ class instruction_reader : public abstract_instruction_reader

} // namespace

-/* If address signing is enabled, mask off the signature bits from ADDR, using
- the register values in THIS_FRAME. */
+/* If address signing is enabled, mask off the signature bits from the link
+ register, which is passed by value in ADDR, using the register values in
+ THIS_FRAME. */

static CORE_ADDR
-aarch64_frame_unmask_address (struct gdbarch_tdep *tdep,
- struct frame_info *this_frame,
- CORE_ADDR addr)
+aarch64_frame_unmask_lr (struct gdbarch_tdep *tdep,
+ struct frame_info *this_frame, CORE_ADDR addr)
{
if (tdep->has_pauth ()
&& frame_unwind_register_unsigned (this_frame,
@@ -265,6 +265,9 @@ aarch64_frame_unmask_address (struct gdbarch_tdep *tdep,
int cmask_num = AARCH64_PAUTH_CMASK_REGNUM (tdep->pauth_reg_base);
CORE_ADDR cmask = frame_unwind_register_unsigned (this_frame, cmask_num);
addr = addr & ~cmask;
+
+ /* Record in the frame that the link register required unmasking. */
+ set_frame_previous_pc_masked (this_frame);
}

return addr;
@@ -952,7 +955,7 @@ aarch64_prologue_prev_register (struct frame_info *this_frame,
if (tdep->has_pauth ()
&& trad_frame_value_p (cache->saved_regs,
tdep->pauth_ra_state_regnum))
- lr = aarch64_frame_unmask_address (tdep, this_frame, lr);
+ lr = aarch64_frame_unmask_lr (tdep, this_frame, lr);

return frame_unwind_got_constant (this_frame, prev_regnum, lr);
}
@@ -1119,7 +1122,7 @@ aarch64_dwarf2_prev_register (struct frame_info *this_frame,
{
case AARCH64_PC_REGNUM:
lr = frame_unwind_register_unsigned (this_frame, AARCH64_LR_REGNUM);
- lr = aarch64_frame_unmask_address (tdep, this_frame, lr);
+ lr = aarch64_frame_unmask_lr (tdep, this_frame, lr);
return frame_unwind_got_constant (this_frame, regnum, lr);

default:
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 6faf5f359d..702bb7c7a0 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2019-08-07 Alan Hayward <alan.hayward@arm.com>
+
+ * gdb.texinfo (AArch64 Pointer Authentication): New subsection.
+
2019-08-05 Christian Biesinger <cbiesinger@google.com>

* python.texi (Blocks In Python): Document dictionary access on blocks.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 89b1eda2c1..7f8c0aff1c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24390,6 +24390,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.
+When GDB prints a backtrace, any addresses that required unmasking will be
+postfixed with the marker [PAC].

@node i386
@subsection x86 Architecture-specific Issues
diff --git a/gdb/frame.c b/gdb/frame.c
index adac24f68c..b62fd5cd85 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -123,6 +123,8 @@ struct frame_info
/* Cached copy of the previous frame's resume address. */
struct {
enum cached_copy_status status;
+ /* Did VALUE require unmasking when being read. */
+ bool masked;
CORE_ADDR value;
} prev_pc;

@@ -161,6 +163,25 @@ struct frame_info
const char *stop_string;
};

+/* See frame.h. */
+
+void
+set_frame_previous_pc_masked (struct frame_info *frame)
+{
+ frame->prev_pc.masked = true;
+}
+
+/* See frame.h. */
+
+bool
+get_frame_pc_masked (const struct frame_info *frame)
+{
+ gdb_assert (frame->next != nullptr);
+ gdb_assert (frame->next->prev_pc.status == CC_VALUE);
+
+ return frame->next->prev_pc.masked;
+}
+
/* A frame stash used to speed up frame lookups. Create a hash table
to stash frames previously accessed from the frame cache for
quicker subsequent retrieval. The hash table is emptied whenever
@@ -429,8 +450,11 @@ fprint_frame (struct ui_file *file, struct frame_info *fi)
if (fi->next == NULL || fi->next->prev_pc.status == CC_UNKNOWN)
fprintf_unfiltered (file, "<unknown>");
else if (fi->next->prev_pc.status == CC_VALUE)
- fprintf_unfiltered (file, "%s",
- hex_string (fi->next->prev_pc.value));
+ {
+ fprintf_unfiltered (file, "%s", hex_string (fi->next->prev_pc.value));
+ if (fi->next->prev_pc.masked)
+ fprintf_unfiltered (file, "[PAC]");
+ }
else if (fi->next->prev_pc.status == CC_NOT_SAVED)
val_print_not_saved (file);
else if (fi->next->prev_pc.status == CC_UNAVAILABLE)
diff --git a/gdb/frame.h b/gdb/frame.h
index ccc285005a..fdb401d84f 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -949,4 +949,13 @@ extern const gdb::option::option_def set_backtrace_option_defs[2];
/* The values behind the global "set backtrace ..." settings. */
extern set_backtrace_options user_set_backtrace_options;

+/* Mark that the PC value is masked for the previous frame. */
+
+extern void set_frame_previous_pc_masked (struct frame_info *frame);
+
+/* Get whether the PC value is masked for the given frame. */
+
+extern bool get_frame_pc_masked (const struct frame_info *frame);
+
+
#endif /* !defined (FRAME_H) */
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 c68b3876d3..0859815baf 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]");
+ }
else
uiout->field_string ("addr", "<unavailable>",
ui_out_style_kind::ADDRESS);