[PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers

[PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers

[Yao Qi]

From: Yao Qi <yao.qi@linaro.org>

watchpoint-reuse-slot.exp sets two hardware breakpoint and/or watchpoint,
to test the same debugging register can be used correctly.  However,
on some targets, such as arm and aarch64, hardware has different
registers for breakpoint and watchpoint, so don't have to do test
if one breakpoint and one watchpoint are requested and target hardware
has different debugging registers for breakpoint and watchpoint.

gdb/testsuite:

2015-03-13  Yao Qi  <yao.qi@linaro.org>

	* gdb.base/watchpoint-reuse-slot.exp (dbg_registers_for_watch_and_break):
	New proc.
	(top level): Skip tests if breakpoint and watchpoint are
	requested and dbg_registers_for_watch_and_break returns false.
---
 gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
index 844bf3a..df6eeb6 100644
--- a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
@@ -157,6 +157,19 @@ proc watch_command {cmd base offset width} {
     }
 }
 
+# Return true if the same debugging register can be used for both
+# watchpoint and breakpoint.
+
+proc dbg_registers_for_watch_and_break {} {
+    if { [istarget "arm*-linux*"] || [istarget "aarch64*-*-linux*"] } {
+	# arm and aarch64 has different registers for watchpoint and
+	# breakpoint.
+	return 0
+    }
+
+    return 1
+}
+
 # Run test proper.  See intro for description.
 
 foreach always_inserted {"off" "on" } {
@@ -171,6 +184,16 @@ foreach always_inserted {"off" "on" } {
 		    continue
 		}
 
+		if {($cmd1 == "hbreak" && $cmd2 != "hbreak"
+		     || $cmd1 != "hbreak" && $cmd2 == "hbreak")
+		    && ![dbg_registers_for_watch_and_break]} {
+		    # One breakpoint and watchpoint is requested, but
+		    # different registers are used for breakpoint and
+		    # watchpoint, then, the same slot can't be reused.
+		    # Skip it.
+		    continue
+		}
+
 		for {set x 0} {$x < 4} {incr x} {
 		    set prefix "always-inserted $always_inserted: "
 		    append prefix "$cmd1 x $cmd2: "
-- 
1.9.1

[PATCH 2/2] watchpoint-reuse-slot.exp: skip when requesting two breakpoints in one slot on aarch64

[Yao Qi]

From: Yao Qi <yao.qi@linaro.org>

watchpoint-reuse-slot.exp sets two hardware breakpoints in contiguous
address to reuse one debug register.  However, requested address for
breakpoint should be 4-byte aligned on aarch64, so it is impossible
to request two hardware breakpoints and use one debug register.  This
patch is to skip the test in the case on aarch64-linux.  Since arm
has Thumb mode, in which instruction address can be 2-byte aligned,
more thoughts are needed for arm in my next step.

gdb/testsuite:

2015-03-13  Yao Qi  <yao.qi@linaro.org>

	* gdb.base/watchpoint-reuse-slot.exp: Skip tests when two
	breakpoints are requested on aarch64-linux.
---
 gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
index df6eeb6..c646f22 100644
--- a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
@@ -178,10 +178,18 @@ foreach always_inserted {"off" "on" } {
 	foreach cmd2 $cmds {
 	    for {set width 1} {$width < 4} {incr width} {
 
-		if {$cmd1 == "hbreak" && $cmd2 == "hbreak" && $width > 1} {
-		    # hbreak ignores WIDTH, no use testing more than
-		    # once.
-		    continue
+		if {$cmd1 == "hbreak" && $cmd2 == "hbreak"} {
+		    if {$width > 1} {
+			# hbreak ignores WIDTH, no use testing more than
+			# once.
+			continue
+		    }
+
+		    if { [istarget "aarch64*-*-linux*"] } {
+			# The address for breakpoint should be 4-byte
+			# aligned, so can't reuse slot.
+			continue
+		    }
 		}
 
 		if {($cmd1 == "hbreak" && $cmd2 != "hbreak"
-- 
1.9.1

Re: [PATCH 2/2] watchpoint-reuse-slot.exp: skip when requesting two breakpoints in one slot on aarch64

[Pedro Alves]

On 03/13/2015 02:41 PM, Yao Qi wrote:
> From: Yao Qi <yao.qi@linaro.org>
> 
> watchpoint-reuse-slot.exp sets two hardware breakpoints in contiguous
> address to reuse one debug register.  However, requested address for
> breakpoint should be 4-byte aligned on aarch64, so it is impossible
> to request two hardware breakpoints and use one debug register.  This
> patch is to skip the test in the case on aarch64-linux.  Since arm
> has Thumb mode, in which instruction address can be 2-byte aligned,
> more thoughts are needed for arm in my next step.

So I take it that GDB complains when the user tries this, and thus
the test is failing on aarch64?  What does the log show?

Thanks,
Pedro Alves

Re: [PATCH 2/2] watchpoint-reuse-slot.exp: skip when requesting two breakpoints in one slot on aarch64

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> So I take it that GDB complains when the user tries this, and thus
> the test is failing on aarch64?  What does the log show?

Yes, some tests fail on aarch64.

stepi^M
Warning:^M
Cannot insert hardware breakpoint 224.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M
^M
(gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: hbreak x hbreak: : width 1, iter 0: base + 1: stepi advanced

hbreak *(buf.byte + 0 + 1)^M
Hardware assisted breakpoint 448 at 0x410a61^M
Warning:^M
Cannot insert hardware breakpoint 448.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M
^M
(gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)

all the fails this patch fixes are:
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: hbreak x hbreak: : width 1, iter 0: base + 1: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: hbreak x hbreak: : width 1, iter 1: base + 0: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: hbreak x hbreak: : width 1, iter 1: base + 1: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: hbreak x hbreak: : width 1, iter 2: base + 0: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: hbreak x hbreak: : width 1, iter 2: base + 1: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: hbreak x hbreak: : width 1, iter 3: base + 0: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 0: base + 1: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 1: base + 0: hbreak *(buf.byte + 1 + 0)
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 1: base + 0: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 1: base + 1: hbreak *(buf.byte + 1 + 1)
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 1: base + 1: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 2: base + 0: hbreak *(buf.byte + 2 + 0)
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 2: base + 0: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 2: base + 1: hbreak *(buf.byte + 2 + 1)
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 2: base + 1: stepi advanced
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 3: base + 0: hbreak *(buf.byte + 3 + 0)
FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: hbreak x hbreak: : width 1, iter 3: base + 0: stepi advanced

-- 
Yao (齐尧)

Re: [PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers

[Pedro Alves]

On 03/13/2015 02:41 PM, Yao Qi wrote:
> From: Yao Qi <yao.qi@linaro.org>
> 
> watchpoint-reuse-slot.exp sets two hardware breakpoint and/or watchpoint,
> to test the same debugging register can be used correctly.  However,
> on some targets, such as arm and aarch64, hardware has different
> registers for breakpoint and watchpoint, so don't have to do test
> if one breakpoint and one watchpoint are requested and target hardware
> has different debugging registers for breakpoint and watchpoint.

Hmm, is this just to save test time?  If so, I'd prefer not skipping,
as it may always catch other bugs, in the target backends or
the kernel.

Despite the test's file name, the test doesn't actually create two
breakpoints/watchpoints at the same time, as mentioned at the top
of the file.

In any case:

> +# Return true if the same debugging register can be used for both

s/can be/is/

> +# watchpoint and breakpoint.
> +
> +proc dbg_registers_for_watch_and_break {} {

I think "same" should be in the proc name:

 same_dbg_registers_for_watch_and_break

> +    if { [istarget "arm*-linux*"] || [istarget "aarch64*-*-linux*"] } {
> +	# arm and aarch64 has different registers for watchpoint and

s/has/have/

Thanks,
Pedro Alves

Re: [PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Hmm, is this just to save test time?  If so, I'd prefer not skipping,
> as it may always catch other bugs, in the target backends or
> the kernel.

No, watchpoint-reuse-slot.exp sets some HW breakpoint/watchpoint on some
address doesn't meet the alignment requirements by kernel, kernel
will reject the ptrace (PTRACE_SETHBPREGS) call, and some fails are
caused, for example:

(gdb) PASS: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 0: delete $bpnum
hbreak *(buf.byte + 0 + 1)^M
Hardware assisted breakpoint 80 at 0x410a61^M
(gdb) PASS: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)
stepi^M
Warning:^M
Cannot insert hardware breakpoint 80.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M
^M
(gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 1: stepi advanced

hbreak *(buf.byte + 0 + 1)^M
Hardware assisted breakpoint 440 at 0x410a61^M
Warning:^M
Cannot insert hardware breakpoint 440.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M
^M
(gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: watch x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)

Do you suggest that we don't skip these tests even requested
breakpoint/watchpoint don't go in the same slot (debugging register)? so
that the test can cover more.  If the requested address of HW
breakpoint/watchpoint doesn't meet the arch/kernel requirements, we can
skip it, is it OK?

The inner loop of test has two parts, "base + 0" and "base + 1",

		    append prefix "$cmd1 x $cmd2: "
		    with_test_prefix "$prefix: width $width, iter $x" {
			with_test_prefix "base + 0" {
			    watch_command $cmd1 $x 0 $width
			    stepi
			    gdb_test_no_output "delete \$bpnum"
			}
			with_test_prefix "base + 1" {
			    watch_command $cmd2 $x 1 $width
			    stepi
			    gdb_test_no_output "delete \$bpnum"
			}
		    }

if we skip "base + 1" part, do we skip "base + 0" too? if not, prefix in
test summary "$cmd1 x $cmd2: " doesn't reflect the fact.

>
> Despite the test's file name, the test doesn't actually create two
> breakpoints/watchpoints at the same time, as mentioned at the top
> of the file.

Yes, only one breakpoint/watchpoint is inserted at a time.

-- 
Yao (齐尧)

Re: [PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers

[Pedro Alves]

On 03/16/2015 02:01 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> Hmm, is this just to save test time?  If so, I'd prefer not skipping,
>> as it may always catch other bugs, in the target backends or
>> the kernel.
> 
> No, watchpoint-reuse-slot.exp sets some HW breakpoint/watchpoint on some
> address doesn't meet the alignment requirements by kernel, kernel
> will reject the ptrace (PTRACE_SETHBPREGS) call, and some fails are
> caused, for example:

OK, then different wp and bp registers really is an orthogonal
predicate.  A better one is around the alignment requirements
of a breakpoint.

> 
> (gdb) PASS: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 0: delete $bpnum
> hbreak *(buf.byte + 0 + 1)^M
> Hardware assisted breakpoint 80 at 0x410a61^M
> (gdb) PASS: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)
> stepi^M
> Warning:^M
> Cannot insert hardware breakpoint 80.^M
> Could not insert hardware breakpoints:^M
> You may have requested too many hardware breakpoints/watchpoints.^M
> ^M
> (gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 1: stepi advanced
> 
> hbreak *(buf.byte + 0 + 1)^M
> Hardware assisted breakpoint 440 at 0x410a61^M
> Warning:^M
> Cannot insert hardware breakpoint 440.^M
> Could not insert hardware breakpoints:^M
> You may have requested too many hardware breakpoints/watchpoints.^M
> ^M
> (gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: watch x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)
> 
> Do you suggest that we don't skip these tests even requested
> breakpoint/watchpoint don't go in the same slot (debugging register)?

Yes.

> so
> that the test can cover more.  If the requested address of HW
> breakpoint/watchpoint doesn't meet the arch/kernel requirements, we can
> skip it, is it OK?

Yes, that makes sense.

> 
> The inner loop of test has two parts, "base + 0" and "base + 1",
> 
> 		    append prefix "$cmd1 x $cmd2: "
> 		    with_test_prefix "$prefix: width $width, iter $x" {
> 			with_test_prefix "base + 0" {
> 			    watch_command $cmd1 $x 0 $width
> 			    stepi
> 			    gdb_test_no_output "delete \$bpnum"
> 			}
> 			with_test_prefix "base + 1" {
> 			    watch_command $cmd2 $x 1 $width
> 			    stepi
> 			    gdb_test_no_output "delete \$bpnum"
> 			}
> 		    }
> 
> if we skip "base + 1" part, do we skip "base + 0" too? if not, prefix in
> test summary "$cmd1 x $cmd2: " doesn't reflect the fact.

I think skipping both is fine.

Thanks,
Pedro Alves

Re: [PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Yes, that makes sense.
>
>> 
>> The inner loop of test has two parts, "base + 0" and "base + 1",
>> 
>> 		    append prefix "$cmd1 x $cmd2: "
>> 		    with_test_prefix "$prefix: width $width, iter $x" {
>> 			with_test_prefix "base + 0" {
>> 			    watch_command $cmd1 $x 0 $width
>> 			    stepi
>> 			    gdb_test_no_output "delete \$bpnum"
>> 			}
>> 			with_test_prefix "base + 1" {
>> 			    watch_command $cmd2 $x 1 $width
>> 			    stepi
>> 			    gdb_test_no_output "delete \$bpnum"
>> 			}
>> 		    }
>> 
>> if we skip "base + 1" part, do we skip "base + 0" too? if not, prefix in
>> test summary "$cmd1 x $cmd2: " doesn't reflect the fact.
>
> I think skipping both is fine.

OK, how about the patch below?

-- 
Yao (齐尧)


From: Yao Qi <yao.qi@linaro.org>
Subject: [PATCH] watchpoint-reuse-slot.exp: skip setting HW breakpoints on some address

We see some fails in watchpoint-reuse-slot.exp on aarch64-linux, because
it sets some HW breakpoint on some address doesn't meet the alignment
requirements by kernel, kernel will reject the
ptrace (PTRACE_SETHBPREGS) call, and some fails are caused, for example:

(gdb) PASS: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 0: delete $bpnum
hbreak *(buf.byte + 0 + 1)^M
Hardware assisted breakpoint 80 at 0x410a61^M
(gdb) PASS: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)
stepi^M
Warning:^M
Cannot insert hardware breakpoint 80.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M
^M
(gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 1: stepi advanced

hbreak *(buf.byte + 0 + 1)^M
Hardware assisted breakpoint 440 at 0x410a61^M
Warning:^M
Cannot insert hardware breakpoint 440.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M
^M
(gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: watch x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)

This patch is to skip some tests by checking proc valid_addr_p.
We can handle other targets in valid_addr_p too.

gdb/testsuite:

2015-03-16  Yao Qi  <yao.qi@linaro.org>

	* gdb.base/watchpoint-reuse-slot.exp: Get the address of
	buf.byte.
	(valid_addr_p): New proc.
	(top level): Skip tests if valid_addr_p returns false for
	$cmd1 or $cmd2.

diff --git a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
index 844bf3a..28839fe 100644
--- a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
@@ -133,6 +133,39 @@ delete_breakpoints
 
 set cur_addr [get_pc]
 
+# The addrss of buf.byte
+set buf_byte_addr ""
+set test "get address of buf.byte"
+gdb_test_multiple "p /d &buf.byte" "$test" {
+    -re " = ($decimal).*$gdb_prompt $" {
+	set buf_byte_addr $expect_out(1,string)
+	pass "$test"
+    }
+}
+
+# Return true if the memory range [buf.byte + OFFSET, +WIDTH] can be
+# monitored by CMD, otherwise return false.
+
+proc  valid_addr_p {cmd offset width} {
+    global buf_byte_addr
+
+    set addr [expr $buf_byte_addr + $offset]
+    if { [istarget "aarch64*-*-linux*"] } {
+	# aarch64 linux kernel accepts 4-byte aligned address for
+	# hardware breakpoint and 8-byte aligned address for hardware
+	# watchpoint.  However, GDB and GDBserver use one or more
+	# watchpoint registers to represent the whole unaligned region
+	# while each individual is properly aligned.
+	if {$cmd == "hbreak" } {
+	    if { [expr $addr % 4] != 0 } {
+		return 0
+	    }
+	}
+    }
+
+    return 1
+}
+
 # Watch WIDTH bytes at BASE + OFFSET.  CMD specifices the specific
 # type of watchpoint to use.  If CMD is "hbreak", WIDTH is ignored.
 
@@ -172,6 +205,15 @@ foreach always_inserted {"off" "on" } {
 		}
 
 		for {set x 0} {$x < 4} {incr x} {
+
+		    if { ![valid_addr_p $cmd1 $x $width]
+			 || ![valid_addr_p $cmd2 $x+1 $width] } {
+			# Skip tests if requested address or length
+			# of breakpoint or watchpoint don't meet
+			# target or kernel requirements.
+			continue
+		    }
+
 		    set prefix "always-inserted $always_inserted: "
 		    append prefix "$cmd1 x $cmd2: "
 		    with_test_prefix "$prefix: width $width, iter $x" {

Re: [PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers

[Pedro Alves]

On 03/16/2015 04:37 PM, Yao Qi wrote:
> gdb/testsuite:
> 
> 2015-03-16  Yao Qi  <yao.qi@linaro.org>
> 
> 	* gdb.base/watchpoint-reuse-slot.exp: Get the address of
> 	buf.byte.
> 	(valid_addr_p): New proc.
> 	(top level): Skip tests if valid_addr_p returns false for
> 	$cmd1 or $cmd2.

Thanks.  This one looks good to me, with the nits below addressed.

> 
> diff --git a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
> index 844bf3a..28839fe 100644
> --- a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
> +++ b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
> @@ -133,6 +133,39 @@ delete_breakpoints
>  
>  set cur_addr [get_pc]
>  
> +# The addrss of buf.byte

"address".  Missing period.

> +set buf_byte_addr ""
> +set test "get address of buf.byte"
> +gdb_test_multiple "p /d &buf.byte" "$test" {
> +    -re " = ($decimal).*$gdb_prompt $" {
> +	set buf_byte_addr $expect_out(1,string)
> +	pass "$test"
> +    }
> +}

Fine with me to add this (some port might need it), but note you
don't actually need it if you're only looking for the alignment,
because 'buf' is always aligned, as that is the whole point of
the union:

union aligned_buf
{
  char byte[12];

  /* So that testing consistently starts on an aligned address.  */
  unsigned long long force_align;
};

> +
> +# Return true if the memory range [buf.byte + OFFSET, +WIDTH] can be
> +# monitored by CMD, otherwise return false.
> +
> +proc  valid_addr_p {cmd offset width} {
       ^^

Spurious double space.

> +    global buf_byte_addr
> +
> +    set addr [expr $buf_byte_addr + $offset]
> +    if { [istarget "aarch64*-*-linux*"] } {
> +	# aarch64 linux kernel accepts 4-byte aligned address for
> +	# hardware breakpoint and 8-byte aligned address for hardware
> +	# watchpoint.  However, GDB and GDBserver use one or more
> +	# watchpoint registers to represent the whole unaligned region
> +	# while each individual is properly aligned.

Suggest:

	# The aarch64 Linux kernel port only accepts 4-byte aligned addresses
        # for hardware breakpoints and 8-byte aligned addresses for hardware
	# watchpoints.  However, both GDB and GDBserver support unaligned
        # watchpoints by using more than one properly aligned watchpoint
        # register to represent the whole unaligned region.  Breakpoint
        # addresses must still be aligned though.

> +	if {$cmd == "hbreak" } {
> +	    if { [expr $addr % 4] != 0 } {
> +		return 0
> +	    }
> +	}
> +    }
> +
> +    return 1
> +}
> +
>  # Watch WIDTH bytes at BASE + OFFSET.  CMD specifices the specific
>  # type of watchpoint to use.  If CMD is "hbreak", WIDTH is ignored.
>  
> @@ -172,6 +205,15 @@ foreach always_inserted {"off" "on" } {
>  		}
>  
>  		for {set x 0} {$x < 4} {incr x} {
> +
> +		    if { ![valid_addr_p $cmd1 $x $width]
> +			 || ![valid_addr_p $cmd2 $x+1 $width] } {
> +			# Skip tests if requested address or length
> +			# of breakpoint or watchpoint don't meet
> +			# target or kernel requirements.
> +			continue
> +		    }
> +
>  		    set prefix "always-inserted $always_inserted: "
>  		    append prefix "$cmd1 x $cmd2: "
>  		    with_test_prefix "$prefix: width $width, iter $x" {


Thanks,
Pedro Alves

Re: [PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Fine with me to add this (some port might need it), but note you
> don't actually need it if you're only looking for the alignment,
> because 'buf' is always aligned, as that is the whole point of
> the union:
>
> union aligned_buf
> {
>   char byte[12];
>
>   /* So that testing consistently starts on an aligned address.  */
>   unsigned long long force_align;
> };
>

OK, let us start from the easy one, checking offset only.

>> +
>> +# Return true if the memory range [buf.byte + OFFSET, +WIDTH] can be
>> +# monitored by CMD, otherwise return false.
>> +
>> +proc  valid_addr_p {cmd offset width} {
>        ^^
>
> Spurious double space.
>

Fixed.

>> +    global buf_byte_addr
>> +
>> +    set addr [expr $buf_byte_addr + $offset]
>> +    if { [istarget "aarch64*-*-linux*"] } {
>> +	# aarch64 linux kernel accepts 4-byte aligned address for
>> +	# hardware breakpoint and 8-byte aligned address for hardware
>> +	# watchpoint.  However, GDB and GDBserver use one or more
>> +	# watchpoint registers to represent the whole unaligned region
>> +	# while each individual is properly aligned.
>
> Suggest:
>
> 	# The aarch64 Linux kernel port only accepts 4-byte aligned addresses
>         # for hardware breakpoints and 8-byte aligned addresses for hardware
> 	# watchpoints.  However, both GDB and GDBserver support unaligned
>         # watchpoints by using more than one properly aligned watchpoint
>         # register to represent the whole unaligned region.  Breakpoint
>         # addresses must still be aligned though.


That is better.  Thanks for tweaking the comments.  Patch below is
pushed in.

-- 
Yao (齐尧)

From: Yao Qi <yao.qi@linaro.org>
Subject: [PATCH] watchpoint-reuse-slot.exp: skip setting HW breakpoints on some address

We see some fails in watchpoint-reuse-slot.exp on aarch64-linux, because
it sets some HW breakpoint on some address doesn't meet the alignment
requirements by kernel, kernel will reject the
ptrace (PTRACE_SETHBPREGS) call, and some fails are caused, for example:

(gdb) PASS: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 0: delete $bpnum
hbreak *(buf.byte + 0 + 1)^M
Hardware assisted breakpoint 80 at 0x410a61^M
(gdb) PASS: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)
stepi^M
Warning:^M
Cannot insert hardware breakpoint 80.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M
^M
(gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted off: watch x hbreak: : width 1, iter 0: base + 1: stepi advanced

hbreak *(buf.byte + 0 + 1)^M
Hardware assisted breakpoint 440 at 0x410a61^M
Warning:^M
Cannot insert hardware breakpoint 440.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M
^M
(gdb) FAIL: gdb.base/watchpoint-reuse-slot.exp: always-inserted on: watch x hbreak: : width 1, iter 0: base + 1: hbreak *(buf.byte + 0 + 1)

This patch is to skip some tests by checking proc valid_addr_p.
We can handle other targets in valid_addr_p too.

gdb/testsuite:

2015-03-16  Yao Qi  <yao.qi@linaro.org>

	* gdb.base/watchpoint-reuse-slot.exp (valid_addr_p): New proc.
	(top level): Skip tests if valid_addr_p returns false for
	$cmd1 or $cmd2.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 76dcde5..c182e7c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2015-03-16  Yao Qi  <yao.qi@linaro.org>
+
+	* gdb.base/watchpoint-reuse-slot.exp (valid_addr_p): New proc.
+	(top level): Skip tests if valid_addr_p returns false for
+	$cmd1 or $cmd2.
+
 2015-03-11  Andy Wingo  <wingo@igalia.com>
 
 	* gdb.guile/scm-objfile.exp: Add objfile-progspace test.
diff --git a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
index 844bf3a..6d2c867 100644
--- a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
@@ -133,6 +133,28 @@ delete_breakpoints
 
 set cur_addr [get_pc]
 
+# Return true if the memory range [buf.byte + OFFSET, +WIDTH] can be
+# monitored by CMD, otherwise return false.
+
+proc valid_addr_p {cmd offset width} {
+
+    if { [istarget "aarch64*-*-linux*"] } {
+	# The aarch64 Linux kernel port only accepts 4-byte aligned addresses
+	# for hardware breakpoints and 8-byte aligned addresses for hardware
+	# watchpoints.  However, both GDB and GDBserver support unaligned
+	# watchpoints by using more than one properly aligned watchpoint
+	# registers to represent the whole unaligned region.  Breakpoint
+	# addresses must still be aligned though.
+	if {$cmd == "hbreak" } {
+	    if { [expr ($offset) % 4] != 0 } {
+		return 0
+	    }
+	}
+    }
+
+    return 1
+}
+
 # Watch WIDTH bytes at BASE + OFFSET.  CMD specifices the specific
 # type of watchpoint to use.  If CMD is "hbreak", WIDTH is ignored.
 
@@ -172,6 +194,15 @@ foreach always_inserted {"off" "on" } {
 		}
 
 		for {set x 0} {$x < 4} {incr x} {
+
+		    if { ![valid_addr_p $cmd1 $x $width]
+			 || ![valid_addr_p $cmd2 $x+1 $width] } {
+			# Skip tests if requested address or length
+			# of breakpoint or watchpoint don't meet
+			# target or kernel requirements.
+			continue
+		    }
+
 		    set prefix "always-inserted $always_inserted: "
 		    append prefix "$cmd1 x $cmd2: "
 		    with_test_prefix "$prefix: width $width, iter $x" {