Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Commit b7d43e7

Browse files
author
Fadi Hanna
committed
Bug fixes for dynamic reflection invoke using the call converter:
1) Rassing InvokeUtils.DynamicInvokeParamType.Ref flag to InvokeUtils.DynamicInvokeParamHelperCore() only when the argument is explicitly passed byref (method signature has 'byref'). 2) When the argument type of the callee was a reference type, we were using typeof(object) as the arg type, and calling InvokeUtils.DynamicInvokeParamHelperCore (which checks for argument types). This is incorrect, and we should use the exact typehandle of the argument type when calling InvokeUtils.DynamicInvokeParamHelperCore. To fix this, I changed all the call converter APIs to always return the TypeHandle of the argument/return type, and not just in the cases of valuetypes. There is no reason to return default(TypeHandle) for reference types. There is an API called IsValueType on TypeHandle. If we need to check if a th is valuetype, just call the IsValueType API instead of .IsNull (which makes a lot more sense really). 3) Reflection invokers emulated by the calling convention converter, and returning void, need to return a 'null' object back to the caller (the signature of the invoker has return type of object. The statically compiled versions of these invokers return null in the "return void" scenarios) [tfs-changeset: 1679620]
1 parent 95630dd commit b7d43e7

File tree

3 files changed

+95
-98
lines changed

3 files changed

+95
-98
lines changed

‎src/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/CallConverterThunk.CallConversionParameters.cs‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -378,10 +378,11 @@ private object BoxedCallerFirstArgument
378378

379379
byte* pSrc = _callerTransitionBlock + ofsCaller;
380380

381-
TypeHandle thValueType;
382-
_callerArgs.GetArgType(out thValueType);
381+
TypeHandle thArgType;
382+
_callerArgs.GetArgType(out thArgType);
383+
Debug.Assert(!thArgType.IsNull());
383384

384-
if (thValueType.IsNull())
385+
if (!thArgType.IsValueType())
385386
{
386387
Debug.Assert(_callerArgs.GetArgSize() == IntPtr.Size);
387388

@@ -395,7 +396,7 @@ private object BoxedCallerFirstArgument
395396
}
396397
else
397398
{
398-
RuntimeTypeHandle argEEType = thValueType.GetRuntimeTypeHandle();
399+
RuntimeTypeHandle argEEType = thArgType.GetRuntimeTypeHandle();
399400

400401
if (_callerArgs.IsArgPassedByRef())
401402
{
@@ -506,7 +507,7 @@ internal void* CallerReturnBuffer
506507
// We'll need to create a return buffer, or assign into the return buffer when the actual call completes.
507508
if (_calleeArgs.HasRetBuffArg())
508509
{
509-
TypeHandle thValueType;
510+
TypeHandle thRetType;
510511
bool forceByRefUnused;
511512
void* callerRetBuffer = null;
512513

@@ -516,8 +517,9 @@ internal void* CallerReturnBuffer
516517
// value, of the same type as the return value type handle in the callee's arguments.
517518
Debug.Assert(!_callerArgs.HasRetBuffArg());
518519

519-
CorElementType returnType = _calleeArgs.GetReturnType(out thValueType, out forceByRefUnused);
520-
RuntimeTypeHandle returnValueType = thValueType.IsNull() ? typeof(object).TypeHandle : thValueType.GetRuntimeTypeHandle();
520+
CorElementType returnType = _calleeArgs.GetReturnType(out thRetType, out forceByRefUnused);
521+
Debug.Assert(!thRetType.IsNull());
522+
RuntimeTypeHandle returnValueType = thRetType.IsValueType() ? thRetType.GetRuntimeTypeHandle() : typeof(object).TypeHandle;
521523
s_pinnedGCHandles._returnObjectHandle.Target = RuntimeAugments.RawNewObject(returnValueType);
522524

523525
// The transition block has a space reserved for storing return buffer data. This is protected conservatively.
@@ -537,8 +539,8 @@ internal void* CallerReturnBuffer
537539
callerRetBuffer = _callerTransitionBlock + TransitionBlock.GetOffsetOfReturnValuesBlock();
538540

539541
// Make sure buffer is nulled out, and setup the return buffer location.
540-
CorElementType returnType = _callerArgs.GetReturnType(out thValueType, out forceByRefUnused);
541-
int returnSize = TypeHandle.GetElemSize(returnType, thValueType);
542+
CorElementType returnType = _callerArgs.GetReturnType(out thRetType, out forceByRefUnused);
543+
int returnSize = TypeHandle.GetElemSize(returnType, thRetType);
542544
CallConverterThunk.memzeroPointerAligned((byte*)callerRetBuffer, returnSize);
543545
}
544546

@@ -630,20 +632,21 @@ internal IntPtr InvokeObjectArrayDelegate(object[] arguments)
630632
Debug.Assert(targetDelegate != null);
631633

632634
object result = targetDelegate(arguments ?? Array.Empty<object>());
633-
TypeHandle thValueType;
634-
bool forceByRefUnused;
635635

636-
_calleeArgs.GetReturnType(out thValueType, out forceByRefUnused);
636+
TypeHandle thArgType;
637+
bool forceByRefUnused;
638+
_calleeArgs.GetReturnType(out thArgType, out forceByRefUnused);
639+
Debug.Assert(!thArgType.IsNull());
637640

638641
unsafe
639642
{
640-
if (!thValueType.IsNull() && thValueType.GetRuntimeTypeHandle().ToEETypePtr()->IsNullable)
643+
if (thArgType.IsValueType() && thArgType.GetRuntimeTypeHandle().ToEETypePtr()->IsNullable)
641644
{
642-
object nullableObj = RuntimeAugments.RawNewObject(thValueType.GetRuntimeTypeHandle());
645+
object nullableObj = RuntimeAugments.RawNewObject(thArgType.GetRuntimeTypeHandle());
643646
s_pinnedGCHandles._returnObjectHandle.Target = nullableObj;
644647
if (result != null)
645648
{
646-
RuntimeAugments.StoreValueTypeField(ref RuntimeAugments.GetRawData(nullableObj), result, thValueType.GetRuntimeTypeHandle());
649+
RuntimeAugments.StoreValueTypeField(ref RuntimeAugments.GetRawData(nullableObj), result, thArgType.GetRuntimeTypeHandle());
647650
}
648651
}
649652
else

‎src/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/CallConverterThunk.cs‎

Lines changed: 43 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,8 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
611611
int ofsCallee;
612612
int ofsCaller;
613613
TypeHandle thDummy;
614-
TypeHandle thValueType;
614+
TypeHandle thArgType;
615+
TypeHandle thRetType;
615616
IntPtr argPtr;
616617
#if CALLDESCR_FPARGREGS
617618
FloatArgumentRegisters* pFloatArgumentRegisters = null;
@@ -688,7 +689,8 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
688689
InvokeUtils.DynamicInvokeParamLookupType paramLookupType;
689690

690691
RuntimeTypeHandle argumentRuntimeTypeHandle;
691-
CorElementType argType = conversionParams._calleeArgs.GetArgType(out thValueType);
692+
CorElementType argType = conversionParams._calleeArgs.GetArgType(out thArgType);
693+
Debug.Assert(!thArgType.IsNull());
692694

693695
if (argType == CorElementType.ELEMENT_TYPE_BYREF)
694696
{
@@ -700,14 +702,15 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
700702
}
701703
else
702704
{
703-
argumentRuntimeTypeHandle = (thValueType.IsNull() ? typeof(object).TypeHandle : thValueType.GetRuntimeTypeHandle());
705+
// We need to check the exact type handle of the argument being passed during reflection invoke scenarios.
706+
argumentRuntimeTypeHandle = thArgType.GetRuntimeTypeHandle();
704707
}
705708

706709
object invokeParam = InvokeUtils.DynamicInvokeParamHelperCore(
707710
argumentRuntimeTypeHandle,
708711
out paramLookupType,
709712
out index,
710-
conversionParams._calleeArgs.IsArgPassedByRef() ? InvokeUtils.DynamicInvokeParamType.Ref : InvokeUtils.DynamicInvokeParamType.In);
713+
argType == CorElementType.ELEMENT_TYPE_BYREF ? InvokeUtils.DynamicInvokeParamType.Ref : InvokeUtils.DynamicInvokeParamType.In);
711714

712715
if (paramLookupType == InvokeUtils.DynamicInvokeParamLookupType.ValuetypeObjectReturned)
713716
{
@@ -786,9 +789,10 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
786789

787790
argumentsAsObjectArray = argumentsAsObjectArray ?? new object[conversionParams._callerArgs.NumFixedArgs()];
788791

789-
conversionParams._callerArgs.GetArgType(out thValueType);
792+
conversionParams._callerArgs.GetArgType(out thArgType);
793+
Debug.Assert(!thArgType.IsNull());
790794

791-
if (thValueType.IsNull())
795+
if (!thArgType.IsValueType())
792796
{
793797
Debug.Assert(!isCallerArgPassedByRef);
794798
Debug.Assert(conversionParams._callerArgs.GetArgSize() == IntPtr.Size);
@@ -798,11 +802,11 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
798802
{
799803
if (isCallerArgPassedByRef)
800804
{
801-
argumentsAsObjectArray[arg] = RuntimeAugments.Box(thValueType.GetRuntimeTypeHandle(), new IntPtr(*((void**)pSrc)));
805+
argumentsAsObjectArray[arg] = RuntimeAugments.Box(thArgType.GetRuntimeTypeHandle(), new IntPtr(*((void**)pSrc)));
802806
}
803807
else
804808
{
805-
argumentsAsObjectArray[arg] = RuntimeAugments.Box(thValueType.GetRuntimeTypeHandle(), new IntPtr(pSrc));
809+
argumentsAsObjectArray[arg] = RuntimeAugments.Box(thArgType.GetRuntimeTypeHandle(), new IntPtr(pSrc));
806810
}
807811
}
808812
}
@@ -817,7 +821,7 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
817821
}
818822
else
819823
{
820-
CorElementType argElemType = conversionParams._calleeArgs.GetArgType(out thValueType);
824+
CorElementType argElemType = conversionParams._calleeArgs.GetArgType(out thArgType);
821825
ExtendingCopy_NoWriteBarrier(pSrc, pDest, argElemType, stackSizeCaller);
822826
}
823827
}
@@ -834,7 +838,7 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
834838
{
835839
// Copy into the destination the data pointed at by the pointer in the source(caller) data.
836840
byte* pRealSrc = *(byte**)pSrc;
837-
CorElementType argElemType = conversionParams._calleeArgs.GetArgType(out thValueType);
841+
CorElementType argElemType = conversionParams._calleeArgs.GetArgType(out thArgType);
838842
ExtendingCopy_NoWriteBarrier(pRealSrc, pDest, argElemType, stackSizeCaller);
839843
}
840844
}
@@ -970,16 +974,17 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
970974
// the target method called by the delegate. Use the callee's ArgIterator instead to get the return type info
971975
if (conversionParams._conversionInfo.IsAnyDynamicInvokerThunk)
972976
{
973-
returnType = conversionParams._calleeArgs.GetReturnType(out thValueType, out forceByRefUnused);
977+
returnType = conversionParams._calleeArgs.GetReturnType(out thRetType, out forceByRefUnused);
974978
}
975979
else
976980
{
977-
returnType = conversionParams._callerArgs.GetReturnType(out thValueType, out forceByRefUnused);
981+
returnType = conversionParams._callerArgs.GetReturnType(out thRetType, out forceByRefUnused);
978982
}
979-
int returnSize = TypeHandle.GetElemSize(returnType, thValueType);
983+
Debug.Assert(!thRetType.IsNull());
984+
int returnSize = TypeHandle.GetElemSize(returnType, thRetType);
980985

981986
// Unbox result of object array delegate call
982-
if (conversionParams._conversionInfo.IsObjectArrayDelegateThunk && !thValueType.IsNull() && pinnedResultObject != IntPtr.Zero)
987+
if (conversionParams._conversionInfo.IsObjectArrayDelegateThunk && thRetType.IsValueType() && pinnedResultObject != IntPtr.Zero)
983988
pinnedResultObject += IntPtr.Size;
984989

985990
// Process return values
@@ -1009,7 +1014,7 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
10091014

10101015
bool useGCSafeCopy = false;
10111016

1012-
if ((returnType == CorElementType.ELEMENT_TYPE_CLASS) || !thValueType.IsNull())
1017+
if ((returnType == CorElementType.ELEMENT_TYPE_CLASS) || thRetType.IsValueType())
10131018
{
10141019
// The GC Safe copy assumes that memory pointers are pointer-aligned and copy length is a multiple of pointer-size
10151020
if (isPointerAligned(incomingRetBufPointer) && isPointerAligned(sourceBuffer) && (returnSize % sizeof(IntPtr) == 0))
@@ -1082,7 +1087,7 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
10821087

10831088
if (conversionParams._conversionInfo.IsObjectArrayDelegateThunk)
10841089
{
1085-
if (!thValueType.IsNull())
1090+
if (thRetType.IsValueType())
10861091
{
10871092
returnValueToCopy = (void*)pinnedResultObject;
10881093
#if _TARGET_X86_
@@ -1107,22 +1112,30 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
11071112
#endif
11081113
}
11091114
}
1110-
else if (conversionParams._conversionInfo.IsAnyDynamicInvokerThunk && !thValueType.IsNull())
1115+
else if (conversionParams._conversionInfo.IsAnyDynamicInvokerThunk && thRetType.IsValueType())
11111116
{
11121117
Debug.Assert(returnValueToCopy != null);
11131118

1114-
if (!conversionParams._callerArgs.HasRetBuffArg() && conversionParams._calleeArgs.HasRetBuffArg())
1115-
returnValueToCopy = (void*)(new IntPtr(*((void**)returnValueToCopy)) + IntPtr.Size);
1119+
if (conversionParams._calleeArgs.GetReturnType(out thDummy, out dummyBool) == CorElementType.ELEMENT_TYPE_VOID)
1120+
{
1121+
// Invokers returning void need to return a null object
1122+
returnValueToCopy = null;
1123+
}
1124+
else
1125+
{
1126+
if (!conversionParams._callerArgs.HasRetBuffArg() && conversionParams._calleeArgs.HasRetBuffArg())
1127+
returnValueToCopy = (void*)(new IntPtr(*((void**)returnValueToCopy)) + IntPtr.Size);
11161128

1117-
// Need to box value type before returning it
1118-
object returnValue = RuntimeAugments.Box(thValueType.GetRuntimeTypeHandle(), new IntPtr(returnValueToCopy));
1119-
CallConversionParameters.s_pinnedGCHandles._returnObjectHandle.Target = returnValue;
1120-
pinnedResultObject = CallConversionParameters.s_pinnedGCHandles._returnObjectHandle.GetRawTargetAddress();
1121-
returnValueToCopy = (void*)&pinnedResultObject;
1129+
// Need to box value type before returning it
1130+
object returnValue = RuntimeAugments.Box(thRetType.GetRuntimeTypeHandle(), new IntPtr(returnValueToCopy));
1131+
CallConversionParameters.s_pinnedGCHandles._returnObjectHandle.Target = returnValue;
1132+
pinnedResultObject = CallConversionParameters.s_pinnedGCHandles._returnObjectHandle.GetRawTargetAddress();
1133+
returnValueToCopy = (void*)&pinnedResultObject;
1134+
}
11221135
// Since we've changed the returnValueToCopy here, we need to update the idea of what we are returning
11231136
returnType = CorElementType.ELEMENT_TYPE_OBJECT;
1124-
thValueType = default(TypeHandle);
1125-
returnSize = TypeHandle.GetElemSize(returnType, thValueType);
1137+
thRetType = default(TypeHandle);
1138+
returnSize = TypeHandle.GetElemSize(returnType, thRetType);
11261139

11271140
#if _TARGET_X86_
11281141
((TransitionBlock*)callerTransitionBlock)->m_returnBlock.returnValue = pinnedResultObject;
@@ -1188,23 +1201,14 @@ unsafe private static void InvokeTarget(void* allocatedStackBuffer, ref CallConv
11881201
return;
11891202
#else
11901203
// If we reach here, we are returning value in the integer registers.
1191-
if (conversionParams._conversionInfo.IsObjectArrayDelegateThunk && (!thValueType.IsNull()))
1204+
if (returnValueToCopy == null)
11921205
{
1193-
if (returnValueToCopy == null)
1194-
{
1195-
// object array delegate thunk result is a null object. We'll fill the return buffer with 'returnSize' zeros in that case
1196-
memzeroPointer(callerTransitionBlock + TransitionBlock.GetOffsetOfArgumentRegisters(), returnSize);
1197-
}
1198-
else
1199-
{
1200-
ExtendingCopy_WriteBarrier(returnValueToCopy, callerTransitionBlock + TransitionBlock.GetOffsetOfArgumentRegisters(), returnType, returnSize);
1201-
}
1206+
// Return result is a null object. We'll fill the return buffer with 'returnSize' zeros in that case
1207+
memzeroPointer(callerTransitionBlock + TransitionBlock.GetOffsetOfArgumentRegisters(), returnSize);
12021208
}
12031209
else
12041210
{
1205-
Debug.Assert(returnValueToCopy != null);
1206-
1207-
ExtendingCopy_NoWriteBarrier(returnValueToCopy, callerTransitionBlock + TransitionBlock.GetOffsetOfArgumentRegisters(), returnType, returnSize);
1211+
ExtendingCopy_WriteBarrier(returnValueToCopy, callerTransitionBlock + TransitionBlock.GetOffsetOfArgumentRegisters(), returnType, returnSize);
12081212
}
12091213
conversionParams._invokeReturnValue = ReturnIntegerPointReturnThunk;
12101214
#endif

0 commit comments

Comments
 (0)