Fix UTF8 string marshalling regression (#69360)
authorJan Kotas <jkotas@microsoft.com>
Sun, 15 May 2022 18:57:22 +0000 (11:57 -0700)
committerGitHub <noreply@github.com>
Sun, 15 May 2022 18:57:22 +0000 (11:57 -0700)
* Fix UTF8 string marshalling regression

We need to compensate for the differences in lifetime management patterns used by built-in marshalling system vs. the publicly explosed marshallers.

Fixes #69349

* Call Marshal.FreeCoTaskMem in the AOT compiler built-in marshalling

* Use Marshal.StringToCoTaskMemUni in Utf16StringMarshaller implementation

src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs
src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.Aot.cs
src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs
src/coreclr/vm/ilmarshalers.cpp
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf16StringMarshaller.cs

index 2216dda..0acf162 100644 (file)
@@ -410,11 +410,6 @@ namespace Internal.Runtime.CompilerHelpers
             return ptr;
         }
 
-        internal static unsafe void CoTaskMemFree(void* p)
-        {
-            Marshal.FreeCoTaskMem((IntPtr)p);
-        }
-
         /// <summary>
         /// Retrieves the function pointer for the current open static delegate that is being called
         /// </summary>
index cd1395d..0322db3 100644 (file)
@@ -182,7 +182,7 @@ namespace Internal.TypeSystem.Interop
         internal override void EmitElementCleanup(ILCodeStream codeStream, ILEmitter emitter)
         {
             codeStream.Emit(ILOpcode.call, emitter.NewToken(
-                                Context.GetHelperEntryPoint("InteropHelpers", "CoTaskMemFree")));
+                                InteropTypes.GetMarshal(Context).GetKnownMethod("FreeCoTaskMem", null)));
         }
 
         protected override void AllocNativeToManaged(ILCodeStream codeStream)
@@ -240,9 +240,10 @@ namespace Internal.TypeSystem.Interop
 
         protected override void EmitCleanupManaged(ILCodeStream codeStream)
         {
+            ILEmitter emitter = _ilCodeStreams.Emitter;
             LoadNativeValue(codeStream);
-            codeStream.Emit(ILOpcode.call, _ilCodeStreams.Emitter.NewToken(
-                                Context.GetHelperEntryPoint("InteropHelpers", "CoTaskMemFree")));
+            codeStream.Emit(ILOpcode.call, emitter.NewToken(
+                                InteropTypes.GetMarshal(Context).GetKnownMethod("FreeCoTaskMem", null)));
         }
     }
 
index a59f595..4cd56ae 100644 (file)
@@ -1491,12 +1491,8 @@ namespace Internal.TypeSystem.Interop
 
         internal override void EmitElementCleanup(ILCodeStream codeStream, ILEmitter emitter)
         {
-#if READYTORUN
-            throw new NotSupportedException();
-#else
             codeStream.Emit(ILOpcode.call, emitter.NewToken(
-                                Context.GetHelperEntryPoint("InteropHelpers", "CoTaskMemFree")));
-#endif
+                                InteropTypes.GetMarshal(Context).GetKnownMethod("FreeCoTaskMem", null)));
         }
 
         protected override void TransformManagedToNative(ILCodeStream codeStream)
@@ -1613,12 +1609,8 @@ namespace Internal.TypeSystem.Interop
 
         internal override void EmitElementCleanup(ILCodeStream codeStream, ILEmitter emitter)
         {
-#if READYTORUN
-            throw new NotSupportedException();
-#else
             codeStream.Emit(ILOpcode.call, emitter.NewToken(
-                                Context.GetHelperEntryPoint("InteropHelpers", "CoTaskMemFree")));
-#endif
+                                InteropTypes.GetMarshal(Context).GetKnownMethod("FreeCoTaskMem", null)));
         }
 
         protected override void TransformManagedToNative(ILCodeStream codeStream)
@@ -1765,7 +1757,7 @@ namespace Internal.TypeSystem.Interop
 
             LoadNativeValue(codeStream);
             codeStream.Emit(ILOpcode.call, emitter.NewToken(
-                                Context.GetHelperEntryPoint("InteropHelpers", "CoTaskMemFree")));
+                                InteropTypes.GetMarshal(Context).GetKnownMethod("FreeCoTaskMem", null)));
 
             codeStream.EmitLabel(lNullCheck);
 #endif
@@ -1787,7 +1779,7 @@ namespace Internal.TypeSystem.Interop
             Debug.Assert(_marshallerInstance is null);
 
             codeStream.Emit(ILOpcode.call, emitter.NewToken(
-                                InteropTypes.GetMarshal(Context).GetKnownMethod("CoTaskMemFree", null)));
+                                InteropTypes.GetMarshal(Context).GetKnownMethod("FreeCoTaskMem", null)));
         }
 
         protected override void TransformManagedToNative(ILCodeStream codeStream)
@@ -1858,11 +1850,22 @@ namespace Internal.TypeSystem.Interop
         {
             ILEmitter emitter = _ilCodeStreams.Emitter;
 
-            Debug.Assert(_marshallerInstance != null);
+            if (In && !Out && !IsManagedByRef)
+            {
+                Debug.Assert(_marshallerInstance != null);
 
-            codeStream.EmitLdLoca(_marshallerInstance.Value);
-            codeStream.Emit(ILOpcode.call, emitter.NewToken(
-                                Marshaller.GetKnownMethod("FreeNative", null)));
+                codeStream.EmitLdLoca(_marshallerInstance.Value);
+                codeStream.Emit(ILOpcode.call, emitter.NewToken(
+                                    Marshaller.GetKnownMethod("FreeNative", null)));
+            }
+            else
+            {
+                // The marshaller instance is not guaranteed to be initialized with the latest native value.
+                // Free  the native value directly.
+                LoadNativeValue(codeStream);
+                codeStream.Emit(ILOpcode.call, emitter.NewToken(
+                                    InteropTypes.GetMarshal(Context).GetKnownMethod("FreeCoTaskMem", null)));
+            }
         }
     }
 
index 3a37699..1a4f265 100644 (file)
@@ -2016,10 +2016,21 @@ void ILCUTF8Marshaler::EmitClearNative(ILCodeStream* pslILEmit)
 {
     STANDARD_VM_CONTRACT;
 
-    _ASSERTE(m_dwInstance != LOCAL_NUM_UNUSED);
+    bool bPassByValueInOnly = IsIn(m_dwMarshalFlags) && !IsOut(m_dwMarshalFlags) && !IsByref(m_dwMarshalFlags);
+    if (bPassByValueInOnly)
+    {
+        _ASSERTE(m_dwInstance != LOCAL_NUM_UNUSED);
 
-    pslILEmit->EmitLDLOCA(m_dwInstance);
-    pslILEmit->EmitCALL(METHOD__UTF8STRINGMARSHALLER__FREE_NATIVE, 1, 0);
+        pslILEmit->EmitLDLOCA(m_dwInstance);
+        pslILEmit->EmitCALL(METHOD__UTF8STRINGMARSHALLER__FREE_NATIVE, 1, 0);
+    }
+    else
+    {
+        // The marshaller instance is not guaranteed to be initialized with the latest native value.
+        // Free the native value directly.
+        EmitLoadNativeValue(pslILEmit);
+        pslILEmit->EmitCALL(METHOD__MARSHAL__FREE_CO_TASK_MEM, 1, 0);
+    }
 }
 
 LocalDesc ILCSTRMarshaler::GetManagedType()
index 7d77bed..e207bec 100644 (file)
@@ -25,17 +25,7 @@ namespace System.Runtime.InteropServices.Marshalling
         /// <param name="str">The string to marshal.</param>
         public Utf16StringMarshaller(string? str)
         {
-            if (str is null)
-            {
-                _nativeValue = null;
-                return;
-            }
-
-            // + 1 for null terminator
-            _nativeValue =  (ushort*)Marshal.AllocCoTaskMem((str.Length + 1) * sizeof(ushort));
-
-            str.CopyTo(new Span<char>(_nativeValue, str.Length));
-            _nativeValue[str.Length] = '\0'; // null-terminate
+            _nativeValue = (ushort*)Marshal.StringToCoTaskMemUni(str);
         }
 
         /// <summary>