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@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.