Hi Qian, On Oct 2 21:27, Qian Hong wrote: > Dear Cygwin developers, > > While testing Cygwin on Wine, there is a random crashing puzzled me > for a wrong time. > [...] > Alternative, we also tried a hack in the Cygwin side, which use > ThreadQuerySetWin32StartAddress to query the thread entry point, as > 0001-hack-use-ThreadQuerySetWin32StartAddress.txt show. I tested this > hack with recent Cygwin git repo and confirming it works for me > (without hack from Wine side). I also tested my own cygwin build with > this hack on Windows to confirm it doesn't break things. > > Is the proposal way accepted by Cygwin? I understand we hate changing > working code (on Windows), but using ThreadQuerySetWin32StartAddress > seems like an improvement than rely on searching result from stack > memory. If we could discuss a solution which makes both Cygwin and > Wine happy that would be great. > > MSDN says, "Note that on versions of Windows prior to Windows Vista, > the returned start address is only reliable before the thread starts > running.". Actually I tested my build on Windows XP sp2 and it works > for me. Additional, since Cygwin is moving to the end of Windows XP > support, maybe we are at the right time to do this change.
Thanks for this patch. I tried it on Windows 10 under 64 bit and it works fine, too. While the existing code seems to work on Windows as desired, it's not really a safe bet, so calling NtQueryInformationThread to get the correct thread start address seems like a nice improvement. Sidenote: During testing it occured to me that it would be even better if we could just call NtSetInformationThread setting the entry point to threadfunc_fe dropping the rather hackish stack walk entirely. Alas, apparently this functionality, calling NtSetInformationThread with the ThreadQuerySetWin32StartAddress information class, has been intentionally broken starting with Windows Vista :-P I simplified your patch, taking out the printf's and added a test in the loop in case NtQueryInformationThread failed. See below. It's not overly large, but it introduces new functionality. It would be nice if you could sign a copyright assignment and send it as PDF. Please see https://cygwin.com/assign.txt. Thanks, Corinna diff --git a/winsup/cygwin/init.cc b/winsup/cygwin/init.cc index 56d4668..69e66a0 100644 --- a/winsup/cygwin/init.cc +++ b/winsup/cygwin/init.cc @@ -55,12 +55,17 @@ munge_threadfunc () if (threadfunc_ix[0]) { - char *threadfunc = ebp[threadfunc_ix[0]]; + char *threadfunc = NULL; + + NtQueryInformationThread (NtCurrentThread (), + ThreadQuerySetWin32StartAddress, + &threadfunc, sizeof threadfunc, NULL); if (!search_for || threadfunc == search_for) { search_for = NULL; for (i = 0; threadfunc_ix[i]; i++) - ebp[threadfunc_ix[i]] = (char *) threadfunc_fe; + if (!threadfunc || ebp[threadfunc_ix[i]] == threadfunc) + ebp[threadfunc_ix[i]] = (char *) threadfunc_fe; TlsSetValue (_my_oldfunc, threadfunc); } } diff --git a/winsup/cygwin/ntdll.h b/winsup/cygwin/ntdll.h index 13a131d..050e848 100644 --- a/winsup/cygwin/ntdll.h +++ b/winsup/cygwin/ntdll.h @@ -1162,7 +1162,8 @@ typedef enum _THREADINFOCLASS { ThreadBasicInformation = 0, ThreadTimes = 1, - ThreadImpersonationToken = 5 + ThreadImpersonationToken = 5, + ThreadQuerySetWin32StartAddress = 9 } THREADINFOCLASS, *PTHREADINFOCLASS; /* Checked on 64 bit. */ -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat
pgpexLwZdH2uB.pgp
Description: PGP signature
