fixed VirtualMemoryLogging::logRecords overflow (#308)

when VirtualMemoryLogging::recordNumber increments from LONG_MAX,
it became negative number, and the result of i % MaxRecords became
a number from -127 to 0.

When that happens we will ovewrite CRITICAL_SECTION virtual_critsec
which are stored in bss right before logRecords with garbage data.
Then most likely the process will have a GC hang with one or more
GC threads stuck trying to enter or leave critical section.

The fix is to ensure ULONG value are passed to modulo operation.

diff --git a/src/coreclr/src/pal/src/map/virtual.cpp b/src/coreclr/src/pal/src/map/virtual.cpp
index 8530ea0..6902153 100644 (file)
--- a/src/coreclr/src/pal/src/map/virtual.cpp
+++ b/src/coreclr/src/pal/src/map/virtual.cpp
@@ -107,7 +107,7 @@ namespace VirtualMemoryLogging
     // An entry in the in-memory log
     struct LogRecord
     {
-        LONG RecordId;
+        ULONG RecordId;
         DWORD Operation;
         LPVOID CurrentThread;
         LPVOID RequestedAddress;
@@ -118,14 +118,14 @@ namespace VirtualMemoryLogging
     };
 
     // Maximum number of records in the in-memory log
-    const LONG MaxRecords = 128;
+    const ULONG MaxRecords = 128;
 
     // Buffer used to store the logged data
     volatile LogRecord logRecords[MaxRecords];
 
     // Current record number. Use (recordNumber % MaxRecords) to determine
     // the current position in the circular buffer.
-    volatile LONG recordNumber = 0;
+    volatile ULONG recordNumber = 0;
 
     // Record an entry in the in-memory log
     void LogVaOperation(
@@ -137,7 +137,7 @@ namespace VirtualMemoryLogging
         IN LPVOID returnedAddress,
         IN BOOL result)
     {
-        LONG i = InterlockedIncrement(&recordNumber) - 1;
+        ULONG i = (ULONG)InterlockedIncrement((LONG *)&recordNumber) - 1;
         LogRecord* curRec = (LogRecord*)&logRecords[i % MaxRecords];
 
         curRec->RecordId = i;