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

Pedro Alves
2012-01-25T18:04:09Z

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