Skip to content

Commit

Permalink
Mop-up changes per Jakob's PR suggestions
Browse files Browse the repository at this point in the history
1) Rename CORINFO_HELP_STATIC_VIRTUAL_AMBIGUOUS_RESOLUTION to
CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION and put it next
to the other CORINFO_HELP_THROW methods;

2) Add the new helper to CorInfoHelpFunc.cs;

3) Remove the jitinterface member doesFieldBelongToClass;

4) Update the JIT EE GUID.

Thanks

Tomas

Fixes: dotnet#69900
  • Loading branch information
trylek committed Jun 2, 2022
1 parent f055c26 commit 8f7697e
Show file tree
Hide file tree
Showing 18 changed files with 39 additions and 172 deletions.
3 changes: 1 addition & 2 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ enum CorInfoHelpFunc
CORINFO_HELP_THROW_NOT_IMPLEMENTED, // throw NotImplementedException
CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED, // throw PlatformNotSupportedException
CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED, // throw TypeNotSupportedException
CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION, // throw AmbiguousResolutionException for failed static virtual method resolution

CORINFO_HELP_JIT_PINVOKE_BEGIN, // Transition to preemptive mode before a P/Invoke, frame is the first argument
CORINFO_HELP_JIT_PINVOKE_END, // Transition to cooperative mode after a P/Invoke, frame is the first argument
Expand All @@ -646,8 +647,6 @@ enum CorInfoHelpFunc
CORINFO_HELP_VALIDATE_INDIRECT_CALL, // CFG: Validate function pointer
CORINFO_HELP_DISPATCH_INDIRECT_CALL, // CFG: Validate and dispatch to pointer

CORINFO_HELP_STATIC_VIRTUAL_AMBIGUOUS_RESOLUTION, // Throw AmbiguousResolutionException for failed static virtual method resolution

CORINFO_HELP_COUNT,
};

Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/inc/corjit.h
Original file line number Diff line number Diff line change
Expand Up @@ -495,12 +495,6 @@ class ICorJitInfo : public ICorDynamicInfo
uint32_t sizeInBytes /* IN: The size of the buffer. Note that this is effectively a
version number for the CORJIT_FLAGS value. */
) = 0;

// Checks if a field belongs to a given class.
virtual bool doesFieldBelongToClass(
CORINFO_FIELD_HANDLE fldHnd, /* IN: the field that we are checking */
CORINFO_CLASS_HANDLE cls /* IN: the class that we are checking */
) = 0;
};

/**********************************************************************************/
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -710,10 +710,6 @@ uint32_t getJitFlags(
CORJIT_FLAGS* flags,
uint32_t sizeInBytes) override;

bool doesFieldBelongToClass(
CORINFO_FIELD_HANDLE fldHnd,
CORINFO_CLASS_HANDLE cls) override;

/**********************************************************************************/
// clang-format on
/**********************************************************************************/
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* f2a217c4-2a69-4308-99ce-8292c6763776 */
0xf2a217c4,
0x2a69,
0x4308,
{0x99, 0xce, 0x82, 0x92, 0xc6, 0x76, 0x37, 0x76}
constexpr GUID JITEEVersionIdentifier = { /* 5868685e-b877-4ef5-83f0-73d601e50013 */
0x5868685e,
0xb877,
0x4ef5,
{0x83, 0xf0, 0x73, 0xd6, 0x01, 0xe5, 0x00, 0x13}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/inc/jithelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@
JITHELPER(CORINFO_HELP_THROW_NOT_IMPLEMENTED, JIT_ThrowNotImplementedException, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED, JIT_ThrowPlatformNotSupportedException, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED, JIT_ThrowTypeNotSupportedException, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION, JIT_ThrowAmbiguousResolutionException, CORINFO_HELP_SIG_REG_ONLY)

JITHELPER(CORINFO_HELP_JIT_PINVOKE_BEGIN, JIT_PInvokeBegin, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_JIT_PINVOKE_END, JIT_PInvokeEnd, CORINFO_HELP_SIG_REG_ONLY)
Expand Down Expand Up @@ -343,8 +344,6 @@
JITHELPER(CORINFO_HELP_DISPATCH_INDIRECT_CALL, NULL, CORINFO_HELP_SIG_REG_ONLY)
#endif

JITHELPER(CORINFO_HELP_STATIC_VIRTUAL_AMBIGUOUS_RESOLUTION, JIT_StaticVirtualAmbiguousResolution, CORINFO_HELP_SIG_REG_ONLY)

#undef JITHELPER
#undef DYNAMICJITHELPER
#undef JITHELPER
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/ICorJitInfo_API_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,5 @@ DEF_CLR_API(recordRelocation)
DEF_CLR_API(getRelocTypeHint)
DEF_CLR_API(getExpectedTargetArchitecture)
DEF_CLR_API(getJitFlags)
DEF_CLR_API(doesFieldBelongToClass)

#undef DEF_CLR_API
10 changes: 0 additions & 10 deletions src/coreclr/jit/ICorJitInfo_API_wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1690,16 +1690,6 @@ uint32_t WrapICorJitInfo::getJitFlags(
return temp;
}

bool WrapICorJitInfo::doesFieldBelongToClass(
CORINFO_FIELD_HANDLE fldHnd,
CORINFO_CLASS_HANDLE cls)
{
API_ENTER(doesFieldBelongToClass);
bool temp = wrapHnd->doesFieldBelongToClass(fldHnd, cls);
API_LEAVE(doesFieldBelongToClass);
return temp;
}

/**********************************************************************************/
// clang-format on
/**********************************************************************************/
18 changes: 1 addition & 17 deletions src/coreclr/tools/Common/JitInterface/CorInfoBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2549,25 +2549,10 @@ static uint _getJitFlags(IntPtr thisHandle, IntPtr* ppException, CORJIT_FLAGS* f
}
}

[UnmanagedCallersOnly]
static byte _doesFieldBelongToClass(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* fldHnd, CORINFO_CLASS_STRUCT_* cls)
{
var _this = GetThis(thisHandle);
try
{
return _this.doesFieldBelongToClass(fldHnd, cls) ? (byte)1 : (byte)0;
}
catch (Exception ex)
{
*ppException = _this.AllocException(ex);
return default;
}
}


static IntPtr GetUnmanagedCallbacks()
{
void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 173);
void** callbacks = (void**)Marshal.AllocCoTaskMem(sizeof(IntPtr) * 172);

callbacks[0] = (delegate* unmanaged<IntPtr, IntPtr*, CORINFO_METHOD_STRUCT_*, byte>)&_isIntrinsic;
callbacks[1] = (delegate* unmanaged<IntPtr, IntPtr*, CORINFO_METHOD_STRUCT_*, uint>)&_getMethodAttribs;
Expand Down Expand Up @@ -2741,7 +2726,6 @@ static IntPtr GetUnmanagedCallbacks()
callbacks[169] = (delegate* unmanaged<IntPtr, IntPtr*, void*, ushort>)&_getRelocTypeHint;
callbacks[170] = (delegate* unmanaged<IntPtr, IntPtr*, uint>)&_getExpectedTargetArchitecture;
callbacks[171] = (delegate* unmanaged<IntPtr, IntPtr*, CORJIT_FLAGS*, uint, uint>)&_getJitFlags;
callbacks[172] = (delegate* unmanaged<IntPtr, IntPtr*, CORINFO_FIELD_STRUCT_*, CORINFO_CLASS_STRUCT_*, byte>)&_doesFieldBelongToClass;

return (IntPtr)callbacks;
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ which is the right helper to use to allocate an object of a given type. */
CORINFO_HELP_THROW_NOT_IMPLEMENTED, // throw NotImplementedException
CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED, // throw PlatformNotSupportedException
CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED, // throw TypeNotSupportedException
CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION, // throw AmbiguousResolutionException for failed static virtual method resolution

CORINFO_HELP_JIT_PINVOKE_BEGIN, // Transition to preemptive mode before a P/Invoke, frame is the first argument
CORINFO_HELP_JIT_PINVOKE_END, // Transition to cooperative mode after a P/Invoke, frame is the first argument
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,4 +325,3 @@ FUNCTIONS
uint16_t getRelocTypeHint(void* target)
uint32_t getExpectedTargetArchitecture()
uint32_t getJitFlags(CORJIT_FLAGS* flags, uint32_t sizeInBytes)
bool doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls)
11 changes: 0 additions & 11 deletions src/coreclr/tools/aot/jitinterface/jitinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ struct JitInterfaceCallbacks
uint16_t (* getRelocTypeHint)(void * thisHandle, CorInfoExceptionClass** ppException, void* target);
uint32_t (* getExpectedTargetArchitecture)(void * thisHandle, CorInfoExceptionClass** ppException);
uint32_t (* getJitFlags)(void * thisHandle, CorInfoExceptionClass** ppException, CORJIT_FLAGS* flags, uint32_t sizeInBytes);
bool (* doesFieldBelongToClass)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls);

};

Expand Down Expand Up @@ -1858,14 +1857,4 @@ class JitInterfaceWrapper : public ICorJitInfo
if (pException != nullptr) throw pException;
return temp;
}

virtual bool doesFieldBelongToClass(
CORINFO_FIELD_HANDLE fldHnd,
CORINFO_CLASS_HANDLE cls)
{
CorInfoExceptionClass* pException = nullptr;
bool temp = _callbacks->doesFieldBelongToClass(_thisHandle, &pException, fldHnd, cls);
if (pException != nullptr) throw pException;
return temp;
}
};
10 changes: 0 additions & 10 deletions src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2034,13 +2034,3 @@ bool interceptor_ICJI::notifyInstructionSetUsage(CORINFO_InstructionSet instruct
{
return original_ICorJitInfo->notifyInstructionSetUsage(instructionSet, supported);
}

bool interceptor_ICJI::doesFieldBelongToClass(
CORINFO_FIELD_HANDLE fldHnd,
CORINFO_CLASS_HANDLE cls)
{
mc->cr->AddCall("doesFieldBelongToClass");
bool result = original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
mc->recDoesFieldBelongToClass(fldHnd, cls, result);
return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -1389,11 +1389,3 @@ uint32_t interceptor_ICJI::getJitFlags(
return original_ICorJitInfo->getJitFlags(flags, sizeInBytes);
}

bool interceptor_ICJI::doesFieldBelongToClass(
CORINFO_FIELD_HANDLE fldHnd,
CORINFO_CLASS_HANDLE cls)
{
mcs->AddCall("doesFieldBelongToClass");
return original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
}

Original file line number Diff line number Diff line change
Expand Up @@ -1217,10 +1217,3 @@ uint32_t interceptor_ICJI::getJitFlags(
return original_ICorJitInfo->getJitFlags(flags, sizeInBytes);
}

bool interceptor_ICJI::doesFieldBelongToClass(
CORINFO_FIELD_HANDLE fldHnd,
CORINFO_CLASS_HANDLE cls)
{
return original_ICorJitInfo->doesFieldBelongToClass(fldHnd, cls);
}

7 changes: 0 additions & 7 deletions src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1576,13 +1576,6 @@ uint32_t MyICJI::getJitFlags(CORJIT_FLAGS* jitFlags, uint32_t sizeInBytes)
return ret;
}

bool MyICJI::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls)
{
jitInstance->mc->cr->AddCall("doesFieldBelongToClass");
bool result = jitInstance->mc->repDoesFieldBelongToClass(fldHnd, cls);
return result;
}

// Runs the given function with the given parameter under an error trap
// and returns true if the function completes successfully. We fake this
// up a bit for SuperPMI and simply catch all exceptions.
Expand Down
57 changes: 29 additions & 28 deletions src/coreclr/vm/jithelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3375,34 +3375,6 @@ NOINLINE HCIMPL3(CORINFO_MethodPtr, JIT_VirtualFunctionPointer_Framed, Object *
}
HCIMPLEND

HCIMPL3(void, JIT_StaticVirtualAmbiguousResolution,
MethodDesc *method,
MethodTable *interfaceType,
MethodTable *targetType)
{
FCALL_CONTRACT;

SString strMethodName;
SString strInterfaceName;
SString strTargetClassName;

HELPER_METHOD_FRAME_BEGIN_0(); // Set up a frame

TypeString::AppendMethod(strMethodName, method, method->GetMethodInstantiation());
TypeString::AppendType(strInterfaceName, TypeHandle(interfaceType));
TypeString::AppendType(strTargetClassName, targetType);

HELPER_METHOD_FRAME_END(); // Set up a frame

FCThrowExVoid(
kAmbiguousImplementationException,
IDS_CLASSLOAD_AMBIGUOUS_OVERRIDE,
strMethodName,
strInterfaceName,
strTargetClassName);
}
HCIMPLEND

HCIMPL1(Object*, JIT_GetRuntimeFieldStub, CORINFO_FIELD_HANDLE field)
{
FCALL_CONTRACT;
Expand Down Expand Up @@ -4181,6 +4153,35 @@ HCIMPL0(void, JIT_ThrowTypeNotSupportedException)
}
HCIMPLEND

/*********************************************************************/
HCIMPL3(void, JIT_ThrowAmbiguousResolutionException,
MethodDesc *method,
MethodTable *interfaceType,
MethodTable *targetType)
{
FCALL_CONTRACT;

SString strMethodName;
SString strInterfaceName;
SString strTargetClassName;

HELPER_METHOD_FRAME_BEGIN_0(); // Set up a frame

TypeString::AppendMethod(strMethodName, method, method->GetMethodInstantiation());
TypeString::AppendType(strInterfaceName, TypeHandle(interfaceType));
TypeString::AppendType(strTargetClassName, targetType);

HELPER_METHOD_FRAME_END(); // Set up a frame

FCThrowExVoid(
kAmbiguousImplementationException,
IDS_CLASSLOAD_AMBIGUOUS_OVERRIDE,
strMethodName,
strInterfaceName,
strTargetClassName);
}
HCIMPLEND

/*********************************************************************/
HCIMPL0(void, JIT_Overflow)
{
Expand Down
52 changes: 1 addition & 51 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5433,7 +5433,7 @@ void CEEInfo::getCallInfo(
// shared generics is covered by the ConstrainedMethodEntrySlot dictionary entry.
pResult->kind = CORINFO_CALL;
pResult->accessAllowed = CORINFO_ACCESS_ILLEGAL;
pResult->callsiteCalloutHelper.helperNum = CORINFO_HELP_STATIC_VIRTUAL_AMBIGUOUS_RESOLUTION;
pResult->callsiteCalloutHelper.helperNum = CORINFO_HELP_THROW_AMBIGUOUS_RESOLUTION_EXCEPTION;
pResult->callsiteCalloutHelper.numArgs = 3;
pResult->callsiteCalloutHelper.args[0].methodHandle = (CORINFO_METHOD_HANDLE)pMD;
pResult->callsiteCalloutHelper.args[0].argType = CORINFO_HELPER_ARG_TYPE_Method;
Expand Down Expand Up @@ -11617,50 +11617,6 @@ uint32_t CEEJitInfo::getExpectedTargetArchitecture()
return IMAGE_FILE_MACHINE_NATIVE;
}

bool CEEJitInfo::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fldHnd, CORINFO_CLASS_HANDLE cls)
{
CONTRACTL {
THROWS;
GC_TRIGGERS;
MODE_PREEMPTIVE;
} CONTRACTL_END;

bool result = false;

JIT_TO_EE_TRANSITION();

FieldDesc* field = (FieldDesc*) fldHnd;
TypeHandle th(cls);

_ASSERTE(!field->IsStatic());

// doesFieldBelongToClass implements the predicate of...
// if field is not associated with the class in any way, return false.
// if field is the only FieldDesc that the JIT might see for a given class handle
// and logical field pair then return true. This is needed as the field handle here
// is used as a key into a hashtable mapping writes to fields to value numbers.
//
// In the CoreCLR VM implementation, verifying that the canonical MethodTable of
// the field matches the type found via GetExactDeclaringType, as all instance fields
// are only held on the canonical MethodTable.
// This yields a truth table such as

// BaseType._field, BaseType -> true
// BaseType._field, DerivedType -> true
// BaseType<__Canon>._field, BaseType<__Canon> -> true
// BaseType<__Canon>._field, BaseType<string> -> true
// BaseType<__Canon>._field, BaseType<object> -> true
// BaseType<sbyte>._field, BaseType<sbyte> -> true
// BaseType<sbyte>._field, BaseType<byte> -> false

MethodTable* pMT = field->GetExactDeclaringType(th.GetMethodTable());
result = (pMT != nullptr) && (pMT->GetCanonicalMethodTable() == field->GetApproxEnclosingMethodTable()->GetCanonicalMethodTable());

EE_TO_JIT_TRANSITION();

return result;
}

void CEEInfo::JitProcessShutdownWork()
{
LIMITED_METHOD_CONTRACT;
Expand Down Expand Up @@ -14283,12 +14239,6 @@ uint32_t CEEInfo::getExpectedTargetArchitecture()
return IMAGE_FILE_MACHINE_NATIVE;
}

bool CEEInfo::doesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls)
{
LIMITED_METHOD_CONTRACT;
UNREACHABLE_RET(); // only called on derived class.
}

void CEEInfo::setBoundaries(CORINFO_METHOD_HANDLE ftn, ULONG32 cMap,
ICorDebugInfo::OffsetMapping *pMap)
{
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/vm/jitinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -670,8 +670,6 @@ class CEEJitInfo : public CEEInfo

uint32_t getExpectedTargetArchitecture() override final;

bool doesFieldBelongToClass(CORINFO_FIELD_HANDLE fld, CORINFO_CLASS_HANDLE cls) override final;

void ResetForJitRetry()
{
CONTRACTL {
Expand Down

0 comments on commit 8f7697e

Please sign in to comment.