Skip to content

Fix TypeLoadException in GetMarshalAs when SafeArray has zero-length user-defined type name #124408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 2 commits into
base: main
Choose a base branch
from
+107 −7
Open

Fix TypeLoadException in GetMarshalAs when SafeArray has zero-length user-defined type name #124408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cac0fc0
Initial plan
Copilot Feb 13, 2026
dc39565
Fix TypeLoadException in GetMarshalAs when SafeArray has zero-length …
Copilot Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/MdImport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +281 to +282
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This catch-block comment says "we return the bad type name", but MarshalAsAttribute doesn’t expose the SafeArray user-defined subtype name string anywhere (only the resolved Type via SafeArrayUserDefinedSubType). Please adjust the comment to reflect the actual behavior (e.g., that we swallow TypeLoadException and leave SafeArrayUserDefinedSubType unset) to avoid misleading future maintainers.

Suggested change
// 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.

Copilot uses AI. Check for mistakes.
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)
{
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/managedmdimport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
Expand Down
90 changes: 89 additions & 1 deletion ...ntime.InteropServices.UnitTests/System/Runtime/InteropServices/MarshalAsAttributeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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);
Expand All @@ -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()
{
Comment on lines +37 to +39
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test relies on COM interop behavior (FEATURE_COMINTEROP) and Reflection.Emit (PersistedAssemblyBuilder). As a plain [Fact], it may run on non-Windows / browser / NativeAOT configurations where it either can’t exercise the regression or may throw PlatformNotSupportedException. Consider switching this to a ConditionalFact guarded by PlatformDetection.IsBuiltInComEnabled and PlatformDetection.IsReflectionEmitSupported (and any other relevant guards used in this test project).

Copilot uses AI. Check for mistakes.
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));
Comment on lines +46 to +53
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These reflection calls return nullable types (e.g., Assembly.GetType / Type.GetMethod / Attribute.GetCustomAttribute). As written, this likely introduces nullable warnings (and potential NullReferenceExceptions if metadata changes). Please add explicit assertions / null-forgiveness where appropriate (or use the overloads that throw on failure) so the test compiles cleanly under nullable and fails with a clear message if the expected members aren’t found.

Suggested change
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));
Type iface = asm.GetType("TestInterface") ?? throw new InvalidOperationException("Test interface 'TestInterface' not found in generated assembly.");
MethodInfo method = iface.GetMethod("TestMethod") ?? throw new InvalidOperationException("Test method 'TestMethod' not found on 'TestInterface'.");
ParameterInfo param = method.GetParameters()[0];
// Must not throw TypeLoadException.
_ = param.GetCustomAttributes(false);
MarshalAsAttribute attr = (MarshalAsAttribute?)Attribute.GetCustomAttribute(param, typeof(MarshalAsAttribute))
?? throw new InvalidOperationException("Expected MarshalAsAttribute not found on parameter.");

Copilot uses AI. Check for mistakes.
Assert.NotNull(attr);
Assert.Equal(UnmanagedType.SafeArray, attr.Value);
Assert.Null(attr.SafeArrayUserDefinedSubType);
Comment on lines +45 to +56
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MemoryStream passed to AssemblyLoadContext.LoadFromStream isn’t disposed. Please wrap it in a using/try-finally so the stream is closed promptly (this can also help collectible ALC unloading behavior).

Suggested change
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);
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));
Assert.NotNull(attr);
Assert.Equal(UnmanagedType.SafeArray, attr.Value);
Assert.Null(attr.SafeArrayUserDefinedSubType);
}

Copilot uses AI. Check for mistakes.
}
finally
{
alc.Unload();
}
}

/// <summary>
/// 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.
/// </summary>
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));
Comment on lines +86 to +105
Copy link

Copilot AI Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid hardcoding ParameterHandle(1) here; the row id depends on how the Params table is emitted and could change (e.g., if a return parameter row gets introduced). Capture the ParameterBuilder returned from DefineParameter and derive the correct ParameterHandle from its metadata token when calling AddMarshallingDescriptor, so the test is resilient to metadata layout changes.

Copilot uses AI. Check for mistakes.

ManagedPEBuilder pe = new(
PEHeaderBuilder.CreateLibraryHeader(),
new MetadataRootBuilder(mdb),
ilBlob,
flags: CorFlags.ILOnly);

BlobBuilder output = new();
pe.Serialize(output);
return output.ToArray();
}
}
}
Loading