Patchwork [V2] AArch64 pauth: Indicate unmasked addresses in backtrace

login
register
mail settings
Submitter Alan Hayward
Date Aug. 7, 2019, 12:35 p.m.
Message ID <C7883044-EBB8-462D-963C-6741AC186444@arm.com>
Download mbox | patch
Permalink /patch/33988/
State New
Headers show

Comments

Alan Hayward - Aug. 7, 2019, 12:35 p.m.
> 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.

Patch

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);