From e7f9221f8b7830f93e04ece046a2a617d69ae152 Mon Sep 17 00:00:00 2001 From: Brian Robbins <brianrob@microsoft.com> Date: Fri, 16 Mar 2018 10:49:49 -0700 Subject: [PATCH 1/5] Handle metadata generation failures. --- .../System/Diagnostics/Tracing/EventSource.cs | 3 +- .../Tracing/TraceLogging/NameInfo.cs | 3 +- .../Eventing/EventPipeMetadataGenerator.cs | 83 +++++++++++-------- 3 files changed, 51 insertions(+), 38 deletions(-) mode change 100644 => 100755 src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs mode change 100644 => 100755 src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs mode change 100644 => 100755 src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs diff --git a/src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs b/src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs old mode 100644 new mode 100755 index 751b0fd4ee1b..21e03e0fd106 --- a/src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs +++ b/src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs @@ -623,6 +623,7 @@ private unsafe void DefineEventPipeEvents() continue; byte[] metadata = EventPipeMetadataGenerator.Instance.GenerateEventMetadata(m_eventData[i]); + uint metadataLength = (metadata != null) ? (uint)metadata.Length : 0; string eventName = m_eventData[i].Name; Int64 keywords = m_eventData[i].Descriptor.Keywords; @@ -638,7 +639,7 @@ private unsafe void DefineEventPipeEvents() eventVersion, level, pMetadata, - (uint)metadata.Length); + metadataLength); Debug.Assert(eventHandle != IntPtr.Zero); m_eventData[i].EventHandle = eventHandle; diff --git a/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs b/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs old mode 100644 new mode 100755 index cf8379ffea37..4711ec6d35d0 --- a/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs +++ b/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs @@ -97,6 +97,7 @@ public IntPtr GetOrCreateEventHandle(EventProvider provider, EventDescriptor des (EventLevel)descriptor.Level, descriptor.Version, eventTypes); + uint metadataLength = (metadataBlob != null) ? (uint)metadataBlob.Length : 0; unsafe { @@ -110,7 +111,7 @@ public IntPtr GetOrCreateEventHandle(EventProvider provider, EventDescriptor des descriptor.Version, descriptor.Level, pMetadataBlob, - (uint)metadataBlob.Length); + metadataLength); } } } diff --git a/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs b/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs old mode 100644 new mode 100755 index a2b609569318..8aae87ea355c --- a/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs +++ b/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs @@ -65,48 +65,59 @@ private unsafe byte[] GenerateMetadata( uint version, EventParameterInfo[] parameters) { - // eventID : 4 bytes - // eventName : (eventName.Length + 1) * 2 bytes - // keywords : 8 bytes - // eventVersion : 4 bytes - // level : 4 bytes - // parameterCount : 4 bytes - uint metadataLength = 24 + ((uint)eventName.Length + 1) * 2; - - // Check for an empty payload. - // Write<T> calls with no arguments by convention have a parameter of - // type NullTypeInfo which is serialized as nothing. - if((parameters.Length == 1) && (parameters[0].ParameterType == typeof(EmptyStruct))) + byte[] metadata = null; + try { - parameters = Array.Empty<EventParameterInfo>(); - } - - // Increase the metadataLength for parameters. - foreach (var parameter in parameters) - { - metadataLength = metadataLength + parameter.GetMetadataLength(); - } - - byte[] metadata = new byte[metadataLength]; - - // Write metadata: eventID, eventName, keywords, eventVersion, level, parameterCount, param1 type, param1 name... - fixed (byte *pMetadata = metadata) - { - uint offset = 0; - WriteToBuffer(pMetadata, metadataLength, ref offset, (uint)eventId); - fixed(char *pEventName = eventName) + // eventID : 4 bytes + // eventName : (eventName.Length + 1) * 2 bytes + // keywords : 8 bytes + // eventVersion : 4 bytes + // level : 4 bytes + // parameterCount : 4 bytes + uint metadataLength = 24 + ((uint)eventName.Length + 1) * 2; + + // Check for an empty payload. + // Write<T> calls with no arguments by convention have a parameter of + // type NullTypeInfo which is serialized as nothing. + if ((parameters.Length == 1) && (parameters[0].ParameterType == typeof(EmptyStruct))) { - WriteToBuffer(pMetadata, metadataLength, ref offset, (byte *)pEventName, ((uint)eventName.Length + 1) * 2); + parameters = Array.Empty<EventParameterInfo>(); } - WriteToBuffer(pMetadata, metadataLength, ref offset, keywords); - WriteToBuffer(pMetadata, metadataLength, ref offset, version); - WriteToBuffer(pMetadata, metadataLength, ref offset, level); - WriteToBuffer(pMetadata, metadataLength, ref offset, (uint)parameters.Length); + + // Increase the metadataLength for parameters. foreach (var parameter in parameters) { - parameter.GenerateMetadata(pMetadata, ref offset, metadataLength); + metadataLength = metadataLength + parameter.GetMetadataLength(); + } + + metadata = new byte[metadataLength]; + + // Write metadata: eventID, eventName, keywords, eventVersion, level, parameterCount, param1 type, param1 name... + fixed (byte* pMetadata = metadata) + { + uint offset = 0; + WriteToBuffer(pMetadata, metadataLength, ref offset, (uint)eventId); + fixed (char* pEventName = eventName) + { + WriteToBuffer(pMetadata, metadataLength, ref offset, (byte*)pEventName, ((uint)eventName.Length + 1) * 2); + } + WriteToBuffer(pMetadata, metadataLength, ref offset, keywords); + WriteToBuffer(pMetadata, metadataLength, ref offset, version); + WriteToBuffer(pMetadata, metadataLength, ref offset, level); + WriteToBuffer(pMetadata, metadataLength, ref offset, (uint)parameters.Length); + foreach (var parameter in parameters) + { + parameter.GenerateMetadata(pMetadata, ref offset, metadataLength); + } + Debug.Assert(metadataLength == offset); } - Debug.Assert(metadataLength == offset); + } + catch + { + // If a failure occurs during metadata generation, make sure that we don't return + // malformed metadata. Instead, return a null metadata blob. + // Consumers can either build in knowledge of the event or skip it entirely. + metadata = null; } return metadata; From 8b6e0b0a1f63b12b2e328711b0e3d24eb05d12ad Mon Sep 17 00:00:00 2001 From: Brian Robbins <brianrob@microsoft.com> Date: Fri, 16 Mar 2018 13:23:25 -0700 Subject: [PATCH 2/5] Move TraceLogging eventHandle lookup to ConcurrentDictionary. --- .../Diagnostics/Tracing/TraceLogging/NameInfo.cs | 9 +++++---- .../Tracing/TraceLogging/TraceLoggingEventSource.cs | 11 ++++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) mode change 100644 => 100755 src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs diff --git a/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs b/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs index 4711ec6d35d0..b1c7327c180f 100755 --- a/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs +++ b/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using System.Collections.Concurrent; using Interlocked = System.Threading.Interlocked; #if ES_BUILD_STANDALONE @@ -42,7 +43,6 @@ internal static void ReserveEventIDsBelow(int eventId) internal readonly byte[] nameMetadata; #if FEATURE_PERFTRACING - private IntPtr eventHandle = IntPtr.Zero; private readonly object eventHandleCreationLock = new object(); #endif @@ -82,13 +82,14 @@ private int Compare(string otherName, EventTags otherTags) } #if FEATURE_PERFTRACING - public IntPtr GetOrCreateEventHandle(EventProvider provider, EventDescriptor descriptor, TraceLoggingEventTypes eventTypes) + public IntPtr GetOrCreateEventHandle(EventProvider provider, ConcurrentDictionary<int, IntPtr> eventHandleMap, EventDescriptor descriptor, TraceLoggingEventTypes eventTypes) { - if (eventHandle == IntPtr.Zero) + IntPtr eventHandle = IntPtr.Zero; + if(!eventHandleMap.TryGetValue(descriptor.EventId, out eventHandle)) { lock (eventHandleCreationLock) { - if (eventHandle == IntPtr.Zero) + if (!eventHandleMap.TryGetValue(descriptor.EventId, out eventHandle)) { byte[] metadataBlob = EventPipeMetadataGenerator.Instance.GenerateEventMetadata( descriptor.EventId, diff --git a/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs b/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs old mode 100644 new mode 100755 index 4ebe3a34c557..ccdc8bf7c4d4 --- a/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs +++ b/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs @@ -24,6 +24,7 @@ using System.Runtime.InteropServices; using System.Security; using System.Collections.ObjectModel; +using System.Collections.Concurrent; #if !ES_BUILD_AGAINST_DOTNET_V35 using Contract = System.Diagnostics.Contracts.Contract; @@ -47,6 +48,10 @@ public partial class EventSource private byte[] providerMetadata; #endif +#if FEATURE_PERFTRACING + private ConcurrentDictionary<int, IntPtr> m_eventHandleMap = new ConcurrentDictionary<int, IntPtr>(); +#endif + /// <summary> /// Construct an EventSource with a given name for non-contract based events (e.g. those using the Write() API). /// </summary> @@ -432,7 +437,7 @@ private unsafe void WriteMultiMergeInner( EventDescriptor descriptor = new EventDescriptor(identity, level, opcode, (long)keywords); #if FEATURE_PERFTRACING - IntPtr eventHandle = nameInfo.GetOrCreateEventHandle(m_provider, descriptor, eventTypes); + IntPtr eventHandle = nameInfo.GetOrCreateEventHandle(m_provider, m_eventHandleMap, descriptor, eventTypes); Debug.Assert(eventHandle != IntPtr.Zero); #else IntPtr eventHandle = IntPtr.Zero; @@ -547,7 +552,7 @@ internal unsafe void WriteMultiMerge( } #if FEATURE_PERFTRACING - IntPtr eventHandle = nameInfo.GetOrCreateEventHandle(m_provider, descriptor, eventTypes); + IntPtr eventHandle = nameInfo.GetOrCreateEventHandle(m_provider, m_eventHandleMap, descriptor, eventTypes); Debug.Assert(eventHandle != IntPtr.Zero); #else IntPtr eventHandle = IntPtr.Zero; @@ -616,7 +621,7 @@ private unsafe void WriteImpl( } #if FEATURE_PERFTRACING - IntPtr eventHandle = nameInfo.GetOrCreateEventHandle(m_provider, descriptor, eventTypes); + IntPtr eventHandle = nameInfo.GetOrCreateEventHandle(m_provider, m_eventHandleMap, descriptor, eventTypes); Debug.Assert(eventHandle != IntPtr.Zero); #else IntPtr eventHandle = IntPtr.Zero; From 8b94caf0c3530faa5ce127d8dc6dfb49bb31de63 Mon Sep 17 00:00:00 2001 From: Brian Robbins <brianrob@microsoft.com> Date: Fri, 16 Mar 2018 18:24:25 -0700 Subject: [PATCH 3/5] Remove obsolete assert. --- src/vm/eventpipe.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vm/eventpipe.cpp b/src/vm/eventpipe.cpp index ab137388dd2d..910dee5b92c8 100644 --- a/src/vm/eventpipe.cpp +++ b/src/vm/eventpipe.cpp @@ -958,7 +958,6 @@ INT_PTR QCALLTYPE EventPipeInternal::DefineEvent( BEGIN_QCALL; _ASSERTE(provHandle != NULL); - _ASSERTE(pMetadata != NULL); EventPipeProvider *pProvider = reinterpret_cast<EventPipeProvider *>(provHandle); pEvent = pProvider->AddEvent(eventID, keywords, eventVersion, (EventPipeEventLevel)level, (BYTE *)pMetadata, metadataLength); _ASSERTE(pEvent != NULL); From 7d2751ff44277a7474f5d80f0c03779772795e61 Mon Sep 17 00:00:00 2001 From: Brian Robbins <brianrob@microsoft.com> Date: Fri, 16 Mar 2018 18:25:38 -0700 Subject: [PATCH 4/5] Remove executable bit from VS. --- src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs | 0 .../shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs | 0 .../Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs | 0 .../src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs | 0 4 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs mode change 100755 => 100644 src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs mode change 100755 => 100644 src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs mode change 100755 => 100644 src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs diff --git a/src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs b/src/mscorlib/shared/System/Diagnostics/Tracing/EventSource.cs old mode 100755 new mode 100644 diff --git a/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs b/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/NameInfo.cs old mode 100755 new mode 100644 diff --git a/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs b/src/mscorlib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs old mode 100755 new mode 100644 diff --git a/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs b/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs old mode 100755 new mode 100644 From c68ce15b5fc66ef1cdca8af924142700c591d3aa Mon Sep 17 00:00:00 2001 From: Brian Robbins <brianrob@microsoft.com> Date: Fri, 16 Mar 2018 23:47:33 -0700 Subject: [PATCH 5/5] Remove improper assert. --- .../Diagnostics/Eventing/EventPipeMetadataGenerator.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs b/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs index 8aae87ea355c..d18610d922ca 100644 --- a/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs +++ b/src/mscorlib/src/System/Diagnostics/Eventing/EventPipeMetadataGenerator.cs @@ -273,7 +273,12 @@ private static unsafe void GenerateMetadataForProperty(PropertyAnalysis property // TypeCode : 4 bytes // PropertyName : NULL-terminated string TypeCode typeCode = GetTypeCodeExtended(property.typeInfo.DataType); - Debug.Assert(typeCode != TypeCode.Object); + + // EventPipe does not support this type. Throw, which will cause no metadata to be registered for this event. + if(typeCode == TypeCode.Object) + { + throw new NotSupportedException(); + } // Write the type code. EventPipeMetadataGenerator.WriteToBuffer(pMetadataBlob, blobSize, ref offset, (uint)typeCode);