[PATCH 0/2] aarch64: skip over stack protector setup in function prologues

[PATCH 0/2] aarch64: skip over stack protector setup in function prologues

[Kyle McMartin]

When debugging code which ends up with stack protector setup in the
function prologue, break points on functions end up stopping in the
prologue, instead of on a useful statement inside the function. To fix
this, do something similar to ARM (which has more cases to handle) and
analyse the instructions we stopped at to see if its the stack protector
sequence.

Also add a simple testcase to check to see if we successfully manage to
end up on the statement we expect or not. (Which could also be used as a
test on ARM, but I don't have hardware handy, will investigate later.)

regards, Kyle

 gdb/aarch64-tdep.c                                 | 125 +++++++++++++++++++--
 gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c   |  28 +++++
 gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp |  43 +++++++
 3 files changed, 189 insertions(+), 7 deletions(-)

[PATCH 1/2] aarch64: extend decode_adrp to decode immediate offset

[Kyle McMartin]

This is needed in order to figure out what an ADRP instruction is
attempting to calculate the address of, for use by a further
functionality.

gdb/
2014-06-03  Kyle McMartin  <kmcmarti@redhat.com>

	* aarch64-tdep.c (decode_adrp): Generalise function to handle
	ADR as well as ADRP, and decode the offset.
	(aarch64_analyze_prologue): Discard unneeded arguments to
	decode_adrp.
---
 gdb/aarch64-tdep.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 4abe36e..9550f42 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -255,7 +255,7 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
   return 0;
 }
 
-/* Decode an opcode if it represents an ADRP instruction.
+/* Decode an opcode if it represents an ADRP/ADR instruction.
 
    ADDR specifies the address of the opcode.
    INSN specifies the opcode to test.
@@ -264,16 +264,28 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
    Return 1 if the opcodes matches and is decoded, otherwise 0.  */
 
 static int
-decode_adrp (CORE_ADDR addr, uint32_t insn, unsigned *rd)
+decode_adrp (CORE_ADDR addr, uint32_t insn, int *page, unsigned *rd,
+	     int64_t *imm)
 {
-  if (decode_masked_match (insn, 0x9f000000, 0x90000000))
+  if (decode_masked_match (insn, 0x1f000000, 0x10000000))
     {
       *rd = (insn >> 0) & 0x1f;
+      *imm = (extract_signed_bitfield (insn, 19, 5) << 2)
+	      | ((insn >> 29) & 0x3);
+      *page = 0;
+
+      if (insn & 0x80000000)
+	{
+	  *page = 1;
+	  *imm <<= 12;
+	}
 
       if (aarch64_debug)
 	fprintf_unfiltered (gdb_stdlog,
-			    "decode: 0x%s 0x%x adrp x%u, #?\n",
-			    core_addr_to_string_nz (addr), insn, *rd);
+			    "decode: 0x%s 0x%x %s x%u, #0x%llx\n",
+			    core_addr_to_string_nz (addr), insn,
+			    *page ? "adrp" : "adr", *rd,
+			    (unsigned long long)*imm);
       return 1;
     }
   return 0;
@@ -681,8 +693,10 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
       unsigned rt2;
       int op_is_sub;
       int32_t imm;
+      int64_t imm64;
       unsigned cond;
       int is64;
+      int page;
       unsigned is_link;
       unsigned op;
       unsigned bit;
@@ -692,7 +706,7 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
 
       if (decode_add_sub_imm (start, insn, &rd, &rn, &imm))
 	regs[rd] = pv_add_constant (regs[rn], imm);
-      else if (decode_adrp (start, insn, &rd))
+      else if (decode_adrp (start, insn, &page, &rd, &imm64))
 	regs[rd] = pv_unknown ();
       else if (decode_b (start, insn, &is_link, &offset))
 	{
-- 
1.9.3

Re: [PATCH 1/2] aarch64: extend decode_adrp to decode immediate offset

[Marcus Shawcroft]

Hi,

On 3 June 2014 06:01, Kyle McMartin <kmcmarti@redhat.com> wrote:

> @@ -264,16 +264,28 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
>     Return 1 if the opcodes matches and is decoded, otherwise 0.  */
>
>  static int
> -decode_adrp (CORE_ADDR addr, uint32_t insn, unsigned *rd)
> +decode_adrp (CORE_ADDR addr, uint32_t insn, int *page, unsigned *rd,
> +            int64_t *imm)

Given that this now decodes both adrp and adr the function name seems
inappropriate, how about following the convention used in the other
decode_ functions and changing to something like decode_adrp_adr ()..
?

Returning both 'page' and 'imm' isn't necessary and I don't think it
makes sense given that logically both adr and adrp are used to
construct an address.  Looking at this patch and the following patch I
think you can achieve the same functionality by having the decodes of
both adrp and adr return a value in 'imm'.  In the case of an adrp
decode just returning: imm = page << 12  ....

Cheers
/Marcus

Re: [PATCH 1/2] aarch64: extend decode_adrp to decode immediate offset

[Kyle McMartin]

On Tue, Jun 03, 2014 at 09:22:28AM +0100, Marcus Shawcroft wrote:
> Hi,
> 
> On 3 June 2014 06:01, Kyle McMartin <kmcmarti@redhat.com> wrote:
> 
> > @@ -264,16 +264,28 @@ decode_add_sub_imm (CORE_ADDR addr, uint32_t insn, unsigned *rd, unsigned *rn,
> >     Return 1 if the opcodes matches and is decoded, otherwise 0.  */
> >
> >  static int
> > -decode_adrp (CORE_ADDR addr, uint32_t insn, unsigned *rd)
> > +decode_adrp (CORE_ADDR addr, uint32_t insn, int *page, unsigned *rd,
> > +            int64_t *imm)
> 
> Given that this now decodes both adrp and adr the function name seems
> inappropriate, how about following the convention used in the other
> decode_ functions and changing to something like decode_adrp_adr ()..
> ?
> 

Sure, sounds good.

> Returning both 'page' and 'imm' isn't necessary and I don't think it
> makes sense given that logically both adr and adrp are used to
> construct an address.  Looking at this patch and the following patch I
> think you can achieve the same functionality by having the decodes of
> both adrp and adr return a value in 'imm'.  In the case of an adrp
> decode just returning: imm = page << 12  ....
> 

Hmm, I was trying to avoid doing the actual computation inside it. I'm
happy one way or another though.

--Kyle

> Cheers
> /Marcus

[PATCH 2/2] aarch64: implement walking over the stack protector

[Kyle McMartin]

Stepping into a function which contains the stack protector sequences
currently stops inside the prologue, resulting in us claiming to be on
the opening bracket or similar, instead of a useful statement inside the
function. Fix that by analysing the prologue instruction, and attempt to
walk through the sequence of instructions which set up the stack
protector canary.

gdb/
2014-06-03  Kyle McMartin  <kmcmarti@redhat.com>

	* aarch64-tdep.c (aarch64_skip_stack_chk_guard): New.
	(aarch64_skip_prologue): Skip over stack protector setup if possible.

gdb/testsuite/
2014-06-03  Kyle McMartin  <kmcmarti@redhat.com>

	* gdb.arch/aarch64-stack_chk_guard.c: New file.
	* gdb.arch/aarch64-stack_chk_guard.exp: New file.
---
 gdb/aarch64-tdep.c                                 | 99 +++++++++++++++++++++-
 gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c   | 28 ++++++
 gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp | 43 ++++++++++
 3 files changed, 169 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 9550f42..0c900ce 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -852,6 +852,99 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
   return start;
 }
 
+/* Attempt to skip the stack protector instructions in a function prologue.
+   If PC points to the first instruction of the sequence, return the
+   address of the instruction after the stack protector sequence.  Otherwise,
+   return the original PC.
+
+   On AArch64, the stack protector sequence is composed of four instructions:
+
+    adrp x0, __stack_chk_guard
+    add  x0, x0, #:lo12:__stack_chk_guard
+    ldr  x0, [x0]
+    str  x0, [x29, #end-of-stack]
+
+   Which loads the address of __stack_chk_guard, then loads the guard from it,
+   and stores it at the end of the stack.  */
+
+static CORE_ADDR
+aarch64_skip_stack_chk_guard (CORE_ADDR pc, struct gdbarch *gdbarch)
+{
+  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
+  CORE_ADDR addr = pc;
+  CORE_ADDR loc = pc;
+  uint32_t insn;
+  int64_t imm;
+  int32_t imm32;
+  unsigned rd, rd2, rn, rt;
+  int page;
+  const int insn_size = 4;
+  struct bound_minimal_symbol stack_chk_guard;
+
+  /* Attempt to find the label formation of __stack_chk_guard.  */
+  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
+  if (!decode_adrp (loc, insn, &page, &rd, &imm))
+    return pc;
+
+  /* Bail if we saw an ADR instruction, not an ADRP.  */
+  if (!page)
+    return pc;
+
+  loc += insn_size;
+  addr &= ~((1 << 12) - 1);
+  addr += imm;
+
+  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
+  if (!decode_add_sub_imm (loc, insn, &rd2, &rn, &imm32))
+    return pc;
+
+  /* Ensure ADD register matches the ADRP instruction.  */
+  if (rn != rd)
+    return pc;
+
+  loc += insn_size;
+  addr += imm32;
+
+  /* See if we calculated the address of the __stack_chk_guard symbol.  */
+  stack_chk_guard = lookup_minimal_symbol_by_pc (addr);
+  if (stack_chk_guard.minsym
+      && strncmp (MSYMBOL_LINKAGE_NAME (stack_chk_guard.minsym),
+		  "__stack_chk_guard", strlen ("__stack_chk_guard")) != 0)
+    return pc;
+
+  /* Check if the next instruction is a load from the same registers.  */
+  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
+  if (decode_masked_match (insn, 0xffc00000, 0xf9400000))
+    {
+      rt = insn & 0x1F;
+      rn = (insn >> 5) & 0x1F;
+
+      if (rn != rd2)
+	return pc;
+    }
+  else
+    return pc;
+
+  /* Finally, look for a store of the guard to the stack.  */
+  loc += insn_size;
+  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
+  if (decode_masked_match (insn, 0xffc00000, 0xf9000000))
+    {
+      unsigned rt2 = insn & 0x1F;
+
+      /* Check we're storing the guard from the previous load instruction.  */
+      if (rt2 != rt)
+        return pc;
+    }
+  else
+    return pc;
+
+  /* If we've made it this far, we've walked through the 4 instruction
+     sequence around __stack_chk_guard, and can skip over it in the function
+     prologue.  */
+  return loc + insn_size;
+}
+
 /* Implement the "skip_prologue" gdbarch method.  */
 
 static CORE_ADDR
@@ -871,7 +964,11 @@ aarch64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
 	= skip_prologue_using_sal (gdbarch, func_addr);
 
       if (post_prologue_pc != 0)
-	return max (pc, post_prologue_pc);
+	{
+	  post_prologue_pc = aarch64_skip_stack_chk_guard (post_prologue_pc,
+							   gdbarch);
+	  return max (pc, post_prologue_pc);
+	}
     }
 
   /* Can't determine prologue from the symbol table, need to examine
diff --git a/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c
new file mode 100644
index 0000000..3cf52d9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c
@@ -0,0 +1,28 @@
+/* This file is part of GDB, the GNU debugger.
+
+   Copyright 2014 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/>.  */
+
+int stack_chk_guard_fn(void)
+{
+  /* Needs to be large enough to trigger -fstack-protector.  */
+  char stack[64];
+  return 0; /* Post function prologue statement.  */
+}
+
+int main(void)
+{
+  return stack_chk_guard_fn();
+}
diff --git a/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp
new file mode 100644
index 0000000..7f60867
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp
@@ -0,0 +1,43 @@
+# Copyright 2014 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+#
+# This file is part of the gdb testsuite.
+
+# Test that we single step past the __stack_chk_guard setup in the
+# prologue of functions.
+
+if {![istarget "aarch64*"]} {
+    verbose "Skipping ${gdb_test_file_name}."
+    return
+}
+
+standard_testfile
+set additional_flags "additional_flags=-fstack-protector"
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
+    unsupported "compiler does not support -fstack-protector"
+    return
+}
+
+clean_restart "${binfile}"
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+gdb_breakpoint "stack_chk_guard_fn"
+
+# Previously, we'd see a { as we're still in the function prologue.
+gdb_continue_to_breakpoint "continue into stack_chk_guard_fn" ".*return 0;.*"
-- 
1.9.3

Re: [PATCH 2/2] aarch64: implement walking over the stack protector

[Andrew Pinski]

On Mon, Jun 2, 2014 at 10:03 PM, Kyle McMartin <kmcmarti@redhat.com> wrote:
> Stepping into a function which contains the stack protector sequences
> currently stops inside the prologue, resulting in us claiming to be on
> the opening bracket or similar, instead of a useful statement inside the
> function. Fix that by analysing the prologue instruction, and attempt to
> walk through the sequence of instructions which set up the stack
> protector canary.
>
> gdb/
> 2014-06-03  Kyle McMartin  <kmcmarti@redhat.com>
>
>         * aarch64-tdep.c (aarch64_skip_stack_chk_guard): New.
>         (aarch64_skip_prologue): Skip over stack protector setup if possible.
>
> gdb/testsuite/
> 2014-06-03  Kyle McMartin  <kmcmarti@redhat.com>
>
>         * gdb.arch/aarch64-stack_chk_guard.c: New file.
>         * gdb.arch/aarch64-stack_chk_guard.exp: New file.
> ---
>  gdb/aarch64-tdep.c                                 | 99 +++++++++++++++++++++-
>  gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c   | 28 ++++++
>  gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp | 43 ++++++++++
>  3 files changed, 169 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c
>  create mode 100644 gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 9550f42..0c900ce 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -852,6 +852,99 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>    return start;
>  }
>
> +/* Attempt to skip the stack protector instructions in a function prologue.
> +   If PC points to the first instruction of the sequence, return the
> +   address of the instruction after the stack protector sequence.  Otherwise,
> +   return the original PC.
> +
> +   On AArch64, the stack protector sequence is composed of four instructions:
> +
> +    adrp x0, __stack_chk_guard
> +    add  x0, x0, #:lo12:__stack_chk_guard
> +    ldr  x0, [x0]
> +    str  x0, [x29, #end-of-stack]

Can you expand this for ILP32?  The sequence is the same except to use
w0 instead of x0.  Otherwise I can put it on my todo list to after I
submit the gdb support for ILP32.

Thanks,
Andrew Pinski

> +
> +   Which loads the address of __stack_chk_guard, then loads the guard from it,
> +   and stores it at the end of the stack.  */
> +
> +static CORE_ADDR
> +aarch64_skip_stack_chk_guard (CORE_ADDR pc, struct gdbarch *gdbarch)
> +{
> +  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
> +  CORE_ADDR addr = pc;
> +  CORE_ADDR loc = pc;
> +  uint32_t insn;
> +  int64_t imm;
> +  int32_t imm32;
> +  unsigned rd, rd2, rn, rt;
> +  int page;
> +  const int insn_size = 4;
> +  struct bound_minimal_symbol stack_chk_guard;
> +
> +  /* Attempt to find the label formation of __stack_chk_guard.  */
> +  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
> +  if (!decode_adrp (loc, insn, &page, &rd, &imm))
> +    return pc;
> +
> +  /* Bail if we saw an ADR instruction, not an ADRP.  */
> +  if (!page)
> +    return pc;
> +
> +  loc += insn_size;
> +  addr &= ~((1 << 12) - 1);
> +  addr += imm;
> +
> +  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
> +  if (!decode_add_sub_imm (loc, insn, &rd2, &rn, &imm32))
> +    return pc;
> +
> +  /* Ensure ADD register matches the ADRP instruction.  */
> +  if (rn != rd)
> +    return pc;
> +
> +  loc += insn_size;
> +  addr += imm32;
> +
> +  /* See if we calculated the address of the __stack_chk_guard symbol.  */
> +  stack_chk_guard = lookup_minimal_symbol_by_pc (addr);
> +  if (stack_chk_guard.minsym
> +      && strncmp (MSYMBOL_LINKAGE_NAME (stack_chk_guard.minsym),
> +                 "__stack_chk_guard", strlen ("__stack_chk_guard")) != 0)
> +    return pc;
> +
> +  /* Check if the next instruction is a load from the same registers.  */
> +  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
> +  if (decode_masked_match (insn, 0xffc00000, 0xf9400000))
> +    {
> +      rt = insn & 0x1F;
> +      rn = (insn >> 5) & 0x1F;
> +
> +      if (rn != rd2)
> +       return pc;
> +    }
> +  else
> +    return pc;
> +
> +  /* Finally, look for a store of the guard to the stack.  */
> +  loc += insn_size;
> +  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
> +  if (decode_masked_match (insn, 0xffc00000, 0xf9000000))
> +    {
> +      unsigned rt2 = insn & 0x1F;
> +
> +      /* Check we're storing the guard from the previous load instruction.  */
> +      if (rt2 != rt)
> +        return pc;
> +    }
> +  else
> +    return pc;
> +
> +  /* If we've made it this far, we've walked through the 4 instruction
> +     sequence around __stack_chk_guard, and can skip over it in the function
> +     prologue.  */
> +  return loc + insn_size;
> +}
> +
>  /* Implement the "skip_prologue" gdbarch method.  */
>
>  static CORE_ADDR
> @@ -871,7 +964,11 @@ aarch64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
>         = skip_prologue_using_sal (gdbarch, func_addr);
>
>        if (post_prologue_pc != 0)
> -       return max (pc, post_prologue_pc);
> +       {
> +         post_prologue_pc = aarch64_skip_stack_chk_guard (post_prologue_pc,
> +                                                          gdbarch);
> +         return max (pc, post_prologue_pc);
> +       }
>      }
>
>    /* Can't determine prologue from the symbol table, need to examine
> diff --git a/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c
> new file mode 100644
> index 0000000..3cf52d9
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c
> @@ -0,0 +1,28 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> +   Copyright 2014 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/>.  */
> +
> +int stack_chk_guard_fn(void)
> +{
> +  /* Needs to be large enough to trigger -fstack-protector.  */
> +  char stack[64];
> +  return 0; /* Post function prologue statement.  */
> +}
> +
> +int main(void)
> +{
> +  return stack_chk_guard_fn();
> +}
> diff --git a/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp
> new file mode 100644
> index 0000000..7f60867
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp
> @@ -0,0 +1,43 @@
> +# Copyright 2014 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, write to the Free Software
> +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> +#
> +# This file is part of the gdb testsuite.
> +
> +# Test that we single step past the __stack_chk_guard setup in the
> +# prologue of functions.
> +
> +if {![istarget "aarch64*"]} {
> +    verbose "Skipping ${gdb_test_file_name}."
> +    return
> +}
> +
> +standard_testfile
> +set additional_flags "additional_flags=-fstack-protector"
> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
> +    unsupported "compiler does not support -fstack-protector"
> +    return
> +}
> +
> +clean_restart "${binfile}"
> +if ![runto_main] {
> +    untested "could not run to main"
> +    return -1
> +}
> +
> +gdb_breakpoint "stack_chk_guard_fn"
> +
> +# Previously, we'd see a { as we're still in the function prologue.
> +gdb_continue_to_breakpoint "continue into stack_chk_guard_fn" ".*return 0;.*"
> --
> 1.9.3
>

Re: [PATCH 2/2] aarch64: implement walking over the stack protector

[Kyle McMartin]

On Mon, Jun 02, 2014 at 10:42:32PM -0700, Andrew Pinski wrote:
> > +    adrp x0, __stack_chk_guard
> > +    add  x0, x0, #:lo12:__stack_chk_guard
> > +    ldr  x0, [x0]
> > +    str  x0, [x29, #end-of-stack]
> 
> Can you expand this for ILP32?  The sequence is the same except to use
> w0 instead of x0.  Otherwise I can put it on my todo list to after I
> submit the gdb support for ILP32.
> 

I'll look into it. Thanks Andrew.

--Kyle

> Thanks,
> Andrew Pinski
> 
> > +
> > +   Which loads the address of __stack_chk_guard, then loads the guard from it,
> > +   and stores it at the end of the stack.  */
> > +
> > +static CORE_ADDR
> > +aarch64_skip_stack_chk_guard (CORE_ADDR pc, struct gdbarch *gdbarch)
> > +{
> > +  enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code (gdbarch);
> > +  CORE_ADDR addr = pc;
> > +  CORE_ADDR loc = pc;
> > +  uint32_t insn;
> > +  int64_t imm;
> > +  int32_t imm32;
> > +  unsigned rd, rd2, rn, rt;
> > +  int page;
> > +  const int insn_size = 4;
> > +  struct bound_minimal_symbol stack_chk_guard;
> > +
> > +  /* Attempt to find the label formation of __stack_chk_guard.  */
> > +  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
> > +  if (!decode_adrp (loc, insn, &page, &rd, &imm))
> > +    return pc;
> > +
> > +  /* Bail if we saw an ADR instruction, not an ADRP.  */
> > +  if (!page)
> > +    return pc;
> > +
> > +  loc += insn_size;
> > +  addr &= ~((1 << 12) - 1);
> > +  addr += imm;
> > +
> > +  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
> > +  if (!decode_add_sub_imm (loc, insn, &rd2, &rn, &imm32))
> > +    return pc;
> > +
> > +  /* Ensure ADD register matches the ADRP instruction.  */
> > +  if (rn != rd)
> > +    return pc;
> > +
> > +  loc += insn_size;
> > +  addr += imm32;
> > +
> > +  /* See if we calculated the address of the __stack_chk_guard symbol.  */
> > +  stack_chk_guard = lookup_minimal_symbol_by_pc (addr);
> > +  if (stack_chk_guard.minsym
> > +      && strncmp (MSYMBOL_LINKAGE_NAME (stack_chk_guard.minsym),
> > +                 "__stack_chk_guard", strlen ("__stack_chk_guard")) != 0)
> > +    return pc;
> > +
> > +  /* Check if the next instruction is a load from the same registers.  */
> > +  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
> > +  if (decode_masked_match (insn, 0xffc00000, 0xf9400000))
> > +    {
> > +      rt = insn & 0x1F;
> > +      rn = (insn >> 5) & 0x1F;
> > +
> > +      if (rn != rd2)
> > +       return pc;
> > +    }
> > +  else
> > +    return pc;
> > +
> > +  /* Finally, look for a store of the guard to the stack.  */
> > +  loc += insn_size;
> > +  insn = read_memory_unsigned_integer (loc, insn_size, byte_order_for_code);
> > +  if (decode_masked_match (insn, 0xffc00000, 0xf9000000))
> > +    {
> > +      unsigned rt2 = insn & 0x1F;
> > +
> > +      /* Check we're storing the guard from the previous load instruction.  */
> > +      if (rt2 != rt)
> > +        return pc;
> > +    }
> > +  else
> > +    return pc;
> > +
> > +  /* If we've made it this far, we've walked through the 4 instruction
> > +     sequence around __stack_chk_guard, and can skip over it in the function
> > +     prologue.  */
> > +  return loc + insn_size;
> > +}
> > +
> >  /* Implement the "skip_prologue" gdbarch method.  */
> >
> >  static CORE_ADDR
> > @@ -871,7 +964,11 @@ aarch64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
> >         = skip_prologue_using_sal (gdbarch, func_addr);
> >
> >        if (post_prologue_pc != 0)
> > -       return max (pc, post_prologue_pc);
> > +       {
> > +         post_prologue_pc = aarch64_skip_stack_chk_guard (post_prologue_pc,
> > +                                                          gdbarch);
> > +         return max (pc, post_prologue_pc);
> > +       }
> >      }
> >
> >    /* Can't determine prologue from the symbol table, need to examine
> > diff --git a/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c
> > new file mode 100644
> > index 0000000..3cf52d9
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.c
> > @@ -0,0 +1,28 @@
> > +/* This file is part of GDB, the GNU debugger.
> > +
> > +   Copyright 2014 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/>.  */
> > +
> > +int stack_chk_guard_fn(void)
> > +{
> > +  /* Needs to be large enough to trigger -fstack-protector.  */
> > +  char stack[64];
> > +  return 0; /* Post function prologue statement.  */
> > +}
> > +
> > +int main(void)
> > +{
> > +  return stack_chk_guard_fn();
> > +}
> > diff --git a/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp
> > new file mode 100644
> > index 0000000..7f60867
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.arch/aarch64-stack_chk_guard.exp
> > @@ -0,0 +1,43 @@
> > +# Copyright 2014 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, write to the Free Software
> > +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> > +#
> > +# This file is part of the gdb testsuite.
> > +
> > +# Test that we single step past the __stack_chk_guard setup in the
> > +# prologue of functions.
> > +
> > +if {![istarget "aarch64*"]} {
> > +    verbose "Skipping ${gdb_test_file_name}."
> > +    return
> > +}
> > +
> > +standard_testfile
> > +set additional_flags "additional_flags=-fstack-protector"
> > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug $additional_flags]] != "" } {
> > +    unsupported "compiler does not support -fstack-protector"
> > +    return
> > +}
> > +
> > +clean_restart "${binfile}"
> > +if ![runto_main] {
> > +    untested "could not run to main"
> > +    return -1
> > +}
> > +
> > +gdb_breakpoint "stack_chk_guard_fn"
> > +
> > +# Previously, we'd see a { as we're still in the function prologue.
> > +gdb_continue_to_breakpoint "continue into stack_chk_guard_fn" ".*return 0;.*"
> > --
> > 1.9.3
> >

Re: [PATCH 2/2] aarch64: implement walking over the stack protector

[Kyle McMartin]

On Tue, Jun 03, 2014 at 10:51:08AM -0400, Kyle McMartin wrote:
> On Mon, Jun 02, 2014 at 10:42:32PM -0700, Andrew Pinski wrote:
> > > +    adrp x0, __stack_chk_guard
> > > +    add  x0, x0, #:lo12:__stack_chk_guard
> > > +    ldr  x0, [x0]
> > > +    str  x0, [x29, #end-of-stack]
> > 
> > Can you expand this for ILP32?  The sequence is the same except to use
> > w0 instead of x0.  Otherwise I can put it on my todo list to after I
> > submit the gdb support for ILP32.
> > 
> 
> I'll look into it. Thanks Andrew.
> 

Finally got a toolchain built that supports ilp32... looks like the
existing code should work for it. The only difference (not sure if it's
because of gcc HEAD or what) is using w1 instead of x0 repeated for the
ldr/str, which looks to be satisfied by the rt/rt2 tests and size=10
included in the decode_masked_match. I'll try to build a static ilp32
binary to test it meanwhile, but I've had some other issues with gcc
HEAD.

regards, Kyle

Re: [PATCH 2/2] aarch64: implement walking over the stack protector

[Andrew Pinski]

On Thu, Jun 5, 2014 at 1:06 PM, Kyle McMartin <kmcmarti@redhat.com> wrote:
> On Tue, Jun 03, 2014 at 10:51:08AM -0400, Kyle McMartin wrote:
>> On Mon, Jun 02, 2014 at 10:42:32PM -0700, Andrew Pinski wrote:
>> > > +    adrp x0, __stack_chk_guard
>> > > +    add  x0, x0, #:lo12:__stack_chk_guard
>> > > +    ldr  x0, [x0]
>> > > +    str  x0, [x29, #end-of-stack]
>> >
>> > Can you expand this for ILP32?  The sequence is the same except to use
>> > w0 instead of x0.  Otherwise I can put it on my todo list to after I
>> > submit the gdb support for ILP32.
>> >
>>
>> I'll look into it. Thanks Andrew.
>>
>
> Finally got a toolchain built that supports ilp32... looks like the
> existing code should work for it. The only difference (not sure if it's
> because of gcc HEAD or what) is using w1 instead of x0 repeated for the
> ldr/str, which looks to be satisfied by the rt/rt2 tests and size=10
> included in the decode_masked_match. I'll try to build a static ilp32
> binary to test it meanwhile, but I've had some other issues with gcc
> HEAD.

Glibc support is not there yet though.  I will take over the issue
with ILP32 then.  When this goes in, can you file a bug and assign it
to me (pinskia@gcc.gnu.org I think is my email for sourceware
bugzilla)?

Thanks,
Andrew Pinski


>
> regards, Kyle

Re: [PATCH 2/2] aarch64: implement walking over the stack protector

[Kyle McMartin]

On Thu, Jun 05, 2014 at 01:35:54PM -0700, Andrew Pinski wrote:
> > Finally got a toolchain built that supports ilp32... looks like the
> > existing code should work for it. The only difference (not sure if it's
> > because of gcc HEAD or what) is using w1 instead of x0 repeated for the
> > ldr/str, which looks to be satisfied by the rt/rt2 tests and size=10
> > included in the decode_masked_match. I'll try to build a static ilp32
> > binary to test it meanwhile, but I've had some other issues with gcc
> > HEAD.
> 
> Glibc support is not there yet though.  I will take over the issue
> with ILP32 then.  When this goes in, can you file a bug and assign it
> to me (pinskia@gcc.gnu.org I think is my email for sourceware
> bugzilla)?
> 

Will do. Just ran this sequence through the code 'by hand' and it seems
to work on ILP32, fwiw.

--Kyle