public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [AArch64] Extend the prologue analyzer to handle the bti instruction
@ 2021-11-11 20:32 Luis Machado
  2021-11-15 13:36 ` Alan Hayward
  0 siblings, 1 reply; 3+ messages in thread
From: Luis Machado @ 2021-11-11 20:32 UTC (permalink / raw)
  To: gdb-patches

Handle the BTI instruction in the prologue analyzer.
---
 gdb/aarch64-tdep.c      | 36 ++++++++++++++++++++++++++++++++++++
 gdb/arch/aarch64-insn.h |  5 +++++
 2 files changed, 41 insertions(+)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 42b8494980c..8c64f56fbe0 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -515,6 +515,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
 	      /* Return addresses are not mangled.  */
 	      ra_state_val = 0;
 	    }
+	  else if (IS_BTI (insn))
+	    /* We don't need to do anything special for a BTI instruction.  */
+	    continue;
 	  else
 	    {
 	      aarch64_debug_printf ("prologue analysis gave up addr=%s"
@@ -869,6 +872,39 @@ aarch64_analyze_prologue_test (void)
 	  SELF_CHECK (cache.saved_regs[regnum].is_value ());
 	}
     }
+
+  /* Test a prologue with a BTI instruction.  */
+    {
+      static const uint32_t insns[] = {
+	0xd503245f, /* bti */
+	0xa9bd7bfd, /* stp	x29, x30, [sp, #-48]! */
+	0x910003fd, /* mov	x29, sp */
+	0xf801c3f3, /* str	x19, [sp, #28] */
+	0xb9401fa0, /* ldr	x19, [x29, #28] */
+      };
+      instruction_reader_test reader (insns);
+
+      trad_frame_reset_saved_regs (gdbarch, cache.saved_regs);
+      CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache,
+						reader);
+
+      SELF_CHECK (end == 4 * 4);
+      SELF_CHECK (cache.framereg == AARCH64_FP_REGNUM);
+      SELF_CHECK (cache.framesize == 48);
+
+      for (int i = 0; i < AARCH64_X_REGISTER_COUNT; i++)
+	{
+	  if (i == 19)
+	    SELF_CHECK (cache.saved_regs[i].addr () == -20);
+	  else if (i == AARCH64_FP_REGNUM)
+	    SELF_CHECK (cache.saved_regs[i].addr () == -48);
+	  else if (i == AARCH64_LR_REGNUM)
+	    SELF_CHECK (cache.saved_regs[i].addr () == -40);
+	  else
+	    SELF_CHECK (cache.saved_regs[i].is_realreg ()
+			&& cache.saved_regs[i].realreg () == i);
+	}
+    }
 }
 } // namespace selftests
 #endif /* GDB_SELF_TEST */
diff --git a/gdb/arch/aarch64-insn.h b/gdb/arch/aarch64-insn.h
index 6f9ec8572b2..a05a6077922 100644
--- a/gdb/arch/aarch64-insn.h
+++ b/gdb/arch/aarch64-insn.h
@@ -42,6 +42,11 @@ extern bool aarch64_debug;
 #define sbits(obj,st,fn) \
   ((long) (bits(obj,st,fn) | ((long) bit(obj,fn) * ~ submask (fn - st))))
 
+/* Prologue analyzer helper macros.  */
+
+/* Is the instruction "bti"?  */
+#define IS_BTI(instruction) ((instruction & 0xffffff3f) == 0xd503241f)
+
 /* List of opcodes that we need for building the jump pad and relocating
    an instruction.  */
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] [AArch64] Extend the prologue analyzer to handle the bti instruction
  2021-11-11 20:32 [PATCH] [AArch64] Extend the prologue analyzer to handle the bti instruction Luis Machado
@ 2021-11-15 13:36 ` Alan Hayward
  2021-11-15 19:01   ` Luis Machado
  0 siblings, 1 reply; 3+ messages in thread
From: Alan Hayward @ 2021-11-15 13:36 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches\@sourceware.org, nd



> On 11 Nov 2021, at 20:32, Luis Machado <luis.machado@linaro.org> wrote:
> 
> Handle the BTI instruction in the prologue analyzer.
> ---
> gdb/aarch64-tdep.c      | 36 ++++++++++++++++++++++++++++++++++++
> gdb/arch/aarch64-insn.h |  5 +++++
> 2 files changed, 41 insertions(+)
> 
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 42b8494980c..8c64f56fbe0 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -515,6 +515,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
> 	      /* Return addresses are not mangled.  */
> 	      ra_state_val = 0;
> 	    }
> +	  else if (IS_BTI (insn))
> +	    /* We don't need to do anything special for a BTI instruction.  */
> +	    continue;
> 	  else
> 	    {
> 	      aarch64_debug_printf ("prologue analysis gave up addr=%s"
> @@ -869,6 +872,39 @@ aarch64_analyze_prologue_test (void)
> 	  SELF_CHECK (cache.saved_regs[regnum].is_value ());
> 	}
>     }
> +
> +  /* Test a prologue with a BTI instruction.  */
> +    {
> +      static const uint32_t insns[] = {
> +	0xd503245f, /* bti */
> +	0xa9bd7bfd, /* stp	x29, x30, [sp, #-48]! */
> +	0x910003fd, /* mov	x29, sp */
> +	0xf801c3f3, /* str	x19, [sp, #28] */
> +	0xb9401fa0, /* ldr	x19, [x29, #28] */
> +      };
> +      instruction_reader_test reader (insns);
> +
> +      trad_frame_reset_saved_regs (gdbarch, cache.saved_regs);
> +      CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache,
> +						reader);
> +
> +      SELF_CHECK (end == 4 * 4);
> +      SELF_CHECK (cache.framereg == AARCH64_FP_REGNUM);
> +      SELF_CHECK (cache.framesize == 48);
> +
> +      for (int i = 0; i < AARCH64_X_REGISTER_COUNT; i++)
> +	{
> +	  if (i == 19)
> +	    SELF_CHECK (cache.saved_regs[i].addr () == -20);
> +	  else if (i == AARCH64_FP_REGNUM)
> +	    SELF_CHECK (cache.saved_regs[i].addr () == -48);
> +	  else if (i == AARCH64_LR_REGNUM)
> +	    SELF_CHECK (cache.saved_regs[i].addr () == -40);
> +	  else
> +	    SELF_CHECK (cache.saved_regs[i].is_realreg ()
> +			&& cache.saved_regs[i].realreg () == i);
> +	}
> +    }
> }
> } // namespace selftests
> #endif /* GDB_SELF_TEST */
> diff --git a/gdb/arch/aarch64-insn.h b/gdb/arch/aarch64-insn.h
> index 6f9ec8572b2..a05a6077922 100644
> --- a/gdb/arch/aarch64-insn.h
> +++ b/gdb/arch/aarch64-insn.h
> @@ -42,6 +42,11 @@ extern bool aarch64_debug;
> #define sbits(obj,st,fn) \
>   ((long) (bits(obj,st,fn) | ((long) bit(obj,fn) * ~ submask (fn - st))))
> 
> +/* Prologue analyzer helper macros.  */
> +
> +/* Is the instruction "bti"?  */
> +#define IS_BTI(instruction) ((instruction & 0xffffff3f) == 0xd503241f)

That mask is everything except the op2 bits. Which means you’re matching all the BTI variants.
Happy with that.

Everything else is straightforward. So, looks ok to me.


Alan.

> +
> /* List of opcodes that we need for building the jump pad and relocating
>    an instruction.  */
> 
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] [AArch64] Extend the prologue analyzer to handle the bti instruction
  2021-11-15 13:36 ` Alan Hayward
@ 2021-11-15 19:01   ` Luis Machado
  0 siblings, 0 replies; 3+ messages in thread
From: Luis Machado @ 2021-11-15 19:01 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches\@sourceware.org, nd

On 11/15/21 10:36 AM, Alan Hayward wrote:
> 
> 
>> On 11 Nov 2021, at 20:32, Luis Machado <luis.machado@linaro.org> wrote:
>>
>> Handle the BTI instruction in the prologue analyzer.
>> ---
>> gdb/aarch64-tdep.c      | 36 ++++++++++++++++++++++++++++++++++++
>> gdb/arch/aarch64-insn.h |  5 +++++
>> 2 files changed, 41 insertions(+)
>>
>> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
>> index 42b8494980c..8c64f56fbe0 100644
>> --- a/gdb/aarch64-tdep.c
>> +++ b/gdb/aarch64-tdep.c
>> @@ -515,6 +515,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>> 	      /* Return addresses are not mangled.  */
>> 	      ra_state_val = 0;
>> 	    }
>> +	  else if (IS_BTI (insn))
>> +	    /* We don't need to do anything special for a BTI instruction.  */
>> +	    continue;
>> 	  else
>> 	    {
>> 	      aarch64_debug_printf ("prologue analysis gave up addr=%s"
>> @@ -869,6 +872,39 @@ aarch64_analyze_prologue_test (void)
>> 	  SELF_CHECK (cache.saved_regs[regnum].is_value ());
>> 	}
>>      }
>> +
>> +  /* Test a prologue with a BTI instruction.  */
>> +    {
>> +      static const uint32_t insns[] = {
>> +	0xd503245f, /* bti */
>> +	0xa9bd7bfd, /* stp	x29, x30, [sp, #-48]! */
>> +	0x910003fd, /* mov	x29, sp */
>> +	0xf801c3f3, /* str	x19, [sp, #28] */
>> +	0xb9401fa0, /* ldr	x19, [x29, #28] */
>> +      };
>> +      instruction_reader_test reader (insns);
>> +
>> +      trad_frame_reset_saved_regs (gdbarch, cache.saved_regs);
>> +      CORE_ADDR end = aarch64_analyze_prologue (gdbarch, 0, 128, &cache,
>> +						reader);
>> +
>> +      SELF_CHECK (end == 4 * 4);
>> +      SELF_CHECK (cache.framereg == AARCH64_FP_REGNUM);
>> +      SELF_CHECK (cache.framesize == 48);
>> +
>> +      for (int i = 0; i < AARCH64_X_REGISTER_COUNT; i++)
>> +	{
>> +	  if (i == 19)
>> +	    SELF_CHECK (cache.saved_regs[i].addr () == -20);
>> +	  else if (i == AARCH64_FP_REGNUM)
>> +	    SELF_CHECK (cache.saved_regs[i].addr () == -48);
>> +	  else if (i == AARCH64_LR_REGNUM)
>> +	    SELF_CHECK (cache.saved_regs[i].addr () == -40);
>> +	  else
>> +	    SELF_CHECK (cache.saved_regs[i].is_realreg ()
>> +			&& cache.saved_regs[i].realreg () == i);
>> +	}
>> +    }
>> }
>> } // namespace selftests
>> #endif /* GDB_SELF_TEST */
>> diff --git a/gdb/arch/aarch64-insn.h b/gdb/arch/aarch64-insn.h
>> index 6f9ec8572b2..a05a6077922 100644
>> --- a/gdb/arch/aarch64-insn.h
>> +++ b/gdb/arch/aarch64-insn.h
>> @@ -42,6 +42,11 @@ extern bool aarch64_debug;
>> #define sbits(obj,st,fn) \
>>    ((long) (bits(obj,st,fn) | ((long) bit(obj,fn) * ~ submask (fn - st))))
>>
>> +/* Prologue analyzer helper macros.  */
>> +
>> +/* Is the instruction "bti"?  */
>> +#define IS_BTI(instruction) ((instruction & 0xffffff3f) == 0xd503241f)
> 
> That mask is everything except the op2 bits. Which means you’re matching all the BTI variants.
> Happy with that.
> 
> Everything else is straightforward. So, looks ok to me.

Thanks Alan. Pushed now.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-15 19:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-11 20:32 [PATCH] [AArch64] Extend the prologue analyzer to handle the bti instruction Luis Machado
2021-11-15 13:36 ` Alan Hayward
2021-11-15 19:01   ` Luis Machado

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