[binutils-gdb] [AArch64] Only check breakpoint alignment on inserting

[Yao Qi <qiyao at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=805035d70cd8637c169caf97800accdd267d1d8e

commit 805035d70cd8637c169caf97800accdd267d1d8e
Author: Yao Qi <yao.qi@linaro.org>
Date:   Fri Nov 27 14:53:32 2015 +0000

    [AArch64] Only check breakpoint alignment on inserting
    
    This patch fixes the GDB internal error on AArch64 when running
    watchpoint-fork.exp
    
     top?bt 15
     internal_error (file=file@entry=0x79d558 "../../binutils-gdb/gdb/linux-nat.c", line=line@entry=4866, fmt=0x793b20 "%s: Assertion `%s' failed.")
        at ../../binutils-gdb/gdb/common/errors.c:51
     #1  0x0000000000495bc4 in linux_nat_thread_address_space (t=<optimized out>, ptid=<error reading variable: Cannot access memory at address 0x1302>)
        at ../../binutils-gdb/gdb/linux-nat.c:4866
     #2  0x00000000005db2c8 in delegate_thread_address_space (self=<optimized out>, arg1=<error reading variable: Cannot access memory at address 0x1302>)
        at ../../binutils-gdb/gdb/target-delegates.c:2447
     #3  0x00000000005e8c7c in target_thread_address_space (ptid=<error reading variable: Cannot access memory at address 0x1302>)
        at ../../binutils-gdb/gdb/target.c:2727
     #4  0x000000000054eef8 in get_thread_arch_regcache (ptid=..., gdbarch=0xad51e0) at ../../binutils-gdb/gdb/regcache.c:529
     #5  0x000000000054efcc in get_thread_regcache (ptid=...) at ../../binutils-gdb/gdb/regcache.c:546
     #6  0x000000000054f120 in get_thread_regcache_for_ptid (ptid=...) at ../../binutils-gdb/gdb/regcache.c:560
     #7  0x00000000004a2278 in aarch64_point_is_aligned (is_watchpoint=0, addr=34168, len=2) at ../../binutils-gdb/gdb/nat/aarch64-linux-hw-point.c:122
     #8  0x00000000004a2e68 in aarch64_handle_breakpoint (type=hw_execute, addr=34168, len=2, is_insert=0, state=0xae8880)
        at ../../binutils-gdb/gdb/nat/aarch64-linux-hw-point.c:465
     #9  0x000000000048edf0 in aarch64_linux_remove_hw_breakpoint (self=<optimized out>, gdbarch=<optimized out>, bp_tgt=<optimized out>)
        at ../../binutils-gdb/gdb/aarch64-linux-nat.c:657
     #10 0x00000000005da8dc in delegate_remove_hw_breakpoint (self=<optimized out>, arg1=<optimized out>, arg2=<optimized out>)
        at ../../binutils-gdb/gdb/target-delegates.c:492
     #11 0x0000000000536a24 in bkpt_remove_location (bl=<optimized out>) at ../../binutils-gdb/gdb/breakpoint.c:13065
     #12 0x000000000053351c in remove_breakpoint_1 (bl=0xb3fe70, is=is@entry=mark_inserted) at ../../binutils-gdb/gdb/breakpoint.c:4026
     #13 0x000000000053ccc0 in detach_breakpoints (ptid=...) at ../../binutils-gdb/gdb/breakpoint.c:3930
     #14 0x00000000005a3ac0 in handle_inferior_event_1 (ecs=0x7ffffff048) at ../../binutils-gdb/gdb/infrun.c:5042
    
    After the fork, GDB will physically remove the breakpoints from the child
    process (in frame #14), but at that time, GDB doesn't create an inferior
    yet for child, but inferior_ptid is set to child's ptid (in frame #13).
    In aarch64_point_is_aligned, we'll get the regcache of current_lwp_ptid
    to determine if the current process is 32-bit or 64-bit, so the inferior
    can't be found, and the internal error is caused.
    
    I don't find a better fix other than not checking alignment on removing
    breakpoint.
    
    gdb:
    
    2015-11-27  Yao Qi  <yao.qi@linaro.org>
    
    	* nat/aarch64-linux-hw-point.c (aarch64_dr_state_remove_one_point):
    	Don't assert on alignment.
    	(aarch64_handle_breakpoint): Only check alignment when IS_INSERT
    	is true.

Diff:
---
 gdb/ChangeLog                    |  7 +++++++
 gdb/nat/aarch64-linux-hw-point.c | 21 ++++++++++++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e883ffc..54642e1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2015-11-27  Yao Qi  <yao.qi@linaro.org>
 
+	* nat/aarch64-linux-hw-point.c (aarch64_dr_state_remove_one_point):
+	Don't assert on alignment.
+	(aarch64_handle_breakpoint): Only check alignment when IS_INSERT
+	is true.
+
+2015-11-27  Yao Qi  <yao.qi@linaro.org>
+
 	* aarch64-tdep.c (is_hfa): Rename to ...
 	(is_hfa_or_hva): ... this.  Handle vector type.  All callers
 	updated.
diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index dcbfa98..466823a 100644
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -411,7 +411,6 @@ aarch64_dr_state_remove_one_point (struct aarch64_debug_reg_state *state,
 
   /* Set up state pointers.  */
   is_watchpoint = (type != hw_execute);
-  gdb_assert (aarch64_point_is_aligned (is_watchpoint, addr, len));
   if (is_watchpoint)
     {
       num_regs = aarch64_num_wp_regs;
@@ -460,13 +459,21 @@ aarch64_handle_breakpoint (enum target_hw_bp_type type, CORE_ADDR addr,
 			   int len, int is_insert,
 			   struct aarch64_debug_reg_state *state)
 {
-  /* The hardware breakpoint on AArch64 should always be 4-byte
-     aligned, but on AArch32, it can be 2-byte aligned.  */
-  if (!aarch64_point_is_aligned (0 /* is_watchpoint */ , addr, len))
-    return -1;
-
   if (is_insert)
-    return aarch64_dr_state_insert_one_point (state, type, addr, len);
+    {
+      /* The hardware breakpoint on AArch64 should always be 4-byte
+	 aligned, but on AArch32, it can be 2-byte aligned.  Note that
+	 we only check the alignment on inserting breakpoint because
+	 aarch64_point_is_aligned needs the inferior_ptid inferior's
+	 regcache to decide whether the inferior is 32-bit or 64-bit.
+	 However when GDB follows the parent process and detach breakpoints
+	 from child process, inferior_ptid is the child ptid, but the
+	 child inferior doesn't exist in GDB's view yet.  */
+      if (!aarch64_point_is_aligned (0 /* is_watchpoint */ , addr, len))
+	return -1;
+
+      return aarch64_dr_state_insert_one_point (state, type, addr, len);
+    }
   else
     return aarch64_dr_state_remove_one_point (state, type, addr, len);
 }