[sginsberg] 69466: [KERNEL32] Fix bug in CreateFiberEx that made it replace the CONTEXT_FULL bits rather than ORing in CONTEXT_FLOATING_POINT when caller wanted FPU state saved. SwitchToFiber check...
Author: sginsberg Date: Thu Oct 8 16:26:29 2015 New Revision: 69466
URL: http://svn.reactos.org/svn/reactos?rev=69466&view=rev Log: [KERNEL32] Fix bug in CreateFiberEx that made it replace the CONTEXT_FULL bits rather than ORing in CONTEXT_FLOATING_POINT when caller wanted FPU state saved. SwitchToFiber checks for CONTEXT_FULL OR CONTEXT_FLOATING_POINT so the save/restore would fail. Moreover, fix BaseInitializeContext that was not checking CONTEXT_FLOATING_POINT correctly for some fibers and, as a result, not initializing FPU context correctly for callers of ConvertThreadToFiberEx. Finally, because trying to access address 0x6 is generally a bad idea, fix SwitchToFiber to use the correct shared user data offsets. Misc cleanup all around. Bonus: Sort TEB/PEB asm offsets and add GDI Batch Count offset, needed soon.
Modified: trunk/reactos/dll/win32/kernel32/client/fiber.c trunk/reactos/dll/win32/kernel32/client/i386/fiber.S trunk/reactos/dll/win32/kernel32/client/utils.c trunk/reactos/include/asm/ks386.template.h trunk/reactos/include/ndk/i386/asm.h
Modified: trunk/reactos/dll/win32/kernel32/client/fiber.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/f... ============================================================================== --- trunk/reactos/dll/win32/kernel32/client/fiber.c [iso-8859-1] (original) +++ trunk/reactos/dll/win32/kernel32/client/fiber.c [iso-8859-1] Thu Oct 8 16:26:29 2015 @@ -27,10 +27,9 @@
VOID WINAPI -BaseRundownFls(IN PVOID FlsData) +BaseRundownFls(_In_ PVOID FlsData) { /* No FLS support yet */ - }
/* PUBLIC FUNCTIONS ***********************************************************/ @@ -55,16 +54,18 @@ return FALSE; }
- /* this thread won't run a fiber anymore */ + /* This thread won't run a fiber anymore */ Teb->HasFiberData = FALSE; FiberData = Teb->NtTib.FiberData; Teb->NtTib.FiberData = NULL;
/* Free the fiber */ ASSERT(FiberData != NULL); - RtlFreeHeap(GetProcessHeap(), 0, FiberData); - - /* success */ + RtlFreeHeap(GetProcessHeap(), + 0, + FiberData); + + /* Success */ return TRUE; }
@@ -73,15 +74,15 @@ */ LPVOID WINAPI -ConvertThreadToFiberEx(LPVOID lpParameter, - DWORD dwFlags) +ConvertThreadToFiberEx(_In_opt_ LPVOID lpParameter, + _In_ DWORD dwFlags) { PTEB Teb; PFIBER Fiber; DPRINT1("Converting Thread to Fiber\n");
/* Check for invalid flags */ - if (dwFlags &~ FIBER_FLAG_FLOAT_SWITCH) + if (dwFlags & ~FIBER_FLAG_FLOAT_SWITCH) { /* Fail */ SetLastError(ERROR_INVALID_PARAMETER); @@ -98,9 +99,12 @@ }
/* Allocate the fiber */ - Fiber = RtlAllocateHeap(RtlGetProcessHeap(), 0, sizeof(FIBER)); + Fiber = RtlAllocateHeap(RtlGetProcessHeap(), + 0, + sizeof(FIBER)); if (!Fiber) { + /* Fail */ SetLastError(ERROR_NOT_ENOUGH_MEMORY); return NULL; } @@ -114,13 +118,11 @@ Fiber->FlsData = Teb->FlsData; Fiber->GuaranteedStackBytes = Teb->GuaranteedStackBytes; Fiber->ActivationContextStackPointer = Teb->ActivationContextStackPointer; - Fiber->FiberContext.ContextFlags = CONTEXT_FULL; - - /* Save FPU State if requested */ - if (dwFlags & FIBER_FLAG_FLOAT_SWITCH) - { - Fiber->FiberContext.ContextFlags |= CONTEXT_FLOATING_POINT; - } + + /* Save FPU State if requested, otherwise just the basic registers */ + Fiber->FiberContext.ContextFlags = (dwFlags & FIBER_FLAG_FLOAT_SWITCH) ? + (CONTEXT_FULL | CONTEXT_FLOATING_POINT) : + CONTEXT_FULL;
/* Associate the fiber to the current thread */ Teb->NtTib.FiberData = Fiber; @@ -135,10 +137,11 @@ */ LPVOID WINAPI -ConvertThreadToFiber(LPVOID lpParameter) +ConvertThreadToFiber(_In_opt_ LPVOID lpParameter) { /* Call the newer function */ - return ConvertThreadToFiberEx(lpParameter, 0); + return ConvertThreadToFiberEx(lpParameter, + 0); }
/* @@ -146,12 +149,16 @@ */ LPVOID WINAPI -CreateFiber(SIZE_T dwStackSize, - LPFIBER_START_ROUTINE lpStartAddress, - LPVOID lpParameter) +CreateFiber(_In_ SIZE_T dwStackSize, + _In_ LPFIBER_START_ROUTINE lpStartAddress, + _In_opt_ LPVOID lpParameter) { /* Call the Newer Function */ - return CreateFiberEx(dwStackSize, 0, 0, lpStartAddress, lpParameter); + return CreateFiberEx(dwStackSize, + 0, + 0, + lpStartAddress, + lpParameter); }
/* @@ -159,20 +166,20 @@ */ LPVOID WINAPI -CreateFiberEx(SIZE_T dwStackCommitSize, - SIZE_T dwStackReserveSize, - DWORD dwFlags, - LPFIBER_START_ROUTINE lpStartAddress, - LPVOID lpParameter) +CreateFiberEx(_In_ SIZE_T dwStackCommitSize, + _In_ SIZE_T dwStackReserveSize, + _In_ DWORD dwFlags, + _In_ LPFIBER_START_ROUTINE lpStartAddress, + _In_opt_ LPVOID lpParameter) { PFIBER Fiber; NTSTATUS Status; INITIAL_TEB InitialTeb; - PACTIVATION_CONTEXT_STACK ActivationContextStackPointer = NULL; + PACTIVATION_CONTEXT_STACK ActivationContextStackPointer; DPRINT("Creating Fiber\n");
/* Check for invalid flags */ - if (dwFlags &~ FIBER_FLAG_FLOAT_SWITCH) + if (dwFlags & ~FIBER_FLAG_FLOAT_SWITCH) { /* Fail */ SetLastError(ERROR_INVALID_PARAMETER); @@ -180,6 +187,7 @@ }
/* Allocate the Activation Context Stack */ + ActivationContextStackPointer = NULL; Status = RtlAllocateActivationContextStack(&ActivationContextStackPointer); if (!NT_SUCCESS(Status)) { @@ -189,7 +197,9 @@ }
/* Allocate the fiber */ - Fiber = RtlAllocateHeap(RtlGetProcessHeap(), 0, sizeof(FIBER)); + Fiber = RtlAllocateHeap(RtlGetProcessHeap(), + 0, + sizeof(FIBER)); if (!Fiber) { /* Free the activation context stack */ @@ -208,7 +218,9 @@ if (!NT_SUCCESS(Status)) { /* Free the fiber */ - RtlFreeHeap(GetProcessHeap(), 0, Fiber); + RtlFreeHeap(GetProcessHeap(), + 0, + Fiber);
/* Free the activation context stack */ RtlFreeActivationContextStack(ActivationContextStackPointer); @@ -219,7 +231,8 @@ }
/* Clear the context */ - RtlZeroMemory(&Fiber->FiberContext, sizeof(CONTEXT)); + RtlZeroMemory(&Fiber->FiberContext, + sizeof(CONTEXT));
/* Copy the data into the fiber */ Fiber->StackBase = InitialTeb.StackBase; @@ -230,12 +243,13 @@ Fiber->GuaranteedStackBytes = 0; Fiber->FlsData = NULL; Fiber->ActivationContextStackPointer = ActivationContextStackPointer; - Fiber->FiberContext.ContextFlags = CONTEXT_FULL; - - /* Save FPU State if requested */ - Fiber->FiberContext.ContextFlags = (dwFlags & FIBER_FLAG_FLOAT_SWITCH) ? CONTEXT_FLOATING_POINT : 0; - - /* initialize the context for the fiber */ + + /* Save FPU State if requested, otherwise just the basic registers */ + Fiber->FiberContext.ContextFlags = (dwFlags & FIBER_FLAG_FLOAT_SWITCH) ? + (CONTEXT_FULL | CONTEXT_FLOATING_POINT) : + CONTEXT_FULL; + + /* Initialize the context for the fiber */ BaseInitializeContext(&Fiber->FiberContext, lpParameter, lpStartAddress, @@ -251,17 +265,24 @@ */ VOID WINAPI -DeleteFiber(LPVOID lpFiber) -{ - SIZE_T Size = 0; - PFIBER Fiber = (PFIBER)lpFiber; +DeleteFiber(_In_ LPVOID lpFiber) +{ + SIZE_T Size; + PFIBER Fiber; PTEB Teb;
- /* First, exit the thread */ + /* Are we deleting ourselves? */ Teb = NtCurrentTeb(); - if ((Teb->HasFiberData) && (Teb->NtTib.FiberData == Fiber)) ExitThread(1); - - /* Now de-allocate the stack */ + Fiber = (PFIBER)lpFiber; + if ((Teb->HasFiberData) && + (Teb->NtTib.FiberData == Fiber)) + { + /* Just exit */ + ExitThread(1); + } + + /* Not ourselves, de-allocate the stack */ + Size = 0 ; NtFreeVirtualMemory(NtCurrentProcess(), &Fiber->DeallocationStack, &Size, @@ -274,7 +295,9 @@ RtlFreeActivationContextStack(Fiber->ActivationContextStackPointer);
/* Free the fiber data */ - RtlFreeHeap(GetProcessHeap(), 0, lpFiber); + RtlFreeHeap(GetProcessHeap(), + 0, + lpFiber); }
/* @@ -284,6 +307,7 @@ WINAPI IsThreadAFiber(VOID) { + /* Return flag in the TEB */ return NtCurrentTeb()->HasFiberData; }
@@ -349,7 +373,8 @@ */ BOOL WINAPI -FlsSetValue(DWORD dwFlsIndex, PVOID lpFlsData) +FlsSetValue(DWORD dwFlsIndex, + PVOID lpFlsData) { PVOID *ppFlsSlots; TEB *pTeb = NtCurrentTeb();
Modified: trunk/reactos/dll/win32/kernel32/client/i386/fiber.S URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/i... ============================================================================== --- trunk/reactos/dll/win32/kernel32/client/i386/fiber.S [iso-8859-1] (original) +++ trunk/reactos/dll/win32/kernel32/client/i386/fiber.S [iso-8859-1] Thu Oct 8 16:26:29 2015 @@ -15,21 +15,26 @@ .code
PUBLIC _BaseFiberStartup@0 -.PROC _BaseFiberStartup@0 - /* Frame pointer is zeroed */ +FUNC _BaseFiberStartup@0 FPO 0, 0, 0, 0, 0, FRAME_FPO + + /* Note that EBP is already zeroed for us during fiber creation */
/* Get the fiber data */ mov eax, fs:[TEB_FIBER_DATA]
- push dword ptr [eax+FIBER_CONTEXT_EBX] /* Parameter */ - push dword ptr [eax+FIBER_CONTEXT_EAX] /* Start Address */ - call _BaseThreadStartup@8 -.ENDP + /* Start the thread with our parameters */ + push dword ptr [eax+FIBER_CONTEXT_EBX] + push dword ptr [eax+FIBER_CONTEXT_EAX] + jmp _BaseThreadStartup@8 + +ENDFUNC
PUBLIC _SwitchToFiber@4 -_SwitchToFiber@4: +FUNC _SwitchToFiber@4 + FPO 0, 0, 0, 0, 0, FRAME_FPO + /* Get the TEB */ mov edx, fs:[TEB_SELF]
@@ -51,7 +56,7 @@ fnstcw [eax+FIBER_CONTEXT_FLOAT_SAVE_CONTROL_WORD]
/* Check if the CPU supports SIMD MXCSR State Save */ - cmp byte ptr ds:[PF_XMMI_INSTRUCTIONS_AVAILABLE], 1 + cmp byte ptr ds:[USER_SHARED_DATA + USER_SHARED_DATA_PROCESSOR_FEATURES + PF_XMMI_INSTRUCTIONS_AVAILABLE], 1 jnz NoFpuStateSave stmxcsr [eax+FIBER_CONTEXT_DR6]
@@ -109,13 +114,13 @@ StatusWordChanged:
/* Load the new one */ - mov word ptr [ecx+FIBER_CONTEXT_FLOAT_SAVE_TAG_WORD], HEX(0FFFF) + mov word ptr [ecx+FIBER_CONTEXT_FLOAT_SAVE_TAG_WORD], HEX(FFFF) fldenv [ecx+FIBER_CONTEXT_FLOAT_SAVE_CONTROL_WORD]
ControlWordEqual:
/* Load the new one */ - cmp byte ptr ds:[PF_XMMI_INSTRUCTIONS_AVAILABLE], 1 + cmp byte ptr ds:[USER_SHARED_DATA + USER_SHARED_DATA_PROCESSOR_FEATURES + PF_XMMI_INSTRUCTIONS_AVAILABLE], 1 jnz NoFpuStateRestore ldmxcsr [ecx+FIBER_CONTEXT_DR6]
@@ -136,6 +141,7 @@ mov esp, [ecx+FIBER_CONTEXT_ESP] ret 4
+ENDFUNC
END /* EOF */
Modified: trunk/reactos/dll/win32/kernel32/client/utils.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/u... ============================================================================== --- trunk/reactos/dll/win32/kernel32/client/utils.c [iso-8859-1] (original) +++ trunk/reactos/dll/win32/kernel32/client/utils.c [iso-8859-1] Thu Oct 8 16:26:29 2015 @@ -548,7 +548,7 @@
/* Is FPU state required? */ Context->ContextFlags |= ContextFlags; - if (ContextFlags == CONTEXT_FLOATING_POINT) + if ((ContextFlags & CONTEXT_FLOATING_POINT) == CONTEXT_FLOATING_POINT) { /* Set an initial state */ Context->FloatSave.ControlWord = 0x27F;
Modified: trunk/reactos/include/asm/ks386.template.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/asm/ks386.template.... ============================================================================== --- trunk/reactos/include/asm/ks386.template.h [iso-8859-1] (original) +++ trunk/reactos/include/asm/ks386.template.h [iso-8859-1] Thu Oct 8 16:26:29 2015 @@ -724,17 +724,20 @@
HEADER("TEB"), OFFSET(TEB_EXCEPTION_LIST, TEB, NtTib.ExceptionList), +OFFSET(TEB_STACK_BASE, TEB, NtTib.StackBase), OFFSET(TEB_STACK_LIMIT, TEB, NtTib.StackLimit), -OFFSET(TEB_STACK_BASE, TEB, NtTib.StackBase), +OFFSET(TEB_FIBER_DATA, TEB, NtTib.FiberData), OFFSET(TEB_SELF, TEB, NtTib.Self), -OFFSET(TEB_FIBER_DATA, TEB, NtTib.FiberData), OFFSET(TEB_PEB, TEB, ProcessEnvironmentBlock), OFFSET(TEB_EXCEPTION_CODE, TEB, ExceptionCode), +OFFSET(TEB_ACTIVATION_CONTEXT_STACK_POINTER, TEB, ActivationContextStackPointer), +OFFSET(TEB_DEALLOCATION_STACK, TEB, DeallocationStack), +OFFSET(TEB_GDI_BATCH_COUNT, TEB, GdiBatchCount), +OFFSET(TEB_GUARANTEED_STACK_BYTES, TEB, GuaranteedStackBytes), +OFFSET(TEB_FLS_DATA, TEB, FlsData), + +HEADER("PEB"), OFFSET(PEB_KERNEL_CALLBACK_TABLE, PEB, KernelCallbackTable), -OFFSET(TEB_FLS_DATA, TEB, FlsData), -OFFSET(TEB_ACTIVATION_CONTEXT_STACK_POINTER, TEB, ActivationContextStackPointer), -OFFSET(TEB_GUARANTEED_STACK_BYTES, TEB, GuaranteedStackBytes), -OFFSET(TEB_DEALLOCATION_STACK, TEB, DeallocationStack),
HEADER("Misc"), CONSTANT(NPX_FRAME_LENGTH), @@ -754,4 +757,6 @@ CONSTANT(CONTEXT_ALIGNED_SIZE), CONSTANT(PROCESSOR_FEATURE_FXSR), CONSTANT(KUSER_SHARED_SYSCALL_RET), - +CONSTANT(USER_SHARED_DATA), +CONSTANT(USER_SHARED_DATA_PROCESSOR_FEATURES), +
Modified: trunk/reactos/include/ndk/i386/asm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/i386/asm.h?rev=... ============================================================================== --- trunk/reactos/include/ndk/i386/asm.h [iso-8859-1] (original) +++ trunk/reactos/include/ndk/i386/asm.h [iso-8859-1] Thu Oct 8 16:26:29 2015 @@ -315,13 +315,14 @@ #define FRAME_EDITED 0xFFF8
// -// KUSER_SHARED_DATA Offsets +// USER_SHARED_DATA Offsets // #ifdef __ASM__ #define USER_SHARED_DATA 0xFFDF0000 #endif #define USER_SHARED_DATA_INTERRUPT_TIME 0x8 #define USER_SHARED_DATA_SYSTEM_TIME 0x14 +#define USER_SHARED_DATA_PROCESSOR_FEATURES 0x274 #define USER_SHARED_DATA_TICK_COUNT 0x320
//
-
sginsberg＠svn.reactos.org