Update TraceLogging to use NULL-terminated strings instead of counted strings (#16672)

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

7 files changed, 84 insertions, 31 deletions

diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventSource.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventSource.cs
index 40d39ced7..40d39ced7 100644..100755
--- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventSource.cs
+++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventSource.cs
diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/DataCollector.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/DataCollector.cs
index 1444c267c..11c18a260 100644..100755
--- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/DataCollector.cs
+++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/DataCollector.cs
@@ -143,6 +143,45 @@ namespace System.Diagnostics.Tracing
             }
         }
 
+        internal unsafe void AddNullTerminatedString(string value)
+        {
+            // Treat null strings as empty strings.
+            if (value == null)
+            {
+                value = string.Empty;
+            }
+
+            // Calculate the size of the string including the trailing NULL char.
+            // Don't use value.Length here because string allows for embedded NULL characters.
+            int nullCharIndex = value.IndexOf((char)0);
+            if (nullCharIndex < 0)
+            {
+                nullCharIndex = value.Length;
+            }
+            int size = (nullCharIndex + 1) * 2;
+
+            if (this.bufferNesting != 0)
+            {
+                this.EnsureBuffer(size);
+            }
+
+            if (this.bufferNesting == 0)
+            {
+                this.ScalarsEnd();
+                this.PinArray(value, size);
+            }
+            else
+            {
+                var oldPos = this.bufferPos;
+                this.bufferPos = checked(this.bufferPos + size);
+                this.EnsureBuffer();
+                fixed (void* p = value)
+                {
+                    Marshal.Copy((IntPtr)p, buffer, oldPos, size);
+                }
+            }
+        }
+
         internal void AddBinary(Array value, int size)
         {
             this.AddArray(value, size, 1);
diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/FieldMetadata.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/FieldMetadata.cs
index 9c7c6369e..f15373475 100644
--- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/FieldMetadata.cs
+++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/FieldMetadata.cs
@@ -135,13 +135,11 @@ namespace System.Diagnostics.Tracing
                 {
                     throw new NotSupportedException(SR.EventSource_NotSupportedArrayOfBinary);
                 }
-#if !BROKEN_UNTIL_M3
                 if (coreType == (int)TraceLoggingDataType.Utf16String ||
                     coreType == (int)TraceLoggingDataType.MbcsString)
                 {
                     throw new NotSupportedException(SR.EventSource_NotSupportedArrayOfNullTerminatedString);
                 }
-#endif
             }
 
             if (((int)this.tags & 0xfffffff) != 0)
diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/SimpleTypeInfos.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/SimpleTypeInfos.cs
index 9b58d8251..2f460d24c 100644..100755
--- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/SimpleTypeInfos.cs
+++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/SimpleTypeInfos.cs
@@ -151,12 +151,12 @@ namespace System.Diagnostics.Tracing
             string name,
             EventFieldFormat format)
         {
-            collector.AddBinary(name, Statics.MakeDataType(TraceLoggingDataType.CountedUtf16String, format));
+            collector.AddNullTerminatedString(name, Statics.MakeDataType(TraceLoggingDataType.Utf16String, format));
         }
 
         public override void WriteData(TraceLoggingDataCollector collector, PropertyValue value)
         {
-            collector.AddBinary((string)value.ReferenceValue);
+            collector.AddNullTerminatedString((string)value.ReferenceValue);
         }
         
         public override object GetData(object value)
diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingDataCollector.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingDataCollector.cs
index 04a047fb3..f6d0a59aa 100644..100755
--- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingDataCollector.cs
+++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingDataCollector.cs
@@ -85,6 +85,17 @@ namespace System.Diagnostics.Tracing
         }
 
         /// <summary>
+        /// Adds a null-terminated String value to the event payload.
+        /// </summary>
+        /// <param name="value">
+        /// Value to be added. A null value is treated as a zero-length string.
+        /// </param>
+        public void AddNullTerminatedString(string value)
+        {
+            DataCollector.ThreadInstance.AddNullTerminatedString(value);
+        }
+
+        /// <summary>
         /// Adds a counted String value to the event payload.
         /// </summary>
         /// <param name="value">
diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs
index 4348df7d6..10b378c2b 100644
--- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs
+++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingEventSource.cs
@@ -557,30 +557,14 @@ namespace System.Diagnostics.Tracing
 
                     for (int i = 0; i < eventTypes.typeInfos.Length; i++)
                     {
-                        // Until M3, we need to morph strings to a counted representation
-                        // When TDH supports null terminated strings, we can remove this.  
-                        if (eventTypes.typeInfos[i].DataType == typeof(string))
-                        {
-                            // Write out the size of the string 
-                            descriptors[numDescrs].DataPointer = (IntPtr) (&descriptors[numDescrs + 1].m_Size);
-                            descriptors[numDescrs].m_Size = 2;
-                            numDescrs++;
-
-                            descriptors[numDescrs].m_Ptr = data[i].m_Ptr;
-                            descriptors[numDescrs].m_Size = data[i].m_Size - 2;   // Remove the null terminator
-                            numDescrs++;
-                        }
-                        else
-                        {
-                            descriptors[numDescrs].m_Ptr = data[i].m_Ptr;
-                            descriptors[numDescrs].m_Size = data[i].m_Size;
+                        descriptors[numDescrs].m_Ptr = data[i].m_Ptr;
+                        descriptors[numDescrs].m_Size = data[i].m_Size;
 
-                            // old conventions for bool is 4 bytes, but meta-data assumes 1.  
-                            if (data[i].m_Size == 4 && eventTypes.typeInfos[i].DataType == typeof(bool))
-                                descriptors[numDescrs].m_Size = 1;
+                        // old conventions for bool is 4 bytes, but meta-data assumes 1.
+                        if (data[i].m_Size == 4 && eventTypes.typeInfos[i].DataType == typeof(bool))
+                            descriptors[numDescrs].m_Size = 1;
 
-                            numDescrs++;
-                        }
+                        numDescrs++;
                     }
 
                     this.WriteEventRaw(
diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingMetadataCollector.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingMetadataCollector.cs
index 1db1a28c9..b5b199dbc 100644..100755
--- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingMetadataCollector.cs
+++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/TraceLogging/TraceLoggingMetadataCollector.cs
@@ -192,22 +192,43 @@ namespace System.Diagnostics.Tracing
         }
 
         /// <summary>
+        /// Adds a null-terminated string field to an event.
+        /// Compatible with core types: Utf16String, MbcsString.
+        /// Compatible with dataCollector method: AddNullTerminatedString(string).
+        /// </summary>
+        /// <param name="name">
+        /// The name to use for the added field. This value must not be null.
+        /// </param>
+        /// <param name="type">
+        /// The type code for the added field. This must be a null-terminated string type.
+        /// </param>
+        public void AddNullTerminatedString(string name, TraceLoggingDataType type)
+        {
+            switch ((TraceLoggingDataType)((int)type & Statics.InTypeMask))
+            {
+                case TraceLoggingDataType.Utf16String:
+                    break;
+                default:
+                    throw new ArgumentOutOfRangeException(nameof(type));
+            }
+
+            this.impl.AddNonscalar();
+            this.AddField(new FieldMetadata(name, type, this.Tags, this.BeginningBufferedArray));
+        }
+
+        /// <summary>
         /// Adds an array field to an event.
         /// </summary>
         /// <param name="name">
         /// The name to use for the added field. This value must not be null.
         /// </param>
         /// <param name="type">
-        /// The type code for the added field. This must be a fixed-size type
-        /// or a string type. In the case of a string type, this adds an array
-        /// of characters, not an array of strings.
+        /// The type code for the added field. This must be a fixed-size type.
         /// </param>
         public void AddArray(string name, TraceLoggingDataType type)
         {
             switch ((TraceLoggingDataType)((int)type & Statics.InTypeMask))
             {
-                case TraceLoggingDataType.Utf16String:
-                case TraceLoggingDataType.MbcsString:
                 case TraceLoggingDataType.Int8:
                 case TraceLoggingDataType.UInt8:
                 case TraceLoggingDataType.Int16: