Fix failures in ETW logging on 4GB aware 32 bit processes (dotnet/coreclr#11941)
authorVance Morrison <vancem@microsoft.com>
Tue, 30 May 2017 16:09:29 +0000 (09:09 -0700)
committerGitHub <noreply@github.com>
Tue, 30 May 2017 16:09:29 +0000 (09:09 -0700)
We incorrectly cast a 32 bit pointer to a 64 bit poitner using as a SIGNED integer.
If this is a 32 bit process that is using more than 2GB of memory this can result in
sign rather than zero extension.  This makes the poitner invalid at the OS level
and causes the OS API to fail.

We disovered this in Visual Studio when debugging large (Rosyln) scenarios.
There were numerous failures which causes sever slowdowns becasue the
EventSource logged OutputDebugString events when the OS API failed.

The fix is to use unsigned extension.   Note that I have confirmed that casting
from a IntPtr or a void* to a ulong does zero extension (that is it uses the target type
to determine whether to use sign or zero extension).

To be useful for Visual Studio, this needs to be ported to the desktop runtime.

Commit migrated from https://github.com/dotnet/coreclr/commit/fef0f8b9c3a29b291894666f19340d99849ab40b

src/coreclr/src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs
src/coreclr/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/DataCollector.cs
src/coreclr/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs

index 3349c06..a4b9d4b 100755 (executable)
@@ -1134,7 +1134,7 @@ namespace System.Diagnostics.Tracing
             /// Address where the one argument lives (if this points to managed memory you must ensure the
             /// managed object is pinned.
             /// </summary>
-            public IntPtr DataPointer { get { return (IntPtr)m_Ptr; } set { m_Ptr = unchecked((long)value); } }
+            public IntPtr DataPointer { get { return (IntPtr)m_Ptr; } set { m_Ptr = unchecked((ulong)value); } }
             /// <summary>
             /// Size of the argument referenced by DataPointer
             /// </summary>
@@ -1150,14 +1150,14 @@ namespace System.Diagnostics.Tracing
             /// <param name="reserved">Value for reserved: 2 for per-provider metadata, 1 for per-event metadata</param>
             internal unsafe void SetMetadata(byte* pointer, int size, int reserved)
             {
-                this.m_Ptr = (long)(ulong)(UIntPtr)pointer;
+                this.m_Ptr = (ulong)pointer;
                 this.m_Size = size;
                 this.m_Reserved = reserved; // Mark this descriptor as containing tracelogging-compatible metadata.
             }
 
             //Important, we pass this structure directly to the Win32 EventWrite API, so this structure must be layed out exactly
             // the way EventWrite wants it.  
-            internal long m_Ptr;
+            internal ulong m_Ptr;
             internal int m_Size;
 #pragma warning disable 0649
             internal int m_Reserved;       // Used to pad the size to match the Win32 API
index 27aae82..87df2d3 100644 (file)
@@ -285,7 +285,7 @@ namespace System.Diagnostics.Tracing
             this.datas = datasTemp + 1;
 
             *pinsTemp = GCHandle.Alloc(value, GCHandleType.Pinned);
-            datasTemp->m_Ptr = (long)(ulong)(UIntPtr)(void*)pinsTemp->AddrOfPinnedObject();
+            datasTemp->DataPointer = pinsTemp->AddrOfPinnedObject();
             datasTemp->m_Size = size;
         }
 
@@ -299,7 +299,7 @@ namespace System.Diagnostics.Tracing
                     throw new IndexOutOfRangeException(Resources.GetResourceString("EventSource_DataDescriptorsOutOfRange"));
                 }
 
-                datasTemp->m_Ptr = (long)(ulong)(UIntPtr)this.scratch;
+                datasTemp->DataPointer = (IntPtr) this.scratch;
                 this.writingScalars = true;
             }
         }
index e733399..aba1671 100644 (file)
@@ -562,7 +562,7 @@ namespace System.Diagnostics.Tracing
                         if (eventTypes.typeInfos[i].DataType == typeof(string))
                         {
                             // Write out the size of the string 
-                            descriptors[numDescrs].m_Ptr = (long)&descriptors[numDescrs + 1].m_Size;
+                            descriptors[numDescrs].DataPointer = (IntPtr) (&descriptors[numDescrs + 1].m_Size);
                             descriptors[numDescrs].m_Size = 2;
                             numDescrs++;