[PATCH] gdb/testsuite: better handle failures in simavr board, reap simavr process
Simon Marchi
simark@simark.ca
Mon Jun 29 14:30:21 GMT 2020
On 2020-06-22 10:50 a.m., Simon Marchi via Gdb-patches wrote:
> This patch makes a few adjustments to the simavr.exp to handle tests
> that error out more gracefully. I put all the changes in the same patch
> because right now it's in a very bad shape, so it's very painful to do
> small incremental adjustements. I found that with these changes, it
> becomes reasonably stable.
>
> For example, the gdb.base/step-over-syscall.exp test is a bit buggy
> (stuff for another patch...), in that it calls gdb_load (through
> clean_restart) with a file that doesn't exist. The gdb_load
> implementation in simavr.exp gets called with a file that doesn't exist,
> and simavr (expectedly) fails to start.
>
> When this happens, we currently leave the $simavr_spawn_id variable set.
> However, because the simavr process has terminated, its spawn id is
> closed. When the next test tries to close the previous connection to
> simavr, it fails to, and this error is thrown:
>
> ERROR: close: spawn id exp86 not open
> while executing
> "close -i $simavr_spawn_id"
> (procedure "gdb_load" line 18)
> invoked from within
>
> In other words, any test ran after step-over-syscall.exp will have
> simavr.exp's gdb_load fail on it.
>
> This patch tries to make sure that simavr.exp's gdb_load only leaves
> simavr_spawn_id set if everything went fine. If we couldn't start
> simavr, don't see the expected output, or fail to connect to it, we
> close the spawn id (if needed) and unset simavr_spawn_id.
>
> As an additional precaution, I added a catch around the "close the
> previous connection" code. Ideally, this shouldn't be necessary, but I
> bet there are other similar scenarios where we might try to close an
> already close spawn id. So I think displaying a warning is better than
> messing up all following tests.
>
> Also, the board never wait'ed for the simavr process, resulting in tons
> of zombie simavr processes hanging around. This patch adds some calls
> to "wait" whenever we close the connection (or realize it is already
> closed), which seems to fix the problem.
>
> Finally I switched a `gdb_expect` to bare `expect`, where we wait for
> the "listening" message from simavr. I found it necessary because
> gdb_expect (through remote_expect) adds a `-i <gdb spawn id> -timeout
> 10`. So if for some reason the GDB process has crashed in the mean
> time, this expect will unexpectedly error out with a `spawn id <gdb
> spawn id> not open`. Therefore, change `gdb_expect` to `expect` so that
> we only deal with simavr's spawn id here.
>
> Here are the results on TESTS="gdb.base/*.exp" before:
>
> # of expected passes 4816
> # of unexpected failures 4433
> # of known failures 2
> # of unresolved testcases 300
> # of untested testcases 143
> # of unsupported tests 7
> # of paths in test names 2
> # of duplicate test names 10
>
> and after:
>
> # of expected passes 21957
> # of unexpected failures 1564
> # of expected failures 8
> # of unknown successes 1
> # of known failures 31
> # of unresolved testcases 532
> # of untested testcases 153
> # of unsupported tests 28
> # of paths in test names 2
> # of duplicate test names 32
>
> I tested using simavr's current master (7c254e2081b5).
>
> gdb/testsuite/ChangeLog:
>
> * boards/simavr.exp (gdb_load): Catch errors when closing
> previous connection. Close connection, wait for process and
> unset simavr_spawn_id on failure.
I pushed this patch.
Simon
More information about the Gdb-patches
mailing list