Fix for gdb.python/py-breakpoint.exp

Message ID d530470d6e715c96833086c528d592d69d6afb5f.camel@us.ibm.com
State New
Headers show
Series
  • Fix for gdb.python/py-breakpoint.exp
Related show

Commit Message

Simon Marchi via Gdb-patches July 21, 2021, 5:15 p.m.
GDB maintainers:

The following patch adds a check to make sure gdb has the needed
hardware break point support for the test.

This patch was tested on Power 9 and Intel without any regression
errors.
Please let me know if it is acceptable.
                            Carl Love

----------------------------------------------------

Fix for gdb.python/py-breakpoint.exp

Not all systems support the hardware breakpoint support.  Added a check
to see if the system supports hardware breakpoints.

gdb/testsuite/ChangeLog
2021-07-20  Carl Love  <cel@us.ibm.com>

	* gdb.python/py-breakpoint.exp: Add check for hardware breakpoint
	support.
---
 gdb/testsuite/gdb.python/py-breakpoint.exp | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

-- 
2.17.1

Comments

Andrew Burgess July 21, 2021, 5:33 p.m. | #1
* Carl Love via Gdb-patches <gdb-patches@sourceware.org> [2021-07-21 10:15:23 -0700]:

> GDB maintainers:

> 

> The following patch adds a check to make sure gdb has the needed

> hardware break point support for the test.

> 

> This patch was tested on Power 9 and Intel without any regression

> errors.

> Please let me know if it is acceptable.

>                             Carl Love

> 

> ----------------------------------------------------

> 

> Fix for gdb.python/py-breakpoint.exp

> 

> Not all systems support the hardware breakpoint support.  Added a check

> to see if the system supports hardware breakpoints.

> 

> gdb/testsuite/ChangeLog

> 2021-07-20  Carl Love  <cel@us.ibm.com>

> 

> 	* gdb.python/py-breakpoint.exp: Add check for hardware breakpoint

> 	support.

> ---

>  gdb/testsuite/gdb.python/py-breakpoint.exp | 17 +++++++++++++++++

>  1 file changed, 17 insertions(+)

> 

> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp

> index d8fb85b784c..b4dcad07eec 100644

> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp

> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp

> @@ -29,6 +29,23 @@ if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${options}]

>  # Skip all tests if Python scripting is not enabled.

>  if { [skip_python_tests] } { continue }

>  

> +set supports_hbreak 0

> +set test "probe hardware breakpoint support"

> +gdb_test_multiple "hbreak -q main" $test {

> +    -re "No hardware breakpoint support in the target.*" {

> +	pass $test

> +    }

> +    -re "Hardware assisted breakpoint.*at.* file .*$srcfile, line.*" {

> +	set supports_hbreak 1

> +	pass $test

> +    }

> +}

> +

> +if {!$supports_hbreak} {

> +    unsupported "hardware breakpoints"

> +    return

> +}


We already have skip_hw_breakpoint_tests in gdb/testsuite/lib/gdb.exp,
we should probably be using this helper function here unless there's a
good reason not too.

Additionally, having this check at the top level feels like we're
skipping too many tests, do all the tests in this file really require
hardware breakpoints?  Could we instead, maybe just have those procs
that require h/w breakpoints (like test_hardware_breakpoints) check
for the feature and just return immediately if needed?

Thanks,
Andrew


> +

>  proc_with_prefix test_bkpt_basic { } {

>      global srcfile testfile hex decimal

>  

> -- 

> 2.17.1

> 

>
Simon Marchi via Gdb-patches July 21, 2021, 5:38 p.m. | #2
Hi,

There is a 'proc skip_hw_breakpoint_tests' helper function in
gdb/testsuite/lib/gdb.exp.  I guess it would make sense to use it here
(and maybe adjust it if required, but I do not think it will be needed
since it only returns true for a fixed list of architectures known to
support this).

Also, would it make sense to only skip test_hardware_breakpoints if
hardware breakpoints are missing instead of the entire file?

Best,
Lancelot.

On Wed, Jul 21, 2021 at 10:15:23AM -0700, Carl Love via Gdb-patches wrote:
> GDB maintainers:

> 

> The following patch adds a check to make sure gdb has the needed

> hardware break point support for the test.

> 

> This patch was tested on Power 9 and Intel without any regression

> errors.

> Please let me know if it is acceptable.

>                             Carl Love

> 

> ----------------------------------------------------

> 

> Fix for gdb.python/py-breakpoint.exp

> 

> Not all systems support the hardware breakpoint support.  Added a check

> to see if the system supports hardware breakpoints.

> 

> gdb/testsuite/ChangeLog

> 2021-07-20  Carl Love  <cel@us.ibm.com>

> 

> 	* gdb.python/py-breakpoint.exp: Add check for hardware breakpoint

> 	support.

> ---

>  gdb/testsuite/gdb.python/py-breakpoint.exp | 17 +++++++++++++++++

>  1 file changed, 17 insertions(+)

> 

> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp

> index d8fb85b784c..b4dcad07eec 100644

> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp

> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp

> @@ -29,6 +29,23 @@ if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${options}]

>  # Skip all tests if Python scripting is not enabled.

>  if { [skip_python_tests] } { continue }

>  

> +set supports_hbreak 0

> +set test "probe hardware breakpoint support"

> +gdb_test_multiple "hbreak -q main" $test {

> +    -re "No hardware breakpoint support in the target.*" {

> +	pass $test

> +    }

> +    -re "Hardware assisted breakpoint.*at.* file .*$srcfile, line.*" {

> +	set supports_hbreak 1

> +	pass $test

> +    }

> +}

> +

> +if {!$supports_hbreak} {

> +    unsupported "hardware breakpoints"

> +    return

> +}

> +

>  proc_with_prefix test_bkpt_basic { } {

>      global srcfile testfile hex decimal

>  

> -- 

> 2.17.1

> 

>
Simon Marchi via Gdb-patches July 21, 2021, 6:35 p.m. | #3
Lancelot, Andrew:

skip_hw_breakpoint_test
OK, I didn't know about skip_hw_breakpoint_tests.  Thanks for the
pointer.  I will rework the patch.


On Wed, 2021-07-21 at 17:38 +0000, Lancelot SIX wrote:
> Hi,

> 

> There is a 'proc skip_hw_breakpoint_tests' helper function in

> gdb/testsuite/lib/gdb.exp.  I guess it would make sense to use it

> here

> (and maybe adjust it if required, but I do not think it will be

> needed

> since it only returns true for a fixed list of architectures known to

> support this).

> 

> Also, would it make sense to only skip test_hardware_breakpoints if

> hardware breakpoints are missing instead of the entire file?



                      Carl
Simon Marchi via Gdb-patches July 22, 2021, 2:32 a.m. | #4
Lancelot, Andrew:

I redid the patch to use the skip_hw_breakpoint_tests per your
suggestions.  I put the test into the proceedure that does the
hardware_breakpoints.  

Please let me know if the patch is OK with you now.  Thanks for the
feedback and help.

Retested on Power 9 and Intel.

                 Carl 



--------------------------------------------------
 Fix for gdb.python/py-breakpoint.exp

Not all systems have hardware breakpoint support.  Added a check
to see if the system supports hardware breakpoints.

gdb/testsuite/ChangeLog
2021-07-21  Carl Love  <cel@us.ibm.com>

	* gdb.python/py-breakpoint.exp (test_hardware_breakpoints): Add
	check for hardware breakpoint support.
---
 gdb/testsuite/gdb.python/py-breakpoint.exp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index d8fb85b784c..797ffbd77e9 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -255,6 +255,9 @@ proc_with_prefix test_bkpt_invisible { } {
 proc_with_prefix test_hardware_breakpoints { } {
     global srcfile testfile hex decimal
 
+    # Skip these tests if the HW does not support hardware breakpoints
+    if { [skip_hw_breakpoint_tests] } { return 0 }
+
     # Start with a fresh gdb.
     clean_restart ${testfile}
 
-- 
2.17.1
Andrew Burgess July 23, 2021, 9:26 a.m. | #5
* Carl Love <cel@us.ibm.com> [2021-07-21 19:32:37 -0700]:

> Lancelot, Andrew:

> 

> I redid the patch to use the skip_hw_breakpoint_tests per your

> suggestions.  I put the test into the proceedure that does the

> hardware_breakpoints.  

> 

> Please let me know if the patch is OK with you now.  Thanks for the

> feedback and help.

> 

> Retested on Power 9 and Intel.

> 

>                  Carl 

> 

> 

> 

> --------------------------------------------------

>  Fix for gdb.python/py-breakpoint.exp

> 

> Not all systems have hardware breakpoint support.  Added a check

> to see if the system supports hardware breakpoints.

> 

> gdb/testsuite/ChangeLog

> 2021-07-21  Carl Love  <cel@us.ibm.com>

> 

> 	* gdb.python/py-breakpoint.exp (test_hardware_breakpoints): Add

> 	check for hardware breakpoint support.


This looks good with just a missing '.' I've pointed out below.

When applying, please remember we no longer apply ChangeLogs in
gdb/testsuite/ChangeLog, though you are welcome to keep the ChangeLog
entry in the commit message.

> ---

>  gdb/testsuite/gdb.python/py-breakpoint.exp | 3 +++

>  1 file changed, 3 insertions(+)

> 

> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp

> index d8fb85b784c..797ffbd77e9 100644

> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp

> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp

> @@ -255,6 +255,9 @@ proc_with_prefix test_bkpt_invisible { } {

>  proc_with_prefix test_hardware_breakpoints { } {

>      global srcfile testfile hex decimal

>  

> +    # Skip these tests if the HW does not support hardware breakpoints


Please add a full stop to the end of this sentence.

Thanks,
Andrew

> +    if { [skip_hw_breakpoint_tests] } { return 0 }

> +

>      # Start with a fresh gdb.

>      clean_restart ${testfile}

>  

> -- 

> 2.17.1

> 

>

Patch

diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
index d8fb85b784c..b4dcad07eec 100644
--- a/gdb/testsuite/gdb.python/py-breakpoint.exp
+++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
@@ -29,6 +29,23 @@  if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${options}]
 # Skip all tests if Python scripting is not enabled.
 if { [skip_python_tests] } { continue }
 
+set supports_hbreak 0
+set test "probe hardware breakpoint support"
+gdb_test_multiple "hbreak -q main" $test {
+    -re "No hardware breakpoint support in the target.*" {
+	pass $test
+    }
+    -re "Hardware assisted breakpoint.*at.* file .*$srcfile, line.*" {
+	set supports_hbreak 1
+	pass $test
+    }
+}
+
+if {!$supports_hbreak} {
+    unsupported "hardware breakpoints"
+    return
+}
+
 proc_with_prefix test_bkpt_basic { } {
     global srcfile testfile hex decimal