diff mbox

[2/2,AArch64] Test handling of additional brk instruction patterns

Message ID 20200113172524.7201-3-luis.machado@linaro.org
State New, archived
Headers

Commit Message

Luis Machado Jan. 13, 2020, 5:25 p.m. UTC 
  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

Comments

Simon Marchi Jan. 14, 2020, 4:25 a.m. UTC | #1 
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] {
> +    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"]
> +}
> +
> +set timeout $old_timeout
> -- 
> 2.17.1
> 

I don't see anything wrong with the test, but I was wondering if we should
assert that $count is the right value in the end, to make sure that we did
hit the expected number of breakpoints.

Simon
Tankut Baris Aktemur Jan. 14, 2020, 8:03 a.m. UTC | #2 
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.

> > +    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

-Baris

> > +}

> > +

> > +set timeout $old_timeout

> > --

> > 2.17.1

> >

> 

> I don't see anything wrong with the test, but I was wondering if we should

> assert that $count is the right value in the end, to make sure that we did

> hit the expected number of breakpoints.

> 

> Simon

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Luis Machado Jan. 14, 2020, 1:30 p.m. UTC | #3 
On 1/14/20 1: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] {
>> +    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"]
>> +}
>> +
>> +set timeout $old_timeout
>> -- 
>> 2.17.1
>>
> 
> I don't see anything wrong with the test, but I was wondering if we should
> assert that $count is the right value in the end, to make sure that we did
> hit the expected number of breakpoints.
> 
> Simon
> 

The idea behind the test is that GDB will get a SIGTRAP and be able to 
step over all of the program breakpoints until the inferior exits. We 
don't count how many of those breakpoints we've hit.

If GDB gets stuck during "continue" and doesn't hit one of the 
breakpoints, then we throw a fail. That failure is an indication that 
something is wrong.

If someone wants to add more breakpoint patterns to the test, the .exp 
file won't need to be changed.

Does that make sense? Or do you think it would be better to have a fixed 
set of instruction the test is bound to?
Luis Machado Jan. 14, 2020, 1:45 p.m. UTC | #4 
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!
Simon Marchi Jan. 14, 2020, 3:22 p.m. UTC | #5 
On 2020-01-14 8:30 a.m., Luis Machado wrote:
> The idea behind the test is that GDB will get a SIGTRAP and be able to 
> step over all of the program breakpoints until the inferior exits. We 
> don't count how many of those breakpoints we've hit.
> 
> If GDB gets stuck during "continue" and doesn't hit one of the 
> breakpoints, then we throw a fail. That failure is an indication that 
> something is wrong.

It would just be one more consistency check.  If we put 4 breakpoints in the
file, but get 3 hits, there is something wrong.

Your test verifies well the failure mode that you have encountered today, but
by adding this check, it could catch some other types failure in the future.

> If someone wants to add more breakpoint patterns to the test, the .exp 
> file won't need to be changed.

I don't really think it's a problem to update the .exp file if you're changing
the .c anyway.

> Does that make sense? Or do you think it would be better to have a fixed 
> set of instruction the test is bound to?
I would add it, but it's not necessary, I'll leave that up to you. :)

Simon
diff mbox

Patch

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] {
+    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"]
+}
+
+set timeout $old_timeout