Re: [rfc] Handle lack of non-stop support more gracefully

[Pedro Alves <pedro at codesourcery dot com>]

On Monday 14 June 2010 15:13:29, Ulrich Weigand wrote:
> Hello,
> 
> on spu-elf I'm now seeing:
> 
> ERROR: mi-ns-stale-regcache.exp tests suppressed
> UNRESOLVED: gdb.mi/mi-nsintrall.exp: Couldn't compile /home/uweigand/fsf/gdb-head/gdb/testsuite/gdb.mi/nsintrall.c: unrecognized error
> 
> Now, on the SPU we don't support non-stop execution (in fact, the SPU
> is always single-threaded anyway).  Therefore, all the non-stop tests
> ought to be marked as UNSUPPORTED.  And in fact, the mi_run_to_main
> routine does that.
> 
> However, various test cases, *in addition* to the actions done by
> the mi_run_to_main routine, themselves call "perror":
>     perror "mi-ns-stale-regcache.exp tests suppressed"
> 
> This seems wrong to me: First of all, an ERROR is supposed to mark some
> sort of unexpected failure of the test harness itself, with unpredictable
> results on the test outcome.  This is not the case for a feature that is
> simply not supported on a target.  Second, because of that, the DejaGNU
> main test harness will use the presence of any perror call as signal to
> mark the next regular test result as UNRESOLVED instead of whatever the
> actual result was -- even if this happens to be in a completely different
> test!  (See the example above.)
> 
> [ Note that the reason this doesn't show up with any of the other non-stop
> tests on SPU is that they already fail during the compile stage due to
> the absence of pthreads support.  mi-ns-stale-regcache is the only non-stop
> test that does not require pthreads.  ]
> 
> It seems to me the correct way to handle this is for mi_run_to_main to
> detect the case where the non-stop feature is unsupported, generate an
> appropriate test result (UNSUPPORTED), which it already does, and then
> have the main test just terminate with no further message.
> 
> The following patch implements this, and fixes the above problems on SPU.
> 
> Pedro, it seems you originally added the perror calls -- was there any
> reason I may be missing why we should need them anyway?

Sorry, I don't recall.  There was probably no good reason.  I probably
copied it from what some CLI tests do:

 if ![runto_main] then {
   perror "Couldn't run to main"
   return -1
 }

I've no objections to your patch.

I took a quick look over mi-support, and I can see how I got a bit confused
about what do the different return codes leading up to mi_run_to_main mean.
It looks like some non-gdbserver targets will still trip on
this problem:

    } elseif { [target_info gdb_protocol] == "remote" } {
	# remote targets
	if { [mi_gdb_target_cmd "remote" [target_info netport]] != 0 } {
	    perror "Unable to connect to remote target"
	    return -1
	}

?

Maybe mi_gdb_target_cmd should return a different error code for
not-supported vs connection error.

I'm still a bit confused over how the non-stop MI handle this.  If
mi_gdb_target_cmd fails to connect, it seems to return 0 anyway, so
the following tests will just cascade in FAILs.  For other, non-remote
targets, mi_gdb_target_load will call perror on connection fail, but
the gdbserver branch at the top doesn't.

-- 
Pedro Alves