[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...
8 Oct
2015
8 Oct
'15
4:26 p.m.
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/…
==============================================================================
--- 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/…
==============================================================================
--- 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/…
==============================================================================
--- 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
//
3270
days inactive
3270
days old
0 comments
1 participants
participants (1)
-
sginsberg＠svn.reactos.org