Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 8 additions & 26 deletions src/mono/mono/mini/interp/interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -3975,7 +3975,12 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
} else if ((m_method_is_virtual (del_imethod->method) && !m_method_is_static (del_imethod->method)) && !del->target && !m_class_is_valuetype (del_imethod->method->klass)) {
// 'this' is passed dynamically, we need to recompute the target method
// with each call
del_imethod = get_virtual_method (del_imethod, LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, MonoObject*)->vtable);
MonoObject *obj = LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, MonoObject*);
del_imethod = get_virtual_method (del_imethod, obj->vtable);
if (m_class_is_valuetype (del_imethod->method->klass)) {
// We are calling into a value type method, `this` needs to be unboxed
LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, gpointer) = mono_object_unbox_internal (obj);
}
} else {
del->interp_invoke_impl = del_imethod;
}
Expand All @@ -3998,7 +4003,7 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
MonoObject *this_arg = del->target;

// replace the MonoDelegate* on the stack with 'this' pointer
if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
if (m_class_is_valuetype (cmethod->method->klass)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The first check was not needed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

To me it seems redundant. I can't think why it would be needed. If you can think on why it would be wrong let me know. I don't plan to backport this part of the change though.

gpointer unboxed = mono_object_unbox_internal (this_arg);
LOCAL_VAR (call_args_offset, gpointer) = unboxed;
} else {
Expand Down Expand Up @@ -4098,7 +4103,7 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
ip += 5;
// FIXME push/pop LMF
cmethod = get_virtual_method_fast (cmethod, this_arg->vtable, slot);
if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
if (m_class_is_valuetype (cmethod->method->klass)) {
/* unbox */
gpointer unboxed = mono_object_unbox_internal (this_arg);
LOCAL_VAR (call_args_offset, gpointer) = unboxed;
Expand Down Expand Up @@ -4148,29 +4153,6 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
goto call;
}

MINT_IN_CASE(MINT_CALLVIRT) {
// FIXME CALLVIRT opcodes are not used on netcore. We should kill them.
cmethod = (InterpMethod*)frame->imethod->data_items [ip [3]];
return_offset = ip [1];
call_args_offset = ip [2];

MonoObject *this_arg = LOCAL_VAR (call_args_offset, MonoObject*);

// FIXME push/pop LMF
cmethod = get_virtual_method (cmethod, this_arg->vtable);
if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
/* unbox */
gpointer unboxed = mono_object_unbox_internal (this_arg);
LOCAL_VAR (call_args_offset, gpointer) = unboxed;
}

#ifdef ENABLE_EXPERIMENT_TIERED
ip += 5;
#else
ip += 4;
#endif
goto call;
}
MINT_IN_CASE(MINT_CALL) {
cmethod = (InterpMethod*)frame->imethod->data_items [ip [3]];
return_offset = ip [1];
Expand Down
1 change: 0 additions & 1 deletion src/mono/mono/mini/interp/mintops.def
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,6 @@ OPDEF(MINT_ARRAY_IS_PRIMITIVE, "array_is_primitive", 3, 1, 1, MintOpNoArgs)

/* Calls */
OPDEF(MINT_CALL, "call", 4, 1, 1, MintOpMethodToken)
OPDEF(MINT_CALLVIRT, "callvirt", 4, 1, 1, MintOpMethodToken)
OPDEF(MINT_CALLVIRT_FAST, "callvirt.fast", 5, 1, 1, MintOpMethodToken)
OPDEF(MINT_CALL_DELEGATE, "call.delegate", 5, 1, 1, MintOpTwoShorts)
OPDEF(MINT_CALLI, "calli", 4, 1, 2, MintOpNoArgs)
Expand Down
2 changes: 0 additions & 2 deletions src/mono/mono/mini/interp/transform.c
Original file line number Diff line number Diff line change
Expand Up @@ -3516,8 +3516,6 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
} else if (is_virtual) {
interp_add_ins (td, MINT_CALLVIRT_FAST);
td->last_ins->data [1] = get_virt_method_slot (target_method);
} else if (is_virtual) {
interp_add_ins (td, MINT_CALLVIRT);
} else {
interp_add_ins (td, MINT_CALL);
}
Expand Down
45 changes: 45 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Reflection;

public interface IGetContents {
(string, int, string) GetContents();
}

public struct MyStruct : IGetContents {
public string s1;
public int a;
public string s2;

public (string, int, string) GetContents()
{
return (s1, a, s2);
}
}

public class Program {

public delegate (string, int, string) MyDelegate(IGetContents arg);

public static int Main(string[] args)
{
MyStruct str = new MyStruct();
str.s1 = "test1";
str.a = 42;
str.s2 = "test2";

MethodInfo mi = typeof(IGetContents).GetMethod("GetContents");
MyDelegate func = (MyDelegate)mi.CreateDelegate(typeof(MyDelegate));

(string c1, int c2, string c3) = func(str);
if (c1 != "test1")
return 1;
if (c2 != 42)
return 2;
if (c3 != "test2")
return 3;
return 100;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>
3 changes: 3 additions & 0 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -2913,6 +2913,9 @@
<ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/DevDiv_461649/DevDiv_461649/**">
<Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354/**">
<Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027/**">
<Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
</ExcludeList>
Expand Down