[clr-interp] Add support for virtual method calls (#114529)

* [clr-interp] Add support for virtual calls (including interface)

This follows a trivial approach where we resolve the MethodDesc of the target method by reusing existing functionality. Long term, virtual dispatching should go through slots continaing interpreter IR code pointer, in a similar fashion to JIT.

* Add test for virtual method dispatch

* Disable test for now

No gc support yet

Author: BrzVlad

src/coreclr/interpreter/compiler.cpp

@@ -1539,12 +1539,18 @@ CORINFO_CLASS_HANDLE InterpCompiler::ResolveClassToken(uint32_t token)
 void InterpCompiler::EmitCall(CORINFO_CLASS_HANDLE constrainedClass, bool readonly, bool tailcall)
 {
     uint32_t token = getU4LittleEndian(m_ip + 1);
+    bool isVirtual = (*m_ip == CEE_CALLVIRT);
 
     CORINFO_METHOD_HANDLE targetMethod = ResolveMethodToken(token);
 
     CORINFO_SIG_INFO targetSignature;
     m_compHnd->getMethodSig(targetMethod, &targetSignature);
 
+    uint32_t mflags = m_compHnd->getMethodAttribs(targetMethod);
+
+    if (isVirtual && (!(mflags & CORINFO_FLG_VIRTUAL) || (mflags & CORINFO_FLG_FINAL)))
+        isVirtual = false;
+
     if (EmitCallIntrinsics(targetMethod, targetSignature))
     {
         m_ip += 5;
@@ -1588,10 +1594,18 @@ void InterpCompiler::EmitCall(CORINFO_CLASS_HANDLE constrainedClass, bool readon
     }
 
     // Emit call instruction
-    AddIns(INTOP_CALL);
+    if (isVirtual)
+    {
+        AddIns(INTOP_CALLVIRT);
+        m_pLastNewIns->data[0] = GetDataItemIndex(targetMethod);
+    }
+    else
+    {
+        AddIns(INTOP_CALL);
+        m_pLastNewIns->data[0] = GetMethodDataItemIndex(targetMethod);
+    }
     m_pLastNewIns->SetDVar(dVar);
     m_pLastNewIns->SetSVar(CALL_ARGS_SVAR);
-    m_pLastNewIns->data[0] = GetMethodDataItemIndex(targetMethod);
 
     m_pLastNewIns->flags |= INTERP_INST_FLAG_CALL;
     m_pLastNewIns->info.pCallInfo = (InterpCallInfo*)AllocMemPool0(sizeof (InterpCallInfo));
@@ -2630,6 +2644,7 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo)
                 EmitUnaryArithmeticOp(INTOP_NOT_I4);
                 m_ip++;
                 break;
+            case CEE_CALLVIRT:
             case CEE_CALL:
                 EmitCall(constrainedClass, readonly, tailcall);
                 constrainedClass = NULL;

src/coreclr/interpreter/intops.def

@@ -246,6 +246,7 @@ OPDEF(INTOP_LDFLDA, "ldflda", 4, 1, 1, InterpOpInt)
 
 // Calls
 OPDEF(INTOP_CALL, "call", 4, 1, 1, InterpOpMethodToken)
+OPDEF(INTOP_CALLVIRT, "callvirt", 4, 1, 1, InterpOpMethodToken)
 OPDEF(INTOP_NEWOBJ, "newobj", 5, 1, 1, InterpOpMethodToken)
 OPDEF(INTOP_NEWOBJ_VT, "newobj.vt", 5, 1, 1, InterpOpMethodToken)
 

src/coreclr/vm/interpexec.cpp

@@ -54,6 +54,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
     stack = pFrame->pStack;
 
     int32_t returnOffset, callArgsOffset, methodSlot;
+    const int32_t *targetIp;
 
 MAIN_LOOP:
     while (true)
@@ -972,6 +973,35 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
                 ip += 5;
                 break;
             }
+            case INTOP_CALLVIRT:
+            {
+                returnOffset = ip[1];
+                callArgsOffset = ip[2];
+                methodSlot = ip[3];
+
+                MethodDesc *pMD = (MethodDesc*)pMethod->pDataItems[methodSlot];
+
+                OBJECTREF *pThisArg = LOCAL_VAR_ADDR(callArgsOffset, OBJECTREF);
+                NULL_CHECK(*pThisArg);
+
+                // Interpreter-TODO
+                // This needs to be optimized, not operating at MethodDesc level, rather with ftnptr
+                // slots containing the interpreter IR pointer
+                pMD = pMD->GetMethodDescOfVirtualizedCode(pThisArg, pMD->GetMethodTable());
+
+                PCODE code = pMD->GetNativeCode();
+                if (!code)
+                {
+                    pInterpreterFrame->SetTopInterpMethodContextFrame(pFrame);
+                    GCX_PREEMP();
+                    pMD->PrepareInitialCode(CallerGCMode::Coop);
+                    code = pMD->GetNativeCode();
+                }
+                targetIp = (const int32_t*)code;
+                ip += 4;
+                // Interpreter-TODO unbox if target method class is valuetype
+                goto CALL_TARGET_IP;
+            }
 
             case INTOP_CALL:
             {
@@ -981,7 +1011,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
 
                 ip += 4;
 CALL_INTERP_SLOT:
-                const int32_t *targetIp;
+                {
                 size_t targetMethod = (size_t)pMethod->pDataItems[methodSlot];
                 if (targetMethod & INTERP_METHOD_DESC_TAG)
                 {
@@ -1012,7 +1042,8 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr
                     // for interpreter call or normal pointer for JIT/R2R call.
                     targetIp = (const int32_t*)targetMethod;
                 }
-
+                }
+CALL_TARGET_IP:
                 // Save current execution state for when we return from called method
                 pFrame->ip = ip;
 

src/coreclr/vm/method.cpp

@@ -2000,13 +2000,9 @@ PCODE MethodDesc::GetSingleCallableAddrOfVirtualizedCode(OBJECTREF *orThis, Type
     return pObjMT->GetRestoredSlot(GetSlot());
 }
 
-//*******************************************************************************
-// The following resolve virtual dispatch for the given method on the given
-// object down to an actual address to call, including any
-// handling of context proxies and other thunking layers.
-PCODE MethodDesc::GetMultiCallableAddrOfVirtualizedCode(OBJECTREF *orThis, TypeHandle staticTH)
+MethodDesc* MethodDesc::GetMethodDescOfVirtualizedCode(OBJECTREF *orThis, TypeHandle staticTH)
 {
-    CONTRACT(PCODE)
+    CONTRACT(MethodDesc*)
     {
         THROWS;
         GC_TRIGGERS;
@@ -2016,41 +2012,43 @@ PCODE MethodDesc::GetMultiCallableAddrOfVirtualizedCode(OBJECTREF *orThis, TypeH
         POSTCONDITION(RETVAL != NULL);
     }
     CONTRACT_END;
-
     // Method table of target (might be instantiated)
     MethodTable *pObjMT = (*orThis)->GetMethodTable();
 
     // This is the static method descriptor describing the call.
     // It is not the destination of the call, which we must compute.
     MethodDesc* pStaticMD = this;
-    MethodDesc *pTargetMD;
 
     if (pStaticMD->HasMethodInstantiation())
     {
         CheckRestore();
-        pTargetMD = ResolveGenericVirtualMethod(orThis);
-
-        // If we're remoting this call we can't call directly on the returned
-        // method desc, we need to go through a stub that guarantees we end up
-        // in the remoting handler. The stub we use below is normally just for
-        // non-virtual calls on virtual methods (that have the same problem
-        // where we could end up bypassing the remoting system), but it serves
-        // our purpose here (basically pushes our correctly instantiated,
-        // resolved method desc on the stack and calls the remoting code).
-
-        RETURN(pTargetMD->GetMultiCallableAddrOfCode());
+        RETURN(ResolveGenericVirtualMethod(orThis));
     }
 
     if (pStaticMD->IsInterface())
     {
-        pTargetMD = MethodTable::GetMethodDescForInterfaceMethodAndServer(staticTH,pStaticMD,orThis);
-        RETURN(pTargetMD->GetMultiCallableAddrOfCode());
+        RETURN(MethodTable::GetMethodDescForInterfaceMethodAndServer(staticTH, pStaticMD, orThis));
     }
 
+    RETURN(pObjMT->GetMethodDescForSlot(pStaticMD->GetSlot()));
+}
 
-    pTargetMD = pObjMT->GetMethodDescForSlot(pStaticMD->GetSlot());
+//*******************************************************************************
+// The following resolve virtual dispatch for the given method on the given
+// object down to an actual address to call, including any
+// handling of context proxies and other thunking layers.
+PCODE MethodDesc::GetMultiCallableAddrOfVirtualizedCode(OBJECTREF *orThis, TypeHandle staticTH)
+{
+    CONTRACT(PCODE)
+    {
+        THROWS;
+        GC_TRIGGERS;
+        POSTCONDITION(RETVAL != NULL);
+    }
+    CONTRACT_END;
 
-    RETURN (pTargetMD->GetMultiCallableAddrOfCode());
+    MethodDesc *pTargetMD = GetMethodDescOfVirtualizedCode(orThis, staticTH);
+    RETURN(pTargetMD->GetMultiCallableAddrOfCode());
 }
 
 //*******************************************************************************

src/coreclr/vm/method.hpp

@@ -1429,6 +1429,7 @@ class MethodDesc
     // indirect call via slot in this case.
     PCODE TryGetMultiCallableAddrOfCode(CORINFO_ACCESS_FLAGS accessFlags);
 
+    MethodDesc* GetMethodDescOfVirtualizedCode(OBJECTREF *orThis, TypeHandle staticTH);
     // These return an address after resolving "virtual methods" correctly, including any
     // handling of context proxies, other thunking layers and also including
     // instantiation of generic virtual methods if required.

src/tests/JIT/interpreter/Interpreter.cs

@@ -4,6 +4,33 @@
 using System;
 using System.Runtime.CompilerServices;
 
+public interface ITest
+{
+    public int VirtualMethod();
+}
+
+public class BaseClass : ITest
+{
+    public int NonVirtualMethod()
+    {
+        return 0xbaba;
+    }
+
+    public virtual int VirtualMethod()
+    {
+        return 0xbebe;
+    }
+}
+
+public class DerivedClass : BaseClass
+{
+    public override int VirtualMethod()
+    {
+        return 0xdede;
+    }
+
+}
+
 public struct MyStruct
 {
     public int a;
@@ -71,6 +98,8 @@ public static void RunInterpreterTests()
 //            Environment.FailFast(null);
         if (!TestFloat())
             Environment.FailFast(null);
+//        if (!TestVirtual())
+//          Environment.FailFast(null);
     }
 
     public static int Mul4(int a, int b, int c, int d)
@@ -209,4 +238,26 @@ public static bool TestFloat()
 
         return true;
     }
+
+    public static bool TestVirtual()
+    {
+        BaseClass bc = new DerivedClass();
+        ITest itest = bc;
+
+        if (bc.NonVirtualMethod() != 0xbaba)
+            return false;
+        if (bc.VirtualMethod() != 0xdede)
+            return false;
+        if (itest.VirtualMethod() != 0xdede)
+            return false;
+        bc = new BaseClass();
+        itest = bc;
+        if (bc.NonVirtualMethod() != 0xbaba)
+            return false;
+        if (bc.VirtualMethod() != 0xbebe)
+            return false;
+        if (itest.VirtualMethod() != 0xbebe)
+            return false;
+        return true;
+    }
 }