Re: [PATCH 3/8] Set general_thread after restart

[Pedro Alves <palves at redhat dot com>]

On 07/20/2015 12:35 PM, Yao Qi wrote:
> When I run gdb.server/ext-restart.exp, I get the following GDB internal
> error,
> 
> run^M
> The program being debugged has been started already.^M
> Start it from the beginning? (y or n) y^M
> Sending packet: $vKill;53c5#3d...Packet received: OK^M
> Packet vKill (kill) is supported^M
> Sending packet: $vFile:close:6#b6...Packet received: F0^M
> Sending packet: $vFile:close:3#b3...Packet received: F0^M
> Starting program: /scratch/yao/gdb/build-git/x86_64/gdb/testsuite/gdb.server/ext-restart ^M
> Sending packet: $QDisableRandomization:1#cf...Packet received: OK^M
> Sending packet: $R0#82...Sending packet: $qC#b4...Packet received: QCp53c5.53c5^M  <-- [1]
> Sending packet: $qAttached:53c5#c9...Packet received: E01^M
> warning: Remote failure reply: E01^M

Curious.  Kevin's original patch to handle bogus qC replies would
have hidden this bug.

Plus, today I wrote a patch that exposed a bunch of stale
general_thread issues (but not this one).

Funny how sometimes we all end up staring at the same things
at the same time.

>  
>  	  /* Wait till we are at 1st instruction in prog.  */
>  	  if (program_argv != NULL)
> -	    start_inferior (program_argv);
> +	    {
> +	      start_inferior (program_argv);
> +	      general_thread = last_ptid;
> +	    }

I think this should be:

  if (last_status.kind == TARGET_WAITKIND_STOPPED)
    {
      /* Stopped at the first instruction of the target process.  */
      general_thread = last_ptid;
    }
  else
    {
      /* Something went wrong.  */
      general_thread = null_ptid;
    }

> +# Test running programs using extended-remote.

Comment looks stale.  Looks like I missed pointing out the same
in patch #2.

Otherwise looks good to me.

(I think it's likely we have lots of stale-data bugs on the
gdb side after R, as we don't resync much.  It previously crossed my mind
that immediately after sending R, gdb should refresh all its
remote state anew, like if it had just disconnected and then reconnected.
That is, do most of what remote_start_remote does, except the
connection-specific details (qSupported, etc.)
Hard to justify the effort though -- I don't think I ever worked with
a stub that relies on R.)

Thanks,
Pedro Alves