From 38798cf1b663c45046fa58df9bf25a016e80d4a0 Mon Sep 17 00:00:00 2001
From: Brian Robbins <brianrob@microsoft.com>
Date: Fri, 10 Aug 2018 10:22:40 -0700
Subject: [PATCH] Align the Contents of EventPipeBuffers (#19375)

---
 src/vm/eventpipebuffer.cpp        | 20 +++++++++++---------
 src/vm/eventpipebuffer.h          | 15 +++++++++++++++
 src/vm/eventpipeeventinstance.cpp |  1 +
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/src/vm/eventpipebuffer.cpp b/src/vm/eventpipebuffer.cpp
index 1639871..e7489cd 100644
--- a/src/vm/eventpipebuffer.cpp
+++ b/src/vm/eventpipebuffer.cpp
@@ -22,8 +22,8 @@ EventPipeBuffer::EventPipeBuffer(unsigned int bufferSize)
 
     m_pBuffer = new BYTE[bufferSize];
     memset(m_pBuffer, 0, bufferSize);
-    m_pCurrent = m_pBuffer;
     m_pLimit = m_pBuffer + bufferSize;
+    m_pCurrent = GetNextAlignedAddress(m_pBuffer);
 
     m_mostRecentTimeStamp.QuadPart = 0;
     m_pLastPoppedEvent = NULL;
@@ -55,6 +55,7 @@ bool EventPipeBuffer::WriteEvent(Thread *pThread, EventPipeSession &session, Eve
         GC_NOTRIGGER;
         MODE_ANY;
         PRECONDITION(pThread != NULL);
+        PRECONDITION(((size_t)m_pCurrent % AlignmentSize) == 0);
     }
     CONTRACTL_END;
 
@@ -110,7 +111,7 @@ bool EventPipeBuffer::WriteEvent(Thread *pThread, EventPipeSession &session, Eve
     if(success)
     {
         // Advance the current pointer past the event.
-        m_pCurrent += eventSize;
+        m_pCurrent = GetNextAlignedAddress(m_pCurrent + eventSize);
     }
 
     return success;
@@ -134,7 +135,7 @@ void EventPipeBuffer::Clear()
     CONTRACTL_END;
 
     memset(m_pBuffer, 0, (size_t)(m_pLimit - m_pBuffer));
-    m_pCurrent = m_pBuffer;
+    m_pCurrent = GetNextAlignedAddress(m_pBuffer);
     m_mostRecentTimeStamp.QuadPart = 0;
     m_pLastPoppedEvent = NULL;
 }
@@ -154,9 +155,10 @@ EventPipeEventInstance* EventPipeBuffer::GetNext(EventPipeEventInstance *pEvent,
     if(pEvent == NULL)
     {
         // If this buffer contains an event, select it.
-        if(m_pCurrent > m_pBuffer)
+        BYTE *pFirstAlignedInstance = GetNextAlignedAddress(m_pBuffer);
+        if(m_pCurrent > pFirstAlignedInstance)
         {
-            pNextInstance = (EventPipeEventInstance*)m_pBuffer;
+            pNextInstance = (EventPipeEventInstance*)pFirstAlignedInstance;
         }
         else
         {
@@ -174,7 +176,7 @@ EventPipeEventInstance* EventPipeBuffer::GetNext(EventPipeEventInstance *pEvent,
 
         // We have a pointer within the bounds of the buffer.
         // Find the next event by skipping the current event with it's data payload immediately after the instance.
-        pNextInstance = (EventPipeEventInstance *)(pEvent->GetData() + pEvent->GetDataLength());
+        pNextInstance = (EventPipeEventInstance *)GetNextAlignedAddress(const_cast<BYTE *>(pEvent->GetData() + pEvent->GetDataLength()));
 
         // Check to see if we've reached the end of the written portion of the buffer.
         if((BYTE*)pNextInstance >= m_pCurrent)
@@ -245,14 +247,14 @@ bool EventPipeBuffer::EnsureConsistency()
     CONTRACTL_END;
 
     // Check to see if the buffer is empty.
-    if(m_pBuffer == m_pCurrent)
+    if(GetNextAlignedAddress(m_pBuffer) == m_pCurrent)
     {
         // Make sure that the buffer size is greater than zero.
         _ASSERTE(m_pBuffer != m_pLimit);
     }
 
     // Validate the contents of the filled portion of the buffer.
-    BYTE *ptr = m_pBuffer;
+    BYTE *ptr = GetNextAlignedAddress(m_pBuffer);
     while(ptr < m_pCurrent)
     {
         // Validate the event.
@@ -263,7 +265,7 @@ bool EventPipeBuffer::EnsureConsistency()
         _ASSERTE((pInstance->GetData() != NULL && pInstance->GetDataLength() > 0) || (pInstance->GetData() == NULL && pInstance->GetDataLength() == 0));
 
         // Skip the event.
-        ptr += sizeof(*pInstance) + pInstance->GetDataLength();
+        ptr = GetNextAlignedAddress(ptr + sizeof(*pInstance) + pInstance->GetDataLength());
     }
 
     // When we're done walking the filled portion of the buffer,
diff --git a/src/vm/eventpipebuffer.h b/src/vm/eventpipebuffer.h
index 7d6287d..7b20883 100644
--- a/src/vm/eventpipebuffer.h
+++ b/src/vm/eventpipebuffer.h
@@ -20,6 +20,10 @@ class EventPipeBuffer
 
 private:
 
+    // Instances of EventPipeEventInstance in the buffer must be 8-byte aligned.
+    // It is OK for the data payloads to be unaligned because they are opaque blobs that are copied via memcpy.
+    const size_t AlignmentSize = 8;
+
     // A pointer to the actual buffer.
     BYTE *m_pBuffer;
 
@@ -72,6 +76,17 @@ private:
         m_pNextBuffer = pBuffer;
     }
 
+    FORCEINLINE BYTE* GetNextAlignedAddress(BYTE *pAddress)
+    {
+        LIMITED_METHOD_CONTRACT;
+        _ASSERTE(m_pBuffer <= pAddress && m_pLimit > pAddress);
+
+        pAddress = (BYTE*)ALIGN_UP(pAddress, AlignmentSize);
+
+        _ASSERTE((size_t)pAddress % AlignmentSize == 0);
+        return pAddress;
+    }
+
 public:
 
     EventPipeBuffer(unsigned int bufferSize);
diff --git a/src/vm/eventpipeeventinstance.cpp b/src/vm/eventpipeeventinstance.cpp
index 023b9b2..1424dbc 100644
--- a/src/vm/eventpipeeventinstance.cpp
+++ b/src/vm/eventpipeeventinstance.cpp
@@ -54,6 +54,7 @@ EventPipeEventInstance::EventPipeEventInstance(
     m_pData = pData;
     m_dataLength = length;
     QueryPerformanceCounter(&m_timeStamp);
+    _ASSERTE(m_timeStamp.QuadPart > 0);
 
     if(event.NeedStack() && !session.RundownEnabled())
     {
-- 
2.7.4