Re: [PING] [PATCH] Enable hardware watchpoints on attach for aarch64

[Simon Marchi <simon dot marchi at polymtl dot ca>]

On 2018-06-07 09:19, Alan Hayward wrote:
> Aarch64 changes look good to me.
> I’m not up to speed enough with .exp files to comment on that part.
> I tested it on an aarch64 box, and it works fine for me.

Hi,

I looked at the test, if Alan is fine with the code changes then I am 
fine with them.  I noted some nits, but otherwise it looks good.  Please 
push after addressing them.

> > > +# Get the PID of the test process.
> > > +set testpid ""
> > > +set test "get inferior process ID"
> > > +    gdb_test_multiple "p mypid" $test {
> > > +    -re " = ($decimal)\r\n$gdb_prompt $" {
> > > +    set testpid $expect_out(1,string)
> > > +    pass $test
> > > +    }
> > > +}

You can use get_integer_valueof for this.

> > > +set test "attach $testpid"
> > > +gdb_test_multiple $test $test {
> > > +    -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt $" 
> > > {
> > > +    pass "$test"
> > > +    }
> > > +    timeout {
> > > +    fail "$test"
> > > +    }
> > > +}

Just wondering, can't simple gdb_test be used here?

> > > +
> > > +gdb_test_no_output "set should_continue = 1"
> > > +# Ensure the test program is in the top frame so the required
> > > +# variables are in scope.
> > > +gdb_breakpoint [gdb_get_line_number "prewatchtrigger"]
> > > +gdb_continue_to_breakpoint "prewatchtrigger"
> > > +
> > > +gdb_test "watch watched_variable" \
> > > +     "Hardware watchpoint $decimal: watched_variable"
> > > +
> > > +gdb_test "continue" \
> > > +     "continue.*Continuing.*\.Hardware watchpoint $decimal:\
> > > +         watched_variable.*Old value = 0.*New value = 4.*"

8 spaces -> tab on the last line.  Though for test patterns, it's fine 
to go over 80 columns, IMO.

Simon