Fix a bug in implementation of ICastable when CoreCLR throws exception on IL isinst...

Correct behavior:   If false is returned when IsInstanceOfInterface() called as part of a castclass then the usual InvalidCastException
will be thrown unless an alternate exception is assigned to the castError output parameter.
This parameter is ignored on successful casts or during the evaluation of an isinst (which returns null rather than throwing on error).

diff --git a/src/vm/interpreter.cpp b/src/vm/interpreter.cpp
index 4d6a47d..614bb87 100644 (file)
--- a/src/vm/interpreter.cpp
+++ b/src/vm/interpreter.cpp
@@ -6172,10 +6172,9 @@ void Interpreter::CastClass()
     Object * pObj = OpStackGet<Object*>(idx);
     if (pObj != NULL)
     {
-        if (!ObjIsInstanceOf(pObj, TypeHandle(cls)))
+        if (!ObjIsInstanceOf(pObj, TypeHandle(cls), TRUE))
         {
-            OBJECTREF oref = ObjectToOBJECTREF(OpStackGet<Object*>(idx));
-            COMPlusThrowInvalidCastException(&oref, TypeHandle(cls));
+            UNREACHABLE(); //ObjIsInstanceOf will throw if cast can't be done
         }
     }
 
@@ -8822,8 +8821,10 @@ void Interpreter::UnboxAny()
     if ((boxTypeAttribs & CORINFO_FLG_VALUECLASS) == 0)
     {
         Object* obj = OpStackGet<Object*>(tos);
-        if (obj != NULL && !ObjIsInstanceOf(obj, TypeHandle(boxTypeClsHnd)))
-            COMPlusThrowInvalidCastException(&ObjectToOBJECTREF(obj), TypeHandle(boxTypeClsHnd));
+        if (obj != NULL && !ObjIsInstanceOf(obj, TypeHandle(boxTypeClsHnd), TRUE))
+        {
+            UNREACHABLE(); //ObjIsInstanceOf will throw if cast can't be done
+        }
     }
     else
     {

diff --git a/src/vm/jithelpers.cpp b/src/vm/jithelpers.cpp
index dc1a76c..01b6715 100644 (file)
--- a/src/vm/jithelpers.cpp
+++ b/src/vm/jithelpers.cpp
@@ -2395,7 +2395,7 @@ TypeHandle::CastResult STDCALL ObjIsInstanceOfNoGC(Object *pObject, TypeHandle t
     return pMT->CanCastToClassOrInterfaceNoGC(toTypeHnd.AsMethodTable());
 }
 
-BOOL ObjIsInstanceOf(Object *pObject, TypeHandle toTypeHnd)
+BOOL ObjIsInstanceOf(Object *pObject, TypeHandle toTypeHnd, BOOL throwCastException)
 {
     CONTRACTL {
         THROWS;
@@ -2464,7 +2464,7 @@ BOOL ObjIsInstanceOf(Object *pObject, TypeHandle toTypeHnd)
         CALL_MANAGED_METHOD(fCast, BOOL, args);
         INDEBUG(managedType = NULL); // managedType isn't protected during the call
 
-        if (exception != NULL)
+        if (!fCast && throwCastException && exception != NULL)
         {
             RealCOMPlusThrow(exception);
         }
@@ -2472,7 +2472,12 @@ BOOL ObjIsInstanceOf(Object *pObject, TypeHandle toTypeHnd)
     }
 #endif // FEATURE_ICASTABLE
 
-    GCPROTECT_END();
+    if (!fCast && throwCastException) 
+    {
+        COMPlusThrowInvalidCastException(&obj, toTypeHnd);
+    }    
+
+    GCPROTECT_END(); // obj
 
     return(fCast);
 }
@@ -2805,8 +2810,10 @@ NOINLINE HCIMPL2(Object *, JITutil_ChkCastAny, CORINFO_CLASS_HANDLE type, Object
     TypeHandle clsHnd(type);
 
     HELPER_METHOD_FRAME_BEGIN_RET_1(oref);
-    if (!ObjIsInstanceOf(OBJECTREFToObject(oref), clsHnd))
-        COMPlusThrowInvalidCastException(&oref, clsHnd);
+    if (!ObjIsInstanceOf(OBJECTREFToObject(oref), clsHnd, TRUE))
+    {
+        UNREACHABLE(); //ObjIsInstanceOf will throw if cast can't be done
+    }
     HELPER_METHOD_FRAME_END();
 
     return OBJECTREFToObject(oref);

diff --git a/src/vm/jitinterface.h b/src/vm/jitinterface.h
index 6c1c7dd..59ddda5 100644 (file)
--- a/src/vm/jitinterface.h
+++ b/src/vm/jitinterface.h
@@ -1616,7 +1616,7 @@ FCDECL0(VOID, JIT_PollGC);
 EXTERN_C FCDECL0(VOID, JIT_PollGC_Nop);
 #endif
 
-BOOL ObjIsInstanceOf(Object *pObject, TypeHandle toTypeHnd);
+BOOL ObjIsInstanceOf(Object *pObject, TypeHandle toTypeHnd, BOOL throwCastException = FALSE);
 EXTERN_C TypeHandle::CastResult STDCALL ObjIsInstanceOfNoGC(Object *pObject, TypeHandle toTypeHnd);
 
 #ifdef _WIN64

diff --git a/src/vm/prestub.cpp b/src/vm/prestub.cpp
index 691be10..c9a45bc 100644 (file)
--- a/src/vm/prestub.cpp
+++ b/src/vm/prestub.cpp
@@ -2617,21 +2617,17 @@ extern "C" SIZE_T STDCALL DynamicHelperWorker(TransitionBlock * pTransitionBlock
         {
         case ENCODE_ISINSTANCEOF_HELPER:
         case ENCODE_CHKCAST_HELPER:
-            if (*(Object **)pArgument == NULL || ObjIsInstanceOf(*(Object **)pArgument, th))
             {
-                result = (SIZE_T)(*(Object **)pArgument);
-            }
-            else
-            {
-                if (kind == ENCODE_CHKCAST_HELPER)
+                BOOL throwInvalidCast = (kind == ENCODE_CHKCAST_HELPER);
+                if (*(Object **)pArgument == NULL || ObjIsInstanceOf(*(Object **)pArgument, th, throwInvalidCast))
                 {
-                    OBJECTREF obj = ObjectToOBJECTREF(*(Object **)pArgument);
-                    GCPROTECT_BEGIN(obj);
-                    COMPlusThrowInvalidCastException(&obj, th);
-                    GCPROTECT_END();
+                    result = (SIZE_T)(*(Object **)pArgument);
+                }
+                else
+                {
+                    _ASSERTE (!throwInvalidCast);
+                    result = NULL;
                 }
- 
-                result = NULL;
             }
             break;
         case ENCODE_STATIC_BASE_NONGC_HELPER:

diff --git a/tests/src/Interop/ICastable/Castable.cs b/tests/src/Interop/ICastable/Castable.cs
index 56a0efb..f7f9475 100644 (file)
--- a/tests/src/Interop/ICastable/Castable.cs
+++ b/tests/src/Interop/ICastable/Castable.cs
@@ -183,6 +183,11 @@ public class Program
             } 
             catch (CastableException) {}
 
+            Assert(!(nullCastable is IRetThis), "null castable shouldn't be allowed to be casted to anything");
+
+            var shouldBeNull = nullCastable as IRetThis;
+            Assert(shouldBeNull == null, "shouldBeNull should be assigned null");
+
             object badCastable = new BadCastable();
             try
             {