Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Yao Qi <qiyaoltc@gmail.com>
To: Pedro Alves <palves@redhat.com>
Cc: Yao Qi <qiyaoltc@gmail.com>,  gdb-patches@sourceware.org
Subject: Re: [PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers
Date: Mon, 16 Mar 2015 17:34:00 -0000	[thread overview]
Message-ID: <86r3sog82r.fsf@gmail.com> (raw)
In-Reply-To: <55070AED.5020502@redhat.com> (Pedro Alves's message of "Mon, 16	Mar 2015 16:55:09 +0000")

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" {


      reply	other threads:[~2015-03-16 17:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-13 14:41 Yao Qi
2015-03-13 14:41 ` [PATCH 2/2] watchpoint-reuse-slot.exp: skip when requesting two breakpoints in one slot on aarch64 Yao Qi
2015-03-16 12:35   ` Pedro Alves
2015-03-16 14:10     ` Yao Qi
2015-03-16 12:35 ` [PATCH 1/2] watchpoint-reuse-slot.exp: skip some tests on targets have different wp and bp registers Pedro Alves
2015-03-16 14:01   ` Yao Qi
2015-03-16 15:23     ` Pedro Alves
2015-03-16 16:37       ` Yao Qi
2015-03-16 16:55         ` Pedro Alves
2015-03-16 17:34           ` Yao Qi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86r3sog82r.fsf@gmail.com \
    --to=qiyaoltc@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox