* [PATCH 1/3] Aarch64: Move pseudo defines to header
2018-09-17 12:53 [PATCH 0/3] Aarch64: Detect FP registers in signal frames Alan Hayward
@ 2018-09-17 12:53 ` Alan Hayward
2018-09-17 12:53 ` [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test Alan Hayward
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Alan Hayward @ 2018-09-17 12:53 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
gdb/ChangeLog:
2018-09-14 Alan Hayward <alan.hayward@arm.com>
* aarch64-tdep.c (AARCH64_Q0_REGNUM): Move to here.
(AARCH64_D0_REGNUM): Likewise.
(AARCH64_S0_REGNUM): Likewise.
(AARCH64_H0_REGNUM): Likewise.
(AARCH64_B0_REGNUM): Likewise.
(AARCH64_SVE_V0_REGNUM): Likewise.
* arch/aarch64.h (AARCH64_Q0_REGNUM): Move from here.
(AARCH64_D0_REGNUM): Likewise.
(AARCH64_S0_REGNUM): Likewise.
(AARCH64_H0_REGNUM): Likewise.
(AARCH64_B0_REGNUM): Likewise.
(AARCH64_SVE_V0_REGNUM): Likewise.
---
gdb/aarch64-tdep.c | 8 --------
gdb/arch/aarch64.h | 8 ++++++++
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index d2e6ac64d5..6993e9061e 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -63,14 +63,6 @@
#define bit(obj,st) (((obj) >> (st)) & 1)
#define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st)))
-/* Pseudo register base numbers. */
-#define AARCH64_Q0_REGNUM 0
-#define AARCH64_D0_REGNUM (AARCH64_Q0_REGNUM + AARCH64_D_REGISTER_COUNT)
-#define AARCH64_S0_REGNUM (AARCH64_D0_REGNUM + 32)
-#define AARCH64_H0_REGNUM (AARCH64_S0_REGNUM + 32)
-#define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32)
-#define AARCH64_SVE_V0_REGNUM (AARCH64_B0_REGNUM + 32)
-
/* A Homogeneous Floating-Point or Short-Vector Aggregate may have at most
four members. */
#define HA_MAX_NUM_FLDS 4
diff --git a/gdb/arch/aarch64.h b/gdb/arch/aarch64.h
index d6b88e6d56..ff9186007b 100644
--- a/gdb/arch/aarch64.h
+++ b/gdb/arch/aarch64.h
@@ -57,6 +57,14 @@ enum aarch64_regnum
AARCH64_LAST_V_ARG_REGNUM = AARCH64_V0_REGNUM + 7
};
+/* Pseudo register base numbers. */
+#define AARCH64_Q0_REGNUM 0
+#define AARCH64_D0_REGNUM (AARCH64_Q0_REGNUM + AARCH64_D_REGISTER_COUNT)
+#define AARCH64_S0_REGNUM (AARCH64_D0_REGNUM + 32)
+#define AARCH64_H0_REGNUM (AARCH64_S0_REGNUM + 32)
+#define AARCH64_B0_REGNUM (AARCH64_H0_REGNUM + 32)
+#define AARCH64_SVE_V0_REGNUM (AARCH64_B0_REGNUM + 32)
+
#define AARCH64_X_REGS_NUM 31
#define AARCH64_V_REGS_NUM 32
#define AARCH64_SVE_Z_REGS_NUM AARCH64_V_REGS_NUM
--
2.15.2 (Apple Git-101.1)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/3] Aarch64: Detect FP registers in signal frames
@ 2018-09-17 12:53 Alan Hayward
2018-09-17 12:53 ` [PATCH 1/3] Aarch64: Move pseudo defines to header Alan Hayward
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Alan Hayward @ 2018-09-17 12:53 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
I was looking at adding code to detect SVE registers for call frames
whilst in a signal handler, and noticed that there was no code to detect
the NEON vfp registers. This set of patches fixes up the code for both
aarch64 and aarch64 with SVE.
Patch 1 simply moves some defines.
Patch 2 is the main work, iterating through the reserved space of the
sigcontext.
Patch 3 adds an aarch64 specific test.
Alan Hayward (3):
Aarch64: Move pseudo defines to header
Aarch64: Detect FP regs in signal frame
Testsuite: Aarch64: Add signal handler registers test
gdb/aarch64-linux-tdep.c | 218 +++++++++++++++++++--
gdb/aarch64-tdep.c | 8 -
gdb/arch/aarch64.h | 8 +
gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c | 170 ++++++++++++++++
gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp | 150 ++++++++++++++
gdb/testsuite/lib/gdb.exp | 51 +++++
6 files changed, 578 insertions(+), 27 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c
create mode 100644 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp
--
2.15.2 (Apple Git-101.1)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] Aarch64: Detect FP regs in signal frames
2018-09-17 12:53 [PATCH 0/3] Aarch64: Detect FP registers in signal frames Alan Hayward
2018-09-17 12:53 ` [PATCH 1/3] Aarch64: Move pseudo defines to header Alan Hayward
2018-09-17 12:53 ` [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test Alan Hayward
@ 2018-09-17 12:53 ` Alan Hayward
2018-09-30 4:37 ` Simon Marchi
2018-09-25 11:54 ` [PING] [PATCH 0/3] Aarch64: Detect FP registers " Alan Hayward
3 siblings, 1 reply; 9+ messages in thread
From: Alan Hayward @ 2018-09-17 12:53 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
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(-)
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));
}
--
2.15.2 (Apple Git-101.1)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test
2018-09-17 12:53 [PATCH 0/3] Aarch64: Detect FP registers in signal frames Alan Hayward
2018-09-17 12:53 ` [PATCH 1/3] Aarch64: Move pseudo defines to header Alan Hayward
@ 2018-09-17 12:53 ` Alan Hayward
2018-09-30 5:01 ` Simon Marchi
2018-09-17 12:53 ` [PATCH 2/3] Aarch64: Detect FP regs in signal frames Alan Hayward
2018-09-25 11:54 ` [PING] [PATCH 0/3] Aarch64: Detect FP registers " Alan Hayward
3 siblings, 1 reply; 9+ messages in thread
From: Alan Hayward @ 2018-09-17 12:53 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, Alan Hayward
Add function to detect if aarch64 SVE is available.
Add Aarch64 test to check register values of a previous frame
can be shown correctly across a signal.
gdb/testsuite/ChangeLog:
2018-09-14 Alan Hayward <alan.hayward@arm.com>
* gdb.arch/aarch64-sighandler-regs.c: New test.
* gdb.arch/aarch64-sighandler-regs.exp: New file.
* lib/gdb.exp (skip_aarch64_sve_tests): New proc.
---
gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c | 170 +++++++++++++++++++++
gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp | 150 ++++++++++++++++++
gdb/testsuite/lib/gdb.exp | 51 +++++++
3 files changed, 371 insertions(+)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c
create mode 100644 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp
diff --git a/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c b/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c
new file mode 100644
index 0000000000..5f3674ae7b
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c
@@ -0,0 +1,170 @@
+#include <signal.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#define OVERWRITE_GP_REGS \
+ "ldr x1, [x0]\n\t" \
+ "ldr x2, [x0]\n\t" \
+ "ldr x3, [x0]\n\t" \
+ "ldr x4, [x0]\n\t" \
+ "ldr x5, [x0]\n\t" \
+ "ldr x6, [x0]\n\t" \
+ "ldr x7, [x0]\n\t" \
+ "ldr x8, [x0]\n\t" \
+ "ldr x9, [x0]\n\t" \
+ "ldr x10, [x0]\n\t" \
+ "ldr x11, [x0]\n\t" \
+ "ldr x12, [x0]\n\t" \
+ "ldr x13, [x0]\n\t" \
+ "ldr x14, [x0]\n\t" \
+ "ldr x15, [x0]\n\t" \
+ "ldr x16, [x0]\n\t" \
+ "ldr x17, [x0]\n\t" \
+ "ldr x18, [x0]\n\t" \
+ "ldr x19, [x0]\n\t" \
+ "ldr x20, [x0]\n\t" \
+ "ldr x21, [x0]\n\t" \
+ "ldr x22, [x0]\n\t" \
+ "ldr x23, [x0]\n\t" \
+ "ldr x24, [x0]\n\t" \
+ "ldr x25, [x0]\n\t" \
+ "ldr x26, [x0]\n\t" \
+ "ldr x27, [x0]\n\t" \
+ "ldr x28, [x0]\n\t"
+
+#ifdef SVE
+#define OVERWRITE_FP_REGS \
+ "ptrue p3.s\n\t" \
+ "ld1w z0.s, p3/z, [x0]\n\t" \
+ "ld1w z1.s, p3/z, [x0]\n\t" \
+ "ld1w z2.s, p3/z, [x0]\n\t" \
+ "ld1w z3.s, p3/z, [x0]\n\t" \
+ "ld1w z4.s, p3/z, [x0]\n\t" \
+ "ld1w z5.s, p3/z, [x0]\n\t" \
+ "ld1w z6.s, p3/z, [x0]\n\t" \
+ "ld1w z7.s, p3/z, [x0]\n\t" \
+ "ld1w z8.s, p3/z, [x0]\n\t" \
+ "ld1w z9.s, p3/z, [x0]\n\t" \
+ "ld1w z10.s, p3/z, [x0]\n\t" \
+ "ld1w z11.s, p3/z, [x0]\n\t" \
+ "ld1w z12.s, p3/z, [x0]\n\t" \
+ "ld1w z13.s, p3/z, [x0]\n\t" \
+ "ld1w z14.s, p3/z, [x0]\n\t" \
+ "ld1w z15.s, p3/z, [x0]\n\t" \
+ "ld1w z16.s, p3/z, [x0]\n\t" \
+ "ld1w z17.s, p3/z, [x0]\n\t" \
+ "ld1w z18.s, p3/z, [x0]\n\t" \
+ "ld1w z19.s, p3/z, [x0]\n\t" \
+ "ld1w z20.s, p3/z, [x0]\n\t" \
+ "ld1w z21.s, p3/z, [x0]\n\t" \
+ "ld1w z22.s, p3/z, [x0]\n\t" \
+ "ld1w z23.s, p3/z, [x0]\n\t" \
+ "ld1w z24.s, p3/z, [x0]\n\t" \
+ "ld1w z25.s, p3/z, [x0]\n\t" \
+ "ld1w z26.s, p3/z, [x0]\n\t" \
+ "ld1w z27.s, p3/z, [x0]\n\t" \
+ "ld1w z28.s, p3/z, [x0]\n\t" \
+ "ld1w z29.s, p3/z, [x0]\n\t" \
+ "ld1w z30.s, p3/z, [x0]\n\t" \
+ "ld1w z31.s, p3/z, [x0]\n\t"
+#else
+#define OVERWRITE_FP_REGS \
+ "ldr q0, [x0]\n\t" \
+ "ldr q1, [x0]\n\t" \
+ "ldr q2, [x0]\n\t" \
+ "ldr q3, [x0]\n\t" \
+ "ldr q4, [x0]\n\t" \
+ "ldr q5, [x0]\n\t" \
+ "ldr q6, [x0]\n\t" \
+ "ldr q7, [x0]\n\t" \
+ "ldr q8, [x0]\n\t" \
+ "ldr q9, [x0]\n\t" \
+ "ldr q10, [x0]\n\t" \
+ "ldr q11, [x0]\n\t" \
+ "ldr q12, [x0]\n\t" \
+ "ldr q13, [x0]\n\t" \
+ "ldr q14, [x0]\n\t" \
+ "ldr q15, [x0]\n\t" \
+ "ldr q16, [x0]\n\t" \
+ "ldr q17, [x0]\n\t" \
+ "ldr q18, [x0]\n\t" \
+ "ldr q19, [x0]\n\t" \
+ "ldr q20, [x0]\n\t" \
+ "ldr q21, [x0]\n\t" \
+ "ldr q22, [x0]\n\t" \
+ "ldr q23, [x0]\n\t" \
+ "ldr q24, [x0]\n\t" \
+ "ldr q25, [x0]\n\t" \
+ "ldr q26, [x0]\n\t" \
+ "ldr q27, [x0]\n\t" \
+ "ldr q28, [x0]\n\t" \
+ "ldr q29, [x0]\n\t" \
+ "ldr q30, [x0]\n\t" \
+ "ldr q31, [x0]\n\t"
+#endif
+
+#ifdef SVE
+#define OVERWRITE_P_REGS(pattern) \
+ "ptrue p0.s, " #pattern "\n\t" \
+ "ptrue p1.s, " #pattern "\n\t" \
+ "ptrue p2.s, " #pattern "\n\t" \
+ "ptrue p3.s, " #pattern "\n\t" \
+ "ptrue p4.s, " #pattern "\n\t" \
+ "ptrue p5.s, " #pattern "\n\t" \
+ "ptrue p6.s, " #pattern "\n\t" \
+ "ptrue p7.s, " #pattern "\n\t" \
+ "ptrue p8.s, " #pattern "\n\t" \
+ "ptrue p9.s, " #pattern "\n\t" \
+ "ptrue p10.s, " #pattern "\n\t" \
+ "ptrue p11.s, " #pattern "\n\t" \
+ "ptrue p12.s, " #pattern "\n\t" \
+ "ptrue p13.s, " #pattern "\n\t" \
+ "ptrue p14.s, " #pattern "\n\t" \
+ "ptrue p15.s, " #pattern "\n\t"
+#else
+#define OVERWRITE_P_REGS(pattern)
+#endif
+
+
+void
+handler (int sig)
+{
+ char buf_handler[] = {0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57,
+ 0x58, 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f};
+
+ __asm __volatile ("mov x0, %0\n\t" \
+ OVERWRITE_GP_REGS \
+ OVERWRITE_FP_REGS \
+ OVERWRITE_P_REGS (MUL3) \
+ : : "r" (buf_handler));
+
+ exit (0);
+}
+
+
+
+int
+main ()
+{
+ /* Ensure all the signals aren't blocked. */
+ sigset_t newset;
+ sigemptyset (&newset);
+ sigprocmask (SIG_SETMASK, &newset, NULL);
+
+ signal (SIGILL, handler);
+
+ char buf_main[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+ 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f};
+
+ /* 0x06000000 : Cause an illegal instruction. Value undefined as per ARM
+ Architecture Reference Manual ARMv8, Section C4.1. */
+
+ __asm __volatile ("mov x0, %0\n\t" \
+ OVERWRITE_GP_REGS \
+ OVERWRITE_FP_REGS \
+ OVERWRITE_P_REGS (VL1) \
+ ".inst 0x06000000"
+ : : "r" (buf_main));
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp b/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp
new file mode 100644
index 0000000000..3fe6a28929
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp
@@ -0,0 +1,150 @@
+# Copyright 2008-2018 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+# This file is part of the gdb testsuite.
+
+
+if {![is_aarch64_target]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return -1
+}
+
+set compile_flags {debug}
+
+if { [skip_aarch64_sve_tests] } {
+ unsupported "target does not support SVE"
+ set sve_hw 0
+} else {
+ set sve_hw 1
+ lappend compile_flags "additional_flags=-DSVE"
+}
+
+standard_testfile
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+set endianness [get_endianness]
+
+if {$endianness == "little"} {
+ set reg_handler_value_128 "0x5f5e5d5c5b5a59585756555453525150"
+ set reg_handler_value_64 "0x5756555453525150"
+ set reg_handler_value_32 "0x53525150"
+ set reg_handler_value_16 "0x5150"
+ set reg_handler_value_8 "0x50"
+ set reg_main_value_128 "0x1f1e1d1c1b1a19181716151413121110"
+ set reg_main_value_64 "0x1716151413121110"
+ set reg_main_value_32 "0x13121110"
+ set reg_main_value_16 "0x1110"
+ set reg_main_value_8 "0x10"
+} else {
+ set reg_handler_value_128 "0x505152535455565758595a5b5c5d5e5f"
+ set reg_handler_value_64 "0x5051525354555657"
+ set reg_handler_value_32 "0x50515253"
+ set reg_handler_value_16 "0x5051"
+ set reg_handler_value_8 "0x50"
+ set reg_main_value_128 "0x101112131415161718191a1b1c1d1e1f"
+ set reg_main_value_64 "0x1011121314151617"
+ set reg_main_value_32 "0x10111213"
+ set reg_main_value_16 "0x1011"
+ set reg_main_value_8 "0x10"
+}
+set zreg_handler_value "\\{0x5756555453525150, .*"
+set zreg_main_value "\\{0x1716151413121110, .*"
+set preg_handler_value "\\{0x11, .*"
+set preg_main_value "\\{0x1, 0x0, .*"
+
+#Ignore x0, and x29 to x31
+set xreg_nums [list 1 2 3 4 5 6 7 8 9 11 12 13 14 15 16 17 18 19 21 22 23 24 \
+ 25 26 27 28 ]
+set vreg_nums [list 0 1 2 3 4 5 6 7 8 9 11 12 13 14 15 16 17 18 19 21 22 23 \
+ 24 25 26 27 28 29 30 31]
+set preg_nums [list 0 1 2 3 4 5 6 7 8 9 11 12 13 14 15]
+
+proc check_regs {regtype regnums value postfix} {
+ foreach regnum $regnums {
+ gdb_test "print /x \$$regtype$regnum$postfix" \
+ ".* = {?$value}?" \
+ "check register \$$regtype$regnum has value $value"
+ }
+}
+
+# Run until end of signal handler
+
+gdb_test "continue" \
+ "Continuing.*Program received signal SIGILL.*" \
+ "continue until signal"
+
+gdb_breakpoint [gdb_get_line_number "exit (0)"]
+gdb_continue_to_breakpoint "exit" ".*exit.*"
+
+set handlerframe [get_current_frame_number]
+set mainframe [expr $handlerframe + 2]
+
+
+# Check register values
+
+check_regs x $xreg_nums $reg_handler_value_64 ""
+check_regs v $vreg_nums $reg_handler_value_128 ".q.u"
+check_regs q $vreg_nums $reg_handler_value_128 ".u"
+check_regs d $vreg_nums $reg_handler_value_64 ".u"
+check_regs s $vreg_nums $reg_handler_value_32 ".u"
+check_regs h $vreg_nums $reg_handler_value_16 ".u"
+check_regs b $vreg_nums $reg_handler_value_8 ".u"
+if { $sve_hw } {
+ check_regs z $vreg_nums $zreg_handler_value ".d.u"
+ check_regs p $preg_nums $preg_handler_value ""
+}
+
+# Switch to the frame for main(), and check register values
+
+gdb_test "frame $mainframe" \
+ "#$mainframe.*in main ().*" \
+ "Set to main frame"
+
+check_regs x $xreg_nums $reg_main_value_64 ""
+check_regs v $vreg_nums $reg_main_value_128 ".q.u"
+check_regs q $vreg_nums $reg_main_value_128 ".u"
+check_regs d $vreg_nums $reg_main_value_64 ".u"
+check_regs s $vreg_nums $reg_main_value_32 ".u"
+check_regs h $vreg_nums $reg_main_value_16 ".u"
+check_regs b $vreg_nums $reg_main_value_8 ".u"
+if { $sve_hw } {
+ check_regs z $vreg_nums $zreg_main_value ".d.u"
+ check_regs p $preg_nums $preg_main_value ""
+}
+
+# Switch back to the signal handler frame, and check register values
+
+gdb_test "frame $handlerframe" \
+ "#$handlerframe.*handler \\\(sig=4\\\).*" \
+ "Set to signal handler frame"
+
+check_regs x $xreg_nums $reg_handler_value_64 ""
+check_regs v $vreg_nums $reg_handler_value_128 ".q.u"
+check_regs q $vreg_nums $reg_handler_value_128 ".u"
+check_regs d $vreg_nums $reg_handler_value_64 ".u"
+check_regs s $vreg_nums $reg_handler_value_32 ".u"
+check_regs h $vreg_nums $reg_handler_value_16 ".u"
+check_regs b $vreg_nums $reg_handler_value_8 ".u"
+if { $sve_hw } {
+ check_regs z $vreg_nums $zreg_handler_value ".d.u"
+ check_regs p $preg_nums $preg_handler_value ""
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f32abfedd5..524cf623b2 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2824,6 +2824,57 @@ gdb_caching_proc skip_btrace_pt_tests {
return $skip_btrace_tests
}
+gdb_caching_proc skip_aarch64_sve_tests {
+ global srcdir subdir gdb_prompt inferior_exited_re
+
+ set me "skip_aarch64_sve_tests"
+
+ if { ![is_aarch64_target]} {
+ return 0
+ }
+
+ set compile_flags "{additional_flags=-march=armv8-a+sve}"
+
+ # Compile a test program containing SVE instructions.
+ set src {
+ int main() {
+ asm volatile ("ptrue p0.b");
+ return 0;
+ }
+ }
+ if {![gdb_simple_compile $me $src executable $compile_flags]} {
+ return 1
+ }
+
+ # Compilation succeeded so now run it via gdb.
+
+ gdb_exit
+ gdb_start
+ gdb_reinitialize_dir $srcdir/$subdir
+ gdb_load "$obj"
+ gdb_run_cmd
+ gdb_expect {
+ -re ".*Illegal instruction.*${gdb_prompt} $" {
+ verbose -log "\n$me sve hardware not detected"
+ set skip_sve_tests 1
+ }
+ -re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
+ verbose -log "\n$me: sve hardware detected"
+ set skip_sve_tests 0
+ }
+ default {
+ warning "\n$me: default case taken"
+ set skip_sve_tests 1
+ }
+ }
+ gdb_exit
+ remote_file build delete $obj
+
+ verbose "$me: returning $skip_sve_tests" 2
+ return $skip_sve_tests
+}
+
+
# A helper that compiles a test case to see if __int128 is supported.
proc gdb_int128_helper {lang} {
return [gdb_can_simple_compile "i128-for-$lang" {
--
2.15.2 (Apple Git-101.1)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PING] [PATCH 0/3] Aarch64: Detect FP registers in signal frames
2018-09-17 12:53 [PATCH 0/3] Aarch64: Detect FP registers in signal frames Alan Hayward
` (2 preceding siblings ...)
2018-09-17 12:53 ` [PATCH 2/3] Aarch64: Detect FP regs in signal frames Alan Hayward
@ 2018-09-25 11:54 ` Alan Hayward
3 siblings, 0 replies; 9+ messages in thread
From: Alan Hayward @ 2018-09-25 11:54 UTC (permalink / raw)
To: GDB Patches; +Cc: nd
Ping!
> On 17 Sep 2018, at 13:53, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> I was looking at adding code to detect SVE registers for call frames
> whilst in a signal handler, and noticed that there was no code to detect
> the NEON vfp registers. This set of patches fixes up the code for both
> aarch64 and aarch64 with SVE.
>
> Patch 1 simply moves some defines.
> Patch 2 is the main work, iterating through the reserved space of the
> sigcontext.
> Patch 3 adds an aarch64 specific test.
>
> Alan Hayward (3):
> Aarch64: Move pseudo defines to header
> Aarch64: Detect FP regs in signal frame
> Testsuite: Aarch64: Add signal handler registers test
>
> gdb/aarch64-linux-tdep.c | 218 +++++++++++++++++++--
> gdb/aarch64-tdep.c | 8 -
> gdb/arch/aarch64.h | 8 +
> gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c | 170 ++++++++++++++++
> gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp | 150 ++++++++++++++
> gdb/testsuite/lib/gdb.exp | 51 +++++
> 6 files changed, 578 insertions(+), 27 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.c
> create mode 100644 gdb/testsuite/gdb.arch/aarch64-sighandler-regs.exp
>
> --
> 2.15.2 (Apple Git-101.1)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] Aarch64: Detect FP regs in signal frames
2018-09-17 12:53 ` [PATCH 2/3] Aarch64: Detect FP regs in signal frames Alan Hayward
@ 2018-09-30 4:37 ` Simon Marchi
2018-10-01 14:26 ` Alan Hayward
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2018-09-30 4:37 UTC (permalink / raw)
To: Alan Hayward, gdb-patches; +Cc: nd
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test
2018-09-17 12:53 ` [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test Alan Hayward
@ 2018-09-30 5:01 ` Simon Marchi
2018-10-01 14:26 ` Alan Hayward
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2018-09-30 5:01 UTC (permalink / raw)
To: Alan Hayward, gdb-patches; +Cc: nd
On 2018-09-17 8:53 a.m., Alan Hayward wrote:
> Add function to detect if aarch64 SVE is available.
>
> Add Aarch64 test to check register values of a previous frame
> can be shown correctly across a signal.
Just some nits here and there:
- The .c file should have a copyright header.
- Make sure test names start with a lowercase letter.
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index f32abfedd5..524cf623b2 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -2824,6 +2824,57 @@ gdb_caching_proc skip_btrace_pt_tests {
> return $skip_btrace_tests
> }
>
> +gdb_caching_proc skip_aarch64_sve_tests {
Can you add a short description of this proc, especially about the side-effects
and the return value?
> + global srcdir subdir gdb_prompt inferior_exited_re
> +
> + set me "skip_aarch64_sve_tests"
> +
> + if { ![is_aarch64_target]} {
> + return 0
> + }
IIUC, you return 1 if SVE tests should be skipped. If the target is not aarch64,
shouldn't we skip the SVE tests?
> +
> + set compile_flags "{additional_flags=-march=armv8-a+sve}"
> +
> + # Compile a test program containing SVE instructions.
> + set src {
> + int main() {
> + asm volatile ("ptrue p0.b");
> + return 0;
> + }
> + }
> + if {![gdb_simple_compile $me $src executable $compile_flags]} {
> + return 1
> + }
> +
> + # Compilation succeeded so now run it via gdb.
> +
> + gdb_exit
> + gdb_start
> + gdb_reinitialize_dir $srcdir/$subdir
> + gdb_load "$obj"
This sequence can probably be replaced with clean_restart.
> + gdb_run_cmd
> + gdb_expect {
> + -re ".*Illegal instruction.*${gdb_prompt} $" {
> + verbose -log "\n$me sve hardware not detected"
> + set skip_sve_tests 1
> + }
> + -re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
> + verbose -log "\n$me: sve hardware detected"
> + set skip_sve_tests 0
> + }
> + default {
> + warning "\n$me: default case taken"
> + set skip_sve_tests 1
> + }
> + }
> + gdb_exit
Instead of executing the test case in gdb and having to mess up
the current debug run, would it be possible to just run the executable
and check the exit code? You could use "remote_Exec target" to execute
the program, and the exit code should be != 0 if the program was terminated
by a signal (SIGILL). Of course, that only works with Linux (and perhaps
FreeBSD), so if you need this to work with bare-metal or other AArch64 targets,
it won't do.
If the side-effect of restarting GDB is clearly stated in the function doc, then
it's fine like this.
LGTM with those fixed.
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] Aarch64: Detect FP regs in signal frames
2018-09-30 4:37 ` Simon Marchi
@ 2018-10-01 14:26 ` Alan Hayward
0 siblings, 0 replies; 9+ messages in thread
From: Alan Hayward @ 2018-10-01 14:26 UTC (permalink / raw)
To: Simon Marchi; +Cc: GDB Patches, nd
> 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test
2018-09-30 5:01 ` Simon Marchi
@ 2018-10-01 14:26 ` Alan Hayward
0 siblings, 0 replies; 9+ messages in thread
From: Alan Hayward @ 2018-10-01 14:26 UTC (permalink / raw)
To: Simon Marchi; +Cc: GDB Patches, nd
> On 30 Sep 2018, at 06:00, Simon Marchi <simark@simark.ca> wrote:
>
> Can you add a short description of this proc, especially about the side-effects
> and the return value?
>
>> + global srcdir subdir gdb_prompt inferior_exited_re
>> +
>> + set me "skip_aarch64_sve_tests"
>> +
>> + if { ![is_aarch64_target]} {
>> + return 0
>> + }
>
>> +
>> + set compile_flags "{additional_flags=-march=armv8-a+sve}"
>> +
>> + # Compile a test program containing SVE instructions.
>> + set src {
>> + int main() {
>> + asm volatile ("ptrue p0.b");
>> + return 0;
>> + }
>> + }
>> + if {![gdb_simple_compile $me $src executable $compile_flags]} {
>> + return 1
>> + }
>> +
>> + # Compilation succeeded so now run it via gdb.
>> +
>> + gdb_exit
>> + gdb_start
>> + gdb_reinitialize_dir $srcdir/$subdir
>> + gdb_load "$obj"
>
> This sequence can probably be replaced with clean_restart.
>
>> + gdb_run_cmd
>> + gdb_expect {
>> + -re ".*Illegal instruction.*${gdb_prompt} $" {
>> + verbose -log "\n$me sve hardware not detected"
>> + set skip_sve_tests 1
>> + }
>> + -re ".*$inferior_exited_re normally.*${gdb_prompt} $" {
>> + verbose -log "\n$me: sve hardware detected"
>> + set skip_sve_tests 0
>> + }
>> + default {
>> + warning "\n$me: default case taken"
>> + set skip_sve_tests 1
>> + }
>> + }
>> + gdb_exit
>
> Instead of executing the test case in gdb and having to mess up
> the current debug run, would it be possible to just run the executable
> and check the exit code? You could use "remote_Exec target" to execute
> the program, and the exit code should be != 0 if the program was terminated
> by a signal (SIGILL). Of course, that only works with Linux (and perhaps
> FreeBSD), so if you need this to work with bare-metal or other AArch64 targets,
> it won't do.
>
> If the side-effect of restarting GDB is clearly stated in the function doc, then
> it's fine like this.
These three comments are all due to me copying the proc from the almost identical
procs skip_altivec_tests, skip_vsx_tests, skip_tsx_tests, skip_btrace_tests,
skip_btrace_pt_tests. It might be worth me opening another quick patch to fix
those up too?
Probably best to keep the restart in to ensure it works on bare metal too.
Fixed all the nits in this patch as requested and pushed.
Thanks,
Alan.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-01 14:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-17 12:53 [PATCH 0/3] Aarch64: Detect FP registers in signal frames Alan Hayward
2018-09-17 12:53 ` [PATCH 1/3] Aarch64: Move pseudo defines to header Alan Hayward
2018-09-17 12:53 ` [PATCH 3/3] Testsuite: Aarch64: Add signal handler registers test Alan Hayward
2018-09-30 5:01 ` Simon Marchi
2018-10-01 14:26 ` Alan Hayward
2018-09-17 12:53 ` [PATCH 2/3] Aarch64: Detect FP regs in signal frames Alan Hayward
2018-09-30 4:37 ` Simon Marchi
2018-10-01 14:26 ` Alan Hayward
2018-09-25 11:54 ` [PING] [PATCH 0/3] Aarch64: Detect FP registers " Alan Hayward
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).