Mail Archives: cygwin/2015/10/21/04:17:39
X-Recipient: | archive-cygwin AT delorie DOT com
|
DomainKey-Signature: | a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id
|
| :list-unsubscribe:list-subscribe:list-archive:list-post
|
| :list-help:sender:date:from:to:subject:message-id:reply-to
|
| :references:mime-version:content-type:in-reply-to; q=dns; s=
|
| default; b=RrwMn5dijkraezBeLTgTgamxZDF4rq6qsNvD/Knv3BcA7LPJmcDVG
|
| kP8sYbX+DAe9zmXmnAIwmhVMVl2y9tjmxbaCuZ4r+ixhu0JCoWa2qkkk09bguGxs
|
| 858Jlz7i9DNV285+JBh27FMKW08TuFXpqTme87/Cqqk/+INfcdo5xQ=
|
DKIM-Signature: | v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id
|
| :list-unsubscribe:list-subscribe:list-archive:list-post
|
| :list-help:sender:date:from:to:subject:message-id:reply-to
|
| :references:mime-version:content-type:in-reply-to; s=default;
|
| bh=q9vwBAFGzRIm/dgZTA4fUW6hmuE=; b=hLyxLwyWY6pKkC+pz3TuPMfn6Bxn
|
| xxbttYTetQmWvgDMcLc03tUxYnIJ1clQQTSbj0sHXM9y4Vj8gAE513i4Fvcycj+Q
|
| SJsItX1uIx9LAiDVz/J9j+uI9hPnM6PUE2bPpC4V+EzyzZSb9ZWmeXlsAVyBp7KA
|
| EPw1YdLxufmyi1s=
|
Mailing-List: | contact cygwin-help AT cygwin DOT com; run by ezmlm
|
List-Id: | <cygwin.cygwin.com>
|
List-Subscribe: | <mailto:cygwin-subscribe AT cygwin DOT com>
|
List-Archive: | <http://sourceware.org/ml/cygwin/>
|
List-Post: | <mailto:cygwin AT cygwin DOT com>
|
List-Help: | <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
|
Sender: | cygwin-owner AT cygwin DOT com
|
Mail-Followup-To: | cygwin AT cygwin DOT com
|
Delivered-To: | mailing list cygwin AT cygwin DOT com
|
Authentication-Results: | sourceware.org; auth=none
|
X-Virus-Found: | No
|
X-Spam-SWARE-Status: | No, score=-5.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2
|
X-HELO: | calimero.vinschen.de
|
Date: | Wed, 21 Oct 2015 10:17:11 +0200
|
From: | Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
|
To: | cygwin AT cygwin DOT com
|
Subject: | Re: Proposal to use ThreadQuerySetWin32StartAddress inside munge_threadfunc (Cygwin randomly crashes on Wine)
|
Message-ID: | <20151021081711.GJ5319@calimero.vinschen.de>
|
Reply-To: | cygwin AT cygwin DOT com
|
Mail-Followup-To: | cygwin AT cygwin DOT com
|
References: | <CALd+sZSPx-bUuq6FQ42vzWpXgGRhuakML87J-N3Wg+3Q0HSFCw AT mail DOT gmail DOT com>
|
MIME-Version: | 1.0
|
In-Reply-To: | <CALd+sZSPx-bUuq6FQ42vzWpXgGRhuakML87J-N3Wg+3Q0HSFCw@mail.gmail.com>
|
User-Agent: | Mutt/1.5.23 (2014-03-12)
|
--uJWb33pM2TcUAXIl
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Hi Qian,
On Oct 2 21:27, Qian Hong wrote:
> Dear Cygwin developers,
>=20
> 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.
>=20
> 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.
>=20
> 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 ()
=20
if (threadfunc_ix[0])
{
- char *threadfunc =3D ebp[threadfunc_ix[0]];
+ char *threadfunc =3D NULL;
+
+ NtQueryInformationThread (NtCurrentThread (),
+ ThreadQuerySetWin32StartAddress,
+ &threadfunc, sizeof threadfunc, NULL);
if (!search_for || threadfunc =3D=3D search_for)
{
search_for =3D NULL;
for (i =3D 0; threadfunc_ix[i]; i++)
- ebp[threadfunc_ix[i]] =3D (char *) threadfunc_fe;
+ if (!threadfunc || ebp[threadfunc_ix[i]] =3D=3D threadfunc)
+ ebp[threadfunc_ix[i]] =3D (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 =3D 0,
ThreadTimes =3D 1,
- ThreadImpersonationToken =3D 5
+ ThreadImpersonationToken =3D 5,
+ ThreadQuerySetWin32StartAddress =3D 9
} THREADINFOCLASS, *PTHREADINFOCLASS;
=20
/* Checked on 64 bit. */
--=20
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
--uJWb33pM2TcUAXIl
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBCAAGBQJWJ0oHAAoJEPU2Bp2uRE+gXIkQAIKN/BsxVDgDj1d3LRlQu43G
Js0GIHJr5KaU97Y1cBt1nsfQAb6XJiDj37Dl4Ja7+6fLasm1lNog6TMVz+A95cpB
XhV+0XpM6DpXt0OGvsu+vEoqD8JkngO+TT55W8kKGUwoZBp4loEXSFeBb5AW35D9
P4jttnohGqLm+7v7x7lmr+bla778X2UWVJjLHEyMWaj+bO+8AXQMIA16BqW3ewM/
drq9Md3DGoO7sA876M/fI77pFMVZymKm+tUYNf9d5x1WcIgBoZsh5aM0LD2FCunD
NOsTQnbQ88utLI4IltRR/DYc1VOXgslaDD75CSSgN6GejhhrW2BIYjuO8Fn/Ok4b
hIdZYduP7302in1z1BmmAQUUitxSu1+UzwSh64mbWknnjO7Vg7Bw2NuPqfa2MTsw
jWpT/pH8E2aMntoyFT9LucRf7xQg3QQk8owgDUgyY6wYwRdDW5KYptQy4vRJCgPO
K9uh18ljwRvecg1EuYLQIkvruY77yog4I01KUvV6Xa6s6vwMHquGAqpA+bQArdTM
dPfILYDpl1WyWZhs3midDrCg68GrYdijDvHixJvXLRV3l5l2DNNQAD887cHzKn1O
qBjcjNMvmOoc+RF8IJ0kAkSZzmUCSk5rWwxW2HM4+73ehywBJcEKE5rZVTBaNrdF
UVpE4kJifdzk5dYQoogW
=aUt4
-----END PGP SIGNATURE-----
--uJWb33pM2TcUAXIl--
- Raw text -