[PATCH v2 0/6] MR2435: Fix activation context switching from SwitchToFiber().
-- v2: kernelbase: Implement activation context switching for fibers. ntdll: Implement RtlFreeActivationContextStack(). ntdll: Use ActivationContextStackPointer instead of referencing ActivationContextStack directly. ntdll: Store current activation context stack pointer into a local variable. ntdll: Factor out reading current activation context into a helper function. kernel32/tests: Test for activation context switching between fibers. https://gitlab.winehq.org/wine/wine/-/merge_requests/2435
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/kernel32/tests/fiber.c | 222 ++++++++++++++++++++++++++++++++++++ 1 file changed, 222 insertions(+) diff --git a/dlls/kernel32/tests/fiber.c b/dlls/kernel32/tests/fiber.c index 433883e5a26..5540305cd1a 100644 --- a/dlls/kernel32/tests/fiber.c +++ b/dlls/kernel32/tests/fiber.c @@ -24,6 +24,7 @@ #define WIN32_NO_STATUS #include <winternl.h> #include "wine/test.h" +#include <winuser.h> static LPVOID (WINAPI *pCreateFiber)(SIZE_T,LPFIBER_START_ROUTINE,LPVOID); static LPVOID (WINAPI *pConvertThreadToFiber)(LPVOID); @@ -791,6 +792,226 @@ static void test_FiberLocalStorageWithFibers(PFLS_CALLBACK_FUNCTION cbfunc) test_ConvertFiberToThread(); } +#define check_current_actctx_is(e,t) check_current_actctx_is_(__LINE__, e, t) +static void check_current_actctx_is_(int line, HANDLE expected_actctx, BOOL todo) +{ + HANDLE cur_actctx; + BOOL ret; + + cur_actctx = (void*)0xdeadbeef; + ret = GetCurrentActCtx(&cur_actctx); + ok_(__FILE__, line)(ret, "thread GetCurrentActCtx failed, %lu\n", GetLastError()); + + todo_wine_if(todo) + ok_(__FILE__, line)(cur_actctx == expected_actctx, "got %p, expected %p\n", cur_actctx, expected_actctx); + + ReleaseActCtx(cur_actctx); +} + +static DWORD WINAPI subthread_actctx_func(void *actctx) +{ + HANDLE fiber; + BOOL ret; + + check_current_actctx_is(actctx, FALSE); + + fiber = pConvertThreadToFiber(NULL); + ok(fiber != NULL, "ConvertThreadToFiber returned error %lu\n", GetLastError()); + check_current_actctx_is(actctx, FALSE); + fibers[2] = fiber; + + SwitchToFiber(fibers[0]); + check_current_actctx_is(actctx, FALSE); + + ok(fibers[2] == fiber, "fibers[2]: expected %p, got %p\n", fiber, fibers[2]); + fibers[2] = NULL; + ret = pConvertFiberToThread(); + ok(ret, "ConvertFiberToThread returned error %lu\n", GetLastError()); + check_current_actctx_is(actctx, FALSE); + + return 0; +} + +static void WINAPI fiber_actctx_func(void *actctx) +{ + ULONG_PTR cookie; + DWORD tid, wait; + HANDLE thread; + BOOL ret; + + check_current_actctx_is(NULL, TRUE); + + ret = ActivateActCtx(actctx, &cookie); + ok(ret, "ActivateActCtx returned error %lu\n", GetLastError()); + check_current_actctx_is(actctx, FALSE); + + SwitchToFiber(fibers[0]); + check_current_actctx_is(actctx, FALSE); + + thread = CreateThread(NULL, 0, subthread_actctx_func, actctx, 0, &tid); + ok(thread != NULL, "CreateThread returned error %lu\n", GetLastError()); + + wait = WaitForSingleObject(thread, INFINITE); + ok(wait == WAIT_OBJECT_0, "WaitForSingleObject returned %lu (last error: %lu)\n", + wait, GetLastError()); + CloseHandle(thread); + + ret = DeactivateActCtx(0, cookie); + ok(ret, "DeactivateActCtx returned error %lu\n", GetLastError()); + check_current_actctx_is(NULL, TRUE); + + SwitchToFiber(fibers[0]); + ok(0, "unreachable\n"); +} + +/* Test that activation context is switched on SwitchToFiber() call */ +static void subtest_fiber_actctx_switch(HANDLE current_actctx, HANDLE child_actctx) +{ + fibers[1] = pCreateFiber(0, fiber_actctx_func, child_actctx); + ok(fibers[1] != NULL, "CreateFiber returned error %lu\n", GetLastError()); + check_current_actctx_is(current_actctx, FALSE); + + SwitchToFiber(fibers[1]); + check_current_actctx_is(current_actctx, TRUE); + + SwitchToFiber(fibers[1]); + check_current_actctx_is(current_actctx, TRUE); + + SwitchToFiber(fibers[2]); + check_current_actctx_is(current_actctx, FALSE); + ok(fibers[2] == NULL, "expected fiber to be deleted (got %p)\n", fibers[2]); + + DeleteFiber(fibers[1]); + fibers[1] = NULL; +} + +static void WINAPI exit_thread_fiber_func(void *unused) +{ + BOOL ret; + + (void)unused; + ret = pConvertFiberToThread(); + ok(ret, "ConvertFiberToThread returned error %lu\n", GetLastError()); + + ExitThread(16); +} + +static DWORD WINAPI thread_actctx_func_early_exit(void *actctx) +{ + HANDLE exit_thread_fiber; + + check_current_actctx_is(actctx, FALSE); + + fibers[1] = pConvertThreadToFiber(NULL); + ok(fibers[1] != NULL, "ConvertThreadToFiber returned error %lu\n", GetLastError()); + check_current_actctx_is(actctx, FALSE); + + exit_thread_fiber = pCreateFiber(0, exit_thread_fiber_func, NULL); + ok(exit_thread_fiber != NULL, "CreateFiber returned error %lu\n", GetLastError()); + + /* exit thread, but keep current fiber */ + SwitchToFiber(exit_thread_fiber); + check_current_actctx_is(actctx, TRUE); + + SwitchToFiber(fibers[0]); + ok(0, "unreachable\n"); + return 17; +} + +/* Test that fiber activation context stack is preserved regardless of creator thread's lifetime */ +static void subtest_fiber_actctx_preservation(HANDLE current_actctx, HANDLE child_actctx) +{ + ULONG_PTR cookie; + DWORD tid, wait; + HANDLE thread; + BOOL ret; + + ret = ActivateActCtx(child_actctx, &cookie); + ok(ret, "ActivateActCtx returned error %lu\n", GetLastError()); + check_current_actctx_is(child_actctx, FALSE); + + thread = CreateThread(NULL, 0, thread_actctx_func_early_exit, child_actctx, 0, &tid); + ok(thread != NULL, "CreateThread returned error %lu\n", GetLastError()); + + ret = DeactivateActCtx(0, cookie); + ok(ret, "DeactivateActCtx returned error %lu\n", GetLastError()); + check_current_actctx_is(current_actctx, FALSE); + + wait = WaitForSingleObject(thread, INFINITE); + ok(wait == WAIT_OBJECT_0, "WaitForSingleObject returned %lu (last error: %lu)\n", + wait, GetLastError()); + CloseHandle(thread); + + /* The exited thread has been converted to a fiber */ + SwitchToFiber(fibers[1]); + check_current_actctx_is(current_actctx, FALSE); + + DeleteFiber(fibers[1]); + fibers[1] = NULL; +} + +static HANDLE create_actctx_from_module_manifest(void) +{ + ACTCTXW actctx; + + memset(&actctx, 0, sizeof(ACTCTXW)); + actctx.cbSize = sizeof(actctx); + actctx.dwFlags = ACTCTX_FLAG_HMODULE_VALID | ACTCTX_FLAG_RESOURCE_NAME_VALID; + actctx.lpResourceName = MAKEINTRESOURCEW(124); + actctx.hModule = GetModuleHandleW(NULL); + + return CreateActCtxW(&actctx); +} + +static void test_fiber_actctx(void) +{ + ULONG_PTR cookies[3]; + HANDLE actctxs[3]; + size_t i, j; + BOOL ret; + + for (i = 0; i < ARRAY_SIZE(actctxs); i++) + { + actctxs[i] = create_actctx_from_module_manifest(); + ok(actctxs[i] != INVALID_HANDLE_VALUE, "failed to create context, error %lu\n", GetLastError()); + for (j = 0; j < i; j++) + { + ok(actctxs[i] != actctxs[j], + "actctxs[%Iu] (%p) and actctxs[%Iu] (%p) should not alias\n", + i, actctxs[i], j, actctxs[j]); + } + } + + ret = ActivateActCtx(actctxs[0], &cookies[0]); + ok(ret, "ActivateActCtx returned error %lu\n", GetLastError()); + check_current_actctx_is(actctxs[0], FALSE); + + test_ConvertThreadToFiber(); + check_current_actctx_is(actctxs[0], FALSE); + + ret = ActivateActCtx(actctxs[1], &cookies[1]); + ok(ret, "ActivateActCtx returned error %lu\n", GetLastError()); + check_current_actctx_is(actctxs[1], FALSE); + + subtest_fiber_actctx_switch(actctxs[1], actctxs[2]); + subtest_fiber_actctx_preservation(actctxs[1], actctxs[2]); + + ret = DeactivateActCtx(0, cookies[1]); + ok(ret, "DeactivateActCtx returned error %lu\n", GetLastError()); + check_current_actctx_is(actctxs[0], FALSE); + + test_ConvertFiberToThread(); + check_current_actctx_is(actctxs[0], FALSE); + + ret = DeactivateActCtx(0, cookies[0]); + ok(ret, "DeactivateActCtx returned error %lu\n", GetLastError()); + check_current_actctx_is(NULL, FALSE); + + for (i = ARRAY_SIZE(actctxs); i > 0; ) + ReleaseActCtx(actctxs[--i]); +} + + static void WINAPI fls_exit_deadlock_callback(void *arg) { if (arg == (void *)1) @@ -873,5 +1094,6 @@ START_TEST(fiber) test_FiberLocalStorage(); test_FiberLocalStorageCallback(FiberLocalStorageProc); test_FiberLocalStorageWithFibers(FiberLocalStorageProc); + test_fiber_actctx(); test_fls_exit_deadlock(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2435
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This refactoring makes it easier to change the algorithm of obtaining the current activation context stack. Note that RtlAddRefActivationContext(NULL) is a no-op, and check_actctx(NULL) returns NULL without doing anything. --- dlls/ntdll/actctx.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index b275a7f5b49..ab4ef378330 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -3384,6 +3384,14 @@ static NTSTATUS parse_depend_manifests(struct actctx_loader* acl) return status; } +static HANDLE get_current_actctx_no_addref(void) +{ + if (NtCurrentTeb()->ActivationContextStack.ActiveFrame) + return NtCurrentTeb()->ActivationContextStack.ActiveFrame->ActivationContext; + + return NULL; +} + /* find the appropriate activation context for RtlQueryInformationActivationContext */ static NTSTATUS find_query_actctx( HANDLE *handle, DWORD flags, ULONG class ) { @@ -3393,8 +3401,7 @@ static NTSTATUS find_query_actctx( HANDLE *handle, DWORD flags, ULONG class ) { if (*handle) return STATUS_INVALID_PARAMETER; - if (NtCurrentTeb()->ActivationContextStack.ActiveFrame) - *handle = NtCurrentTeb()->ActivationContextStack.ActiveFrame->ActivationContext; + *handle = get_current_actctx_no_addref(); } else if (flags & (QUERY_ACTCTX_FLAG_ACTCTX_IS_ADDRESS|QUERY_ACTCTX_FLAG_ACTCTX_IS_HMODULE)) { @@ -5462,14 +5469,7 @@ void WINAPI RtlFreeThreadActivationContextStack(void) */ NTSTATUS WINAPI RtlGetActiveActivationContext( HANDLE *handle ) { - if (NtCurrentTeb()->ActivationContextStack.ActiveFrame) - { - *handle = NtCurrentTeb()->ActivationContextStack.ActiveFrame->ActivationContext; - RtlAddRefActivationContext( *handle ); - } - else - *handle = 0; - + RtlAddRefActivationContext( *handle = get_current_actctx_no_addref() ); return STATUS_SUCCESS; } @@ -5754,6 +5754,7 @@ NTSTATUS WINAPI RtlFindActivationContextSectionString( ULONG flags, const GUID * { PACTCTX_SECTION_KEYED_DATA data = ptr; NTSTATUS status = STATUS_SXS_KEY_NOT_FOUND; + ACTIVATION_CONTEXT *actctx; TRACE("%08lx %s %lu %s %p\n", flags, debugstr_guid(guid), section_kind, debugstr_us(section_name), data); @@ -5775,11 +5776,8 @@ NTSTATUS WINAPI RtlFindActivationContextSectionString( ULONG flags, const GUID * return STATUS_INVALID_PARAMETER; } - if (NtCurrentTeb()->ActivationContextStack.ActiveFrame) - { - ACTIVATION_CONTEXT *actctx = check_actctx(NtCurrentTeb()->ActivationContextStack.ActiveFrame->ActivationContext); - if (actctx) status = find_string( actctx, section_kind, section_name, flags, data ); - } + actctx = check_actctx( get_current_actctx_no_addref() ); + if (actctx) status = find_string( actctx, section_kind, section_name, flags, data ); if (status != STATUS_SUCCESS) status = find_string( process_actctx, section_kind, section_name, flags, data ); @@ -5798,6 +5796,7 @@ NTSTATUS WINAPI RtlFindActivationContextSectionGuid( ULONG flags, const GUID *ex { ACTCTX_SECTION_KEYED_DATA *data = ptr; NTSTATUS status = STATUS_SXS_KEY_NOT_FOUND; + ACTIVATION_CONTEXT *actctx; TRACE("%08lx %s %lu %s %p\n", flags, debugstr_guid(extguid), section_kind, debugstr_guid(guid), data); @@ -5816,11 +5815,8 @@ NTSTATUS WINAPI RtlFindActivationContextSectionGuid( ULONG flags, const GUID *ex if (!data || data->cbSize < FIELD_OFFSET(ACTCTX_SECTION_KEYED_DATA, ulAssemblyRosterIndex) || !guid) return STATUS_INVALID_PARAMETER; - if (NtCurrentTeb()->ActivationContextStack.ActiveFrame) - { - ACTIVATION_CONTEXT *actctx = check_actctx(NtCurrentTeb()->ActivationContextStack.ActiveFrame->ActivationContext); - if (actctx) status = find_guid( actctx, section_kind, guid, flags, data ); - } + actctx = check_actctx( get_current_actctx_no_addref() ); + if (actctx) status = find_guid( actctx, section_kind, guid, flags, data ); if (status != STATUS_SUCCESS) status = find_guid( process_actctx, section_kind, guid, flags, data ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2435
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This refactoring makes it easier to change the algorithm of obtaining the current activation context stack. --- dlls/ntdll/actctx.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index ab4ef378330..ea7bf15b4ce 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -3386,8 +3386,10 @@ static NTSTATUS parse_depend_manifests(struct actctx_loader* acl) static HANDLE get_current_actctx_no_addref(void) { - if (NtCurrentTeb()->ActivationContextStack.ActiveFrame) - return NtCurrentTeb()->ActivationContextStack.ActiveFrame->ActivationContext; + ACTIVATION_CONTEXT_STACK *actctx_stack = &NtCurrentTeb()->ActivationContextStack; + + if (actctx_stack->ActiveFrame) + return actctx_stack->ActiveFrame->ActivationContext; return NULL; } @@ -5396,14 +5398,15 @@ NTSTATUS WINAPI RtlActivateActivationContext( ULONG unknown, HANDLE handle, PULO NTSTATUS WINAPI RtlActivateActivationContextEx( ULONG flags, TEB *teb, HANDLE handle, ULONG_PTR *cookie ) { RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; + ACTIVATION_CONTEXT_STACK *actctx_stack = &teb->ActivationContextStack; if (!(frame = RtlAllocateHeap( GetProcessHeap(), 0, sizeof(*frame) ))) return STATUS_NO_MEMORY; - frame->Previous = teb->ActivationContextStack.ActiveFrame; + frame->Previous = actctx_stack->ActiveFrame; frame->ActivationContext = handle; frame->Flags = 0; - teb->ActivationContextStack.ActiveFrame = frame; + actctx_stack->ActiveFrame = frame; RtlAddRefActivationContext( handle ); *cookie = (ULONG_PTR)frame; @@ -5417,12 +5420,13 @@ NTSTATUS WINAPI RtlActivateActivationContextEx( ULONG flags, TEB *teb, HANDLE ha */ void WINAPI RtlDeactivateActivationContext( ULONG flags, ULONG_PTR cookie ) { + ACTIVATION_CONTEXT_STACK *actctx_stack = &NtCurrentTeb()->ActivationContextStack; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame, *top; TRACE( "%lx cookie=%Ix\n", flags, cookie ); /* find the right frame */ - top = NtCurrentTeb()->ActivationContextStack.ActiveFrame; + top = actctx_stack->ActiveFrame; for (frame = top; frame; frame = frame->Previous) if ((ULONG_PTR)frame == cookie) break; @@ -5433,9 +5437,9 @@ void WINAPI RtlDeactivateActivationContext( ULONG flags, ULONG_PTR cookie ) RtlRaiseStatus( STATUS_SXS_EARLY_DEACTIVATION ); /* pop everything up to and including frame */ - NtCurrentTeb()->ActivationContextStack.ActiveFrame = frame->Previous; + actctx_stack->ActiveFrame = frame->Previous; - while (top != NtCurrentTeb()->ActivationContextStack.ActiveFrame) + while (top != actctx_stack->ActiveFrame) { frame = top->Previous; RtlReleaseActivationContext( top->ActivationContext ); @@ -5450,9 +5454,10 @@ void WINAPI RtlDeactivateActivationContext( ULONG flags, ULONG_PTR cookie ) */ void WINAPI RtlFreeThreadActivationContextStack(void) { + ACTIVATION_CONTEXT_STACK *actctx_stack = &NtCurrentTeb()->ActivationContextStack; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; - frame = NtCurrentTeb()->ActivationContextStack.ActiveFrame; + frame = actctx_stack->ActiveFrame; while (frame) { RTL_ACTIVATION_CONTEXT_STACK_FRAME *prev = frame->Previous; @@ -5460,7 +5465,7 @@ void WINAPI RtlFreeThreadActivationContextStack(void) RtlFreeHeap( GetProcessHeap(), 0, frame ); frame = prev; } - NtCurrentTeb()->ActivationContextStack.ActiveFrame = NULL; + actctx_stack->ActiveFrame = NULL; } @@ -5479,9 +5484,10 @@ NTSTATUS WINAPI RtlGetActiveActivationContext( HANDLE *handle ) */ BOOLEAN WINAPI RtlIsActivationContextActive( HANDLE handle ) { + ACTIVATION_CONTEXT_STACK *actctx_stack = &NtCurrentTeb()->ActivationContextStack; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; - for (frame = NtCurrentTeb()->ActivationContextStack.ActiveFrame; frame; frame = frame->Previous) + for (frame = actctx_stack->ActiveFrame; frame; frame = frame->Previous) if (frame->ActivationContext == handle) return TRUE; return FALSE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2435
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This allows changing the location of activation context stack if it should be put somewhere else (e.g. inside the fiber structure). --- dlls/ntdll/actctx.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index ea7bf15b4ce..eaa15907132 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -3386,7 +3386,7 @@ static NTSTATUS parse_depend_manifests(struct actctx_loader* acl) static HANDLE get_current_actctx_no_addref(void) { - ACTIVATION_CONTEXT_STACK *actctx_stack = &NtCurrentTeb()->ActivationContextStack; + ACTIVATION_CONTEXT_STACK *actctx_stack = NtCurrentTeb()->ActivationContextStackPointer; if (actctx_stack->ActiveFrame) return actctx_stack->ActiveFrame->ActivationContext; @@ -5398,7 +5398,7 @@ NTSTATUS WINAPI RtlActivateActivationContext( ULONG unknown, HANDLE handle, PULO NTSTATUS WINAPI RtlActivateActivationContextEx( ULONG flags, TEB *teb, HANDLE handle, ULONG_PTR *cookie ) { RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; - ACTIVATION_CONTEXT_STACK *actctx_stack = &teb->ActivationContextStack; + ACTIVATION_CONTEXT_STACK *actctx_stack = teb->ActivationContextStackPointer; if (!(frame = RtlAllocateHeap( GetProcessHeap(), 0, sizeof(*frame) ))) return STATUS_NO_MEMORY; @@ -5420,7 +5420,7 @@ NTSTATUS WINAPI RtlActivateActivationContextEx( ULONG flags, TEB *teb, HANDLE ha */ void WINAPI RtlDeactivateActivationContext( ULONG flags, ULONG_PTR cookie ) { - ACTIVATION_CONTEXT_STACK *actctx_stack = &NtCurrentTeb()->ActivationContextStack; + ACTIVATION_CONTEXT_STACK *actctx_stack = NtCurrentTeb()->ActivationContextStackPointer; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame, *top; TRACE( "%lx cookie=%Ix\n", flags, cookie ); @@ -5454,7 +5454,7 @@ void WINAPI RtlDeactivateActivationContext( ULONG flags, ULONG_PTR cookie ) */ void WINAPI RtlFreeThreadActivationContextStack(void) { - ACTIVATION_CONTEXT_STACK *actctx_stack = &NtCurrentTeb()->ActivationContextStack; + ACTIVATION_CONTEXT_STACK *actctx_stack = NtCurrentTeb()->ActivationContextStackPointer; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; frame = actctx_stack->ActiveFrame; @@ -5484,7 +5484,7 @@ NTSTATUS WINAPI RtlGetActiveActivationContext( HANDLE *handle ) */ BOOLEAN WINAPI RtlIsActivationContextActive( HANDLE handle ) { - ACTIVATION_CONTEXT_STACK *actctx_stack = &NtCurrentTeb()->ActivationContextStack; + ACTIVATION_CONTEXT_STACK *actctx_stack = NtCurrentTeb()->ActivationContextStackPointer; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; for (frame = actctx_stack->ActiveFrame; frame; frame = frame->Previous) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2435
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/ntdll/actctx.c | 10 +++++++++- dlls/ntdll/ntdll.spec | 1 + include/winternl.h | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index eaa15907132..5903389fd4b 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -5454,7 +5454,15 @@ void WINAPI RtlDeactivateActivationContext( ULONG flags, ULONG_PTR cookie ) */ void WINAPI RtlFreeThreadActivationContextStack(void) { - ACTIVATION_CONTEXT_STACK *actctx_stack = NtCurrentTeb()->ActivationContextStackPointer; + RtlFreeActivationContextStack( NtCurrentTeb()->ActivationContextStackPointer ); +} + + +/****************************************************************** + * RtlFreeActivationContextStack (NTDLL.@) + */ +void WINAPI RtlFreeActivationContextStack( ACTIVATION_CONTEXT_STACK *actctx_stack ) +{ RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; frame = actctx_stack->ActiveFrame; diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index d5f6737e1aa..b7659ac8c49 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -700,6 +700,7 @@ @ stdcall RtlFormatCurrentUserKeyPath(ptr) @ stdcall RtlFormatMessage(ptr long long long long ptr ptr long ptr) @ stdcall RtlFormatMessageEx(ptr long long long long ptr ptr long ptr long) +@ stdcall RtlFreeActivationContextStack(ptr) @ stdcall RtlFreeAnsiString(ptr) @ stdcall RtlFreeHandle(ptr ptr) @ stdcall RtlFreeHeap(long long ptr) diff --git a/include/winternl.h b/include/winternl.h index ed0e51bae24..b96ca5c3af6 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -4426,6 +4426,7 @@ NTSYSAPI NTSTATUS WINAPI RtlFlsSetValue(ULONG,void *); NTSYSAPI NTSTATUS WINAPI RtlFormatCurrentUserKeyPath(PUNICODE_STRING); NTSYSAPI NTSTATUS WINAPI RtlFormatMessage(LPCWSTR,ULONG,BOOLEAN,BOOLEAN,BOOLEAN,__ms_va_list *,LPWSTR,ULONG,ULONG*); NTSYSAPI NTSTATUS WINAPI RtlFormatMessageEx(LPCWSTR,ULONG,BOOLEAN,BOOLEAN,BOOLEAN,__ms_va_list *,LPWSTR,ULONG,ULONG*,ULONG); +NTSYSAPI void WINAPI RtlFreeActivationContextStack(ACTIVATION_CONTEXT_STACK *); NTSYSAPI void WINAPI RtlFreeAnsiString(PANSI_STRING); NTSYSAPI BOOLEAN WINAPI RtlFreeHandle(RTL_HANDLE_TABLE *,RTL_HANDLE *); NTSYSAPI void WINAPI RtlFreeOemString(POEM_STRING); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2435
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/kernel32/tests/fiber.c | 10 ++++---- dlls/kernelbase/thread.c | 50 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/dlls/kernel32/tests/fiber.c b/dlls/kernel32/tests/fiber.c index 5540305cd1a..a5b55c70af3 100644 --- a/dlls/kernel32/tests/fiber.c +++ b/dlls/kernel32/tests/fiber.c @@ -839,7 +839,7 @@ static void WINAPI fiber_actctx_func(void *actctx) HANDLE thread; BOOL ret; - check_current_actctx_is(NULL, TRUE); + check_current_actctx_is(NULL, FALSE); ret = ActivateActCtx(actctx, &cookie); ok(ret, "ActivateActCtx returned error %lu\n", GetLastError()); @@ -858,7 +858,7 @@ static void WINAPI fiber_actctx_func(void *actctx) ret = DeactivateActCtx(0, cookie); ok(ret, "DeactivateActCtx returned error %lu\n", GetLastError()); - check_current_actctx_is(NULL, TRUE); + check_current_actctx_is(NULL, FALSE); SwitchToFiber(fibers[0]); ok(0, "unreachable\n"); @@ -872,10 +872,10 @@ static void subtest_fiber_actctx_switch(HANDLE current_actctx, HANDLE child_actc check_current_actctx_is(current_actctx, FALSE); SwitchToFiber(fibers[1]); - check_current_actctx_is(current_actctx, TRUE); + check_current_actctx_is(current_actctx, FALSE); SwitchToFiber(fibers[1]); - check_current_actctx_is(current_actctx, TRUE); + check_current_actctx_is(current_actctx, FALSE); SwitchToFiber(fibers[2]); check_current_actctx_is(current_actctx, FALSE); @@ -911,7 +911,7 @@ static DWORD WINAPI thread_actctx_func_early_exit(void *actctx) /* exit thread, but keep current fiber */ SwitchToFiber(exit_thread_fiber); - check_current_actctx_is(actctx, TRUE); + check_current_actctx_is(actctx, FALSE); SwitchToFiber(fibers[0]); ok(0, "unreachable\n"); diff --git a/dlls/kernelbase/thread.c b/dlls/kernelbase/thread.c index 235624811f5..50e50fd0230 100644 --- a/dlls/kernelbase/thread.c +++ b/dlls/kernelbase/thread.c @@ -768,6 +768,12 @@ BOOL WINAPI DECLSPEC_HOTPATCH TlsSetValue( DWORD index, LPVOID value ) ***********************************************************************/ +struct fiber_actctx +{ + ACTIVATION_CONTEXT_STACK stack_space; /* activation context stack space */ + ACTIVATION_CONTEXT_STACK *stack_ptr; /* last value of ActivationContextStackPointer */ +}; + struct fiber_data { LPVOID param; /* 00/00 fiber param */ @@ -779,6 +785,7 @@ struct fiber_data DWORD flags; /* fiber flags */ LPFIBER_START_ROUTINE start; /* start routine */ void *fls_slots; /* fiber storage slots */ + struct fiber_actctx actctx; /* activation context state */ }; extern void WINAPI switch_fiber( CONTEXT *old, CONTEXT *new ); @@ -931,6 +938,41 @@ static void init_fiber_context( struct fiber_data *fiber ) #endif } +static void move_list( LIST_ENTRY *dest, LIST_ENTRY *src ) +{ + LIST_ENTRY *head = src->Flink; + LIST_ENTRY *tail = src->Blink; + + if (src != head) + { + dest->Flink = head; + dest->Blink = tail; + head->Blink = dest; + tail->Flink = dest; + } + else InitializeListHead( dest ); +} + +static void relocate_thread_actctx_stack( ACTIVATION_CONTEXT_STACK *dest ) +{ + ACTIVATION_CONTEXT_STACK *src = NtCurrentTeb()->ActivationContextStackPointer; + + if (dest != src) + { + C_ASSERT(sizeof(*dest) == sizeof(dest->ActiveFrame) + sizeof(dest->FrameListCache) + + sizeof(dest->Flags) + sizeof(dest->NextCookieSequenceNumber) + + sizeof(dest->StackId)); + + dest->ActiveFrame = src->ActiveFrame; + move_list( &dest->FrameListCache, &src->FrameListCache ); + dest->Flags = src->Flags; + dest->NextCookieSequenceNumber = src->NextCookieSequenceNumber; + dest->StackId = src->StackId; + + NtCurrentTeb()->ActivationContextStackPointer = dest; + } +} + /*********************************************************************** * CreateFiber (kernelbase.@) @@ -969,6 +1011,8 @@ LPVOID WINAPI DECLSPEC_HOTPATCH CreateFiberEx( SIZE_T stack_commit, SIZE_T stack fiber->except = (void *)-1; fiber->start = start; fiber->flags = flags; + InitializeListHead( &fiber->actctx.stack_space.FrameListCache ); + fiber->actctx.stack_ptr = &fiber->actctx.stack_space; init_fiber_context( fiber ); return fiber; } @@ -983,6 +1027,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH ConvertFiberToThread(void) if (fiber) { + relocate_thread_actctx_stack( &NtCurrentTeb()->ActivationContextStack ); NtCurrentTeb()->Tib.u.FiberData = NULL; HeapFree( GetProcessHeap(), 0, fiber ); } @@ -1025,6 +1070,7 @@ LPVOID WINAPI DECLSPEC_HOTPATCH ConvertThreadToFiberEx( LPVOID param, DWORD flag fiber->start = NULL; fiber->flags = flags; fiber->fls_slots = NtCurrentTeb()->FlsSlots; + relocate_thread_actctx_stack( &fiber->actctx.stack_space ); NtCurrentTeb()->Tib.u.FiberData = fiber; return fiber; } @@ -1040,11 +1086,13 @@ void WINAPI DECLSPEC_HOTPATCH DeleteFiber( LPVOID fiber_ptr ) if (!fiber) return; if (fiber == NtCurrentTeb()->Tib.u.FiberData) { + relocate_thread_actctx_stack( &NtCurrentTeb()->ActivationContextStack ); HeapFree( GetProcessHeap(), 0, fiber ); RtlExitUserThread( 1 ); } RtlFreeUserStack( fiber->stack_allocation ); RtlProcessFlsData( fiber->fls_slots, 3 ); + RtlFreeActivationContextStack( &fiber->actctx.stack_space ); HeapFree( GetProcessHeap(), 0, fiber ); } @@ -1069,6 +1117,7 @@ void WINAPI DECLSPEC_HOTPATCH SwitchToFiber( LPVOID fiber ) current_fiber->except = NtCurrentTeb()->Tib.ExceptionList; current_fiber->stack_limit = NtCurrentTeb()->Tib.StackLimit; current_fiber->fls_slots = NtCurrentTeb()->FlsSlots; + current_fiber->actctx.stack_ptr = NtCurrentTeb()->ActivationContextStackPointer; /* stack_allocation and stack_base never change */ /* FIXME: should save floating point context if requested in fiber->flags */ @@ -1078,6 +1127,7 @@ void WINAPI DECLSPEC_HOTPATCH SwitchToFiber( LPVOID fiber ) NtCurrentTeb()->Tib.StackLimit = new_fiber->stack_limit; NtCurrentTeb()->DeallocationStack = new_fiber->stack_allocation; NtCurrentTeb()->FlsSlots = new_fiber->fls_slots; + NtCurrentTeb()->ActivationContextStackPointer = new_fiber->actctx.stack_ptr; switch_fiber( ¤t_fiber->context, &new_fiber->context ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2435
v2: rebase to fix kernel32:fiber crashes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2435#note_28505
Zebediah Figura (@zfigura) commented about dlls/kernel32/tests/fiber.c:
+ SwitchToFiber(fibers[1]); + check_current_actctx_is(current_actctx, TRUE); + + SwitchToFiber(fibers[2]); + check_current_actctx_is(current_actctx, FALSE); + ok(fibers[2] == NULL, "expected fiber to be deleted (got %p)\n", fibers[2]); + + DeleteFiber(fibers[1]); + fibers[1] = NULL; +} + +static void WINAPI exit_thread_fiber_func(void *unused) +{ + BOOL ret; + + (void)unused; What's this for?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2435#note_30301
Zebediah Figura (@zfigura) commented about dlls/kernelbase/thread.c:
+ + if (src != head) + { + dest->Flink = head; + dest->Blink = tail; + head->Blink = dest; + tail->Flink = dest; + } + else InitializeListHead( dest ); +} + +static void relocate_thread_actctx_stack( ACTIVATION_CONTEXT_STACK *dest ) +{ + ACTIVATION_CONTEXT_STACK *src = NtCurrentTeb()->ActivationContextStackPointer; + + if (dest != src) I must be missing something, when can dest == src? Aren't we always moving from the fiber data to the TEB in ConvertFiberToThread()?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2435#note_30302
Sorry for the delay; I wasn't expecting to be asked to review this. I don't have a familiarity with activation contexts, but this looks reasonable and holistic enough. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2435#note_30303
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Zebediah Figura (＠zfigura)