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

[Pedro Alves <palves@redhat.com>]

On 01/25/2012 05:27 PM, Pedro Alves wrote:
> 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.

Bah, no, it is still not sufficient.  Don't know why it passed for me
before.  Looking again...

> 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.

-- 
Pedro Alves