[2/3] Aarch64: Detect FP regs in signal frames

Message ID 20180917125314.71795-3-alan.hayward@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Sept. 17, 2018, 12:53 p.m. UTC
  Both the VFP and SVE registers may be contained within the reserved space of
the sigcontext and can be found by seraching for MAGIC values. Detect these
and add the registers (including pseudos) to the trad frame cache.

gdb/ChangeLog:

2018-09-14  Alan Hayward  <alan.hayward@arm.com>

	* aarch64-linux-tdep.c (AARCH64_SIGCONTEXT_RESERVED_OFFSET): Add
	define.
	(AARCH64_EXTRA_MAGIC): Likewise.
	(AARCH64_FPSIMD_MAGIC): Likewise.
	(AARCH64_SVE_MAGIC): Likewise.
	(AARCH64_EXTRA_DATAP_OFFSET): Likewise.
	(AARCH64_FPSIMD_FPSR_OFFSET): Likewise.
	(AARCH64_FPSIMD_FPCR_OFFSET): Likewise.
	(AARCH64_FPSIMD_V0_OFFSET): Likewise.
	(AARCH64_FPSIMD_VREG_SIZE): Likewise.
	(AARCH64_SVE_CONTEXT_VL_OFFSET): Likewise.
	(AARCH64_SVE_CONTEXT_REGS_OFFSET): Likewise.
	(AARCH64_SVE_CONTEXT_P_REGS_OFFSET): Likewise.
	(AARCH64_SVE_CONTEXT_FFR_OFFSET): Likewise.
	(AARCH64_SVE_CONTEXT_SIZE): Likewise.
	(read_aarch64_ctx): Add function.
	(aarch64_linux_sigframe_init): Detect FP registers.
---
 gdb/aarch64-linux-tdep.c | 218 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 199 insertions(+), 19 deletions(-)
  

Comments

Simon Marchi Sept. 30, 2018, 4:36 a.m. UTC | #1
On 2018-09-17 8:53 a.m., Alan Hayward wrote:
> Both the VFP and SVE registers may be contained within the reserved space of
> the sigcontext and can be found by seraching for MAGIC values. Detect these
> and add the registers (including pseudos) to the trad frame cache.

Hi Alan,

I don't know by heart all the intricacies of aarch64/sve, so I wouldn't spot
a subtle mistake, but overall this made sense to me.  I noted a few formatting
nits.  Unless somebody with more AArche64 knowledge wants to take a look, I'd say
this is ok to go in with the nits fixed.

I was a bit surprised that it was necessary to provide the value even for pseudo registers.
I would have expected that only raw registers would be sufficient, and that
tramp_frame_prev_register would reconstruct the value of pseudo ones based on the value
of raw ones.  But there's nothing dealing with pseudo registers in there, so I guess
it's necessary to provide all of them.

> +/* Read an aarch64_ctx, returning the magic value, and setting size to the size,
> +   or return 0 on error.  */

"and setting *SIZE to the size"

> @@ -145,14 +190,20 @@ aarch64_linux_sigframe_init (const struct tramp_frame *self,
>  			     struct trad_frame_cache *this_cache,
>  			     CORE_ADDR func)
>  {
> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>    CORE_ADDR sp = get_frame_register_unsigned (this_frame, AARCH64_SP_REGNUM);
> -  CORE_ADDR sigcontext_addr =
> -    sp
> -    + AARCH64_RT_SIGFRAME_UCONTEXT_OFFSET
> -    + AARCH64_UCONTEXT_SIGCONTEXT_OFFSET;
> -  int i;
> -
> -  for (i = 0; i < 31; i++)
> +  CORE_ADDR sigcontext_addr = sp + AARCH64_RT_SIGFRAME_UCONTEXT_OFFSET
> +			      + AARCH64_UCONTEXT_SIGCONTEXT_OFFSET;

Use parentheses around an expression when breaking it:

  CORE_ADDR sigcontext_addr = (sp + AARCH64_RT_SIGFRAME_UCONTEXT_OFFSET
			       + AARCH64_UCONTEXT_SIGCONTEXT_OFFSET);

Search for "Insert extra parentheses" in:

  https://www.gnu.org/prep/standards/standards.html

There's at least one other occurrence of this, so maybe go over the patch quickly
to fix other cases.

> +	case AARCH64_EXTRA_MAGIC:
> +	  {
> +	    /* Extra is always the penultimate section in reserved and points to
> +	       an block of memory block of sections.  Reset the address to the

"an block".  Also, not sure if "a block of memory block of sections" is really what you meant,
or just "a memory block of sections"?

> +	       extra section and continue looking for more sections.  */
> +	    gdb_byte buf[8];
> +
> +	    if (target_read_memory (section + AARCH64_EXTRA_DATAP_OFFSET,
> +				    buf, 8) != 0)
> +	      {
> +		section += size;
> +		break;
> +	      }
> +
> +	    section = extract_unsigned_integer (buf, 8, byte_order);
> +	    break;
> +	  }
> +
> +	default:
> +	  break;
> +	}
> +    }
> +
> +  /* If there was an SVE section, record the location of all the FP regs.  */
> +  if (sve_regs)

sve_regs != 0

> +    {
> +      CORE_ADDR offset;
> +
> +      for (int i = 0; i < 32; i++)
> +	{
> +	  offset = sve_regs + (i * tdep->vq * 16);
> +	  trad_frame_set_reg_addr (this_cache, AARCH64_SVE_Z0_REGNUM + i,
> +				   offset);
> +	  trad_frame_set_reg_addr (this_cache,
> +				   num_regs + AARCH64_SVE_V0_REGNUM + i,
> +				   offset);
> +	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_Q0_REGNUM + i,
> +				   offset);
> +	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_D0_REGNUM + i,
> +				   offset);
> +	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_S0_REGNUM + i,
> +				   offset);
> +	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_H0_REGNUM + i,
> +				   offset);
> +	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_B0_REGNUM + i,
> +				   offset);
> +	}
> +
> +      offset = sve_regs + AARCH64_SVE_CONTEXT_P_REGS_OFFSET (tdep->vq);
> +      for (int i = 0; i < 16; i++)
> +	trad_frame_set_reg_addr (this_cache, AARCH64_SVE_P0_REGNUM + i,
> +				 offset + (i * tdep->vq * 2));
> +
> +      offset = sve_regs + AARCH64_SVE_CONTEXT_FFR_OFFSET (tdep->vq);
> +      trad_frame_set_reg_addr (this_cache, AARCH64_SVE_FFR_REGNUM, offset);
> +    }
> +
> +  /* If there was a fpsimd section, record the location of all the VFP regs.  */
> +  if (fpsimd)

fpsimd != 0

Thanks,

Simon
  
Alan Hayward Oct. 1, 2018, 2:25 p.m. UTC | #2
> On 30 Sep 2018, at 05:36, Simon Marchi <simark@simark.ca> wrote:

> 

> On 2018-09-17 8:53 a.m., Alan Hayward wrote:

>> Both the VFP and SVE registers may be contained within the reserved space of

>> the sigcontext and can be found by seraching for MAGIC values. Detect these

>> and add the registers (including pseudos) to the trad frame cache.

> 

> Hi Alan,

> 

> I don't know by heart all the intricacies of aarch64/sve, so I wouldn't spot

> a subtle mistake, but overall this made sense to me.  I noted a few formatting

> nits.  Unless somebody with more AArche64 knowledge wants to take a look, I'd say

> this is ok to go in with the nits fixed.

> 

> I was a bit surprised that it was necessary to provide the value even for pseudo registers.

> I would have expected that only raw registers would be sufficient, and that

> tramp_frame_prev_register would reconstruct the value of pseudo ones based on the value

> of raw ones.  But there's nothing dealing with pseudo registers in there, so I guess

> it's necessary to provide all of them.


Yes, this confused me for a while too.
I guess the ultimate solution would be to fix trap frames to understand pseudo registers.
Not sure how tricky that would be.

> 

> 

>> +	case AARCH64_EXTRA_MAGIC:

>> +	  {

>> +	    /* Extra is always the penultimate section in reserved and points to

>> +	       an block of memory block of sections.  Reset the address to the

> 

> "an block".  Also, not sure if "a block of memory block of sections" is really what you meant,

> or just "a memory block of sections”?


Updated this to
	    /* Extra is always the last valid section in reserved and points to
	       an additional block of memory filled with more sections. Reset
	       the address to the extra section and continue looking for more
	       structures.  */

And pushed with the additional nits fixed too.

Thanks for the review!

Alan.
  

Patch

diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index dc2b89121a..3d60fa4040 100644
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -85,11 +85,6 @@ 
     struct ucontext uc;
   };
 
-  typedef struct
-  {
-    ...                                    128 bytes
-  } siginfo_t;
-
   The ucontext has the following form:
   struct ucontext
   {
@@ -100,13 +95,6 @@ 
     struct sigcontext uc_mcontext;
   };
 
-  typedef struct sigaltstack
-  {
-    void *ss_sp;
-    int ss_flags;
-    size_t ss_size;
-  } stack_t;
-
   struct sigcontext
   {
     unsigned long fault_address;
@@ -117,6 +105,17 @@ 
     __u8 __reserved[4096]
   };
 
+  The reserved space in sigcontext contains additional structures, each starting
+  with a aarch64_ctx, which specifies a unique identifier and the total size of
+  the structure.  The final structure in reserved will start will a null
+  aarch64_ctx.  The penultimate entry in reserved may be a extra_context which
+  then points to a further block of reserved space.
+
+  struct aarch64_ctx {
+	u32 magic;
+	u32 size;
+  };
+
   The restorer stub will always have the form:
 
   d28015a8        movz    x8, #0xad
@@ -136,6 +135,52 @@ 
 #define AARCH64_RT_SIGFRAME_UCONTEXT_OFFSET     128
 #define AARCH64_UCONTEXT_SIGCONTEXT_OFFSET      176
 #define AARCH64_SIGCONTEXT_XO_OFFSET            8
+#define AARCH64_SIGCONTEXT_RESERVED_OFFSET      288
+
+/* Unique identifiers that may be used for aarch64_ctx.magic.  */
+#define AARCH64_EXTRA_MAGIC			0x45585401
+#define AARCH64_FPSIMD_MAGIC			0x46508001
+#define AARCH64_SVE_MAGIC			0x53564501
+
+/* Defines for the extra_context that follows an AARCH64_EXTRA_MAGIC.  */
+#define AARCH64_EXTRA_DATAP_OFFSET		8
+
+/* Defines for the fpsimd that follows an AARCH64_FPSIMD_MAGIC.  */
+#define AARCH64_FPSIMD_FPSR_OFFSET		8
+#define AARCH64_FPSIMD_FPCR_OFFSET		12
+#define AARCH64_FPSIMD_V0_OFFSET		16
+#define AARCH64_FPSIMD_VREG_SIZE		16
+
+/* Defines for the sve structure that follows an AARCH64_SVE_MAGIC.  */
+#define AARCH64_SVE_CONTEXT_VL_OFFSET		8
+#define AARCH64_SVE_CONTEXT_REGS_OFFSET		16
+#define AARCH64_SVE_CONTEXT_P_REGS_OFFSET(vq) (32 * vq * 16)
+#define AARCH64_SVE_CONTEXT_FFR_OFFSET(vq) \
+  (AARCH64_SVE_CONTEXT_P_REGS_OFFSET (vq) + (16 * vq * 2))
+#define AARCH64_SVE_CONTEXT_SIZE(vq) \
+  (AARCH64_SVE_CONTEXT_FFR_OFFSET (vq) + (vq * 2))
+
+
+/* Read an aarch64_ctx, returning the magic value, and setting size to the size,
+   or return 0 on error.  */
+
+static uint32_t
+read_aarch64_ctx (CORE_ADDR ctx_addr, enum bfd_endian byte_order,
+		  uint32_t *size)
+{
+  uint32_t magic = 0;
+  gdb_byte buf[4];
+
+  if (target_read_memory (ctx_addr, buf, 4) != 0)
+    return 0;
+  magic = extract_unsigned_integer (buf, 4, byte_order);
+
+  if (target_read_memory (ctx_addr + 4, buf, 4) != 0)
+    return 0;
+  *size = extract_unsigned_integer (buf, 4, byte_order);
+
+  return magic;
+}
 
 /* Implement the "init" method of struct tramp_frame.  */
 
@@ -145,14 +190,20 @@  aarch64_linux_sigframe_init (const struct tramp_frame *self,
 			     struct trad_frame_cache *this_cache,
 			     CORE_ADDR func)
 {
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   CORE_ADDR sp = get_frame_register_unsigned (this_frame, AARCH64_SP_REGNUM);
-  CORE_ADDR sigcontext_addr =
-    sp
-    + AARCH64_RT_SIGFRAME_UCONTEXT_OFFSET
-    + AARCH64_UCONTEXT_SIGCONTEXT_OFFSET;
-  int i;
-
-  for (i = 0; i < 31; i++)
+  CORE_ADDR sigcontext_addr = sp + AARCH64_RT_SIGFRAME_UCONTEXT_OFFSET
+			      + AARCH64_UCONTEXT_SIGCONTEXT_OFFSET;
+  CORE_ADDR section = sigcontext_addr + AARCH64_SIGCONTEXT_RESERVED_OFFSET;
+  CORE_ADDR fpsimd = 0;
+  CORE_ADDR sve_regs = 0;
+  uint32_t size, magic;
+  int num_regs = gdbarch_num_regs (gdbarch);
+
+  /* Read in the integer registers.  */
+  for (int i = 0; i < 31; i++)
     {
       trad_frame_set_reg_addr (this_cache,
 			       AARCH64_X0_REGNUM + i,
@@ -166,6 +217,135 @@  aarch64_linux_sigframe_init (const struct tramp_frame *self,
 			   sigcontext_addr + AARCH64_SIGCONTEXT_XO_OFFSET
 			     + 32 * AARCH64_SIGCONTEXT_REG_SIZE);
 
+  /* Find the FP and SVE sections.  */
+  while ((magic = read_aarch64_ctx (section, byte_order, &size)) != 0)
+    {
+      switch (magic)
+	{
+	case AARCH64_FPSIMD_MAGIC:
+	  fpsimd = section;
+	  section += size;
+	  break;
+
+	case AARCH64_SVE_MAGIC:
+	  {
+	    /* Check if the section is followed by a full SVE dump, and set
+	       sve_regs if it is.  */
+	    gdb_byte buf[4];
+	    uint16_t vq;
+
+	    if (!tdep->has_sve ())
+	      break;
+
+	    if (target_read_memory (section + AARCH64_SVE_CONTEXT_VL_OFFSET,
+				    buf, 2) != 0)
+	      {
+		section += size;
+		break;
+	      }
+	    vq = sve_vq_from_vl (extract_unsigned_integer (buf, 2, byte_order));
+
+	    if (vq != tdep->vq)
+	      error (_("Invalid vector length in signal frame %d vs %ld."), vq,
+		     tdep->vq);
+
+	    if (size >= AARCH64_SVE_CONTEXT_SIZE (vq))
+	      sve_regs = section + AARCH64_SVE_CONTEXT_REGS_OFFSET;
+
+	    section += size;
+	    break;
+	  }
+
+	case AARCH64_EXTRA_MAGIC:
+	  {
+	    /* Extra is always the penultimate section in reserved and points to
+	       an block of memory block of sections.  Reset the address to the
+	       extra section and continue looking for more sections.  */
+	    gdb_byte buf[8];
+
+	    if (target_read_memory (section + AARCH64_EXTRA_DATAP_OFFSET,
+				    buf, 8) != 0)
+	      {
+		section += size;
+		break;
+	      }
+
+	    section = extract_unsigned_integer (buf, 8, byte_order);
+	    break;
+	  }
+
+	default:
+	  break;
+	}
+    }
+
+  /* If there was an SVE section, record the location of all the FP regs.  */
+  if (sve_regs)
+    {
+      CORE_ADDR offset;
+
+      for (int i = 0; i < 32; i++)
+	{
+	  offset = sve_regs + (i * tdep->vq * 16);
+	  trad_frame_set_reg_addr (this_cache, AARCH64_SVE_Z0_REGNUM + i,
+				   offset);
+	  trad_frame_set_reg_addr (this_cache,
+				   num_regs + AARCH64_SVE_V0_REGNUM + i,
+				   offset);
+	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_Q0_REGNUM + i,
+				   offset);
+	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_D0_REGNUM + i,
+				   offset);
+	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_S0_REGNUM + i,
+				   offset);
+	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_H0_REGNUM + i,
+				   offset);
+	  trad_frame_set_reg_addr (this_cache, num_regs + AARCH64_B0_REGNUM + i,
+				   offset);
+	}
+
+      offset = sve_regs + AARCH64_SVE_CONTEXT_P_REGS_OFFSET (tdep->vq);
+      for (int i = 0; i < 16; i++)
+	trad_frame_set_reg_addr (this_cache, AARCH64_SVE_P0_REGNUM + i,
+				 offset + (i * tdep->vq * 2));
+
+      offset = sve_regs + AARCH64_SVE_CONTEXT_FFR_OFFSET (tdep->vq);
+      trad_frame_set_reg_addr (this_cache, AARCH64_SVE_FFR_REGNUM, offset);
+    }
+
+  /* If there was a fpsimd section, record the location of all the VFP regs.  */
+  if (fpsimd)
+    {
+      trad_frame_set_reg_addr (this_cache, AARCH64_FPSR_REGNUM,
+			       fpsimd + AARCH64_FPSIMD_FPSR_OFFSET);
+      trad_frame_set_reg_addr (this_cache, AARCH64_FPCR_REGNUM,
+			       fpsimd + AARCH64_FPSIMD_FPCR_OFFSET);
+
+      /* If there was no SVE section then set up the V registers.  */
+      if (sve_regs == 0)
+	for (int i = 0; i < 32; i++)
+	  {
+	    CORE_ADDR offset = fpsimd + AARCH64_FPSIMD_V0_OFFSET
+				 + (i * AARCH64_FPSIMD_VREG_SIZE);
+
+	    trad_frame_set_reg_addr (this_cache, AARCH64_V0_REGNUM + i, offset);
+	    trad_frame_set_reg_addr (this_cache,
+				     num_regs + AARCH64_Q0_REGNUM + i, offset);
+	    trad_frame_set_reg_addr (this_cache,
+				     num_regs + AARCH64_D0_REGNUM + i, offset);
+	    trad_frame_set_reg_addr (this_cache,
+				     num_regs + AARCH64_S0_REGNUM + i, offset);
+	    trad_frame_set_reg_addr (this_cache,
+				     num_regs + AARCH64_H0_REGNUM + i, offset);
+	    trad_frame_set_reg_addr (this_cache,
+				     num_regs + AARCH64_B0_REGNUM + i, offset);
+	    if (tdep->has_sve ())
+	      trad_frame_set_reg_addr (this_cache,
+				       num_regs + AARCH64_SVE_V0_REGNUM + i,
+				       offset);
+	  }
+    }
+
   trad_frame_set_id (this_cache, frame_id_build (sp, func));
 }