Fix x86 only ICastable feature bug.

When we call ICastable.IsInstanceOfInterface, we treat RuntimeTypeHandle as a OBJECTREF, which is incorrect as-per x86 calling convention since RuntimeTypehandle is a struct that contains a RuntimeType ref field and needs to be passed in stack. Our VM simple call helpers CALL_MANAGED_METHOD doesn't handle this correctly (it does the simple thing that always assume all the arguments are passed in register first). I'm fixing this by using a static method that takes RuntimeType instead of RuntimeTypeHandle, then convert it to RuntimeTypehandle.
Also switch to use PREPARE_NONVIRTUAL_CALLSITE(METHODID) as per Jan's suggestion.
ICastable test is now enabled in x86 for both JITs.

diff --git a/src/mscorlib/model.CoreLib.xml b/src/mscorlib/model.CoreLib.xml
index 6ea5768..114b450 100644 (file)
--- a/src/mscorlib/model.CoreLib.xml
+++ b/src/mscorlib/model.CoreLib.xml
@@ -5539,6 +5539,10 @@
       <Member Name="IsInstanceOfInterface(System.RuntimeTypeHandle,System.Exception@)" /> <!-- EE -->
       <Member Name="GetImplType(System.RuntimeTypeHandle)" /> <!-- EE -->
     </Type>
+    <Type Name="System.Runtime.CompilerServices.ICastableHelpers"> 
+      <Member Name="IsInstanceOfInterface(System.Runtime.CompilerServices.ICastable,System.RuntimeType,System.Exception@)" /> <!-- EE -->
+      <Member Name="GetImplType(System.Runtime.CompilerServices.ICastable,System.RuntimeType)" /> <!-- EE -->
+    </Type>
     <Type Name="System.Runtime.CompilerServices.IsExplicitlyDereferenced"> <!-- for MC++ -->
     </Type>
     <Type Name="System.Runtime.CompilerServices.IsImplicitlyDereferenced">  <!-- for MC++ -->

diff --git a/src/mscorlib/model.xml b/src/mscorlib/model.xml
index ef2774a..671da7e 100644 (file)
--- a/src/mscorlib/model.xml
+++ b/src/mscorlib/model.xml
@@ -5539,6 +5539,10 @@
       <Member Name="IsInstanceOfInterface(System.RuntimeTypeHandle,System.Exception@)" /> <!-- EE -->
       <Member Name="GetImplType(System.RuntimeTypeHandle)" /> <!-- EE -->
     </Type>
+    <Type Name="System.Runtime.CompilerServices.ICastableHelpers"> 
+      <Member Name="IsInstanceOfInterface(System.Runtime.CompilerServices.ICastable,System.RuntimeType, System.Exception@)" /> <!-- EE -->
+      <Member Name="GetImplType(System.Runtime.CompilerServices.ICastable,System.RuntimeType)" /> <!-- EE -->
+    </Type>  
     <Type Name="System.Runtime.CompilerServices.IsExplicitlyDereferenced"> <!-- for MC++ -->
     </Type>
     <Type Name="System.Runtime.CompilerServices.IsImplicitlyDereferenced">  <!-- for MC++ -->

diff --git a/src/mscorlib/src/System/Runtime/CompilerServices/ICastable.cs b/src/mscorlib/src/System/Runtime/CompilerServices/ICastable.cs
index bd04647..7ba9434 100644 (file)
--- a/src/mscorlib/src/System/Runtime/CompilerServices/ICastable.cs
+++ b/src/mscorlib/src/System/Runtime/CompilerServices/ICastable.cs
@@ -61,4 +61,22 @@ namespace System.Runtime.CompilerServices
         // IsInstanceOfInterface.
         RuntimeTypeHandle GetImplType(RuntimeTypeHandle interfaceType);
     }
+    
+    /// <summary>
+    /// Helpers that allows VM to call into ICastable methods without having to deal with RuntimeTypeHandle.
+    /// RuntimeTypeHandle is a struct and is always passed in stack in x86, which our VM call helpers don't
+    /// particularly like.
+    /// </summary>
+    class ICastableHelpers
+    {
+        internal static bool IsInstanceOfInterface(ICastable castable, RuntimeType type, out Exception castError)
+        {
+            return castable.IsInstanceOfInterface(new RuntimeTypeHandle(type), out castError);
+        }    
+        
+        internal static RuntimeType GetImplType(ICastable castable, RuntimeType interfaceType)
+        {
+            return castable.GetImplType(new RuntimeTypeHandle(interfaceType)).GetRuntimeType(); 
+        }    
+    }
 }

diff --git a/src/vm/jithelpers.cpp b/src/vm/jithelpers.cpp
index 340d61b..fffc6ce 100644 (file)
--- a/src/vm/jithelpers.cpp
+++ b/src/vm/jithelpers.cpp
@@ -2446,14 +2446,13 @@ BOOL ObjIsInstanceOf(Object *pObject, TypeHandle toTypeHnd, BOOL throwCastExcept
     // to a given type.
     else if (toTypeHnd.IsInterface() && fromTypeHnd.GetMethodTable()->IsICastable())
     {
-        // Make actuall call to obj.IsInstanceOfInterface(interfaceTypeObj, out exception)
+        // Make actuall call to ICastableHelpers.IsInstanceOfInterface(obj, interfaceTypeObj, out exception)
         OBJECTREF exception = NULL;
         GCPROTECT_BEGIN(exception);
-        MethodTable *pFromTypeMT = fromTypeHnd.GetMethodTable();
-        MethodDesc *pIsInstanceOfMD = pFromTypeMT->GetMethodDescForInterfaceMethod(MscorlibBinder::GetMethod(METHOD__ICASTABLE__ISINSTANCEOF)); //GC triggers
-        OBJECTREF managedType = toTypeHnd.GetManagedClassObject(); //GC triggers
+        
+        PREPARE_NONVIRTUAL_CALLSITE(METHOD__ICASTABLEHELPERS__ISINSTANCEOF);
 
-        PREPARE_NONVIRTUAL_CALLSITE_USING_METHODDESC(pIsInstanceOfMD);
+        OBJECTREF managedType = toTypeHnd.GetManagedClassObject(); //GC triggers
 
         DECLARE_ARGHOLDER_ARRAY(args, 3);
         args[ARGNUM_0] = OBJECTREF_TO_ARGHOLDER(obj);

diff --git a/src/vm/metasig.h b/src/vm/metasig.h
index 4edfe68..4509307 100644 (file)
--- a/src/vm/metasig.h
+++ b/src/vm/metasig.h
@@ -675,8 +675,8 @@ DEFINE_METASIG(SM(RefObject_Object_Object_RetObject, r(j) j j, j))
 DEFINE_METASIG_T(SM(RefCleanupWorkList_RetVoid, r(C(CLEANUP_WORK_LIST)), v))
 DEFINE_METASIG_T(SM(RefCleanupWorkList_SafeHandle_RetIntPtr, r(C(CLEANUP_WORK_LIST)) C(SAFE_HANDLE), I))
 
-DEFINE_METASIG_T(IM(RuntimeTypeHandle_RefException_RetBool, g(RT_TYPE_HANDLE) r(C(EXCEPTION)), F))
-DEFINE_METASIG_T(IM(RuntimeTypeHandle_RetRuntimeTypeHandle, g(RT_TYPE_HANDLE), g(RT_TYPE_HANDLE)))
+DEFINE_METASIG_T(SM(ICastable_RtType_RefException_RetBool, C(ICASTABLE) C(CLASS) r(C(EXCEPTION)), F))
+DEFINE_METASIG_T(SM(ICastable_RtType_RetRtType, C(ICASTABLE) C(CLASS), C(CLASS)))
 
 DEFINE_METASIG_T(IM(ArrByte_Int_Int_AsyncCallback_Object_RetIAsyncResult, a(b) i i C(ASYNCCALLBACK) j, C(IASYNCRESULT)))
 DEFINE_METASIG_T(IM(IAsyncResult_RetInt, C(IASYNCRESULT), i))

diff --git a/src/vm/methodtable.cpp b/src/vm/methodtable.cpp
index c620046..bb7e44c 100644 (file)
--- a/src/vm/methodtable.cpp
+++ b/src/vm/methodtable.cpp
@@ -805,11 +805,10 @@ PTR_MethodTable InterfaceInfo_t::GetApproxMethodTable(Module * pContainingModule
     {
         GCStress<cfg_any>::MaybeTrigger();
 
-        // Make call to obj.GetImplType(interfaceTypeObj)
-        MethodDesc *pGetImplTypeMD = pServerMT->GetMethodDescForInterfaceMethod(MscorlibBinder::GetMethod(METHOD__ICASTABLE__GETIMPLTYPE));
-        OBJECTREF ownerManagedType = ownerType.GetManagedClassObject(); //GC triggers
+        // Make call to ICastableHelpers.GetImplType(obj, interfaceTypeObj)
+        PREPARE_NONVIRTUAL_CALLSITE(METHOD__ICASTABLEHELPERS__GETIMPLTYPE);
 
-        PREPARE_NONVIRTUAL_CALLSITE_USING_METHODDESC(pGetImplTypeMD);
+        OBJECTREF ownerManagedType = ownerType.GetManagedClassObject(); //GC triggers
         
         DECLARE_ARGHOLDER_ARRAY(args, 2);
         args[ARGNUM_0] = OBJECTREF_TO_ARGHOLDER(*pServer);

diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h
index 4c27965..bc1b557 100644 (file)
--- a/src/vm/mscorlib.h
+++ b/src/vm/mscorlib.h
@@ -2204,8 +2204,11 @@ DEFINE_CLASS(MODULEBASE,        Reflection,         Module)
 
 #ifdef FEATURE_ICASTABLE
 DEFINE_CLASS(ICASTABLE,         CompilerServices,   ICastable)
-DEFINE_METHOD(ICASTABLE,        ISINSTANCEOF,       IsInstanceOfInterface, IM_RuntimeTypeHandle_RefException_RetBool)
-DEFINE_METHOD(ICASTABLE,        GETIMPLTYPE,        GetImplType, IM_RuntimeTypeHandle_RetRuntimeTypeHandle)
+
+DEFINE_CLASS(ICASTABLEHELPERS,         CompilerServices,   ICastableHelpers)
+DEFINE_METHOD(ICASTABLEHELPERS,        ISINSTANCEOF,       IsInstanceOfInterface, SM_ICastable_RtType_RefException_RetBool)
+DEFINE_METHOD(ICASTABLEHELPERS,        GETIMPLTYPE,        GetImplType, SM_ICastable_RtType_RetRtType)
+
 #endif // FEATURE_ICASTABLE
 
 DEFINE_CLASS(CUTF8MARSHALER, StubHelpers, UTF8Marshaler)

diff --git a/src/vm/virtualcallstub.cpp b/src/vm/virtualcallstub.cpp
index 82de5ac..512b4f2 100644 (file)
--- a/src/vm/virtualcallstub.cpp
+++ b/src/vm/virtualcallstub.cpp
@@ -2277,12 +2277,11 @@ VirtualCallStubManager::Resolver(
         // It allows objects that implement ICastable to mimic behavior of other types. 
         MethodTable * pTokenMT = GetTypeFromToken(token);
 
-        // Make call to obj.GetImplType(interfaceTypeObj)
-        MethodDesc *pGetImplTypeMD = pMT->GetMethodDescForInterfaceMethod(MscorlibBinder::GetMethod(METHOD__ICASTABLE__GETIMPLTYPE));
-        OBJECTREF tokenManagedType = pTokenMT->GetManagedClassObject(); //GC triggers
-
-        PREPARE_NONVIRTUAL_CALLSITE_USING_METHODDESC(pGetImplTypeMD);
+        // Make call to ICastableHelpers.GetImplType(this, interfaceTypeObj)
+        PREPARE_NONVIRTUAL_CALLSITE(METHOD__ICASTABLEHELPERS__GETIMPLTYPE);
 
+        OBJECTREF tokenManagedType = pTokenMT->GetManagedClassObject(); //GC triggers
+        
         DECLARE_ARGHOLDER_ARRAY(args, 2);
         args[ARGNUM_0] = OBJECTREF_TO_ARGHOLDER(*protectedObj);
         args[ARGNUM_1] = OBJECTREF_TO_ARGHOLDER(tokenManagedType);

diff --git a/tests/issues.targets b/tests/issues.targets
index b7301e0..c59ffd0 100644 (file)
--- a/tests/issues.targets
+++ b/tests/issues.targets
@@ -228,9 +228,6 @@
         <ExcludeList Include="$(XunitTestBinBase)\GC\LargeMemory\API\gc\suppressfinalize\suppressfinalize.cmd">
              <Issue>3392</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)\Interop\ICastable\Castable\Castable.cmd">
-             <Issue>needs triage</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)\JIT\Methodical\divrem\div\u8div_cs_do\u8div_cs_do.cmd">
              <Issue>needs triage</Issue>
         </ExcludeList>

diff --git a/tests/ryujit_x86_no_fallback_issues.targets b/tests/ryujit_x86_no_fallback_issues.targets
index 1af80e0..5519a28 100644 (file)
--- a/tests/ryujit_x86_no_fallback_issues.targets
+++ b/tests/ryujit_x86_no_fallback_issues.targets
@@ -14625,9 +14625,6 @@
         <ExcludeList Include="$(XunitTestBinBase)\Interop\MarshalAPI\Copy\CopySingleArray\CopySingleArray.cmd" >
              <Issue>4179</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)\Interop\ICastable\Castable\Castable.cmd" >
-             <Issue>4179</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)\Interop\MarshalAPI\OffsetOf\OffsetOf\OffsetOf.cmd" >
              <Issue>4179</Issue>
         </ExcludeList>

diff --git a/tests/x86_jit32_issues.targets b/tests/x86_jit32_issues.targets
index adee8b3..3bc3b16 100644 (file)
--- a/tests/x86_jit32_issues.targets
+++ b/tests/x86_jit32_issues.targets
@@ -1,9 +1,6 @@
 <Project DefaultTargets = "GetListOfTestCmds"
     xmlns="http://schemas.microsoft.com/developer/msbuild/2003" >
     <ItemGroup Condition="'$(XunitTestBinBase)' != ''">
-        <ExcludeList Include="$(XunitTestBinBase)\Interop\ICastable\Castable\Castable.cmd" >
-         <Issue> Assert failure(PID 4872 [0x00001308], Thread: 13744 [0x35b0]): Consistency check failed: AV in clr at this callstack:</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)\JIT\opt\Inline\regression\mismatch64\mismatch64\mismatch64.cmd" >
              <Issue>'Arg type mismatch: Wanted long (size 8), Got struct (size 4) for call at IL offset 0x16'</Issue>
         </ExcludeList>
@@ -278,4 +275,4 @@
              <Issue>needs triage</Issue>
         </ExcludeList>
     </ItemGroup>
-</Project>
\ No newline at end of file
+</Project>

diff --git a/tests/x86_legacy_backend_issues.targets b/tests/x86_legacy_backend_issues.targets
index a99dc6b..ee66502 100644 (file)
--- a/tests/x86_legacy_backend_issues.targets
+++ b/tests/x86_legacy_backend_issues.targets
@@ -31,9 +31,6 @@
     <ExcludeList Include="$(XunitTestBinBase)\JIT\Directed\rvastatics\rvastatic1\rvastatic1.cmd">
       <Issue>needs triage</Issue>
     </ExcludeList>
-    <ExcludeList Include="$(XunitTestBinBase)\Interop\ICastable\Castable\Castable.cmd">
-      <Issue>needs triage</Issue>
-    </ExcludeList>
     <ExcludeList Include="$(XunitTestBinBase)\JIT\Methodical\Boxing\xlang\_odbgsin_il_cs\_odbgsin_il_cs.cmd">
       <Issue>needs triage</Issue>
     </ExcludeList>
@@ -172,9 +169,6 @@
     <ExcludeList Include="$(XunitTestBinBase)\JIT\Methodical\tailcall\_il_dbgtest_implicit\_il_dbgtest_implicit.cmd">
       <Issue>needs triage</Issue>
     </ExcludeList>
-    <ExcludeList Include="$(XunitTestBinBase)\Interop\ICastable\Castable\Castable.cmd Timed Out">
-      <Issue>needs triage</Issue>
-    </ExcludeList>
     <ExcludeList Include="$(XunitTestBinBase)\JIT\Directed\rvastatics\rvastatic3\rvastatic3.cmd">
       <Issue>needs triage</Issue>
     </ExcludeList>