[PATCH 1/2] ntdll/tests: Check that creating huge thread stacks should work.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- I'm not completely sure what 789c1db18a4e192425da3771cac4726cda77130b was for, but it seems to be causing spurious failures. This fix is trying to keep the current behavior, but it may be better to completely revert the commit as, although less likely, TEB allocation could also fail much more often. dlls/ntdll/tests/virtual.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 8f5b0092bea..686b4076801 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -488,6 +488,11 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) return 0; } +static DWORD WINAPI test_stack_size_dummy_thread(void *ptr) +{ + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); @@ -563,6 +568,11 @@ static void test_RtlCreateUserStack(void) thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); + + thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); } static void test_NtMapViewOfSection(void) -- 2.31.0
When creating it in the lower 2G fails. This fixes a regression from 789c1db18a4e192425da3771cac4726cda77130b, which causes several apps to fail when the lower 2G are getting crowded, or when they have unusually large default thread stack sizes. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 2 +- dlls/ntdll/unix/virtual.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 686b4076801..853ff9c27a7 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -570,7 +570,7 @@ static void test_RtlCreateUserStack(void) CloseHandle(thread); thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); - todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + ok(thread != NULL, "CreateThread with huge stack failed\n"); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); } diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index c3baa5f09c7..e036ebdec86 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -3051,6 +3051,7 @@ NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, SI NTSTATUS status; sigset_t sigset; SIZE_T size, extra_size = 0; + unsigned int vprot = VPROT_READ | VPROT_WRITE | VPROT_COMMITTED; if (!reserve_size) reserve_size = main_image_info.MaximumStackSize; if (!commit_size) commit_size = main_image_info.CommittedStackSize; @@ -3062,9 +3063,9 @@ NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, SIZE_T reserve_size, SI server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - if ((status = map_view( &view, NULL, size + extra_size, FALSE, - VPROT_READ | VPROT_WRITE | VPROT_COMMITTED, 33 )) != STATUS_SUCCESS) - goto done; + status = map_view( &view, NULL, size + extra_size, FALSE, vprot, 33 ); + if (status == STATUS_NO_MEMORY) status = map_view( &view, NULL, size + extra_size, FALSE, vprot, 0 ); + if (status != STATUS_SUCCESS) goto done; #ifdef VALGRIND_STACK_REGISTER VALGRIND_STACK_REGISTER( view->base, (char *)view->base + view->size ); -- 2.31.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=90057 Your paranoid android. === w2008s64 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w7u_2qxl (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w7u_adm (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w7u_el (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w8 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w8adm (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w864 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w1064v1507 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w1064v1809 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w1064 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w1064_tsign (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w10pro64 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === debiant2 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === debiant2 (32 bit French report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === debiant2 (32 bit Japanese:Japan report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === debiant2 (32 bit Chinese:China report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === debiant2 (32 bit WoW report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed
On 5/7/21 12:08 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
I'm not completely sure what 789c1db18a4e192425da3771cac4726cda77130b was for, but it seems to be causing spurious failures. This fix is trying to keep the current behavior, but it may be better to completely revert the commit as, although less likely, TEB allocation could also fail much more often.
dlls/ntdll/tests/virtual.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 8f5b0092bea..686b4076801 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -488,6 +488,11 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) return 0; }
+static DWORD WINAPI test_stack_size_dummy_thread(void *ptr) +{ + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); @@ -563,6 +568,11 @@ static void test_RtlCreateUserStack(void) thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); + + thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); }
static void test_NtMapViewOfSection(void)
Eh obviously that fails on 32bit. Anyway, I'm not sure at all about the fix, it's maybe something related to allocations that should be bottom-up by default? -- Rémi Bernon <rbernon(a)codeweavers.com>
On 07/05/2021 13:20, Rémi Bernon wrote:
On 5/7/21 12:08 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
I'm not completely sure what 789c1db18a4e192425da3771cac4726cda77130b was for, but it seems to be causing spurious failures. This fix is trying to keep the current behavior, but it may be better to completely revert the commit as, although less likely, TEB allocation could also fail much more often.
dlls/ntdll/tests/virtual.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 8f5b0092bea..686b4076801 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -488,6 +488,11 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) return 0; } +static DWORD WINAPI test_stack_size_dummy_thread(void *ptr) +{ + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); @@ -563,6 +568,11 @@ static void test_RtlCreateUserStack(void) thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); + + thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); } static void test_NtMapViewOfSection(void)
Eh obviously that fails on 32bit. Anyway, I'm not sure at all about the fix, it's maybe something related to allocations that should be bottom-up by default?
There's a wine-staging patch that does this and fixes some bugs: ntdll-ForceBottomUpAlloc Does it help? Also what happens on Windows if you force top-down allocation?
On 5/7/21 1:38 PM, Gabriel Ivăncescu wrote:
On 07/05/2021 13:20, Rémi Bernon wrote:
On 5/7/21 12:08 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
I'm not completely sure what 789c1db18a4e192425da3771cac4726cda77130b was for, but it seems to be causing spurious failures. This fix is trying to keep the current behavior, but it may be better to completely revert the commit as, although less likely, TEB allocation could also fail much more often.
dlls/ntdll/tests/virtual.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 8f5b0092bea..686b4076801 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -488,6 +488,11 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) return 0; } +static DWORD WINAPI test_stack_size_dummy_thread(void *ptr) +{ + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); @@ -563,6 +568,11 @@ static void test_RtlCreateUserStack(void) thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); + + thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); } static void test_NtMapViewOfSection(void)
Eh obviously that fails on 32bit. Anyway, I'm not sure at all about the fix, it's maybe something related to allocations that should be bottom-up by default?
There's a wine-staging patch that does this and fixes some bugs:
ntdll-ForceBottomUpAlloc
Does it help? Also what happens on Windows if you force top-down allocation?
It doesn't help with the regression, because we now forcefully limit the address space where the thread stack / teb can be allocated. What I meant was that although I don't know why it's been added, and if the reason was to make sure early threads are allocated on low addresses, maybe making default allocation strategy bottom-up is the correct fix (instead of trying to forcefully put them in the low 2G). I think that patch set has been sent already before but it was considered a bit over complicated. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 07/05/2021 14:44, Rémi Bernon wrote:
On 5/7/21 1:38 PM, Gabriel Ivăncescu wrote:
On 07/05/2021 13:20, Rémi Bernon wrote:
On 5/7/21 12:08 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
I'm not completely sure what 789c1db18a4e192425da3771cac4726cda77130b was for, but it seems to be causing spurious failures. This fix is trying to keep the current behavior, but it may be better to completely revert the commit as, although less likely, TEB allocation could also fail much more often.
dlls/ntdll/tests/virtual.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 8f5b0092bea..686b4076801 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -488,6 +488,11 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) return 0; } +static DWORD WINAPI test_stack_size_dummy_thread(void *ptr) +{ + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); @@ -563,6 +568,11 @@ static void test_RtlCreateUserStack(void) thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); + + thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); } static void test_NtMapViewOfSection(void)
Eh obviously that fails on 32bit. Anyway, I'm not sure at all about the fix, it's maybe something related to allocations that should be bottom-up by default?
There's a wine-staging patch that does this and fixes some bugs:
ntdll-ForceBottomUpAlloc
Does it help? Also what happens on Windows if you force top-down allocation?
It doesn't help with the regression, because we now forcefully limit the address space where the thread stack / teb can be allocated.
What I meant was that although I don't know why it's been added, and if the reason was to make sure early threads are allocated on low addresses, maybe making default allocation strategy bottom-up is the correct fix (instead of trying to forcefully put them in the low 2G).
I think that patch set has been sent already before but it was considered a bit over complicated.
Yeah, but if all allocations are forced bottom-up, not just the thread stack/TEB, then the staging patch should already fix this, implicitly. An interesting thing is if you forced top-down allocation on Windows, do they get allocated in low 2G? Info here: https://tedwvc.wordpress.com/2013/07/16/porting-win32-code-to-64-bit-watch-o... I think that would explain Windows behavior and whether it special cases them or not.
On 5/7/21 1:56 PM, Gabriel Ivăncescu wrote:
On 07/05/2021 14:44, Rémi Bernon wrote:
On 5/7/21 1:38 PM, Gabriel Ivăncescu wrote:
On 07/05/2021 13:20, Rémi Bernon wrote:
On 5/7/21 12:08 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
I'm not completely sure what 789c1db18a4e192425da3771cac4726cda77130b was for, but it seems to be causing spurious failures. This fix is trying to keep the current behavior, but it may be better to completely revert the commit as, although less likely, TEB allocation could also fail much more often.
dlls/ntdll/tests/virtual.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 8f5b0092bea..686b4076801 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -488,6 +488,11 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) return 0; } +static DWORD WINAPI test_stack_size_dummy_thread(void *ptr) +{ + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); @@ -563,6 +568,11 @@ static void test_RtlCreateUserStack(void) thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); + + thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); } static void test_NtMapViewOfSection(void)
Eh obviously that fails on 32bit. Anyway, I'm not sure at all about the fix, it's maybe something related to allocations that should be bottom-up by default?
There's a wine-staging patch that does this and fixes some bugs:
ntdll-ForceBottomUpAlloc
Does it help? Also what happens on Windows if you force top-down allocation?
It doesn't help with the regression, because we now forcefully limit the address space where the thread stack / teb can be allocated.
What I meant was that although I don't know why it's been added, and if the reason was to make sure early threads are allocated on low addresses, maybe making default allocation strategy bottom-up is the correct fix (instead of trying to forcefully put them in the low 2G).
I think that patch set has been sent already before but it was considered a bit over complicated.
Yeah, but if all allocations are forced bottom-up, not just the thread stack/TEB, then the staging patch should already fix this, implicitly.
I think it makes things ever worse as more things will be allocated in the low address space?
An interesting thing is if you forced top-down allocation on Windows, do they get allocated in low 2G? Info here:
https://tedwvc.wordpress.com/2013/07/16/porting-win32-code-to-64-bit-watch-o...
I think that would explain Windows behavior and whether it special cases them or not.
I don't understand, you can't control thread stack allocation strategy. At least not using CreateThread. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 5/7/21 1:58 PM, Rémi Bernon wrote:
On 5/7/21 1:56 PM, Gabriel Ivăncescu wrote:
On 07/05/2021 14:44, Rémi Bernon wrote:
On 5/7/21 1:38 PM, Gabriel Ivăncescu wrote:
On 07/05/2021 13:20, Rémi Bernon wrote:
On 5/7/21 12:08 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
I'm not completely sure what 789c1db18a4e192425da3771cac4726cda77130b was for, but it seems to be causing spurious failures. This fix is trying to keep the current behavior, but it may be better to completely revert the commit as, although less likely, TEB allocation could also fail much more often.
dlls/ntdll/tests/virtual.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 8f5b0092bea..686b4076801 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -488,6 +488,11 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) return 0; } +static DWORD WINAPI test_stack_size_dummy_thread(void *ptr) +{ + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); @@ -563,6 +568,11 @@ static void test_RtlCreateUserStack(void) thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); + + thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); } static void test_NtMapViewOfSection(void)
Eh obviously that fails on 32bit. Anyway, I'm not sure at all about the fix, it's maybe something related to allocations that should be bottom-up by default?
There's a wine-staging patch that does this and fixes some bugs:
ntdll-ForceBottomUpAlloc
Does it help? Also what happens on Windows if you force top-down allocation?
It doesn't help with the regression, because we now forcefully limit the address space where the thread stack / teb can be allocated.
What I meant was that although I don't know why it's been added, and if the reason was to make sure early threads are allocated on low addresses, maybe making default allocation strategy bottom-up is the correct fix (instead of trying to forcefully put them in the low 2G).
I think that patch set has been sent already before but it was considered a bit over complicated.
Yeah, but if all allocations are forced bottom-up, not just the thread stack/TEB, then the staging patch should already fix this, implicitly.
I think it makes things ever worse as more things will be allocated in the low address space?
An interesting thing is if you forced top-down allocation on Windows, do they get allocated in low 2G? Info here:
https://tedwvc.wordpress.com/2013/07/16/porting-win32-code-to-64-bit-watch-o...
I think that would explain Windows behavior and whether it special cases them or not.
I don't understand, you can't control thread stack allocation strategy. At least not using CreateThread.
Oh sorry, just reading that link now. I guess you can. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 07/05/2021 14:58, Rémi Bernon wrote:
On 5/7/21 1:56 PM, Gabriel Ivăncescu wrote:
On 07/05/2021 14:44, Rémi Bernon wrote:
On 5/7/21 1:38 PM, Gabriel Ivăncescu wrote:
On 07/05/2021 13:20, Rémi Bernon wrote:
On 5/7/21 12:08 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
I'm not completely sure what 789c1db18a4e192425da3771cac4726cda77130b was for, but it seems to be causing spurious failures. This fix is trying to keep the current behavior, but it may be better to completely revert the commit as, although less likely, TEB allocation could also fail much more often.
dlls/ntdll/tests/virtual.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 8f5b0092bea..686b4076801 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -488,6 +488,11 @@ static DWORD WINAPI test_stack_size_thread(void *ptr) return 0; } +static DWORD WINAPI test_stack_size_dummy_thread(void *ptr) +{ + return 0; +} + static void test_RtlCreateUserStack(void) { IMAGE_NT_HEADERS *nt = RtlImageNtHeader( NtCurrentTeb()->Peb->ImageBaseAddress ); @@ -563,6 +568,11 @@ static void test_RtlCreateUserStack(void) thread = CreateThread(NULL, 0x3ff000, test_stack_size_thread, &args, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); + + thread = CreateThread(NULL, 0x80000000, test_stack_size_dummy_thread, NULL, STACK_SIZE_PARAM_IS_A_RESERVATION, NULL); + todo_wine ok(thread != NULL, "CreateThread with huge stack failed\n"); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); } static void test_NtMapViewOfSection(void)
Eh obviously that fails on 32bit. Anyway, I'm not sure at all about the fix, it's maybe something related to allocations that should be bottom-up by default?
There's a wine-staging patch that does this and fixes some bugs:
ntdll-ForceBottomUpAlloc
Does it help? Also what happens on Windows if you force top-down allocation?
It doesn't help with the regression, because we now forcefully limit the address space where the thread stack / teb can be allocated.
What I meant was that although I don't know why it's been added, and if the reason was to make sure early threads are allocated on low addresses, maybe making default allocation strategy bottom-up is the correct fix (instead of trying to forcefully put them in the low 2G).
I think that patch set has been sent already before but it was considered a bit over complicated.
Yeah, but if all allocations are forced bottom-up, not just the thread stack/TEB, then the staging patch should already fix this, implicitly.
I think it makes things ever worse as more things will be allocated in the low address space?
AFAIK that's already the case on Windows (bottom-up default allocation behavior), and some things are known to break if top-down is forced in the registry. Even Microsoft's own xaudio breaks. Not related to stack or teb. I don't know the purpose of the current code, I'm just assuming it might be a semi-hack (probably dealing with apps that expect stack in lower 2G) and the "correct" fix would be to force bottom up allocation, as that also fixes other apps and matches Windows default behavior. Then we won't need this. And of course, I doubt Windows restricts the stack to low 2G if it doesn't have any more room there. Anyway this will need investigation.
Hi Rémi, Sorry, it seems my last email didn't get sent for some reason. Anyway, I wanted to say I did a quick test on Windows 10. Setting that registry value will make all stacks be allocated from top-down, even the main thread's stack (i.e. far above 4 GB). So it seems that Windows doesn't really treat the stack special here. The fact it's allocated below 2G for most apps is due to the side-effect of the bottom-up allocation behavior on Windows by default. The current Wine code probably does that since some apps depend on it, but IMO correct fix would be to have all allocations bottom-up like the staging patchset, since other apps depend on that.
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=90056 Your paranoid android. === w2008s64 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w7u_2qxl (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w7u_adm (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w7u_el (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w8 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w8adm (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w864 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w1064v1507 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w1064v1809 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w1064 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w1064_tsign (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed === w10pro64 (32 bit report) === ntdll: virtual.c:573: Test failed: CreateThread with huge stack failed
participants (3)
-
Gabriel Ivăncescu -
Marvin -
Rémi Bernon