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

[Andrew Burgess <aburgess@redhat.com>]

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