Re: [patch 2/2] Fix watchpoints for multi-inferior #2

[Pedro Alves <palves@redhat.com>]

On 01/25/2012 03:22 PM, Jan Kratochvil wrote:
> On Tue, 24 Jan 2012 14:19:34 +0100, Pedro Alves wrote:
>> On 01/20/2012 09:31 PM, Jan Kratochvil wrote:
>>> @@ -2107,7 +2090,14 @@ retry:
>>>        if (thread == NULL)
>>>  	{
>>>  	  struct thread_resume resume_info;
>>> -	  resume_info.thread = minus_one_ptid;
>>> +
>>> +	  /* Resume only a single process if requested so.  */
>>> +	  if (!ptid_equal (cont_thread, minus_one_ptid)
>>> +	      && ptid_get_lwp (cont_thread) == -1)
>>> +	    resume_info.thread = cont_thread;
>>
>> Just above we see:
>>
>>       thread = (struct thread_info *) find_inferior_id (&all_threads,
>> 							cont_thread);
>>
>>       /* No stepping, no signal - unless one is pending already, of course.  */
>>       if (thread == NULL)
>>
>> So, cont_thread does not exist, which was the whole point of reaching
>> here.  Therefore there's no use trying to resuming it (at first sight).
>>
>> BTW, I have just recently stumbled on this:
>>
>>  http://sourceware.org/ml/gdb-patches/2012-01/msg00502.html
>>
>> But as said, I'll need to take a better look at the gdbserver bits.
> 
> FYI I did not repost this patch part as it needs to be implemented by some
> larger code rewrite IMO now, anyway this patch chunk is not good according to
> your review.

Yeah.  That cont_thread bit should really go away.

I've been looking at your other patch (the linux_wait_for_event_1 one), and
seeing:

(gdb) run
Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.multi/watchpoint-multi
handling possible serial event
getpkt ("QDisableRandomization:1");  [no ack sent]
[address space randomization disabled]
putpkt ("$OK#9a"); [noack mode]
handling possible serial event
getpkt ("vRun;2f686f6d652f706564726f2f6764622f6d796769742f6275696c642f6764622f7465737473756974652f6764622e6d756c74692f7761746368706f696e742d6d756c7469");  [no ack sent]
new_argv[0] = "/home/pedro/gdb/mygit/build/gdb/testsuite/gdb.multi/watchpoint-multi"
Process /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.multi/watchpoint-multi created; pid = 21270
linux_wait: [Process 21270]
pc is 0x40060f
Need step over [LWP 21269]? yes, but found GDB breakpoint at 0x40060f; skipping step over
Need step over [LWP 21270]? Ignoring, not stopped
Resuming, no pending status or step over needed
resuming LWP 21269
pc is 0x40060f
Resuming lwp 21269 (continue, signal 0, stop not expected)
  resuming from pc 0x40060f
resuming LWP 21270
linux_wait_for_lwp: <all threads>

And I had a wth moment -- Why are we resuming 21269 at all, since
we just spawned 21270.  I then realized that it is resumed exactly
that by broken cont_thread code in linux_wait_1...

I really would like to get back to getting rid of those cont_thread
bits, but, this patch, very similar to the one linked above (which fixed
it for vAttach), completely fixes this testcase as well.  The issue is
that cont_thread is also stale from the previous run, when we
start a new vRun.  So I think the patch below is correct, and should
be applied.

You patch may _also_ be correct, and necessary for other cases.
I'll go back to staring at it.

-- 
Pedro Alves

2012-01-25  Pedro Alves  <palves@redhat.com>

	* server.c (start_inferior): Clear `cont_thread'.

---

 0 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index c1788a9..9c50ecc 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -259,6 +259,10 @@ start_inferior (char **argv)
   signal (SIGTTIN, SIG_DFL);
 #endif

+  /* Clear this so the backend doesn't get confused, thinking
+     CONT_THREAD died, and it needs to resume all threads.  */
+  cont_thread = null_ptid;
+
   signal_pid = create_inferior (new_argv[0], new_argv);

   /* FIXME: we don't actually know at this point that the create