[ros-diffs] [reactos] 12/13: [NTOS:KE] Test spinlock ownership on both UP & MP build

[Jérôme Gardou]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f30136bc79109af5df335…

commit f30136bc79109af5df33526974e24b39353773c7
Author:     Jérôme Gardou <jerome.gardou(a)reactos.org>
AuthorDate: Thu May 20 10:13:40 2021 +0200
Commit:     Jérôme Gardou <zefklop(a)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.ionescu(a)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