From cac0fc07a56bb151991de7361a010643e10de51c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 23:22:10 +0000 Subject: [PATCH 1/4] Initial plan From dc39565fff547533be3d9cb799b04b3ddec6eb1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 23:48:20 +0000 Subject: [PATCH 2/4] Fix TypeLoadException in GetMarshalAs when SafeArray has zero-length user-defined type name Return NULL in native code when byte count is 0 for safeArrayUserDefinedSubType, marshalType, and marshalCookie. Add try/catch for safeArrayUserDefinedType resolution in managed code. Use IsNullOrEmpty consistently for marshalTypeName. Add regression test with poison bytes to reliably reproduce the bug. Fixes #124346 Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../src/System/Reflection/MdImport.cs | 18 +++- src/coreclr/vm/managedmdimport.cpp | 6 +- .../MarshalAsAttributeTests.cs | 90 ++++++++++++++++++- 3 files changed, 107 insertions(+), 7 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs index 7d53f6b24f6e77..311eba8d9e6c41 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs @@ -269,13 +269,25 @@ internal static unsafe MarshalAsAttribute GetMarshalAs(ConstArray nativeType, Ru ? null : Text.Encoding.UTF8.GetString(MemoryMarshal.CreateReadOnlySpanFromNullTerminated(marshalCookieRaw)); - RuntimeType? safeArrayUserDefinedType = string.IsNullOrEmpty(safeArrayUserDefinedTypeName) ? null : - TypeNameResolver.GetTypeReferencedByCustomAttribute(safeArrayUserDefinedTypeName, scope); + RuntimeType? safeArrayUserDefinedType = null; + + try + { + safeArrayUserDefinedType = string.IsNullOrEmpty(safeArrayUserDefinedTypeName) ? null : + TypeNameResolver.GetTypeReferencedByCustomAttribute(safeArrayUserDefinedTypeName, scope); + } + catch (TypeLoadException) + { + // The user may have supplied a bad type name string causing this TypeLoadException + // Regardless, we return the bad type name + Debug.Assert(safeArrayUserDefinedTypeName is not null); + } + RuntimeType? marshalTypeRef = null; try { - marshalTypeRef = marshalTypeName is null ? null : TypeNameResolver.GetTypeReferencedByCustomAttribute(marshalTypeName, scope); + marshalTypeRef = string.IsNullOrEmpty(marshalTypeName) ? null : TypeNameResolver.GetTypeReferencedByCustomAttribute(marshalTypeName, scope); } catch (TypeLoadException) { diff --git a/src/coreclr/vm/managedmdimport.cpp b/src/coreclr/vm/managedmdimport.cpp index 0bada0526d991b..4739e8e27d15e6 100644 --- a/src/coreclr/vm/managedmdimport.cpp +++ b/src/coreclr/vm/managedmdimport.cpp @@ -50,7 +50,7 @@ FCIMPL11(FC_BOOL_RET, MetaDataImport::GetMarshalAs, *safeArraySubType = info.m_SafeArrayElementVT; - *safeArrayUserDefinedSubType = info.m_strSafeArrayUserDefTypeName; + *safeArrayUserDefinedSubType = info.m_cSafeArrayUserDefTypeNameBytes > 0 ? info.m_strSafeArrayUserDefTypeName : NULL; #else *iidParamIndex = 0; @@ -59,9 +59,9 @@ FCIMPL11(FC_BOOL_RET, MetaDataImport::GetMarshalAs, *safeArrayUserDefinedSubType = NULL; #endif - *marshalType = info.m_strCMMarshalerTypeName; + *marshalType = info.m_cCMMarshalerTypeNameBytes > 0 ? info.m_strCMMarshalerTypeName : NULL; - *marshalCookie = info.m_strCMCookie; + *marshalCookie = info.m_cCMCookieStrBytes > 0 ? info.m_strCMCookie : NULL; FC_RETURN_BOOL(TRUE); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs index 25df6e09a591ea..966cbef79bb57f 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs @@ -1,6 +1,13 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO; +using System.Reflection; +using System.Reflection.Emit; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; +using System.Reflection.PortableExecutable; +using System.Runtime.Loader; using Xunit; namespace System.Runtime.InteropServices.Tests @@ -11,7 +18,7 @@ public class MarshalAsAttributeTests [InlineData((UnmanagedType)(-1))] [InlineData(UnmanagedType.HString)] [InlineData((UnmanagedType)int.MaxValue)] - public void Ctor_UmanagedTye(UnmanagedType unmanagedType) + public void Ctor_UnmanagedType(UnmanagedType unmanagedType) { var attribute = new MarshalAsAttribute(unmanagedType); Assert.Equal(unmanagedType, attribute.Value); @@ -26,5 +33,86 @@ public void Ctor_ShortUnmanagedType(short umanagedType) var attribute = new MarshalAsAttribute(umanagedType); Assert.Equal((UnmanagedType)umanagedType, attribute.Value); } + + [Fact] + public void SafeArrayParameter_ZeroLengthUserDefinedSubType_DoesNotThrow() + { + byte[] peBytes = BuildPEWithSafeArrayMarshalBlob(); + + AssemblyLoadContext alc = new(nameof(SafeArrayParameter_ZeroLengthUserDefinedSubType_DoesNotThrow), isCollectible: true); + try + { + Assembly asm = alc.LoadFromStream(new MemoryStream(peBytes)); + Type iface = asm.GetType("TestInterface"); + MethodInfo method = iface.GetMethod("TestMethod"); + ParameterInfo param = method.GetParameters()[0]; + + // Must not throw TypeLoadException. + _ = param.GetCustomAttributes(false); + + MarshalAsAttribute attr = (MarshalAsAttribute)Attribute.GetCustomAttribute(param, typeof(MarshalAsAttribute)); + Assert.NotNull(attr); + Assert.Equal(UnmanagedType.SafeArray, attr.Value); + Assert.Null(attr.SafeArrayUserDefinedSubType); + } + finally + { + alc.Unload(); + } + } + + /// + /// Creates a PE whose parameter has a FieldMarshal blob that + /// reproduces the native bug: NATIVE_TYPE_SAFEARRAY (0x1D), + /// VT_DISPATCH (0x09), zero-length string (0x00), followed by + /// poison byte 'X' (0x58) and null terminator (0x00). + /// Without the native fix the FCALL returns a dangling pointer + /// into the poison byte region causing TypeLoadException. + /// + private static byte[] BuildPEWithSafeArrayMarshalBlob() + { + PersistedAssemblyBuilder ab = new( + new AssemblyName("SafeArrayTestAsm"), typeof(object).Assembly); + ModuleBuilder mod = ab.DefineDynamicModule("SafeArrayTestAsm.dll"); + TypeBuilder td = mod.DefineType("TestInterface", + TypeAttributes.Interface | TypeAttributes.Abstract | TypeAttributes.Public); + + MethodBuilder md = td.DefineMethod("TestMethod", + MethodAttributes.Public | MethodAttributes.Abstract | + MethodAttributes.Virtual | MethodAttributes.NewSlot | + MethodAttributes.HideBySig, + typeof(void), new[] { typeof(object[]) }); + + md.DefineParameter(1, ParameterAttributes.HasFieldMarshal, "args"); + td.CreateType(); + + MetadataBuilder mdb = ab.GenerateMetadata(out BlobBuilder ilBlob, out _); + + // Blob bytes: + // 0x1D NATIVE_TYPE_SAFEARRAY + // 0x09 VT_DISPATCH + // 0x00 compressed string length 0 + // 0x58 'X' poison (not consumed by parser) + // 0x00 null terminator + BlobBuilder marshalBlob = new(); + marshalBlob.WriteByte(0x1D); + marshalBlob.WriteByte(0x09); + marshalBlob.WriteByte(0x00); + marshalBlob.WriteByte(0x58); + marshalBlob.WriteByte(0x00); + mdb.AddMarshallingDescriptor( + MetadataTokens.ParameterHandle(1), + mdb.GetOrAddBlob(marshalBlob)); + + ManagedPEBuilder pe = new( + PEHeaderBuilder.CreateLibraryHeader(), + new MetadataRootBuilder(mdb), + ilBlob, + flags: CorFlags.ILOnly); + + BlobBuilder output = new(); + pe.Serialize(output); + return output.ToArray(); + } } } From 81224a0eb22784413942574be9b7c8dd953f0ef7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 00:12:03 +0000 Subject: [PATCH 3/4] Address PR review feedback: improve test guards, nullable handling, dispose stream, and comment accuracy Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../src/System/Reflection/MdImport.cs | 5 +++-- .../InteropServices/MarshalAsAttributeTests.cs | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs index 311eba8d9e6c41..39ea24b454104c 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs @@ -278,8 +278,9 @@ internal static unsafe MarshalAsAttribute GetMarshalAs(ConstArray nativeType, Ru } catch (TypeLoadException) { - // The user may have supplied a bad type name string causing this TypeLoadException - // Regardless, we return the bad type name + // The user may have supplied a bad type name string causing this TypeLoadException. + // Swallow the exception and leave SafeArrayUserDefinedSubType unset; the original + // type name remains in safeArrayUserDefinedTypeName for potential diagnostics. Debug.Assert(safeArrayUserDefinedTypeName is not null); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs index 966cbef79bb57f..6f7126df290cc2 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs @@ -34,7 +34,7 @@ public void Ctor_ShortUnmanagedType(short umanagedType) Assert.Equal((UnmanagedType)umanagedType, attribute.Value); } - [Fact] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled), nameof(PlatformDetection.IsReflectionEmitSupported))] public void SafeArrayParameter_ZeroLengthUserDefinedSubType_DoesNotThrow() { byte[] peBytes = BuildPEWithSafeArrayMarshalBlob(); @@ -42,15 +42,16 @@ public void SafeArrayParameter_ZeroLengthUserDefinedSubType_DoesNotThrow() AssemblyLoadContext alc = new(nameof(SafeArrayParameter_ZeroLengthUserDefinedSubType_DoesNotThrow), isCollectible: true); try { - Assembly asm = alc.LoadFromStream(new MemoryStream(peBytes)); - Type iface = asm.GetType("TestInterface"); - MethodInfo method = iface.GetMethod("TestMethod"); + using MemoryStream peStream = new(peBytes); + Assembly asm = alc.LoadFromStream(peStream); + Type iface = asm.GetType("TestInterface")!; + MethodInfo method = iface.GetMethod("TestMethod")!; ParameterInfo param = method.GetParameters()[0]; // Must not throw TypeLoadException. _ = param.GetCustomAttributes(false); - MarshalAsAttribute attr = (MarshalAsAttribute)Attribute.GetCustomAttribute(param, typeof(MarshalAsAttribute)); + MarshalAsAttribute? attr = (MarshalAsAttribute?)Attribute.GetCustomAttribute(param, typeof(MarshalAsAttribute)); Assert.NotNull(attr); Assert.Equal(UnmanagedType.SafeArray, attr.Value); Assert.Null(attr.SafeArrayUserDefinedSubType); @@ -88,6 +89,11 @@ private static byte[] BuildPEWithSafeArrayMarshalBlob() MetadataBuilder mdb = ab.GenerateMetadata(out BlobBuilder ilBlob, out _); + // This is the only parameter defined on the only method, so it + // occupies row 1 in the Param table. PersistedAssemblyBuilder + // emits parameters in definition order deterministically. + const int paramRowNumber = 1; + // Blob bytes: // 0x1D NATIVE_TYPE_SAFEARRAY // 0x09 VT_DISPATCH @@ -101,7 +107,7 @@ private static byte[] BuildPEWithSafeArrayMarshalBlob() marshalBlob.WriteByte(0x58); marshalBlob.WriteByte(0x00); mdb.AddMarshallingDescriptor( - MetadataTokens.ParameterHandle(1), + MetadataTokens.ParameterHandle(paramRowNumber), mdb.GetOrAddBlob(marshalBlob)); ManagedPEBuilder pe = new( From 3c8d57022eab910b17dc9c4d5021fc1e95809419 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 16 Feb 2026 01:24:06 +0000 Subject: [PATCH 4/4] Simplify safeArrayUserDefinedType catch block comment per jkotas feedback Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com> --- .../System.Private.CoreLib/src/System/Reflection/MdImport.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs index 39ea24b454104c..a4fff3f762cf33 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs @@ -278,9 +278,7 @@ internal static unsafe MarshalAsAttribute GetMarshalAs(ConstArray nativeType, Ru } catch (TypeLoadException) { - // The user may have supplied a bad type name string causing this TypeLoadException. - // Swallow the exception and leave SafeArrayUserDefinedSubType unset; the original - // type name remains in safeArrayUserDefinedTypeName for potential diagnostics. + // The user may have supplied a bad type name string causing this TypeLoadException Debug.Assert(safeArrayUserDefinedTypeName is not null); }