How did our Fiber tests ever pass? Best regards, Alex Ionescu
On Thu, Oct 8, 2015 at 9:26 AM, <sginsb...@svn.reactos.org> wrote: > 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/fiber.c?rev=69466&r1=69465&r2=69466&view=diff > > ============================================================================== > --- 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/i386/fiber.S?rev=69466&r1=69465&r2=69466&view=diff > > ============================================================================== > --- 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/utils.c?rev=69466&r1=69465&r2=69466&view=diff > > ============================================================================== > --- 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.h?rev=69466&r1=69465&r2=69466&view=diff > > ============================================================================== > --- 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=69466&r1=69465&r2=69466&view=diff > > ============================================================================== > --- 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 > > // > > >
_______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
