[PATCH] gdb/testsuite: fix gdb.base/break-idempotent.exp on ppc

[PATCH] gdb/testsuite: fix gdb.base/break-idempotent.exp on ppc

[Andrew Burgess]

When running the gdb.base/break-idempotent.exp test on ppc, I was
seeing some test failures (or rather errors), that looked like this:

  (gdb) watch local
  Hardware watchpoint 2: local

  has_hw_wp_support: Hardware watchpoint detected
  ERROR: no fileid for gcc2-power8
  ERROR: Couldn't send delete breakpoints to GDB.
  ERROR OCCURED: can't read "gdb_spawn_id": no such variable
      while executing
  "expect {
  -i 1000 -timeout 100
          -re ".*A problem internal to GDB has been detected" {
              fail "$message (GDB internal error)"
              gdb_internal_erro..."
      ("uplevel" body line 1)
      invoked from within

What happens is that in break-idempotent.exp we basically do this:

    if {[prepare_for_testing "failed to prepare" $binfile $srcfile $opts]} {
        continue
    }

    # ....

    if {![skip_hw_watchpoint_tests]} {
        test_break $always_inserted "watch"
    }

The problem with this is that skip_hw_watchpoint_tests, includes this:

    if { [istarget "i?86-*-*"]
	 || [istarget "x86_64-*-*"]
	 || [istarget "ia64-*-*"]
	 || [istarget "arm*-*-*"]
	 || [istarget "aarch64*-*-*"]
	 || ([istarget "powerpc*-*-linux*"] && [has_hw_wp_support])
	 || [istarget "s390*-*-*"] } {
	return 0
    }

For powerpc only we call has_hw_wp_support.  This is a caching proc
that runs a test within GDB to detect if we have hardware watchpoint
support or not.

Unfortunately, to run this test we kill the current GDB session (call
gdb_exit), start a new session, and, when the test is completed, we
kill the test session.  This leave DejaGNU with no running GDB
session.

Now has_hw_wp_support is a caching proc, so we only do the kill
session, run test, and cleanup the first time has_hw_wp_support is
called, but, this means that the first time we call
skip_hw_watchpoint_tests in a single DejaGNU run, we carry out these
extra steps.

Every other use of skip_hw_watchpoint_tests is just at the stop of a
test script, and is used to return early, entirely skipping the script
if hardware watchpoint support is not available.

Only in break-idempotent.exp and process-dies-while-detaching.exp do
we call skip_hw_watchpoint_tests from the middle of a test script.  In
the process-dies-while-detaching.exp case we immediately call
clean_restart anyway, so if the skip_hw_watchpoint_tests call does
close the current GDB session we're fine.

But in break-idempotent.exp we call skip_hw_watchpoint_tests with GDB
running, and we expect GDB to be running when the call finishes.  On
powerpc this might not be the case, and as a result we see the errors
above.

To fix this I propose adding an extra call to skip_hw_watchpoint_tests
to the top of break-idempotent.exp, this call will prime the cache
with the result of the check.  Now, when we later make a call to
skip_hw_watchpoint_tests, instead of running the test we will simply
reuse the cached result.

After this change break-idempotent.exp runs fine on powerpc.
---
 gdb/testsuite/gdb.base/break-idempotent.exp | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/gdb/testsuite/gdb.base/break-idempotent.exp b/gdb/testsuite/gdb.base/break-idempotent.exp
index 29002f103a8..8866c65e883 100644
--- a/gdb/testsuite/gdb.base/break-idempotent.exp
+++ b/gdb/testsuite/gdb.base/break-idempotent.exp
@@ -36,6 +36,23 @@
 
 standard_testfile
 
+# For some architectures (just powerpc right now), this test will
+# invoke a caching proc that closes the current GDB session, and
+# starts a new session in which to run the test.  Once the test is
+# complete, GDB is exited, leaving no GDB running.
+#
+# Later in this test script we will call this function (directly, and
+# indirectly) multiple times, after starting GDB, and we expect GDB to
+# still be running after this test is complete.  As a result of the
+# above, the first time this test is run, we will break this
+# assumption, and GDB will not be running when we expect it to be.
+#
+# Luckily, the test is a caching proc, and so it is only the first
+# call to this function that causes GDB to exit.  Any later calls will
+# simply use the cached result.  So, we call this function now, before
+# we've started GDB, and this will prime the cache.
+skip_hw_breakpoint_tests
+
 # Force a breakpoint re-set in GDB.  Currently this is done by
 # reloading symbols with the "file" command.
 
-- 
2.25.4

[PATCHv2] gdb/testsuite: fix gdb.base/break-idempotent.exp on ppc

[Andrew Burgess]

Please ignore patch v1, there was a stupid error.

Here's the real patch.

Thanks,
Andrew

---

When running the gdb.base/break-idempotent.exp test on ppc, I was
seeing some test failures (or rather errors), that looked like this:

  (gdb) watch local
  Hardware watchpoint 2: local

  has_hw_wp_support: Hardware watchpoint detected
  ERROR: no fileid for gcc2-power8
  ERROR: Couldn't send delete breakpoints to GDB.
  ERROR OCCURED: can't read "gdb_spawn_id": no such variable
      while executing
  "expect {
  -i 1000 -timeout 100
          -re ".*A problem internal to GDB has been detected" {
              fail "$message (GDB internal error)"
              gdb_internal_erro..."
      ("uplevel" body line 1)
      invoked from within

What happens is that in break-idempotent.exp we basically do this:

    if {[prepare_for_testing "failed to prepare" $binfile $srcfile $opts]} {
        continue
    }

    # ....

    if {![skip_hw_watchpoint_tests]} {
        test_break $always_inserted "watch"
    }

The problem with this is that skip_hw_watchpoint_tests, includes this:

    if { [istarget "i?86-*-*"]
	 || [istarget "x86_64-*-*"]
	 || [istarget "ia64-*-*"]
	 || [istarget "arm*-*-*"]
	 || [istarget "aarch64*-*-*"]
	 || ([istarget "powerpc*-*-linux*"] && [has_hw_wp_support])
	 || [istarget "s390*-*-*"] } {
	return 0
    }

For powerpc only we call has_hw_wp_support.  This is a caching proc
that runs a test within GDB to detect if we have hardware watchpoint
support or not.

Unfortunately, to run this test we kill the current GDB session (call
gdb_exit), start a new session, and, when the test is completed, we
kill the test session.  This leave DejaGNU with no running GDB
session.

Now has_hw_wp_support is a caching proc, so we only do the kill
session, run test, and cleanup the first time has_hw_wp_support is
called, but, this means that the first time we call
skip_hw_watchpoint_tests in a single DejaGNU run, we carry out these
extra steps.

Every other use of skip_hw_watchpoint_tests is just at the stop of a
test script, and is used to return early, entirely skipping the script
if hardware watchpoint support is not available.

Only in break-idempotent.exp and process-dies-while-detaching.exp do
we call skip_hw_watchpoint_tests from the middle of a test script.  In
the process-dies-while-detaching.exp case we immediately call
clean_restart anyway, so if the skip_hw_watchpoint_tests call does
close the current GDB session we're fine.

But in break-idempotent.exp we call skip_hw_watchpoint_tests with GDB
running, and we expect GDB to be running when the call finishes.  On
powerpc this might not be the case, and as a result we see the errors
above.

To fix this I propose adding an extra call to skip_hw_watchpoint_tests
to the top of break-idempotent.exp, this call will prime the cache
with the result of the check.  Now, when we later make a call to
skip_hw_watchpoint_tests, instead of running the test we will simply
reuse the cached result.

After this change break-idempotent.exp runs fine on powerpc.
---
 gdb/testsuite/gdb.base/break-idempotent.exp | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/gdb/testsuite/gdb.base/break-idempotent.exp b/gdb/testsuite/gdb.base/break-idempotent.exp
index 29002f103a8..a51b59e251e 100644
--- a/gdb/testsuite/gdb.base/break-idempotent.exp
+++ b/gdb/testsuite/gdb.base/break-idempotent.exp
@@ -36,6 +36,23 @@
 
 standard_testfile
 
+# For some architectures (just powerpc right now), this test will
+# invoke a caching proc that closes the current GDB session, and
+# starts a new session in which to run the test.  Once the test is
+# complete, GDB is exited, leaving no GDB running.
+#
+# Later in this test script we will call this function (directly, and
+# indirectly) multiple times, after starting GDB, and we expect GDB to
+# still be running after this test is complete.  As a result of the
+# above, the first time this test is run, we will break this
+# assumption, and GDB will not be running when we expect it to be.
+#
+# Luckily, the test is a caching proc, and so it is only the first
+# call to this function that causes GDB to exit.  Any later calls will
+# simply use the cached result.  So, we call this function now, before
+# we've started GDB, and this will prime the cache.
+skip_hw_watchpoint_tests
+
 # Force a breakpoint re-set in GDB.  Currently this is done by
 # reloading symbols with the "file" command.
 
-- 
2.25.4

Re: [PATCHv2] gdb/testsuite: fix gdb.base/break-idempotent.exp on ppc

[Andrew Burgess]

Andrew Burgess <aburgess@redhat.com> writes:

> Please ignore patch v1, there was a stupid error.
>
> Here's the real patch.

Carl already posted a patch for this issue, which is being discussed
here:

  https://sourceware.org/pipermail/gdb-patches/2022-June/190094.html

Thanks,
Andrew

>
> Thanks,
> Andrew
>
> ---
>
> When running the gdb.base/break-idempotent.exp test on ppc, I was
> seeing some test failures (or rather errors), that looked like this:
>
>   (gdb) watch local
>   Hardware watchpoint 2: local
>
>   has_hw_wp_support: Hardware watchpoint detected
>   ERROR: no fileid for gcc2-power8
>   ERROR: Couldn't send delete breakpoints to GDB.
>   ERROR OCCURED: can't read "gdb_spawn_id": no such variable
>       while executing
>   "expect {
>   -i 1000 -timeout 100
>           -re ".*A problem internal to GDB has been detected" {
>               fail "$message (GDB internal error)"
>               gdb_internal_erro..."
>       ("uplevel" body line 1)
>       invoked from within
>
> What happens is that in break-idempotent.exp we basically do this:
>
>     if {[prepare_for_testing "failed to prepare" $binfile $srcfile $opts]} {
>         continue
>     }
>
>     # ....
>
>     if {![skip_hw_watchpoint_tests]} {
>         test_break $always_inserted "watch"
>     }
>
> The problem with this is that skip_hw_watchpoint_tests, includes this:
>
>     if { [istarget "i?86-*-*"]
> 	 || [istarget "x86_64-*-*"]
> 	 || [istarget "ia64-*-*"]
> 	 || [istarget "arm*-*-*"]
> 	 || [istarget "aarch64*-*-*"]
> 	 || ([istarget "powerpc*-*-linux*"] && [has_hw_wp_support])
> 	 || [istarget "s390*-*-*"] } {
> 	return 0
>     }
>
> For powerpc only we call has_hw_wp_support.  This is a caching proc
> that runs a test within GDB to detect if we have hardware watchpoint
> support or not.
>
> Unfortunately, to run this test we kill the current GDB session (call
> gdb_exit), start a new session, and, when the test is completed, we
> kill the test session.  This leave DejaGNU with no running GDB
> session.
>
> Now has_hw_wp_support is a caching proc, so we only do the kill
> session, run test, and cleanup the first time has_hw_wp_support is
> called, but, this means that the first time we call
> skip_hw_watchpoint_tests in a single DejaGNU run, we carry out these
> extra steps.
>
> Every other use of skip_hw_watchpoint_tests is just at the stop of a
> test script, and is used to return early, entirely skipping the script
> if hardware watchpoint support is not available.
>
> Only in break-idempotent.exp and process-dies-while-detaching.exp do
> we call skip_hw_watchpoint_tests from the middle of a test script.  In
> the process-dies-while-detaching.exp case we immediately call
> clean_restart anyway, so if the skip_hw_watchpoint_tests call does
> close the current GDB session we're fine.
>
> But in break-idempotent.exp we call skip_hw_watchpoint_tests with GDB
> running, and we expect GDB to be running when the call finishes.  On
> powerpc this might not be the case, and as a result we see the errors
> above.
>
> To fix this I propose adding an extra call to skip_hw_watchpoint_tests
> to the top of break-idempotent.exp, this call will prime the cache
> with the result of the check.  Now, when we later make a call to
> skip_hw_watchpoint_tests, instead of running the test we will simply
> reuse the cached result.
>
> After this change break-idempotent.exp runs fine on powerpc.
> ---
>  gdb/testsuite/gdb.base/break-idempotent.exp | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.base/break-idempotent.exp b/gdb/testsuite/gdb.base/break-idempotent.exp
> index 29002f103a8..a51b59e251e 100644
> --- a/gdb/testsuite/gdb.base/break-idempotent.exp
> +++ b/gdb/testsuite/gdb.base/break-idempotent.exp
> @@ -36,6 +36,23 @@
>  
>  standard_testfile
>  
> +# For some architectures (just powerpc right now), this test will
> +# invoke a caching proc that closes the current GDB session, and
> +# starts a new session in which to run the test.  Once the test is
> +# complete, GDB is exited, leaving no GDB running.
> +#
> +# Later in this test script we will call this function (directly, and
> +# indirectly) multiple times, after starting GDB, and we expect GDB to
> +# still be running after this test is complete.  As a result of the
> +# above, the first time this test is run, we will break this
> +# assumption, and GDB will not be running when we expect it to be.
> +#
> +# Luckily, the test is a caching proc, and so it is only the first
> +# call to this function that causes GDB to exit.  Any later calls will
> +# simply use the cached result.  So, we call this function now, before
> +# we've started GDB, and this will prime the cache.
> +skip_hw_watchpoint_tests
> +
>  # Force a breakpoint re-set in GDB.  Currently this is done by
>  # reloading symbols with the "file" command.
>  
> -- 
> 2.25.4