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

[Pedro Alves <palves@redhat.com>]

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