https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f30136bc79109af5df33526974e24b39353773c7
commit f30136bc79109af5df33526974e24b39353773c7
Author: Jérôme Gardou <jerome.gar...@reactos.org>
AuthorDate: Thu May 20 10:13:40 2021 +0200
Commit: Jérôme Gardou <zefk...@users.noreply.github.com>
CommitDate: Tue Jun 29 11:49:20 2021 +0200
[NTOS:KE] Test spinlock ownership on both UP & MP build
There is no reason not to, and this avoids introducing bugs stupidly.
---
boot/freeldr/freeldr/arch/i386/ntoskrnl.c | 8 ++++
ntoskrnl/include/internal/spinlock.h | 72 ++++++++++++++-----------------
ntoskrnl/ke/spinlock.c | 25 +++++++----
sdk/lib/crt/crt.cmake | 3 +-
sdk/lib/crt/libcntpr.cmake | 3 +-
5 files changed, 62 insertions(+), 49 deletions(-)
diff --git a/boot/freeldr/freeldr/arch/i386/ntoskrnl.c
b/boot/freeldr/freeldr/arch/i386/ntoskrnl.c
index e44fa770771..4255e5cf0e0 100644
--- a/boot/freeldr/freeldr/arch/i386/ntoskrnl.c
+++ b/boot/freeldr/freeldr/arch/i386/ntoskrnl.c
@@ -28,6 +28,14 @@ FASTCALL
KefAcquireSpinLockAtDpcLevel(
IN PKSPIN_LOCK SpinLock)
{
+#if DBG
+ /* To be on par with HAL/NTOSKRNL */
+#ifdef _M_AMD64
+ *SpinLock = (KSPIN_LOCK)KeGetCurrentThread() | 1;
+#else
+ *SpinLock = (KSPIN_LOCK)(((PKIPCR)KeGetPcr())->PrcbData.CurrentThread) | 1;
+#endif
+#endif
}
VOID
diff --git a/ntoskrnl/include/internal/spinlock.h
b/ntoskrnl/include/internal/spinlock.h
index e126dbe0b03..013ad913c34 100644
--- a/ntoskrnl/include/internal/spinlock.h
+++ b/ntoskrnl/include/internal/spinlock.h
@@ -6,50 +6,25 @@
* PROGRAMMERS: Alex Ionescu (alex.ione...@reactos.org)
*/
+#if defined(_M_IX86)
VOID
NTAPI
Kii386SpinOnSpinLock(PKSPIN_LOCK SpinLock, ULONG Flags);
-
-#ifndef CONFIG_SMP
-
-//
-// Spinlock Acquire at IRQL >= DISPATCH_LEVEL
-//
-FORCEINLINE
-VOID
-KxAcquireSpinLock(IN PKSPIN_LOCK SpinLock)
-{
- /* On UP builds, spinlocks don't exist at IRQL >= DISPATCH */
- UNREFERENCED_PARAMETER(SpinLock);
-
- /* Add an explicit memory barrier to prevent the compiler from reordering
- memory accesses across the borders of spinlocks */
- KeMemoryBarrierWithoutFence();
-}
-
-//
-// Spinlock Release at IRQL >= DISPATCH_LEVEL
-//
-FORCEINLINE
-VOID
-KxReleaseSpinLock(IN PKSPIN_LOCK SpinLock)
-{
- /* On UP builds, spinlocks don't exist at IRQL >= DISPATCH */
- UNREFERENCED_PARAMETER(SpinLock);
-
- /* Add an explicit memory barrier to prevent the compiler from reordering
- memory accesses across the borders of spinlocks */
- KeMemoryBarrierWithoutFence();
-}
-
-#else
+#endif
//
// Spinlock Acquisition at IRQL >= DISPATCH_LEVEL
//
+_Acquires_nonreentrant_lock_(SpinLock)
FORCEINLINE
VOID
-KxAcquireSpinLock(IN PKSPIN_LOCK SpinLock)
+KxAcquireSpinLock(
+#if defined(CONFIG_SMP) || DBG
+ _Inout_
+#else
+ _Unreferenced_parameter_
+#endif
+ PKSPIN_LOCK SpinLock)
{
#if DBG
/* Make sure that we don't own the lock already */
@@ -60,6 +35,7 @@ KxAcquireSpinLock(IN PKSPIN_LOCK SpinLock)
}
#endif
+#ifdef CONFIG_SMP
/* Try to acquire the lock */
while (InterlockedBitTestAndSet((PLONG)SpinLock, 0))
{
@@ -75,6 +51,12 @@ KxAcquireSpinLock(IN PKSPIN_LOCK SpinLock)
}
#endif
}
+#endif
+
+ /* Add an explicit memory barrier to prevent the compiler from reordering
+ memory accesses across the borders of spinlocks */
+ KeMemoryBarrierWithoutFence();
+
#if DBG
/* On debug builds, we OR in the KTHREAD */
*SpinLock = (KSPIN_LOCK)KeGetCurrentThread() | 1;
@@ -84,9 +66,16 @@ KxAcquireSpinLock(IN PKSPIN_LOCK SpinLock)
//
// Spinlock Release at IRQL >= DISPATCH_LEVEL
//
+_Releases_nonreentrant_lock_(SpinLock)
FORCEINLINE
VOID
-KxReleaseSpinLock(IN PKSPIN_LOCK SpinLock)
+KxReleaseSpinLock(
+#if defined(CONFIG_SMP) || DBG
+ _Inout_
+#else
+ _Unreferenced_parameter_
+#endif
+ PKSPIN_LOCK SpinLock)
{
#if DBG
/* Make sure that the threads match */
@@ -96,12 +85,17 @@ KxReleaseSpinLock(IN PKSPIN_LOCK SpinLock)
KeBugCheckEx(SPIN_LOCK_NOT_OWNED, (ULONG_PTR)SpinLock, 0, 0, 0);
}
#endif
- /* Clear the lock */
+
+#if defined(CONFIG_SMP) || DBG
+ /* Clear the lock */
#ifdef _WIN64
InterlockedAnd64((PLONG64)SpinLock, 0);
#else
InterlockedAnd((PLONG)SpinLock, 0);
#endif
-}
-
#endif
+
+ /* Add an explicit memory barrier to prevent the compiler from reordering
+ memory accesses across the borders of spinlocks */
+ KeMemoryBarrierWithoutFence();
+}
diff --git a/ntoskrnl/ke/spinlock.c b/ntoskrnl/ke/spinlock.c
index 6648f75e5a9..deb52b85d29 100644
--- a/ntoskrnl/ke/spinlock.c
+++ b/ntoskrnl/ke/spinlock.c
@@ -109,10 +109,10 @@ KeAcquireQueuedSpinLockAtDpcLevel(IN PKSPIN_LOCK_QUEUE
LockHandle)
0,
0);
}
+#endif
/* Do the inlined function */
KxAcquireSpinLock(LockHandle->Lock);
-#endif
}
VOID
@@ -130,10 +130,10 @@ KeReleaseQueuedSpinLockFromDpcLevel(IN PKSPIN_LOCK_QUEUE
LockHandle)
0,
0);
}
+#endif
/* Do the inlined function */
KxReleaseSpinLock(LockHandle->Lock);
-#endif
}
#endif
@@ -302,6 +302,15 @@ BOOLEAN
FASTCALL
KeTryToAcquireSpinLockAtDpcLevel(IN OUT PKSPIN_LOCK SpinLock)
{
+#if DBG
+ /* Make sure that we don't own the lock already */
+ if (((KSPIN_LOCK)KeGetCurrentThread() | 1) == *SpinLock)
+ {
+ /* We do, bugcheck! */
+ KeBugCheckEx(SPIN_LOCK_ALREADY_OWNED, (ULONG_PTR)SpinLock, 0, 0, 0);
+ }
+#endif
+
#ifdef CONFIG_SMP
/* Check if it's already acquired */
if (!(*SpinLock))
@@ -318,11 +327,11 @@ KeTryToAcquireSpinLockAtDpcLevel(IN OUT PKSPIN_LOCK
SpinLock)
/* It was already acquired */
return FALSE;
}
+#endif
#if DBG
/* On debug builds, we OR in the KTHREAD */
*SpinLock = (ULONG_PTR)KeGetCurrentThread() | 1;
-#endif
#endif
/* All is well, return TRUE */
@@ -337,10 +346,10 @@ FASTCALL
KeAcquireInStackQueuedSpinLockAtDpcLevel(IN PKSPIN_LOCK SpinLock,
IN PKLOCK_QUEUE_HANDLE LockHandle)
{
-#ifdef CONFIG_SMP
/* Set it up properly */
LockHandle->LockQueue.Next = NULL;
LockHandle->LockQueue.Lock = SpinLock;
+#ifdef CONFIG_SMP
#if 0
KeAcquireQueuedSpinLockAtDpcLevel(LockHandle->LockQueue.Next);
#else
@@ -354,11 +363,11 @@ KeAcquireInStackQueuedSpinLockAtDpcLevel(IN PKSPIN_LOCK
SpinLock,
0,
0);
}
+#endif
+#endif
/* Acquire the lock */
KxAcquireSpinLock(LockHandle->LockQueue.Lock); // HACK
-#endif
-#endif
}
/*
@@ -383,11 +392,11 @@ KeReleaseInStackQueuedSpinLockFromDpcLevel(IN
PKLOCK_QUEUE_HANDLE LockHandle)
0,
0);
}
+#endif
+#endif
/* Release the lock */
KxReleaseSpinLock(LockHandle->LockQueue.Lock); // HACK
-#endif
-#endif
}
/*
diff --git a/sdk/lib/crt/crt.cmake b/sdk/lib/crt/crt.cmake
index f3b9160a9b5..1090ff33a78 100644
--- a/sdk/lib/crt/crt.cmake
+++ b/sdk/lib/crt/crt.cmake
@@ -117,6 +117,7 @@ list(APPEND CRT_SOURCE
mem/memcmp.c
mem/memccpy.c
mem/memicmp.c
+ mem/memset.c
misc/__crt_MessageBoxA.c
misc/amsg.c
misc/assert.c
@@ -399,7 +400,7 @@ if(ARCH STREQUAL "i386")
math/i386/fmodf_asm.s
mem/i386/memchr_asm.s
mem/i386/memmove_asm.s
- mem/i386/memset_asm.s
+ # mem/i386/memset_asm.s
misc/i386/readcr4.S
setjmp/i386/setjmp.s
string/i386/strcat_asm.s
diff --git a/sdk/lib/crt/libcntpr.cmake b/sdk/lib/crt/libcntpr.cmake
index 178a18cd704..cff2a2e7286 100644
--- a/sdk/lib/crt/libcntpr.cmake
+++ b/sdk/lib/crt/libcntpr.cmake
@@ -9,6 +9,7 @@ list(APPEND LIBCNTPR_SOURCE
mem/memccpy.c
mem/memcmp.c
mem/memicmp.c
+ mem/memset.c
misc/fltused.c
printf/_snprintf.c
printf/_snwprintf.c
@@ -186,7 +187,7 @@ if(ARCH STREQUAL "i386")
list(APPEND LIBCNTPR_ASM_SOURCE
mem/i386/memchr_asm.s
mem/i386/memmove_asm.s
- mem/i386/memset_asm.s
+ # mem/i386/memset_asm.s
string/i386/strcat_asm.s
string/i386/strchr_asm.s
string/i386/strcmp_asm.s
