Re: [PATCH] Fix "PC register is not available" issue

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Fri, 28 Mar 2014 14:50:31 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: Joel Brobecker <brobecker@adacore.com>, gdb-patches@sourceware.org
> 
> On 03/26/2014 06:49 PM, Eli Zaretskii wrote:
> > This describes the results of my looking into this issue, given the
> > comments and suggestions by Joel and Pedro.  Sorry about the length.
> > 
> >> I didn't mean to change the behavior - only hide the warning.
> >> In this case, if it is normal that we can't suspend the thread,
> >> then there is no point in warning (scaring) the user about it.
> >> I would only generate a warning if something abnormal that we should
> >> fix occured.
> > 
> > The patch near the end of this message indeed includes code to ignore
> > the warning in these cases.
> > 
> >> I see that the GetThreadContext call (do_windows_fetch_inferior_registers)
> >> doesn't check for errors (I think it should (*)).  It'd be interesting to know whether gdb can
> >> actually read the registers off of this thread, and if so, what's the
> >> thread's backtrace like.
> > 
> > I added CHECK to that call to GetThreadContext.  It never produced a
> > warning in all my testing, and it looks like we do succeed to get the
> > registers.  At least the registers of 2 such threads show different
> > contents, and the EIP value is consistent with what "info threads"
> > displays.
> 
> It isn't clear to me whether you're saying that you saw the
> SuspendThread failure trigger in all your new testing, so that
> we'd know for sure whether GetThreadContext suceeds in that case,
> or whether it might have been that you just were "lucky" enough
> to not trigger the SuspendThread failure issue.

The former.

> Does your patch fix the test case in PR14018, without producing
> a CHECK warning from the new CHECK in GetThreadContext you've
> added?

Yes.

> > I can show you 2 typical examples.  This is from Emacs, where the
> > application has 3 threads, and one more is started by the debugger.
> > The rest, threads 5 and 6 in these examples, are those mysterious
> > threads we are talking about.
> > 
> >   (gdb) info threads
> >     Id   Target Id         Frame
> >     6    Thread 15492.0x1f28 0x77a41f46 in ntdll!ZwWaitForWorkViaWorkerFactory
> >       () from C:\Windows\system32\ntdll.dll
> >     5    Thread 15492.0x73c0 0x77a41f46 in ntdll!ZwWaitForWorkViaWorkerFactory
> >       () from C:\Windows\system32\ntdll.dll
> >     4    Thread 15492.0x2300 0x75ac78d7 in USER32!DispatchMessageW ()
> >      from C:\Windows\syswow64\user32.dll
> >     3    Thread 15492.0x1860 0x77a3fd91 in ntdll!ZwDelayExecution ()
> >      from C:\Windows\system32\ntdll.dll
> >     2    Thread 15492.0x2410 0x77a4015d in ntdll!ZwWaitForMultipleObjects ()
> >      from C:\Windows\system32\ntdll.dll
> >   * 1    Thread 15492.0x44a0 cleanup_vector (vector=0x62daeb0) at alloc.c:2917
> > 
> >   (gdb) info threads
> >     Id   Target Id         Frame
> >     6    Thread 15492.0x1f28 0x77a3f8d1 in ntdll!ZwWaitForSingleObject ()
> >      from C:\Windows\system32\ntdll.dll
> >     5    Thread 15492.0x73c0 0x77a72880 in ntdll!RtlFillMemoryUlong ()
> >      from C:\Windows\system32\ntdll.dll
> >     4    Thread 15492.0x2300 0x75ac78d7 in USER32!DispatchMessageW ()
> >      from C:\Windows\syswow64\user32.dll
> >     3    Thread 15492.0x1860 0x77a3fd91 in ntdll!ZwDelayExecution ()
> >      from C:\Windows\system32\ntdll.dll
> >     2    Thread 15492.0x2410 0x77a4015d in ntdll!ZwWaitForMultipleObjects ()
> >      from C:\Windows\system32\ntdll.dll
> >   * 1    Thread 15492.0x44a0 cleanup_vector (vector=0x388ca58) at alloc.c:2917
> >
> > The first display is what I usually see: several (I've seen up to 4)
> > threads waiting inside ZwWaitForWorkViaWorkerFactory.  But sometimes
> > they do perform some work, as can be seen from the second display.
> 
> OK, but these don't appear to be backtraces taken right after
> SuspendThread failed.

Yes, they are after SuspendThread failed.

> Why bother calling SetThreadContext at all if we just killed
> the process?

See my other mail and Joel's response.

> > Finally, here's the full patch.  I hope this research answered all the
> > questions, and we can now get the patch in.
> 
> I'm not sure it did, but in any case the patch looks good to me.

If that's an approval, I will happily commit the changes.

> Sounds like GDBserver might have this problem too.

If there's an easy way to verify that, without having 2 systems
talking via some communications line, please tell how, and I will try
that.

Thanks.