Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Enable FEATURE_STUBS_AS_IL for ARM/Linux #6500

Merged
merged 1 commit into from
Aug 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clr.coreclr.props
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
<FeaturePal>true</FeaturePal>
<FeatureXplatEventSource>true</FeatureXplatEventSource>

<FeatureStubsAsIL Condition="'$(TargetArch)' != 'arm'">true</FeatureStubsAsIL>
<FeatureStubsAsIL>true</FeatureStubsAsIL>

<!-- Windows specific features -->
<FeatureWin32Registry>false</FeatureWin32Registry>
Expand Down
2 changes: 1 addition & 1 deletion clrdefinitions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ endif(CLR_CMAKE_TARGET_ARCH_AMD64 OR CLR_CMAKE_TARGET_ARCH_I386)
add_definitions(-DFEATURE_STANDALONE_SN)
add_definitions(-DFEATURE_STRONGNAME_DELAY_SIGNING_ALLOWED)
add_definitions(-DFEATURE_STRONGNAME_MIGRATION)
if ((CLR_CMAKE_PLATFORM_UNIX OR CLR_CMAKE_TARGET_ARCH_ARM64) AND NOT CLR_CMAKE_TARGET_ARCH_ARM)
if (CLR_CMAKE_PLATFORM_UNIX OR CLR_CMAKE_TARGET_ARCH_ARM64)
add_definitions(-DFEATURE_STUBS_AS_IL)
endif ()
add_definitions(-DFEATURE_SVR_GC)
Expand Down
15 changes: 10 additions & 5 deletions src/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,11 @@ TODO: Talk about initializing strutures before use
#if COR_JIT_EE_VERSION > 460

// Update this one
SELECTANY const GUID JITEEVersionIdentifier = { /* 718c4238-2a85-45de-88ad-9b1fed806547 */
0x718c4238,
0x2a85,
0x45de,
{ 0x88, 0xad, 0x9b, 0x1f, 0xed, 0x80, 0x65, 0x47 }
SELECTANY const GUID JITEEVersionIdentifier = { /* 0b17dfeb-1ead-4e06-b025-d60d3a493b53 */
0x0b17dfeb,
0x1ead,
0x4e06,
{ 0xb0, 0x25, 0xd6, 0x0d, 0x3a, 0x49, 0x3b, 0x53 }
};

#else
Expand Down Expand Up @@ -1686,6 +1686,8 @@ struct CORINFO_CALL_INFO
};

CORINFO_CONST_LOOKUP instParamLookup; // Used by Ready-to-Run

BOOL secureDelegateInvoke;
};

//----------------------------------------------------------------------------
Expand Down Expand Up @@ -1808,6 +1810,9 @@ struct CORINFO_EE_INFO
unsigned offsetOfDelegateInstance;
unsigned offsetOfDelegateFirstTarget;

// Secure delegate offsets
unsigned offsetOfSecureDelegateIndirectCell;

// Remoting offsets
unsigned offsetOfTransparentProxyRP;
unsigned offsetOfRealProxyServer;
Expand Down
9 changes: 8 additions & 1 deletion src/jit/codegenlegacy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18418,7 +18418,6 @@ regMaskTP CodeGen::genLoadIndirectCallTarget(GenTreePtr call)
* register mask that describes where the result will be found is returned;
* otherwise, RBM_NONE is returned.
*/

#ifdef _PREFAST_
#pragma warning(push)
#pragma warning(disable : 21000) // Suppress PREFast warning about overly large function
Expand Down Expand Up @@ -18837,6 +18836,14 @@ regMaskTP CodeGen::genCodeForCall(GenTreePtr call, bool valUsed)
instOffs = pInfo->offsetOfDelegateInstance;
firstTgtOffs = pInfo->offsetOfDelegateFirstTarget;

#ifdef _TARGET_ARM_
if ((call->gtCall.gtCallMoreFlags & GTF_CALL_M_SECURE_DELEGATE_INV))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It maybe a good idea to put it under ARM ifdef since this is ARM-specific code.

{
getEmitter()->emitIns_R_R_I(INS_add, EA_PTRSIZE, REG_VIRTUAL_STUB_PARAM, regThis, pInfo->offsetOfSecureDelegateIndirectCell);
regTracker.rsTrackRegTrash(REG_VIRTUAL_STUB_PARAM);
}
#endif // _TARGET_ARM_

// Grab an available register to use for the CALL indirection
regNumber indCallReg = regSet.rsGrabReg(RBM_ALLINT);

Expand Down
5 changes: 3 additions & 2 deletions src/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -2929,8 +2929,9 @@ struct GenTreeCall final : public GenTree
// a Pinvoke but not as an unmanaged call. See impCheckForPInvokeCall() to
// know when these flags are set.

#define GTF_CALL_M_R2R_REL_INDIRECT 0x2000 // GT_CALL -- ready to run call is indirected through a relative address
#define GTF_CALL_M_DOES_NOT_RETURN 0x4000 // GT_CALL -- call does not return
#define GTF_CALL_M_R2R_REL_INDIRECT 0x2000 // GT_CALL -- ready to run call is indirected through a relative address
#define GTF_CALL_M_DOES_NOT_RETURN 0x4000 // GT_CALL -- call does not return
#define GTF_CALL_M_SECURE_DELEGATE_INV 0x8000 // GT_CALL -- call is in secure delegate

bool IsUnmanaged() const
{
Expand Down
5 changes: 5 additions & 0 deletions src/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6586,6 +6586,11 @@ var_types Compiler::impImportCall(OPCODE opcode,
/* Set the delegate flag */
call->gtCall.gtCallMoreFlags |= GTF_CALL_M_DELEGATE_INV;

if (callInfo->secureDelegateInvoke)
{
call->gtCall.gtCallMoreFlags |= GTF_CALL_M_SECURE_DELEGATE_INV;
}

if (opcode == CEE_CALLVIRT)
{
assert(mflags & CORINFO_FLG_FINAL);
Expand Down
44 changes: 42 additions & 2 deletions src/vm/comdelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2596,7 +2596,7 @@ BOOL COMDelegate::NeedsWrapperDelegate(MethodDesc* pTargetMD)
#ifdef _TARGET_ARM_
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it is ok to just return false for ARM from here. You should see crashes in delegate tests with this change. This change would need matching changes in arm\virtualcallstubcpu.hpp and JIT.

Copy link
Author

@parjong parjong Jul 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jkotas Without this, I encountered UNREACHABLE call as COMDelegate::GetSecureInvoke is currently not implemented.

Could you let me know which changes are required for arm\virtualcallstubcpu.hpp and JIT, or how to implement COMDelegate::GetSecureInvoke?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that implementing COMDelegate::GetSecureInvoke is easier. It should be implemented similar to how COMDelegate::GetMulticastInvoke is implemented, except that there will be no loop. It should just re-push the arguments and call the actual target. The call to actual target generated by the JIT has to pass the indirection cell in R4. There will need be a new flag passed to the JIT to make it happen.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove my email address from DW? I think this discussion does not
concern me.
Cheers
Julian

28 lip 2016 4:06 PM "Jan Kotas" [email protected] napisał(a):

In src/vm/comdelegate.cpp
#6500 (comment):

@@ -2593,15 +2593,6 @@ BOOL COMDelegate::NeedsWrapperDelegate(MethodDesc* pTargetMD)
{
LIMITED_METHOD_CONTRACT;

-#ifdef TARGET_ARM

I believe that implementing COMDelegate::GetSecureInvoke is easier. It
should be implemented similar to how COMDelegate::GetMulticastInvoke is
implemented, except that there will be no loop. It should just re-push the
arguments and call the actual target. The call to actual target generated
by the JIT has to pass the indirection cell in R4. There will need be a new
flag passed to the JIT to make it happen.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/coreclr/pull/6500/files/c7d62d06126218318efee2d7bf5ed0158ffb58d5#r72628027,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAA1nOgyJ-Ik3RQUtmQvlY0BKE3L3gPYks5qaLeBgaJpZM4JW0HY
.

// For arm VSD expects r4 to contain the indirection cell. However r4 is a non-volatile register
// and its value must be preserved. So we need to erect a frame and store indirection cell in r4 before calling
// virtual stub dispatch. Erecting frame is already done by secure delegates so the secureDelegate infrastructure
// virtual stub dispatch. Erecting frame is already done by secure delegates so the secureDelegate infrastructure
// can easliy be used for our purpose.
// set needsSecureDelegate flag in order to erect a frame. (Secure Delegate stub also loads the right value in r4)
if (!pTargetMD->IsStatic() && pTargetMD->IsVirtual() && !pTargetMD->GetMethodTable()->IsValueType())
Expand Down Expand Up @@ -2931,7 +2931,47 @@ PCODE COMDelegate::GetSecureInvoke(MethodDesc* pMD)
#ifdef FEATURE_CAS_POLICY
#error GetSecureInvoke not implemented
#else
UNREACHABLE();
GCX_PREEMP();

MetaSig sig(pMD);

BOOL fReturnVal = !sig.IsReturnTypeVoid();

SigTypeContext emptyContext;
ILStubLinker sl(pMD->GetModule(), pMD->GetSignature(), &emptyContext, pMD, TRUE, TRUE, FALSE);

ILCodeStream *pCode = sl.NewCodeStream(ILStubLinker::kDispatch);

// Load the "real" delegate
pCode->EmitLoadThis();
pCode->EmitLDFLD(pCode->GetToken(MscorlibBinder::GetField(FIELD__MULTICAST_DELEGATE__INVOCATION_LIST)));

// Load the arguments
UINT paramCount = 0;
while(paramCount < sig.NumFixedArgs())
pCode->EmitLDARG(paramCount++);

// Call the delegate
pCode->EmitCALL(pCode->GetToken(pMD), sig.NumFixedArgs(), fReturnVal);

// Return
pCode->EmitRET();

PCCOR_SIGNATURE pSig;
DWORD cbSig;

pMD->GetSig(&pSig,&cbSig);

MethodDesc* pStubMD =
ILStubCache::CreateAndLinkNewILStubMethodDesc(pMD->GetLoaderAllocator(),
pMD->GetMethodTable(),
ILSTUB_SECUREDELEGATE_INVOKE,
pMD->GetModule(),
pSig, cbSig,
NULL,
&sl);

return Stub::NewStub(JitILStub(pStubMD))->GetEntryPoint();
#endif
}
#else // FEATURE_STUBS_AS_IL
Expand Down
2 changes: 2 additions & 0 deletions src/vm/dllimport.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ enum ILStubTypes
ILSTUB_MULTICASTDELEGATE_INVOKE = 0x80000010,
ILSTUB_UNBOXINGILSTUB = 0x80000020,
ILSTUB_INSTANTIATINGSTUB = 0x80000040,
ILSTUB_SECUREDELEGATE_INVOKE = 0x80000080,
#endif
};

Expand Down Expand Up @@ -245,6 +246,7 @@ inline bool SF_IsArrayOpStub (DWORD dwStubFlags) { LIMITED_METHOD_CONT
#endif

#ifdef FEATURE_STUBS_AS_IL
inline bool SF_IsSecureDelegateStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_SECUREDELEGATE_INVOKE); }
inline bool SF_IsMulticastDelegateStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_MULTICASTDELEGATE_INVOKE); }
inline bool SF_IsUnboxingILStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_UNBOXINGILSTUB); }
inline bool SF_IsInstantiatingStub (DWORD dwStubFlags) { LIMITED_METHOD_CONTRACT; return (dwStubFlags == ILSTUB_INSTANTIATINGSTUB); }
Expand Down
6 changes: 6 additions & 0 deletions src/vm/ilstubcache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ MethodDesc* ILStubCache::CreateNewMethodDesc(LoaderHeap* pCreationHeap, MethodTa
else
#endif
#ifdef FEATURE_STUBS_AS_IL
if (SF_IsSecureDelegateStub(dwStubFlags))
{
pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdSecureDelegateStub;
pMD->GetILStubResolver()->SetStubType(ILStubResolver::SecureDelegateStub);
}
else
if (SF_IsMulticastDelegateStub(dwStubFlags))
{
pMD->m_dwExtendedFlags |= DynamicMethodDesc::nomdMulticastStub;
Expand Down
1 change: 1 addition & 0 deletions src/vm/ilstubresolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ LPCUTF8 ILStubResolver::GetStubMethodName()
case MulticastDelegateStub: return "IL_STUB_MulticastDelegate_Invoke";
case UnboxingILStub: return "IL_STUB_UnboxingStub";
case InstantiatingStub: return "IL_STUB_InstantiatingStub";
case SecureDelegateStub: return "IL_STUB_SecureDelegate_Invoke";
#endif
default:
UNREACHABLE_MSG("Unknown stub type");
Expand Down
1 change: 1 addition & 0 deletions src/vm/ilstubresolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class ILStubResolver : DynamicResolver
ArrayOpStub,
#endif
#ifdef FEATURE_STUBS_AS_IL
SecureDelegateStub,
MulticastDelegateStub,
UnboxingILStub,
InstantiatingStub,
Expand Down
17 changes: 16 additions & 1 deletion src/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5169,7 +5169,6 @@ void CEEInfo::getCallInfo(
bool resolvedCallVirt = false;
bool callVirtCrossingVersionBubble = false;


// Delegate targets are always treated as direct calls here. (It would be nice to clean it up...).
if (flags & CORINFO_CALLINFO_LDFTN)
{
Expand Down Expand Up @@ -5686,6 +5685,19 @@ void CEEInfo::getCallInfo(
}
}

pResult->secureDelegateInvoke = FALSE;

#ifdef FEATURE_STUBS_AS_IL
if (m_pMethodBeingCompiled->IsDynamicMethod())
{
auto pMD = m_pMethodBeingCompiled->AsDynamicMethodDesc();
if (pMD->IsILStub() && pMD->IsSecureDelegateStub())
{
pResult->secureDelegateInvoke = TRUE;
}
}
#endif

EE_TO_JIT_TRANSITION();
}

Expand Down Expand Up @@ -9630,6 +9642,9 @@ void CEEInfo::getEEInfo(CORINFO_EE_INFO *pEEInfoOut)
pEEInfoOut->offsetOfDelegateInstance = DelegateObject::GetOffsetOfTarget();
pEEInfoOut->offsetOfDelegateFirstTarget = DelegateObject::GetOffsetOfMethodPtr();

// Secure delegate offsets
pEEInfoOut->offsetOfSecureDelegateIndirectCell = DelegateObject::GetOffsetOfMethodPtrAux();

// Remoting offsets
pEEInfoOut->offsetOfTransparentProxyRP = TransparentProxyObject::GetOffsetOfRP();
pEEInfoOut->offsetOfRealProxyServer = RealProxyObject::GetOffsetOfServerObject();
Expand Down
6 changes: 6 additions & 0 deletions src/vm/method.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2304,6 +2304,7 @@ class DynamicMethodDesc : public StoredSigMethodDesc
nomdStubNeedsCOMStarted = 0x0800, // EnsureComStarted must be called before executing the method
nomdMulticastStub = 0x1000,
nomdUnboxingILStub = 0x2000,
nomdSecureDelegateStub = 0x4000,

nomdILStub = 0x00010000,
nomdLCGMethod = 0x00020000,
Expand Down Expand Up @@ -2412,6 +2413,11 @@ class DynamicMethodDesc : public StoredSigMethodDesc
bool IsSignatureNeedsRestore() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdSignatureNeedsRestore)); }
bool IsStubNeedsCOMStarted() { LIMITED_METHOD_CONTRACT; _ASSERTE(IsILStub()); return (0 != (m_dwExtendedFlags & nomdStubNeedsCOMStarted)); }
#ifdef FEATURE_STUBS_AS_IL
bool IsSecureDelegateStub() {
LIMITED_METHOD_DAC_CONTRACT;
_ASSERTE(IsILStub());
return !!(m_dwExtendedFlags & nomdSecureDelegateStub);
}
bool IsMulticastStub() {
LIMITED_METHOD_DAC_CONTRACT;
_ASSERTE(IsILStub());
Expand Down