Skip to content

Commit 0e60130

Browse files
BrzVladBrzVlad
authored
[mono][interp] Add unbox when calling valuetype method through delegate (#79445)
* [mono][interp] Add unbox when calling valuetype method through delegate If we are calling an open instance delegate, where the target method is on a valuetype, we will need to unbox this pointer. * [mono][interp] Remove redundant check If we are calling a method of a valuetype, then we already know `this` pointer is an instantiation of a valuetype. * [mono][interp] Remove dead opcode * [tests] Add regression test * [tests] Disable test on fullaot
1 parent 84d58ec commit 0e60130

File tree

6 files changed

+65
-29
lines changed

6 files changed

+65
-29
lines changed

‎src/mono/mono/mini/interp/interp.c‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3975,7 +3975,12 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
39753975
} 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)) {
39763976
// 'this' is passed dynamically, we need to recompute the target method
39773977
// with each call
3978-
del_imethod = get_virtual_method (del_imethod, LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, MonoObject*)->vtable);
3978+
MonoObject *obj = LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, MonoObject*);
3979+
del_imethod = get_virtual_method (del_imethod, obj->vtable);
3980+
if (m_class_is_valuetype (del_imethod->method->klass)) {
3981+
// We are calling into a value type method, `this` needs to be unboxed
3982+
LOCAL_VAR (call_args_offset + MINT_STACK_SLOT_SIZE, gpointer) = mono_object_unbox_internal (obj);
3983+
}
39793984
} else {
39803985
del->interp_invoke_impl = del_imethod;
39813986
}
@@ -3998,7 +4003,7 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
39984003
MonoObject *this_arg = del->target;
39994004

40004005
// replace the MonoDelegate* on the stack with 'this' pointer
4001-
if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
4006+
if (m_class_is_valuetype (cmethod->method->klass)) {
40024007
gpointer unboxed = mono_object_unbox_internal (this_arg);
40034008
LOCAL_VAR (call_args_offset, gpointer) = unboxed;
40044009
} else {
@@ -4098,7 +4103,7 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
40984103
ip += 5;
40994104
// FIXME push/pop LMF
41004105
cmethod = get_virtual_method_fast (cmethod, this_arg->vtable, slot);
4101-
if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
4106+
if (m_class_is_valuetype (cmethod->method->klass)) {
41024107
/* unbox */
41034108
gpointer unboxed = mono_object_unbox_internal (this_arg);
41044109
LOCAL_VAR (call_args_offset, gpointer) = unboxed;
@@ -4148,29 +4153,6 @@ mono_interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClause
41484153
goto call;
41494154
}
41504155

4151-
MINT_IN_CASE(MINT_CALLVIRT) {
4152-
// FIXME CALLVIRT opcodes are not used on netcore. We should kill them.
4153-
cmethod = (InterpMethod*)frame->imethod->data_items [ip [3]];
4154-
return_offset = ip [1];
4155-
call_args_offset = ip [2];
4156-
4157-
MonoObject *this_arg = LOCAL_VAR (call_args_offset, MonoObject*);
4158-
4159-
// FIXME push/pop LMF
4160-
cmethod = get_virtual_method (cmethod, this_arg->vtable);
4161-
if (m_class_is_valuetype (this_arg->vtable->klass) && m_class_is_valuetype (cmethod->method->klass)) {
4162-
/* unbox */
4163-
gpointer unboxed = mono_object_unbox_internal (this_arg);
4164-
LOCAL_VAR (call_args_offset, gpointer) = unboxed;
4165-
}
4166-
4167-
#ifdef ENABLE_EXPERIMENT_TIERED
4168-
ip += 5;
4169-
#else
4170-
ip += 4;
4171-
#endif
4172-
goto call;
4173-
}
41744156
MINT_IN_CASE(MINT_CALL) {
41754157
cmethod = (InterpMethod*)frame->imethod->data_items [ip [3]];
41764158
return_offset = ip [1];

‎src/mono/mono/mini/interp/mintops.def‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,6 @@ OPDEF(MINT_ARRAY_ELEMENT_SIZE, "array_element_size", 3, 1, 1, MintOpNoArgs)
670670

671671
/* Calls */
672672
OPDEF(MINT_CALL, "call", 4, 1, 1, MintOpMethodToken)
673-
OPDEF(MINT_CALLVIRT, "callvirt", 4, 1, 1, MintOpMethodToken)
674673
OPDEF(MINT_CALLVIRT_FAST, "callvirt.fast", 5, 1, 1, MintOpMethodToken)
675674
OPDEF(MINT_CALL_DELEGATE, "call.delegate", 5, 1, 1, MintOpTwoShorts)
676675
OPDEF(MINT_CALLI, "calli", 4, 1, 2, MintOpNoArgs)

‎src/mono/mono/mini/interp/transform.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3541,8 +3541,6 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
35413541
} else if (is_virtual) {
35423542
interp_add_ins (td, MINT_CALLVIRT_FAST);
35433543
td->last_ins->data [1] = get_virt_method_slot (target_method);
3544-
} else if (is_virtual) {
3545-
interp_add_ins (td, MINT_CALLVIRT);
35463544
} else {
35473545
interp_add_ins (td, MINT_CALL);
35483546
}

‎src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Reflection;
6+
7+
public interface IGetContents {
8+
(string, int, string) GetContents();
9+
}
10+
11+
public struct MyStruct : IGetContents {
12+
public string s1;
13+
public int a;
14+
public string s2;
15+
16+
public (string, int, string) GetContents()
17+
{
18+
return (s1, a, s2);
19+
}
20+
}
21+
22+
public class Program {
23+
24+
public delegate (string, int, string) MyDelegate(IGetContents arg);
25+
26+
public static int Main(string[] args)
27+
{
28+
MyStruct str = new MyStruct();
29+
str.s1 = "test1";
30+
str.a = 42;
31+
str.s2 = "test2";
32+
33+
MethodInfo mi = typeof(IGetContents).GetMethod("GetContents");
34+
MyDelegate func = (MyDelegate)mi.CreateDelegate(typeof(MyDelegate));
35+
36+
(string c1, int c2, string c3) = func(str);
37+
if (c1 != "test1")
38+
return 1;
39+
if (c2 != 42)
40+
return 2;
41+
if (c3 != "test2")
42+
return 3;
43+
return 100;
44+
}
45+
}

‎src/tests/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354.csproj‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
<Optimize>True</Optimize>
5+
</PropertyGroup>
6+
<ItemGroup>
7+
<Compile Include="$(MSBuildProjectName).cs" />
8+
</ItemGroup>
9+
</Project>

‎src/tests/issues.targets‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2910,6 +2910,9 @@
29102910
<ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/DevDiv_461649/DevDiv_461649/**">
29112911
<Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
29122912
</ExcludeList>
2913+
<ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_79354/Runtime_79354/**">
2914+
<Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
2915+
</ExcludeList>
29132916
<ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/GitHub_25027/GitHub_25027/**">
29142917
<Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
29152918
</ExcludeList>

0 commit comments

Comments
 (0)