[binutils-gdb] Adjust breakpoint address by clearing non-significant bits

[Yao Qi <qiyao at sourceware dot org>]

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

commit a0de8c21baf46c40ed8e62faef5f750b1e5453ea
Author: Yao Qi <yao.qi@linaro.org>
Date:   Fri Dec 8 17:27:03 2017 +0000

    Adjust breakpoint address by clearing non-significant bits
    
    Tag in tagged address on AArch64 is treated as a non-significant bits of
    address, which can be got by gdbarch method significant_addr_bit, and gdb
    can clear these bits.
    
    With this patch, when user sets a breakpoint on tagged address on AArch64,
    GDB will drop the top byte of address, and put breakpoint at the new place,
    as shown below,
    
    (gdb) hbreak *func_ptr
    warning: Breakpoint address adjusted from 0xf000000000400690 to 0x00400690.
    Hardware assisted breakpoint 2 at 0x400690
    
    (gdb) break *func_ptr
    warning: Breakpoint address adjusted from 0xf000000000400690 to 0x00400690.
    Breakpoint 3 at 0x400690
    
    When program hits a breakpoint, the stopped pc reported by Linux kernel is
    the address *without* tag, so it is better the address recorded in
    breakpoint location is the one without tag too, so we can still match
    breakpoint location address and stopped pc reported by Linux kernel, by
    simple compare.
    
    gdb:
    
    2017-12-08  Yao Qi  <yao.qi@linaro.org>
    
    	* breakpoint.c (adjust_breakpoint_address): Call
    	address_significant.
    
    gdb/testsuite:
    
    2017-12-08  Yao Qi  <yao.qi@linaro.org>
    
    	* gdb.arch/aarch64-tagged-pointer.c (main): Update.
    	* gdb.arch/aarch64-tagged-pointer.exp: Add test for breakpoint.

Diff:
---
 gdb/ChangeLog                                     |  5 ++++
 gdb/breakpoint.c                                  | 28 +++++++++++------------
 gdb/testsuite/ChangeLog                           |  5 ++++
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c   |  8 +++++++
 gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp | 24 +++++++++++++++++++
 5 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 952a2ff..134f7b4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-12-08  Yao Qi  <yao.qi@linaro.org>
 
+	* breakpoint.c (adjust_breakpoint_address): Call
+	address_significant.
+
+2017-12-08  Yao Qi  <yao.qi@linaro.org>
+
 	* aarch64-tdep.c (aarch64_gdbarch_init): Install gdbarch
 	significant_addr_bit.
 	* gdbarch.sh (significant_addr_bit): New.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b4353d2..1cee730 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6973,16 +6973,11 @@ static CORE_ADDR
 adjust_breakpoint_address (struct gdbarch *gdbarch,
 			   CORE_ADDR bpaddr, enum bptype bptype)
 {
-  if (!gdbarch_adjust_breakpoint_address_p (gdbarch))
-    {
-      /* Very few targets need any kind of breakpoint adjustment.  */
-      return bpaddr;
-    }
-  else if (bptype == bp_watchpoint
-           || bptype == bp_hardware_watchpoint
-           || bptype == bp_read_watchpoint
-           || bptype == bp_access_watchpoint
-           || bptype == bp_catchpoint)
+  if (bptype == bp_watchpoint
+      || bptype == bp_hardware_watchpoint
+      || bptype == bp_read_watchpoint
+      || bptype == bp_access_watchpoint
+      || bptype == bp_catchpoint)
     {
       /* Watchpoints and the various bp_catch_* eventpoints should not
          have their addresses modified.  */
@@ -7000,11 +6995,16 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
     }
   else
     {
-      CORE_ADDR adjusted_bpaddr;
+      CORE_ADDR adjusted_bpaddr = bpaddr;
+
+      if (gdbarch_adjust_breakpoint_address_p (gdbarch))
+	{
+	  /* Some targets have architectural constraints on the placement
+	     of breakpoint instructions.  Obtain the adjusted address.  */
+	  adjusted_bpaddr = gdbarch_adjust_breakpoint_address (gdbarch, bpaddr);
+	}
 
-      /* Some targets have architectural constraints on the placement
-         of breakpoint instructions.  Obtain the adjusted address.  */
-      adjusted_bpaddr = gdbarch_adjust_breakpoint_address (gdbarch, bpaddr);
+      adjusted_bpaddr = address_significant (gdbarch, adjusted_bpaddr);
 
       /* An adjusted breakpoint address can significantly alter
          a user's expectations.  Print a warning if an adjustment
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index cc834de..17fbe4f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2017-12-08  Yao Qi  <yao.qi@linaro.org>
 
+	* gdb.arch/aarch64-tagged-pointer.c (main): Update.
+	* gdb.arch/aarch64-tagged-pointer.exp: Add test for breakpoint.
+
+2017-12-08  Yao Qi  <yao.qi@linaro.org>
+
 	* gdb.arch/aarch64-tagged-pointer.c: New file.
 	* gdb.arch/aarch64-tagged-pointer.exp: New file.
 
diff --git a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
index 7c90132..9bfe41e 100644
--- a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
+++ b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.c
@@ -45,4 +45,12 @@ main (void)
   void (*func_ptr) (void) = foo;
   func_ptr = (void (*) (void)) ((uintptr_t) func_ptr | 0xf000000000000000ULL);
   sp2->i = 4321; /* breakpoint here.  */
+
+  for (int i = 0; i < 2; i++)
+    {
+      foo ();
+      (*func_ptr) ();
+    }
+
+  sp1->i = 8765;
 }
diff --git a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
index 4f2b44c..fcab1b7 100644
--- a/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
+++ b/gdb/testsuite/gdb.arch/aarch64-tagged-pointer.exp
@@ -65,3 +65,27 @@ gdb_test_multiple $test $test {
 
 gdb_test "disassemble func_ptr,+8" \
     ":\[\t \]+$insn1\[ \r\n\]+.*:\[\t \]+$insn2.*"
+
+foreach_with_prefix bptype {"hbreak" "break"} {
+
+    # Set a breakpoint on a tagged address, func_ptr,
+    gdb_test "$bptype *func_ptr" \
+	"warning: Breakpoint address adjusted from .*reakpoint $decimal at .*" \
+	"breakpoint at *func_ptr"
+    # Resume the program and expect it hits foo,
+    gdb_test "continue" \
+	"Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\) at .*" \
+	"run until breakpoint set *func_ptr"
+    gdb_test "up" "foo \\(\\).*" "caller is foo"
+    delete_breakpoints
+
+    # Set a breakpoint on normal function, call it through tagged
+    # function pointer.
+    gdb_test "$bptype foo" "reakpoint $decimal at .*" \
+	"hardware breakpoint at foo"
+    gdb_test "continue" \
+	"Continuing\\..*Breakpoint \[0-9\]+, foo \\(\\) at .*" \
+	"run until breakpoint set foo"
+    gdb_test "up" "\\(\*func_ptr\\) \\(\\).*" "caller is *func_ptr"
+    delete_breakpoints
+}