[PATCH v2 0/5] MR8326: ntdll: Implement RtlActivateActivationContextUnsafeFast().
For React Native. -- v2: kernel32/tests: Add tests for RtlActivateActivationContextUnsafe() and RtlDeactivateActivationContextUnsafeFast(). ntdll: Implement RtlDeactivateActivationContextUnsafeFast(). ntdll: Implement RtlActivateActivationContextUnsafeFast(). ntdll: Set and check flags for activation context stack frames. kernel32/tests: Add tests for normal activation context stack frame flags. https://gitlab.winehq.org/wine/wine/-/merge_requests/8326
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Test that the flags for normal activation context stack frames are 0x28. --- dlls/kernel32/tests/actctx.c | 98 ++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index ca24ed10549..b35e2f3f418 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -31,6 +31,8 @@ static BOOL (WINAPI *pQueryActCtxSettingsW)(DWORD,HANDLE,LPCWSTR,LPCWSTR,LPWSTR,SIZE_T,SIZE_T*); +static NTSTATUS (NTAPI *pRtlActivateActivationContext)(ULONG,HANDLE,ULONG_PTR *); +static NTSTATUS (NTAPI *pRtlActivateActivationContextEx)(ULONG,TEB *,HANDLE,ULONG_PTR *); static NTSTATUS(NTAPI *pRtlFindActivationContextSectionString)(DWORD,const GUID *,ULONG,PUNICODE_STRING,PACTCTX_SECTION_KEYED_DATA); static BOOLEAN (NTAPI *pRtlCreateUnicodeStringFromAsciiz)(PUNICODE_STRING, PCSZ); static VOID (NTAPI *pRtlFreeUnicodeString)(PUNICODE_STRING); @@ -2197,7 +2199,9 @@ static void test_allowDelayedBinding(void) static void test_actctx(void) { + RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; ULONG_PTR cookie; + NTSTATUS status; HANDLE handle; BOOL b; @@ -2213,6 +2217,98 @@ static void test_actctx(void) ReleaseActCtx(handle); } + /* Test flags for normal frames */ + if (!create_manifest_file("test1.manifest", manifest1, -1, NULL, NULL)) + { + skip("Could not create manifest file\n"); + return; + } + handle = test_create("test1.manifest"); + ok(handle != INVALID_HANDLE_VALUE, "handle == INVALID_HANDLE_VALUE, error %lu\n", GetLastError()); + DeleteFileA("test1.manifest"); + if (handle != INVALID_HANDLE_VALUE) + { + b = ActivateActCtx(handle, &cookie); + ok(b, "ActivateActCtx failed: %lu\n", GetLastError()); + + frame = NtCurrentTeb()->ActivationContextStackPointer->ActiveFrame; + ok(!frame->Previous, "Got unexpected Previous.\n"); + ok(frame->ActivationContext == handle, "Got unexpected ActivationContext.\n"); + todo_wine + ok(frame->Flags == 0x28, "Got unexpected Flags %#lx.\n", frame->Flags); + + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + + status = pRtlActivateActivationContext(0x20, handle, &cookie); + todo_wine + ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); + if (status == STATUS_SUCCESS) + { + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + } + + status = pRtlActivateActivationContext(0x28, handle, &cookie); + todo_wine + ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); + if (status == STATUS_SUCCESS) + { + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + } + + status = pRtlActivateActivationContext(0, handle, &cookie); + ok(status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status); + + frame = NtCurrentTeb()->ActivationContextStackPointer->ActiveFrame; + ok(!frame->Previous, "Got unexpected Previous.\n"); + ok(frame->ActivationContext == handle, "Got unexpected ActivationContext.\n"); + todo_wine + ok(frame->Flags == 0x28, "Got unexpected Flags %#lx.\n", frame->Flags); + + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + b = GetCurrentActCtx(&handle); + ok(handle == NULL, "handle = %p, expected NULL\n", handle); + ok(b, "GetCurrentActCtx failed: %lu\n", GetLastError()); + + status = pRtlActivateActivationContextEx(0x20, NtCurrentTeb(), handle, &cookie); + todo_wine + ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); + if (status == STATUS_SUCCESS) + { + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + } + + status = pRtlActivateActivationContextEx(0x28, NtCurrentTeb(), handle, &cookie); + todo_wine + ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); + if (status == STATUS_SUCCESS) + { + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + } + + status = pRtlActivateActivationContextEx(0, NtCurrentTeb(), handle, &cookie); + ok(status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status); + + frame = NtCurrentTeb()->ActivationContextStackPointer->ActiveFrame; + ok(!frame->Previous, "Got unexpected Previous.\n"); + ok(frame->ActivationContext == handle, "Got unexpected ActivationContext.\n"); + todo_wine + ok(frame->Flags == 0x28, "Got unexpected Flags %#lx.\n", frame->Flags); + + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + b = GetCurrentActCtx(&handle); + ok(handle == NULL, "handle = %p, expected NULL\n", handle); + ok(b, "GetCurrentActCtx failed: %lu\n", GetLastError()); + + ReleaseActCtx(handle); + } + /* test for whitespace handling in Eq ::= S? '=' S? */ create_manifest_file("test1_1.manifest", manifest1_1, -1, NULL, NULL); handle = test_create("test1_1.manifest"); @@ -3096,6 +3192,8 @@ static BOOL init_funcs(void) pQueryActCtxSettingsW = (void *)GetProcAddress( hLibrary, "QueryActCtxSettingsW" ); hLibrary = GetModuleHandleA("ntdll.dll"); + X(RtlActivateActivationContext); + X(RtlActivateActivationContextEx); X(RtlFindActivationContextSectionString); X(RtlCreateUnicodeStringFromAsciiz); X(RtlFreeUnicodeString); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8326
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Prepare for RtlActivateActivationContextUnsafeFast(), which adds frames with flags 0x20. Normal frame flags are 0x28. So 0x8 is most likely to note that it's callee-allocated and needs to be freed in RtlFreeActivationContextStack(). Whereas missing 0x8 probably means a frame is caller-allocated. --- dlls/kernel32/tests/actctx.c | 27 --------------------------- dlls/ntdll/actctx.c | 12 ++++++++---- dlls/ntdll/ntdll_misc.h | 3 +++ 3 files changed, 11 insertions(+), 31 deletions(-) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index b35e2f3f418..8bd459dccca 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -2234,29 +2234,16 @@ static void test_actctx(void) frame = NtCurrentTeb()->ActivationContextStackPointer->ActiveFrame; ok(!frame->Previous, "Got unexpected Previous.\n"); ok(frame->ActivationContext == handle, "Got unexpected ActivationContext.\n"); - todo_wine ok(frame->Flags == 0x28, "Got unexpected Flags %#lx.\n", frame->Flags); b = DeactivateActCtx(0, cookie); ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); status = pRtlActivateActivationContext(0x20, handle, &cookie); - todo_wine ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); - if (status == STATUS_SUCCESS) - { - b = DeactivateActCtx(0, cookie); - ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); - } status = pRtlActivateActivationContext(0x28, handle, &cookie); - todo_wine ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); - if (status == STATUS_SUCCESS) - { - b = DeactivateActCtx(0, cookie); - ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); - } status = pRtlActivateActivationContext(0, handle, &cookie); ok(status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status); @@ -2264,7 +2251,6 @@ static void test_actctx(void) frame = NtCurrentTeb()->ActivationContextStackPointer->ActiveFrame; ok(!frame->Previous, "Got unexpected Previous.\n"); ok(frame->ActivationContext == handle, "Got unexpected ActivationContext.\n"); - todo_wine ok(frame->Flags == 0x28, "Got unexpected Flags %#lx.\n", frame->Flags); b = DeactivateActCtx(0, cookie); @@ -2274,22 +2260,10 @@ static void test_actctx(void) ok(b, "GetCurrentActCtx failed: %lu\n", GetLastError()); status = pRtlActivateActivationContextEx(0x20, NtCurrentTeb(), handle, &cookie); - todo_wine ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); - if (status == STATUS_SUCCESS) - { - b = DeactivateActCtx(0, cookie); - ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); - } status = pRtlActivateActivationContextEx(0x28, NtCurrentTeb(), handle, &cookie); - todo_wine ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); - if (status == STATUS_SUCCESS) - { - b = DeactivateActCtx(0, cookie); - ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); - } status = pRtlActivateActivationContextEx(0, NtCurrentTeb(), handle, &cookie); ok(status == STATUS_SUCCESS, "Got unexpected status %#lx.\n", status); @@ -2297,7 +2271,6 @@ static void test_actctx(void) frame = NtCurrentTeb()->ActivationContextStackPointer->ActiveFrame; ok(!frame->Previous, "Got unexpected Previous.\n"); ok(frame->ActivationContext == handle, "Got unexpected ActivationContext.\n"); - todo_wine ok(frame->Flags == 0x28, "Got unexpected Flags %#lx.\n", frame->Flags); b = DeactivateActCtx(0, cookie); diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index 9828d90d11d..20d22bbe4de 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -5430,9 +5430,9 @@ NTSTATUS WINAPI RtlZombifyActivationContext( HANDLE handle ) /****************************************************************** * RtlActivateActivationContext (NTDLL.@) */ -NTSTATUS WINAPI RtlActivateActivationContext( ULONG unknown, HANDLE handle, PULONG_PTR cookie ) +NTSTATUS WINAPI RtlActivateActivationContext( ULONG flags, HANDLE handle, PULONG_PTR cookie ) { - return RtlActivateActivationContextEx( 0, NtCurrentTeb(), handle, cookie ); + return RtlActivateActivationContextEx( flags, NtCurrentTeb(), handle, cookie ); } @@ -5444,10 +5444,13 @@ NTSTATUS WINAPI RtlActivateActivationContextEx( ULONG flags, TEB *teb, HANDLE ha ACTIVATION_CONTEXT_STACK *actctx_stack = teb->ActivationContextStackPointer; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; + if (flags != 0) + return STATUS_INVALID_PARAMETER; + frame = RtlAllocateHeap( GetProcessHeap(), HEAP_GENERATE_EXCEPTIONS, sizeof(*frame) ); frame->Previous = actctx_stack->ActiveFrame; frame->ActivationContext = handle; - frame->Flags = 0; + frame->Flags = 0x20 | NTDLL_STACK_FRAME_ALLOCATED; actctx_stack->ActiveFrame = frame; RtlAddRefActivationContext( handle ); @@ -5512,7 +5515,8 @@ void WINAPI RtlFreeActivationContextStack( ACTIVATION_CONTEXT_STACK *actctx_stac { RTL_ACTIVATION_CONTEXT_STACK_FRAME *prev = frame->Previous; RtlReleaseActivationContext( frame->ActivationContext ); - RtlFreeHeap( GetProcessHeap(), 0, frame ); + if (frame->Flags & NTDLL_STACK_FRAME_ALLOCATED) + RtlFreeHeap( GetProcessHeap(), 0, frame ); frame = prev; } actctx_stack->ActiveFrame = NULL; diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index f82edef6b65..7e9b39fbc53 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -35,6 +35,9 @@ #define NTDLL_TLS_ERRNO 16 /* TLS slot for _errno() */ +/* undocumented */ +#define NTDLL_STACK_FRAME_ALLOCATED 0x8 /* For RtlActivateActivationContextEx()/RtlFreeActivationContextStack() */ + #ifdef __i386__ static const USHORT current_machine = IMAGE_FILE_MACHINE_I386; #elif defined(__x86_64__) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8326
From: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/ntdll/actctx.c | 19 +++++++++++++++++++ dlls/ntdll/ntdll.spec | 2 +- include/winternl.h | 11 +++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index 20d22bbe4de..5c7aa6936ac 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -5435,6 +5435,25 @@ NTSTATUS WINAPI RtlActivateActivationContext( ULONG flags, HANDLE handle, PULONG return RtlActivateActivationContextEx( flags, NtCurrentTeb(), handle, cookie ); } +/****************************************************************** + * RtlActivateActivationContextUnsafeFast (NTDLL.@) + * + * FIXME: function prototype might be wrong + */ +RTL_ACTIVATION_CONTEXT_STACK_FRAME * FASTCALL RtlActivateActivationContextUnsafeFast( RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED *frame_extended, + PVOID context ) +{ + ACTIVATION_CONTEXT_STACK *actctx_stack = NtCurrentTeb()->ActivationContextStackPointer; + + TRACE( "%p %p\n", frame_extended, context ); + + frame_extended->Frame.Previous = actctx_stack->ActiveFrame; + frame_extended->Frame.ActivationContext = context; + frame_extended->Frame.Flags = 0x20; + actctx_stack->ActiveFrame = &frame_extended->Frame; + RtlAddRefActivationContext( context ); + return &frame_extended->Frame; +} /****************************************************************** * RtlActivateActivationContextEx (NTDLL.@) diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 4e36bfe2662..f79dfd50da5 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -484,7 +484,7 @@ @ stdcall RtlAcquireSRWLockShared(ptr) @ stdcall RtlActivateActivationContext(long ptr ptr) @ stdcall RtlActivateActivationContextEx(long ptr ptr ptr) -@ stub RtlActivateActivationContextUnsafeFast +@ stdcall -fastcall RtlActivateActivationContextUnsafeFast(ptr ptr) @ stdcall RtlAddAccessAllowedAce(ptr long long ptr) @ stdcall RtlAddAccessAllowedAceEx(ptr long long long ptr) @ stdcall RtlAddAccessAllowedObjectAce(ptr long long long ptr ptr ptr) diff --git a/include/winternl.h b/include/winternl.h index 86f5f4e5480..76d719ffac9 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -231,6 +231,17 @@ typedef struct _RTL_ACTIVATION_CONTEXT_STACK_FRAME ULONG Flags; } RTL_ACTIVATION_CONTEXT_STACK_FRAME, *PRTL_ACTIVATION_CONTEXT_STACK_FRAME; +typedef struct _RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED +{ + SIZE_T Size; + ULONG Format; + RTL_ACTIVATION_CONTEXT_STACK_FRAME Frame; + PVOID Extra1; + PVOID Extra2; + PVOID Extra3; + PVOID Extra4; +} RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED, *PRTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED; + typedef struct _ACTIVATION_CONTEXT_STACK { RTL_ACTIVATION_CONTEXT_STACK_FRAME *ActiveFrame; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8326
From: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/ntdll/actctx.c | 17 +++++++++++++++++ dlls/ntdll/ntdll.spec | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index 5c7aa6936ac..0f76b95d961 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -5455,6 +5455,23 @@ RTL_ACTIVATION_CONTEXT_STACK_FRAME * FASTCALL RtlActivateActivationContextUnsafe return &frame_extended->Frame; } +/****************************************************************** + * RtlDeactivateActivationContextUnsafeFast (NTDLL.@) + * + * FIXME: function prototype might be wrong + */ +VOID FASTCALL RtlDeactivateActivationContextUnsafeFast( RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED *frame_extended ) +{ + ACTIVATION_CONTEXT_STACK *actctx_stack = NtCurrentTeb()->ActivationContextStackPointer; + ACTIVATION_CONTEXT *context; + + TRACE( "%p\n", frame_extended ); + + context = actctx_stack->ActiveFrame->ActivationContext; + actctx_stack->ActiveFrame = frame_extended->Frame.Previous; + RtlReleaseActivationContext( context ); +} + /****************************************************************** * RtlActivateActivationContextEx (NTDLL.@) */ diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index f79dfd50da5..78da963b969 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -606,7 +606,7 @@ @ stub RtlCutoverTimeToSystemTime @ stdcall RtlDeNormalizeProcessParams(ptr) @ stdcall RtlDeactivateActivationContext(long long) -@ stub RtlDeactivateActivationContextUnsafeFast +@ stdcall -fastcall RtlDeactivateActivationContextUnsafeFast(ptr) @ stub RtlDebugPrintTimes @ stdcall RtlDecodePointer(ptr) @ stdcall RtlDecodeSystemPointer(ptr) RtlDecodePointer -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8326
From: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/kernel32/tests/actctx.c | 115 +++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index 8bd459dccca..66360284d42 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -33,7 +33,12 @@ static BOOL (WINAPI *pQueryActCtxSettingsW)(DWORD,HANDLE,LPCWSTR,LPCWSTR,LPWST static NTSTATUS (NTAPI *pRtlActivateActivationContext)(ULONG,HANDLE,ULONG_PTR *); static NTSTATUS (NTAPI *pRtlActivateActivationContextEx)(ULONG,TEB *,HANDLE,ULONG_PTR *); +static PRTL_ACTIVATION_CONTEXT_STACK_FRAME (FASTCALL *pRtlActivateActivationContextUnsafeFast)(PRTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED, + PVOID); +static VOID (FASTCALL *pRtlDeactivateActivationContextUnsafeFast)(PRTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED); +static NTSTATUS (WINAPI *pRtlGetVersion)(RTL_OSVERSIONINFOEXW *); static NTSTATUS(NTAPI *pRtlFindActivationContextSectionString)(DWORD,const GUID *,ULONG,PUNICODE_STRING,PACTCTX_SECTION_KEYED_DATA); +static VOID (NTAPI *pRtlFreeThreadActivationContextStack)(VOID); static BOOLEAN (NTAPI *pRtlCreateUnicodeStringFromAsciiz)(PUNICODE_STRING, PCSZ); static VOID (NTAPI *pRtlFreeUnicodeString)(PUNICODE_STRING); static NTSTATUS(NTAPI *pRtlQueryInformationActiveActivationContext)(ULONG,PVOID,SIZE_T,SIZE_T *); @@ -604,6 +609,16 @@ static const WCHAR wndClass3W[] = static WCHAR app_dir[MAX_PATH], exe_path[MAX_PATH], work_dir[MAX_PATH], work_dir_subdir[MAX_PATH]; static WCHAR app_manifest_path[MAX_PATH], manifest_path[MAX_PATH], depmanifest_path[MAX_PATH]; +#define is_win8_plus() check_win_version(6, 2) +static BOOL check_win_version(int min_major, int min_minor) +{ + RTL_OSVERSIONINFOEXW ver; + + ver.dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW); + pRtlGetVersion(&ver); + return ver.dwMajorVersion > min_major || (ver.dwMajorVersion == min_major && ver.dwMinorVersion >= min_minor); +} + static BOOL create_manifest_file(const char *filename, const char *manifest, int manifest_len, const char *depfile, const char *depmanifest) { @@ -3167,7 +3182,11 @@ static BOOL init_funcs(void) hLibrary = GetModuleHandleA("ntdll.dll"); X(RtlActivateActivationContext); X(RtlActivateActivationContextEx); + X(RtlActivateActivationContextUnsafeFast); + X(RtlDeactivateActivationContextUnsafeFast); + X(RtlGetVersion); X(RtlFindActivationContextSectionString); + X(RtlFreeThreadActivationContextStack); X(RtlCreateUnicodeStringFromAsciiz); X(RtlFreeUnicodeString); X(RtlQueryInformationActiveActivationContext); @@ -4597,6 +4616,101 @@ static void test_RtlQueryInformationActiveActivationContext(void) ReleaseActCtx( context ); } +static void test_RtlActivateActivationContextUnsafeFast(void) +{ + RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED frame_extended1 = {0}, frame_extended2 = {0}; + HANDLE context1, context2, current_context; + RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; + BOOL ret; + + /* RtlActivateActivationContextUnsafeFast() has a different prototype on <= Win7 */ + if (!is_win8_plus()) + { + skip("Test requires Win8+.\n"); + return; + } + + ret = GetCurrentActCtx(¤t_context); + ok(ret, "GetCurrentActCtx failed.\n"); + ok(!current_context, "Got unexpected handle.\n"); + + if (!create_manifest_file("test1.manifest", manifest1, -1, NULL, NULL)) + { + skip("Could not create manifest file 1.\n"); + return; + } + if (!create_manifest_file("test2.manifest", manifest1_1, -1, NULL, NULL)) + { + skip("Could not create manifest file 2.\n"); + DeleteFileA("test1.manifest"); + return; + } + context1 = test_create("test1.manifest"); + ok(context1 != INVALID_HANDLE_VALUE, "Failed to create context, error %lu.\n", GetLastError()); + DeleteFileA("test1.manifest"); + context2 = test_create("test2.manifest"); + ok(context2 != INVALID_HANDLE_VALUE, "Failed to create context, error %lu.\n", GetLastError()); + DeleteFileA("test2.manifest"); + + frame_extended1.Size = sizeof(frame_extended1); + frame_extended1.Format = 0xdeadbeef; + frame = pRtlActivateActivationContextUnsafeFast(&frame_extended1, context1); + ok(frame == &frame_extended1.Frame, "Got unexpected frame.\n"); + ok(frame_extended1.Size == sizeof(frame_extended1), "Got unexpected Size %#Ix.\n", frame_extended1.Size); + ok(frame_extended1.Format == 0xdeadbeef, "Got unexpected Format %#lx.\n", frame_extended1.Format); + ok(!frame_extended1.Frame.Previous, "Got unexpected Previous.\n"); + ok(frame_extended1.Frame.ActivationContext == context1, "Got unexpected ActivationContext.\n"); + ok(frame_extended1.Frame.Flags == 0x20, "Got unexpected Flags %#lx.\n", frame_extended1.Frame.Flags); + + ret = GetCurrentActCtx(¤t_context); + ok(current_context == context1, "Got unexpected handle.\n"); + ReleaseActCtx(current_context); + + frame_extended2.Size = sizeof(frame_extended2); + frame_extended2.Format = 0xdeadbeef; + frame = pRtlActivateActivationContextUnsafeFast(&frame_extended2, context2); + ok(frame == &frame_extended2.Frame, "Got unexpected frame %p.\n", frame); + ok(frame_extended2.Size == sizeof(frame_extended2), "Got unexpected Size %#Ix.\n", frame_extended2.Size); + ok(frame_extended2.Format == 0xdeadbeef, "Got unexpected Format %#lx.\n", frame_extended2.Format); + ok(frame_extended2.Frame.Previous == &frame_extended1.Frame, "Got unexpected Previous.\n"); + ok(frame_extended2.Frame.ActivationContext == context2, "Got unexpected ActivationContext.\n"); + ok(frame_extended2.Frame.Flags == 0x20, "Got unexpected Flags %#lx.\n", frame_extended2.Frame.Flags); + + ret = GetCurrentActCtx(¤t_context); + ok(current_context == context2, "Got unexpected handle.\n"); + ReleaseActCtx(current_context); + + pRtlDeactivateActivationContextUnsafeFast(&frame_extended2); + + ret = GetCurrentActCtx(¤t_context); + ok(current_context == context1, "Got unexpected handle.\n"); + ReleaseActCtx(current_context); + + pRtlDeactivateActivationContextUnsafeFast(&frame_extended1); + + ret = GetCurrentActCtx(¤t_context); + ok(ret, "GetCurrentActCtx failed.\n"); + ok(!current_context, "Got unexpected handle.\n"); + + /* Test freeing an unsafe frame. Not crashing means RtlFreeThreadActivationContextStack() must + * be able to handle it, most likely by checking the frame flags */ + frame_extended1.Size = sizeof(frame_extended1); + frame = pRtlActivateActivationContextUnsafeFast(&frame_extended1, context1); + ok(frame == &frame_extended1.Frame, "Got unexpected frame.\n"); + + ret = GetCurrentActCtx(¤t_context); + ok(current_context == context1, "Got unexpected handle.\n"); + ReleaseActCtx(current_context); + + pRtlFreeThreadActivationContextStack(); + ret = GetCurrentActCtx(¤t_context); + ok(ret, "GetCurrentActCtx failed.\n"); + ok(!current_context, "Got unexpected handle.\n"); + + ReleaseActCtx(context2); + ReleaseActCtx(context1); +} + START_TEST(actctx) { int argc; @@ -4637,5 +4751,6 @@ START_TEST(actctx) test_compatibility(); test_settings(); test_RtlQueryInformationActiveActivationContext(); + test_RtlActivateActivationContextUnsafeFast(); for (int i = 1; i <= 6; i++) run_child_process_two_dll(i); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8326
Nikolay Sivov (@nsivov) commented about dlls/kernel32/tests/actctx.c:
+ skip("Could not create manifest file\n"); + return; + } + handle = test_create("test1.manifest"); + ok(handle != INVALID_HANDLE_VALUE, "handle == INVALID_HANDLE_VALUE, error %lu\n", GetLastError()); + DeleteFileA("test1.manifest"); + if (handle != INVALID_HANDLE_VALUE) + { + b = ActivateActCtx(handle, &cookie); + ok(b, "ActivateActCtx failed: %lu\n", GetLastError()); + + frame = NtCurrentTeb()->ActivationContextStackPointer->ActiveFrame; + ok(!frame->Previous, "Got unexpected Previous.\n"); + ok(frame->ActivationContext == handle, "Got unexpected ActivationContext.\n"); + todo_wine + ok(frame->Flags == 0x28, "Got unexpected Flags %#lx.\n", frame->Flags); We don't know what 0x20 means in this mask. Important difference here is 0x8 bit. I would test only for what we know the meaning of.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106826
Nikolay Sivov (@nsivov) commented about dlls/kernel32/tests/actctx.c:
+ todo_wine + ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); + if (status == STATUS_SUCCESS) + { + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + } + + status = pRtlActivateActivationContext(0x28, handle, &cookie); + todo_wine + ok(status == STATUS_INVALID_PARAMETER, "Got unexpected status %#lx.\n", status); + if (status == STATUS_SUCCESS) + { + b = DeactivateActCtx(0, cookie); + ok(b, "DeactivateActCtx failed: %lu\n", GetLastError()); + } I don't see why we need to test this. It's good that you verified that this argument means something else, but now that we know we don't need to run it as a test. We don't use it internally and unless application is starts using it, I don't think we need to care.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106825
Nikolay Sivov (@nsivov) commented about dlls/kernel32/tests/actctx.c:
static BOOL (WINAPI *pQueryActCtxSettingsW)(DWORD,HANDLE,LPCWSTR,LPCWSTR,LPWSTR,SIZE_T,SIZE_T*);
+static NTSTATUS (NTAPI *pRtlActivateActivationContext)(ULONG,HANDLE,ULONG_PTR *); +static NTSTATUS (NTAPI *pRtlActivateActivationContextEx)(ULONG,TEB *,HANDLE,ULONG_PTR *); static NTSTATUS(NTAPI *pRtlFindActivationContextSectionString)(DWORD,const GUID *,ULONG,PUNICODE_STRING,PACTCTX_SECTION_KEYED_DATA);
We should stop using HANDLE here, it's causing confusion, and mistakes like wrong release call in the previous version. We'll keep a HANDLE type in kernelbase of course as it's public, but for Rtl* functions it's fine to use structure pointers. This could be done as a separate commit or MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106827
Nikolay Sivov (@nsivov) commented about dlls/ntdll/actctx.c:
/****************************************************************** * RtlActivateActivationContext (NTDLL.@) */ -NTSTATUS WINAPI RtlActivateActivationContext( ULONG unknown, HANDLE handle, PULONG_PTR cookie ) +NTSTATUS WINAPI RtlActivateActivationContext( ULONG flags, HANDLE handle, PULONG_PTR cookie ) { - return RtlActivateActivationContextEx( 0, NtCurrentTeb(), handle, cookie ); + return RtlActivateActivationContextEx( flags, NtCurrentTeb(), handle, cookie ); } I don't think there is any evidence we should do that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106828
Nikolay Sivov (@nsivov) commented about dlls/ntdll/actctx.c:
ACTIVATION_CONTEXT_STACK *actctx_stack = teb->ActivationContextStackPointer; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame;
+ if (flags != 0) + return STATUS_INVALID_PARAMETER; +
We don't need that change, and it's not clear if it's correct. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106830
Nikolay Sivov (@nsivov) commented about dlls/ntdll/ntdll_misc.h:
#define NTDLL_TLS_ERRNO 16 /* TLS slot for _errno() */
+/* undocumented */ +#define NTDLL_STACK_FRAME_ALLOCATED 0x8 /* For RtlActivateActivationContextEx()/RtlFreeActivationContextStack() */ + #ifdef __i386__
Comment is misleading, this flag is not used with those functions directly. Just mention _STACK_FRAME.Flags field. Less important suggestion would be to use an enum instead, but it's up to you. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106831
Nikolay Sivov (@nsivov) commented about dlls/ntdll/actctx.c:
ACTIVATION_CONTEXT_STACK *actctx_stack = teb->ActivationContextStackPointer; RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame;
+ if (flags != 0) + return STATUS_INVALID_PARAMETER; + frame = RtlAllocateHeap( GetProcessHeap(), HEAP_GENERATE_EXCEPTIONS, sizeof(*frame) ); frame->Previous = actctx_stack->ActiveFrame; frame->ActivationContext = handle; - frame->Flags = 0; + frame->Flags = 0x20 | NTDLL_STACK_FRAME_ALLOCATED; actctx_stack->ActiveFrame = frame;
This needs a better name to make it clear, like _HEAP_ALLOCATED. Stack ones are also allocated in a sense. And same as for tests, we don't need to include 0x20 mask until we know what it does. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106829
Nikolay Sivov (@nsivov) commented about include/winternl.h:
ULONG Flags; } RTL_ACTIVATION_CONTEXT_STACK_FRAME, *PRTL_ACTIVATION_CONTEXT_STACK_FRAME;
+typedef struct _RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED +{ + SIZE_T Size;
Do we need to check size field? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106833
Nikolay Sivov (@nsivov) commented about dlls/ntdll/actctx.c:
+/****************************************************************** + * RtlActivateActivationContextUnsafeFast (NTDLL.@) + * + * FIXME: function prototype might be wrong + */ +RTL_ACTIVATION_CONTEXT_STACK_FRAME * FASTCALL RtlActivateActivationContextUnsafeFast( RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED *frame_extended, + PVOID context ) +{ + ACTIVATION_CONTEXT_STACK *actctx_stack = NtCurrentTeb()->ActivationContextStackPointer; + + TRACE( "%p %p\n", frame_extended, context ); + + frame_extended->Frame.Previous = actctx_stack->ActiveFrame; + frame_extended->Frame.ActivationContext = context; + frame_extended->Frame.Flags = 0x20;
Same here, we only need it not to contain 0x8 bit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106832
Nikolay Sivov (@nsivov) commented about dlls/kernel32/tests/actctx.c:
ReleaseActCtx( context ); }
+static void test_RtlActivateActivationContextUnsafeFast(void) +{ + RTL_CALLER_ALLOCATED_ACTIVATION_CONTEXT_STACK_FRAME_EXTENDED frame_extended1 = {0}, frame_extended2 = {0}; + HANDLE context1, context2, current_context; + RTL_ACTIVATION_CONTEXT_STACK_FRAME *frame; + BOOL ret; + + /* RtlActivateActivationContextUnsafeFast() has a different prototype on <= Win7 */ + if (!is_win8_plus()) + { + skip("Test requires Win8+.\n"); + return; + }
This will skip on Wine and we don't need to do that. How different is it? If it's only about return value I would still use current prototype, but adjusted tests to use frame_extended.Frame instead of return value, if it's the same value. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8326#note_106834
participants (3)
-
Nikolay Sivov (＠nsivov) -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)