Skip to content

Commit

Permalink
Fix TypeBuilder.DefineMethodOverride validation error (#101246)
Browse files Browse the repository at this point in the history 
* Fix DefineMethodOverride validation error

* Add more checks and tests

* Remove DefineMethodOverride validations that not exist in RuntimeTypeBuilder
  • Loading branch information
@buyaa-n
buyaa-n authored May 16, 2024
1 parent ab3ac4b commit 9c710c4
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 127 deletions.
61 changes: 0 additions & 61 deletions src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,6 @@ protected override void DefineMethodOverrideCore(MethodInfo methodInfoBody, Meth
throw new ArgumentException(SR.ArgumentException_BadMethodImplBody);
}
Type baseType = methodInfoDeclaration.DeclaringType!;
ValidateBaseType(baseType, methodInfoDeclaration.Name);
ValidateImplementedMethod(methodInfoBody, methodInfoDeclaration);
_methodOverrides ??= new();

if (_methodOverrides.TryGetValue(baseType, out List<(MethodInfo ifaceMethod, MethodInfo targetMethod)>? im))
Expand All @@ -342,24 +340,6 @@ protected override void DefineMethodOverrideCore(MethodInfo methodInfoBody, Meth
}
}

private void ValidateBaseType(Type baseType, string methodName)
{
if (baseType.IsInterface)
{
if (!IsInterfaceImplemented(baseType))
{
throw ArgumentExceptionInvalidMethodOverride(methodName);
}

return;
}

if (!baseType.IsAssignableFrom(_typeParent))
{
throw ArgumentExceptionInvalidMethodOverride(methodName);
}
}

private bool IsInterfaceImplemented(Type interfaceType)
{
if (_interfaces != null)
Expand Down Expand Up @@ -390,47 +370,6 @@ private bool IsInterfaceImplemented(Type interfaceType)
private ArgumentException ArgumentExceptionInvalidMethodOverride(string methodName) =>
new ArgumentException(SR.Format(SR.Argument_InvalidMethodOverride, FullName, methodName), "methodInfoBody");

private void ValidateImplementedMethod(MethodInfo methodInfoBody, MethodInfo methodInfoDeclaration)
{
if ((methodInfoBody.IsVirtual || methodInfoBody.IsStatic) &&
(methodInfoDeclaration.IsAbstract || methodInfoDeclaration.IsVirtual) &&
methodInfoDeclaration.ReturnType.IsAssignableFrom(methodInfoBody.ReturnType))
{
ParameterInfo[] bodyParameters = methodInfoBody.GetParameters();
ParameterInfo[] declarationParameters = methodInfoDeclaration.GetParameters();
if (bodyParameters.Length == declarationParameters.Length)
{
for (int i = 0; i < bodyParameters.Length; i++)
{
Type? bodyType = bodyParameters[i].ParameterType;
Type? declType = declarationParameters[i].ParameterType;

if (bodyType.IsArray != declType.IsArray ||
bodyType.IsByRef != declType.IsByRef ||
bodyType.IsPointer != declType.IsPointer)
{
throw ArgumentExceptionInvalidMethodOverride(methodInfoDeclaration.Name);
}

if (bodyType.HasElementType || declType.HasElementType)
{
bodyType = bodyType.GetElementType();
declType = declType.GetElementType();
}

if (bodyType == null || !bodyType.Equals(declType))
{
throw ArgumentExceptionInvalidMethodOverride(methodInfoDeclaration.Name);
}
}

return;
}
}

throw ArgumentExceptionInvalidMethodOverride(methodInfoDeclaration.Name);
}

protected override TypeBuilder DefineNestedTypeCore(string name, TypeAttributes attr,
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? parent, Type[]? interfaces, PackingSize packSize, int typeSize)
{
Expand Down
66 changes: 0 additions & 66 deletions ...System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderAPIsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,38 +134,6 @@ public void DefineMethodOverride_TypeCreated_ThrowsInvalidOperationException()
Assert.Throws<InvalidOperationException>(() => type.DefineMethodOverride(method, declaration));
}

[Fact]
public void DefineMethodOverride_MethodNotVirtual_ThrowsArgumentException()
{
AssemblySaveTools.PopulateAssemblyBuilderAndTypeBuilder(out TypeBuilder type);
MethodBuilder method = type.DefineMethod("M", MethodAttributes.Public, typeof(int), null);
ILGenerator ilGenerator = method.GetILGenerator();
ilGenerator.Emit(OpCodes.Ldc_I4, 2);
ilGenerator.Emit(OpCodes.Ret);

type.AddInterfaceImplementation(typeof(DefineMethodOverrideInterface));
MethodInfo declaration = typeof(DefineMethodOverrideInterface).GetMethod(method.Name);

Assert.Throws<ArgumentException>("methodInfoBody", () => type.DefineMethodOverride(method, declaration));
}

[Fact]
public void DefineMethodOverride_TypeDoesNotImplementOrInheritMethod_ThrowsArgumentException()
{
AssemblySaveTools.PopulateAssemblyBuilderAndTypeBuilder(out TypeBuilder type);
MethodBuilder method = type.DefineMethod("M", MethodAttributes.Public | MethodAttributes.Virtual, typeof(int), null);
method.GetILGenerator().Emit(OpCodes.Ret);
MethodInfo interfaceMethod = typeof(DefineMethodOverrideInterface).GetMethod("M");
MethodInfo baseTypeMethod = typeof(DefineMethodOverrideClass).GetMethod("M");

Assert.Throws<ArgumentException>("methodInfoBody", () => type.DefineMethodOverride(method, interfaceMethod));
Assert.Throws<ArgumentException>("methodInfoBody", () => type.DefineMethodOverride(method, baseTypeMethod));

type.AddInterfaceImplementation(typeof(GenericInterface<string>));
MethodInfo implementingMethod = typeof(GenericInterface<string>).GetMethod(nameof(GenericInterface<string>.Method));
Assert.Throws<ArgumentException>("methodInfoBody", () => type.DefineMethodOverride(method, implementingMethod));
}

[Fact]
public void DefineMethodOverride_CalledAgainWithSameDeclaration_ThrowsArgumentException()
{
Expand All @@ -188,24 +156,6 @@ public void DefineMethodOverride_CalledAgainWithSameDeclaration_ThrowsArgumentEx
Assert.Throws<ArgumentException>(() => type.DefineMethodOverride(method2, declaration));
}

[Theory]
[InlineData(typeof(int), new Type[0])]
[InlineData(typeof(int), new Type[] { typeof(int), typeof(int) })]
[InlineData(typeof(int), new Type[] { typeof(string), typeof(string) })]
[InlineData(typeof(int), new Type[] { typeof(int), typeof(string), typeof(bool) })]
[InlineData(typeof(string), new Type[] { typeof(string), typeof(int) })]
public void DefineMethodOverride_BodyAndDeclarationHaveDifferentSignatures_ThrowsArgumentException(Type returnType, Type[] parameterTypes)
{
AssemblySaveTools.PopulateAssemblyBuilderAndTypeBuilder(out TypeBuilder type);
MethodBuilder method = type.DefineMethod("M", MethodAttributes.Public | MethodAttributes.Virtual, returnType, parameterTypes);
method.GetILGenerator().Emit(OpCodes.Ret);
type.AddInterfaceImplementation(typeof(InterfaceWithMethod));

MethodInfo declaration = typeof(InterfaceWithMethod).GetMethod(nameof(InterfaceWithMethod.Method));

Assert.Throws<ArgumentException>(() => type.DefineMethodOverride(method, declaration));
}

public interface GenericInterface<T>
{
T Method();
Expand Down Expand Up @@ -429,22 +379,6 @@ public interface InterfaceDerivedFromOtherInterface : DefineMethodOverrideInterf
public string M2(int a);
}

public abstract class PartialImplementation : InterfaceDerivedFromOtherInterface
{
public int M() => 1;
public abstract string M2(int a);
}

public interface IDefaultImplementation
{
void Method() => Console.WriteLine("Hello");
}

public interface IStaticAbstract
{
static abstract void Method();
}

[Fact]
public void CreateType_ValidateMethods()
{
Expand Down

0 comments on commit 9c710c4

Please sign in to comment.