[PATCH] ntdll: Workaround for requesting thread affinity mask that isn't a subset of the system's mask

[Roger Zoellner]

This patch is related to the game 'Divinity Original Sin 2' which crashes on startup, if the system only
has 4 or less logical cpus.

By Microsoft specification, requesting a thread affinity mask requires it be be a subset of the process affiniy mask.
However the game requests a mask F...FE (-2) which makes wine to return STATUS_INVALID_PARAMETER, which in turn  makes
the game hang.

This patch will make wine to adjust the mask in this case, which makes the game start correctly.


Signed-off-by: Roger Zoellner <zoellner.roger(a)gmail.com>
---
 dlls/ntdll/thread.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c
index 59d64e174d..6beb8c1a3d 100644
--- a/dlls/ntdll/thread.c
+++ b/dlls/ntdll/thread.c
@@ -1290,6 +1290,7 @@ NTSTATUS WINAPI NtSetInformationThread( HANDLE handle, THREADINFOCLASS class,
             if (length != sizeof(ULONG_PTR)) return STATUS_INVALID_PARAMETER;
             req_aff = *(const ULONG_PTR *)data;
             if ((ULONG)req_aff == ~0u) req_aff = affinity_mask;
+            else if ((LONG) req_aff < 0) req_aff = affinity_mask & req_aff;
             else if (req_aff & ~affinity_mask) return STATUS_INVALID_PARAMETER;
             else if (!req_aff) return STATUS_INVALID_PARAMETER;
             SERVER_START_REQ( set_thread_info )
-- 
2.18.0

[Dmitry Timoshkov]

Roger Zoellner <zoellner.roger(a)gmail.com> wrote:
> @@ -1290,6 +1290,7 @@ NTSTATUS WINAPI NtSetInformationThread( HANDLE handle, THREADINFOCLASS class,
>              if (length != sizeof(ULONG_PTR)) return STATUS_INVALID_PARAMETER;
>              req_aff = *(const ULONG_PTR *)data;
>              if ((ULONG)req_aff == ~0u) req_aff = affinity_mask;
> +            else if ((LONG) req_aff < 0) req_aff = affinity_mask & req_aff;

It should be possible to add a test case for this. Also there should be
nothing special about negative values unless there's a test case that
shows that all negative values get accepted.

-- 
Dmitry.

[Chris Robinson]

On 09/08/2018 06:26 AM, Roger Zoellner wrote:
> if ((ULONG)req_aff == ~0u) req_aff = affinity_mask;
> +            else if ((LONG) req_aff < 0) req_aff = affinity_mask & req_aff;

In addition to Dmitry's comment, it's notable that these two cases 
basically become the same thing. ~0u is -1, which would make req_aff a 
full mask. Though it may also be worth a test if the sign check actually 
applies to LONG_PTR instead of LONG, since LONG_PTR is 64-bit on 64-bit 
targets while LONG is 32-bit even on 64-bit targets.

[Roger Zoellner]

I've created a test case that shows, that Wine indeed behaves incorrectly
when the highest bit of the requested mask is set.
With the (obvious) exception of ~0u, Wine returns STATUS_INVALID_PARAMETER
while Windows returns 0 (SUCCESS).

You're right. Casting to LONG is probably wrong here.

Am So., 9. Sep. 2018 um 10:31 Uhr schrieb Chris Robinson <
chris.kcat(a)gmail.com>:
> On 09/08/2018 06:26 AM, Roger Zoellner wrote:
> > if ((ULONG)req_aff == ~0u) req_aff = affinity_mask;
> > +            else if ((LONG) req_aff < 0) req_aff = affinity_mask &
>
> req_aff;
>
> In addition to Dmitry's comment, it's notable that these two cases
> basically become the same thing. ~0u is -1, which would make req_aff a
> full mask. Though it may also be worth a test if the sign check actually
> applies to LONG_PTR instead of LONG, since LONG_PTR is 64-bit on 64-bit
> targets while LONG is 32-bit even on 64-bit targets.

[Roger Zoellner]

This line is definately a problem:
> else if (req_aff & ~affinity_mask) return STATUS_INVALID_PARAMETER;

After inverting affinity_mask, it will contain leading 1s. So the
comparison will always return true when req_aff > system_mask


Am So., 9. Sep. 2018 um 16:12 Uhr schrieb Roger Zoellner <
zoellner.roger(a)gmail.com>:
> I've created a test case that shows, that Wine indeed behaves incorrectly
> when the highest bit of the requested mask is set.
> With the (obvious) exception of ~0u, Wine returns STATUS_INVALID_PARAMETER
> while Windows returns 0 (SUCCESS).
>
> You're right. Casting to LONG is probably wrong here.
>
> Am So., 9. Sep. 2018 um 10:31 Uhr schrieb Chris Robinson <
> chris.kcat(a)gmail.com>:
> > On 09/08/2018 06:26 AM, Roger Zoellner wrote:
> > > if ((ULONG)req_aff == ~0u) req_aff = affinity_mask;
> > > +            else if ((LONG) req_aff < 0) req_aff = affinity_mask &
> >
> > req_aff;
> >
> > In addition to Dmitry's comment, it's notable that these two cases
> > basically become the same thing. ~0u is -1, which would make req_aff a
> > full mask. Though it may also be worth a test if the sign check actually
> > applies to LONG_PTR instead of LONG, since LONG_PTR is 64-bit on 64-bit
> > targets while LONG is 32-bit even on 64-bit targets.