Re: [patch] Forbid "run" etc. for use_gdb_stub targets [Re: [patch] gdb.python/py-events.exp: Disable multi-inferior for gdbserver]

[Pedro Alves <pedro@codesourcery.com>]

On Thursday 27 October 2011 11:17:00, Jan Kratochvil wrote:
> On Wed, 26 Oct 2011 15:48:58 +0200, Pedro Alves wrote:
> > "run" doesn't work with "target remote" either.  If that is succeeding,
> > it's because it's actually running the default native target,
> 
> Thanks for catching it.  I have seen these false runs already before,
> therefore I find it unmanageable to always catch it at least myself.

Funny, they tend to pop and ring alarm bells right at me.  :-)

> Maybe some more testfiles could be fixed instead of skipped below, I fixed
> one, still I find that as a possible different patch.
> 
> I am not sure what is the correct way to handle gdbserver_start_extended.
> gdb.server/ext-*.exp run gdbserver even in the default native run.  They
> either should not run in the default native or they should not run in the
> non-extended gdbserver mode.  Currently they run in both modes the same way.

These tests were written to smoke test gdbserver on native test
runs.  Not sure what you mean.  Is is that use_gdb_stub will be wrong?
Maybe we can override it?

> AFAIK there is currently no way to run the whole testsuite in extended
> gdbserver mode.  

Yeah.  Maybe it's possible today with a clever board file.  But it'd be
nice to have it integrated without hacks (even if it means a different
board file, which probabably does).

> (There were some attempts for it on the archer list IIRC.)

Missed those.

> No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu and with
> gdbsever.
> 
> 
> Thanks,
> Jan
> 
> 
> gdb/testsuite/
> 2011-10-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/async-shell.exp: Skip the testfile for is_remote target.
> 	* gdb.base/attach-pie-misread.exp: Skip the testfile for use_gdb_stub.
> 	* gdb.base/break-interp.exp: Likewise.
> 	* gdb.base/default.exp (attach, run "r" abbreviation, run): Skip these
> 	testcases for use_gdb_stub.
> 	* gdb.dwarf2/dw2-restore.exp: Fix the run for use_gdb_stub.
> 	(continue): New testcase.
> 	* gdb.python/py-events.exp: Skip the testfile for is_remote target.
> 	* gdb.threads/attach-into-signal.exp: Likewise.
> 	* gdb.threads/attach-stopped.exp: Likewise.
> 	* gdb.threads/attachstop-mt.exp: Likewise.
> 	* lib/gdb.exp (gdb_test_multiple): Forbid run, start or attach for
> 	use_gdb_stub without gdbserver_start_extended.
> 	* lib/gdbserver-support.exp (gdbserver_start_extended): Set global
> 	gdbserver_protocol and gdbserver_gdbport.
> 
> --- a/gdb/testsuite/gdb.base/async-shell.exp
> +++ b/gdb/testsuite/gdb.base/async-shell.exp
> @@ -20,6 +20,10 @@ if { ![support_displaced_stepping] } {
>      return -1
>  }
>  
> +if [is_remote target] { 
> +    return -1
> +}

Shouldn't this be a check for use_gdb_stub instead?  (I know
the bug was originally linux native.)  Doesn't matter much though.

The test fixes look right to me.  IMO, you should put them
in upfront and separately from the new smart detection.

> +    if {[regexp -nocase {^\s*(r|run|star|start|at|att|atta|attac|attach)\M} \
> +	 $command]
> +        && [target_info exists use_gdb_stub]} {
> +	global gdbserver_protocol
> +
> +	# gdbserver_start_extended runs as a part of non-extended configuration.
> +	if {![info exists gdbserver_protocol]
> +	    || ![string match "extended-*" $gdbserver_protocol]} {
> +	    error "gdbserver does not support $command without extended-remote"
> +	}
> +    }

I think this should be:

   if {[info exists gdbserver_protocol]
	    && ![string match "extended-*" $gdbserver_protocol]} {
	    error "gdbserver does not support $command without extended-remote"

We don't want to throw an error if gdbserver_protocol does not exist?

-- 
Pedro Alves