[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