Re: [PATCH 2/2][AArch64] Test handling of additional brk instruction patterns

[Luis Machado <luis.machado@linaro.org>]

On 1/14/20 5:03 AM, Aktemur, Tankut Baris wrote:
> On Tuesday, January 14, 2020 5:25 AM, Simon Marchi wrote:
>> On 2020-01-13 12:25 p.m., Luis Machado wrote:
>>> This test exercises the previous patch's code and makes sure GDB can
>>> properly get a SIGTRAP from various brk instruction patterns.
>>>
>>> GDB needs to be able to see the program exiting normally. If GDB doesn't
>>> support the additional brk instructions, we will see timeouts.
>>>
>>> We bail out with the first timeout since we won't be able to step through
>>> the program breakpoint anyway, so it is no use carrying on.
>>>
>>> gdb/testsuite/ChangeLog:
>>>
>>> 2020-01-13  Luis Machado  <luis.machado@linaro.org>
>>>
>>> 	* gdb.arch/aarch64-brk-patterns.c: New source file.
>>> 	* gdb.arch/aarch64-brk-patterns.exp: New test.
>>> ---
>>>   gdb/testsuite/gdb.arch/aarch64-brk-patterns.c | 30 +++++++++
>>>   .../gdb.arch/aarch64-brk-patterns.exp         | 67 +++++++++++++++++++
>>>   2 files changed, 97 insertions(+)
>>>   create mode 100644 gdb/testsuite/gdb.arch/aarch64-brk-patterns.c
>>>   create mode 100644 gdb/testsuite/gdb.arch/aarch64-brk-patterns.exp
>>>
>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-brk-patterns.c b/gdb/testsuite/gdb.arch/aarch64-brk-patterns.c
>>> new file mode 100644
>>> index 0000000000..ccf9a35a94
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.arch/aarch64-brk-patterns.c
>>> @@ -0,0 +1,30 @@
>>> +/* This file is part of GDB, the GNU debugger.
>>> +
>>> +   Copyright 2020 Free Software Foundation, Inc.
>>> +
>>> +   This program is free software; you can redistribute it and/or modify
>>> +   it under the terms of the GNU General Public License as published by
>>> +   the Free Software Foundation; either version 3 of the License, or
>>> +   (at your option) any later version.
>>> +
>>> +   This program is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +   GNU General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU General Public License
>>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>> +
>>> +int main(void)
>>> +{
>>> +  /* Dummy instruction just so GDB doesn't stop at the first breakpoint
>>> +     instruction.  */
>>> +  __asm __volatile ("nop\n\t");
>>> +
>>> +  /* Multiple BRK instruction patterns.  */
>>> +  __asm __volatile ("brk %0\n\t" ::"n"(0x0));
>>> +  __asm __volatile ("brk %0\n\t" ::"n"(0x900 + 0xf));
>>> +  __asm __volatile ("brk %0\n\t" ::"n"(0xf000));
>>> +
>>> +  return 0;
>>> +}
>>> diff --git a/gdb/testsuite/gdb.arch/aarch64-brk-patterns.exp b/gdb/testsuite/gdb.arch/aarch64-brk-patterns.exp
>>> new file mode 100644
>>> index 0000000000..3532a0df95
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.arch/aarch64-brk-patterns.exp
>>> @@ -0,0 +1,67 @@
>>> +# Copyright 2020 Free Software Foundation, Inc.
>>> +#
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 3 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +#
>>> +# This file is part of the gdb testsuite.
>>> +
>>> +# Test if GDB stops at various BRK instruction patterns inserted into
>>> +# the code.
>>> +
>>> +if {![is_aarch64_target]} {
>>> +    verbose "Skipping ${gdb_test_file_name}."
>>> +    return
>>> +}
>>> +
>>> +standard_testfile
>>> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
>>> +    return -1
>>> +}
>>> +
>>> +if ![runto_main] {
> 
> I'm not aware of a common convention of use of braces and spaces, but adding braces
> as in {![runto_main]} and removing the spaces after/before the opening/closing braces
> in { [prepare_for_testing...] } above could make the use more consistent within the
> file.
> 

That's fair. I've fixed it now to make it consistent.

>>> +    untested "could not run to main"
>>> +    return -1
>>> +}
>>> +
>>> +set keep_going 1
>>> +set count 0
>>> +set old_timeout $timeout
>>> +set timeout 10
>>> +
>>> +while { $keep_going } {
>>> +
>>> +  set test "brk instruction $count causes SIGTRAP"
>>> +
>>> +  # Continue to next program breakpoint instruction.
>>> +  gdb_test_multiple "continue" $test {
>>> +      -re "Program received signal SIGTRAP, Trace/breakpoint trap.*$gdb_prompt $" {
>>> +	  pass $test
>>> +
>>> +	  # Insert a breakpoint at the program breakpoint instruction so GDB
>>> +	  # can step over it.
>>> +	  gdb_test "break" \
>>> +	    "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \
>>> +	    "insert breakpoint at brk instruction $count"
>>> +      }
>>> +      -re "exited normally.*$gdb_prompt $" {
>>> +	  set keep_going 0
>>> +      }
>>> +      timeout {
>>> +	  fail $test
>>> +	  set keep_going 0
>>> +      }
>>> +  }
>>> +  set count [expr "$count + 1"]
> 
> A minor thing.  This could be simplified as
> 
>    incr count
> 

Fixed as well. Thanks!