GDB: aarch64: Add ability to displaced step over a BR/BLR instruction

Enable displaced stepping over a BR/BLR instruction

Displaced stepping over an instruction executes a instruction in a
scratch area and then manually fixes up the PC address to leave
execution where it would have been if the instruction were in its
original location.

The BR instruction does not need modification in order to run correctly
at a different address, but the displaced step fixup method should not
manually adjust the PC since the BR instruction sets that value already.

The BLR instruction should also avoid such a fixup, but must also have
the link register modified to point to just after the original code
location rather than back to the scratch location.

This patch adds the above functionality.
We add this functionality by modifying aarch64_displaced_step_others
rather than by adding a new visitor method to aarch64_insn_visitor.
We choose this since it seems that visitor approach is designed
specifically for PC relative instructions (which must always be modified
when executed in a different location).

It seems that the BR and BLR instructions are more like the RET
instruction which is already handled specially in
aarch64_displaced_step_others.

This also means the gdbserver code to relocate an instruction when
creating a fast tracepoint does not need to be modified, since nothing
special is needed for the BR and BLR instructions there.

Regression tests showed nothing untoward on native aarch64 (though it
took a while for me to get the testcase to account for PIE).

------#####
Original observed (mis)behaviour before was that displaced stepping over
a BR or BLR instruction would not execute the function they called.
Most easily seen by putting a breakpoint with a condition on such an
instruction and a print statement in the functions they called.
When run with the breakpoint enabled the function is not called and
"numargs called" is not printed.
When run with the breakpoint disabled the function is called and the
message is printed.

--- GDB Session
~ [15:57:14] % gdb ../using-blr
Reading symbols from ../using-blr...done.
(gdb) disassemble blr_call_value
Dump of assembler code for function blr_call_value:
...
   0x0000000000400560 <+28>:    blr     x2
...
   0x00000000004005b8 <+116>:   ret
End of assembler dump.
(gdb) break *0x0000000000400560
Breakpoint 1 at 0x400560: file ../using-blr.c, line 22.
(gdb) condition 1 10 == 0
(gdb) run
Starting program: /home/matmal01/using-blr
[Inferior 1 (process 33279) exited with code 012]
(gdb) disable 1
(gdb) run
Starting program: /home/matmal01/using-blr
numargs called
[Inferior 1 (process 33289) exited with code 012]
(gdb)

Test program:
---- using-blr ----
\#include <stdio.h>
typedef int (foo) (int, int);
typedef void (bar) (int, int);
struct sls_testclass {
    foo *x;
    bar *y;
    int left;
    int right;
};

__attribute__ ((noinline))
int blr_call_value (struct sls_testclass x)
{
  int retval = x.x(x.left, x.right);
  if (retval % 10)
    return 100;
  return 9;
}

__attribute__ ((noinline))
int blr_call (struct sls_testclass x)
{
  x.y(x.left, x.right);
  if (x.left % 10)
    return 100;
  return 9;
}

int
numargs (__attribute__ ((unused)) int left, __attribute__ ((unused)) int right)
{
        printf("numargs called\n");
        return 10;
}

void
altfunc (__attribute__ ((unused)) int left, __attribute__ ((unused)) int right)
{
        printf("altfunc called\n");
}

int main(int argc, char **argv)
{
  struct sls_testclass x = { .x = numargs, .y = altfunc, .left = 1, .right = 2 };
  if (argc > 2)
  {
        blr_call (x);
  }
  else
        blr_call_value (x);
  return 10;
}

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4b1f21cc91d130df8dbd7267192398dfc0bbe871..087e2041ba225e63fe764977979d80a9b2a244c9 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2021-01-27  Matthew Malcomson  <matthew.malcomson@arm.com>
+
+       * aarch64-tdep.c (aarch64_displaced_step_others): Account for
+       BLR and BR instructions.
+       * arch/aarch64-insn.h (enum aarch64_opcodes): Add BR opcode.
+       (enum aarch64_masks): New.
+
 2021-01-26  Tom Tromey  <tromey@adacore.com>
 
        * windows-nat.c (DEBUG_EXEC, DEBUG_EVENTS, DEBUG_MEM)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index d1e15497a46ca250d606f4da77ad653fecc41e1c..3ac0564dd9a1bf01a7df60059b3a7b353e3b042f 100644 (file)
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3099,14 +3099,21 @@ aarch64_displaced_step_others (const uint32_t insn,
   struct aarch64_displaced_step_data *dsd
     = (struct aarch64_displaced_step_data *) data;
 
-  aarch64_emit_insn (dsd->insn_buf, insn);
-  dsd->insn_count = 1;
-
-  if ((insn & 0xfffffc1f) == 0xd65f0000)
+  uint32_t masked_insn = (insn & CLEAR_Rn_MASK);
+  if (masked_insn == BLR)
     {
-      /* RET */
-      dsd->dsc->pc_adjust = 0;
+      /* Emit a BR to the same register and then update LR to the original
+        address (similar to aarch64_displaced_step_b).  */
+      aarch64_emit_insn (dsd->insn_buf, insn & 0xffdfffff);
+      regcache_cooked_write_unsigned (dsd->regs, AARCH64_LR_REGNUM,
+                                     data->insn_addr + 4);
     }
+  else
+    aarch64_emit_insn (dsd->insn_buf, insn);
+  dsd->insn_count = 1;
+
+  if (masked_insn == RET || masked_insn == BR || masked_insn == BLR)
+    dsd->dsc->pc_adjust = 0;
   else
     dsd->dsc->pc_adjust = 4;
 }

diff --git a/gdb/arch/aarch64-insn.h b/gdb/arch/aarch64-insn.h
index 1e8c5eac940ee017f4b6d2685bbcbaeed0617e3a..6f9ec8572b22c1d61ad96b317090da36dd14f27d 100644 (file)
--- a/gdb/arch/aarch64-insn.h
+++ b/gdb/arch/aarch64-insn.h
@@ -61,7 +61,9 @@ enum aarch64_opcodes
   CBNZ            = 0x21000000 | B,
   TBZ             = 0x36000000 | B,
   TBNZ            = 0x37000000 | B,
+  /* BR             1101 0110 0001 1111 0000 00rr rrr0 0000 */
   /* BLR            1101 0110 0011 1111 0000 00rr rrr0 0000 */
+  BR              = 0xd61f0000,
   BLR             = 0xd63f0000,
   /* RET            1101 0110 0101 1111 0000 00rr rrr0 0000 */
   RET             = 0xd65f0000,
@@ -128,6 +130,13 @@ enum aarch64_opcodes
   NOP             = (0 << 5) | HINT,
 };
 
+/* List of useful masks.  */
+enum aarch64_masks
+{
+  /* Used for masking out an Rn argument from an opcode.  */
+  CLEAR_Rn_MASK = 0xfffffc1f,
+};
+
 /* Representation of a general purpose register of the form xN or wN.
 
    This type is used by emitting functions that take registers as operands.  */

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index ec5db0612e0bb5f4eeb9a6b8a69aa4289c89be86..f20f041878ee0d1cf8860897abff7d772309c85e 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2021-01-27  Matthew Malcomson  <matthew.malcomson@arm.com>
+
+       * gdb.arch/insn-reloc.c: Add tests for BR and BLR.
+
 2021-01-26  Tom de Vries  <tdevries@suse.de>
 
        * gdb.threads/killed-outside.exp: Allow regular output.

diff --git a/gdb/testsuite/gdb.arch/insn-reloc.c b/gdb/testsuite/gdb.arch/insn-reloc.c
index bfbb3161b3aa3dcf8a5ff00e4341c4d68e473f26..62f13a9275439c8e2abdedd5cfbd88d06d863d03 100644 (file)
--- a/gdb/testsuite/gdb.arch/insn-reloc.c
+++ b/gdb/testsuite/gdb.arch/insn-reloc.c
@@ -512,6 +512,84 @@ can_relocate_bl (void)
        : : : "x30"); /* Test that LR is updated correctly.  */
 }
 
+/* Make sure we can relocate a BR instruction.
+
+     ... Set x0 to target
+   set_point12:
+     BR x0 ; jump to target (tracepoint here).
+     fail()
+     return
+   target:
+     pass()
+   end
+
+   */
+
+static void
+can_relocate_br (void)
+{
+  int ok = 0;
+
+  asm goto ("  adr x0, %l0\n"
+            "set_point12:\n"
+            "  br x0\n"
+            :
+            :
+            : "x0"
+            : madejump);
+
+  fail ();
+  return;
+madejump:
+  pass ();
+}
+
+/* Make sure we can relocate a BLR instruction.
+
+   We use two different functions since the test runner expects one breakpoint
+   per function and we want to test two different things.
+   For BLR we want to test that the BLR actually jumps to the relevant
+   function, *and* that it sets the LR register correctly.
+
+   Hence we create one testcase that jumps to `pass` using BLR, and one
+   testcase that jumps to `pass` if BLR has set the LR correctly.
+
+  -- can_relocate_blr_jumps
+     ... Set x0 to pass
+   set_point13:
+     BLR x0        ; jump to pass (tracepoint here).
+
+  -- can_relocate_blr_sets_lr
+     ... Set x0 to foo
+   set_point14:
+     BLR x0        ; jumps somewhere else (tracepoint here).
+     BL pass       ; ensures the LR was set correctly by the BLR.
+
+   */
+
+static void
+can_relocate_blr_jumps (void)
+{
+  int ok = 0;
+
+  /* Test BLR indeed jumps to the target.  */
+  asm ("set_point13:\n"
+       "  blr %[address]\n"
+       : : [address] "r" (&pass) : "x30");
+}
+
+static void
+can_relocate_blr_sets_lr (void)
+{
+  int ok = 0;
+
+  /* Test BLR sets the LR correctly.  */
+  asm ("set_point14:\n"
+       "  blr %[address]\n"
+       "  bl pass\n"
+       : : [address] "r" (&foo) : "x30");
+}
+
 #endif
 
 /* Functions testing relocations need to be placed here.  GDB will read
@@ -536,6 +614,9 @@ static testcase_ftype testcases[] = {
   can_relocate_ldr,
   can_relocate_bcond_false,
   can_relocate_bl,
+  can_relocate_br,
+  can_relocate_blr_jumps,
+  can_relocate_blr_sets_lr,
 #endif
 };