Skip to content
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.

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

Add ldvirtftn #141

Closed
wants to merge 2 commits into from
Closed

Add ldvirtftn #141

wants to merge 2 commits into from

Conversation

BadRyuner
Copy link
Contributor

  • Adds support for Ldvirtftn OpCode
  • Adds test for Ldvirtftn OpCode using delegates

Also unexpected, but ((object)anyInteger).ToString() throws exception +_+

Copy link
Owner

@Washi1337 Washi1337 left a comment

Choose a reason for hiding this comment

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

This does not cover the semantics of ldvirtftn adequately. It has to follow the exact same behavior as callvirt when it comes to method devirtualization.

We may have to somehow extract the method devirtualization of callvirt into its own separate unit such that it can be reused by both callvirt and ldvirtftn without having to copy implementations.

Comment on lines 34 to 52
do
{
// try resolve function
var resolvedVirtualFunction = thisObjectType.Methods
.FirstOrDefault(method => method.Name == virtualFunctionName
&& SignatureComparer.Default.Equals(method.Signature, virtualFunctionSignature));

// if resolved then push function pointer
if (resolvedVirtualFunction != null)
{
var functionPointer = methods.GetAddress(resolvedVirtualFunction);
stack.Push(factory.CreateNativeInteger(functionPointer), type);
return CilDispatchResult.Success();
}

// else switch to BaseType and try resolve again
thisObjectType = thisObjectType.BaseType?.Resolve();
} // or exit and throw CilEmulationException
while (thisObjectType != null);
Copy link
Owner

Choose a reason for hiding this comment

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

This is not sufficient. Ldvirtftn follows the exact same method devirtualization process as callvirt, with all its edge cases.

@Washi1337
Copy link
Owner

Also unexpected, but ((object)anyInteger).ToString() throws exception

If that is the case, we need a test case for this too.

@BadRyuner
Copy link
Contributor Author

BadRyuner commented Jun 5, 2024

Also unexpected, but ((object)anyInteger).ToString() throws exception

If that is the case, we need a test case for this too.

System.ArgumentException: "The type parameter !0 could not be resolved to a concrete type argument in the current context."
- AsmResolver.DotNet.Memory.TypeAlignmentDetector.VisitGenericParameter(GenericParameterSignature signature) in AsmResolver.DotNet.Memory\TypeAlignmentDetector.cs:line 94
   - AsmResolver.DotNet.Signatures.Types.GenericParameterSignature.AcceptVisitor[TResult](ITypeSignatureVisitor`1 visitor) in AsmResolver.DotNet.Signatures.Types\GenericParameterSignature.cs:line 70
   - AsmResolver.DotNet.Memory.TypeAlignmentDetector.VisitTypeDefinition(TypeDefinition type) in AsmResolver.DotNet.Memory\TypeAlignmentDetector.cs:line 165
   - AsmResolver.DotNet.Memory.TypeMemoryLayoutDetector.VisitValueTypeDefinition(TypeDefinition type) in AsmResolver.DotNet.Memory\TypeMemoryLayoutDetector.cs:line 182
   - AsmResolver.DotNet.Memory.TypeMemoryLayoutDetector.VisitTypeDefinition(TypeDefinition type) in AsmResolver.DotNet.Memory\TypeMemoryLayoutDetector.cs:line 172
   - AsmResolver.DotNet.Memory.TypeMemoryLayoutDetector.VisitTypeDefOrRef(ITypeDefOrRef type) in AsmResolver.DotNet.Memory\TypeMemoryLayoutDetector.cs:line 147
   - AsmResolver.DotNet.Memory.TypeMemoryLayoutDetection.GetImpliedMemoryLayout(ITypeDefOrRef type, Boolean is32Bit) in AsmResolver.DotNet.Memory\TypeMemoryLayoutDetection.cs:line 29
   - Echo.Platforms.AsmResolver.Emulation.ValueFactory.GetTypeDefOrRefContentsLayout(ITypeDefOrRef type, GenericContext context, UInt32 alignment) in D:\Work\Echo\src\Platforms\Echo.Platforms.AsmResolver\Emulation\ValueFactory.cs:line 544
   - Echo.Platforms.AsmResolver.Emulation.ValueFactory.GetTypeSignatureContentsLayout(TypeSignature type) in D:\Work\Echo\src\Platforms\Echo.Platforms.AsmResolver\Emulation\ValueFactory.cs:line 603
   - Echo.Platforms.AsmResolver.Emulation.ValueFactory.GetTypeContentsMemoryLayout(ITypeDescriptor type) in D:\Work\Echo\src\Platforms\Echo.Platforms.AsmResolver\Emulation\ValueFactory.cs:line 498
   - Echo.Platforms.AsmResolver.Emulation.Dispatch.ObjectModel.NewObjHandler.HandleValueTypeNewObj(CilExecutionContext context, CilInstruction instruction, IMethodDescriptor constructor, TypeSignature instanceType, IList`1 arguments) in D:\Work\Echo\src\Platforms\Echo.Platforms.AsmResolver\Emulation\Dispatch\ObjectModel\NewObjHandler.cs:line 50
   - Echo.Platforms.AsmResolver.Emulation.Dispatch.ObjectModel.NewObjHandler.Dispatch(CilExecutionContext context, CilInstruction instruction) in D:\Work\Echo\src\Platforms\Echo.Platforms.AsmResolver\Emulation\Dispatch\ObjectModel\NewObjHandler.cs:line 27
   - Echo.Platforms.AsmResolver.Emulation.Dispatch.CilDispatcher.Dispatch(CilExecutionContext context, CilInstruction instruction) in D:\Work\Echo\src\Platforms\Echo.Platforms.AsmResolver\Emulation\Dispatch\CilDispatcher.cs:line 72

Virtual callstack:
image

looks like a new issue for AsmResolver

@Washi1337
Copy link
Owner

Washi1337 commented Jun 5, 2024

looks like a new issue for AsmResolver

If that is the case, it would be great if you could make a minimal reproducible test case and put it on the AsmResolver issue board.

However I often found this to be the case of Echo simply not always instantiating the generic types properly everywhere. In that case, we still need a minimal unit test, but specifically using Echo instead :)

In either case, that probably warrants a separate issue, independent of the ldvirtftn implementation PR.

@BadRyuner
Copy link
Contributor Author

var functionPointer = methods.GetAddress(resolvedVirtualFunction);
stack.Push(factory.CreateNativeInteger(functionPointer), type);
return CilDispatchResult.Success();

I poked around the error a bit and fixed it (almost hehehe). I don't know if the error is in AsmResolver or Echo +_+
But I figured it out:

  • The (st|ld)fld instruction handler does not pay attention to the GenericContext
  • If you fix this and change instruction.Operand (field.DeclaringType + field.Signature) with GenericContext in mind, WriteField will drop because: field.Signature is Int32 (as an example), but this field in TypeMemoryLayout (given the substitution of !0 for Int32 in field.DeclaringType) is of type !0. EXCEPTION go BRRR.
  • If you fix this and change instruction.Operand (field.DeclaringType only) to take GenericContext into account, Push/Pop drops because you can't get MemoryLayout for !0 (see field.Signature). EXCEPTION go BRRR.
  • Same with instruction.Operand (field.Signature only) as with (Type + Sig)
  • If you fix this and change instruction.Operand (field.DeclaringType only) and send the corrected FieldSignature only to Push/Pop but not to WriteField, then everything works fine (almost, hehehe). A new trap appears: cursed TypeMemoryLayout. I haven't gone into detail as to why this is the case, but the test (in the commit below) I have int32 123 turns into ‘268435712’ instead of ‘123’. Very cursed, but it works without exceptions lol.
    cursed commit

@Washi1337
Copy link
Owner

This is very likely an Echo bug. I don't see any evidence of the actual type memory layout algorithm being problematic here.

This problem was partially addressed for the call opcode. See CallHandlerBase.cs below:

private static IMethodDescriptor InstantiateDeclaringType(IMethodDescriptor caller, IMethodDescriptor callee)
{
// The caller may pass along generic type arguments to the callee.
// Note: Since this is a hot path, try to delay allocations when possible.
var context = GenericContext.FromMethod(caller);
var result = callee;
// Instantiate any args in the declaring type.
if (callee.DeclaringType?.ToTypeDefOrRef() is TypeSpecification { Signature: {} typeSignature })
{
var newType = typeSignature.InstantiateGenericTypes(context);
if (newType != typeSignature)
result = newType.ToTypeDefOrRef().CreateMemberReference(callee.Name!, callee.Signature!);
}
// When generic instance method call, instantiate any type arguments when necessary.
if (callee is MethodSpecification { Method: {} method, Signature: { } signature })
{
// Delay the allocation of a completely new method specification unless we absolutely need to.
// Try finding the first argument that needed instantiation.
TypeSignature? firstInstantiated = null;
int index = -1;
for (int i = 0; i < signature.TypeArguments.Count; i++)
{
var original = signature.TypeArguments[i];
var instantiated = original.InstantiateGenericTypes(context);
if (instantiated != original)
{
firstInstantiated = instantiated;
index = i;
break;
}
}
// If there is one, create the new signature.
if (firstInstantiated is not null)
{
var args = new TypeSignature[signature.TypeArguments.Count];
for (int i = 0; i < index; i++)
args[i] = signature.TypeArguments[i];
args[index] = firstInstantiated;
for (int i = index + 1; i < args.Length; i++)
args[i] = signature.TypeArguments[i].InstantiateGenericTypes(context);
if (result is MemberReference member)
result = member.MakeGenericInstanceMethod(args);
else
result = method.MakeGenericInstanceMethod(args);
}
}
return result;
}

We should probably copy/extend this to work for fields as well.

Regardless, this is definitely out of scope for this PR, and really should be in a separate issue.

@BadRyuner BadRyuner closed this Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants