PR feedback

* Refactored GC and EH info allocation
* Added Interpreter-TODO at some places
* Reverted some changes that were not needed anymore

Author: janvorli

src/coreclr/inc/eetwain.h

@@ -625,10 +625,6 @@ class InterpreterCodeManager : public ICodeManager {
 
 #ifndef DACCESS_COMPILE
 #ifndef FEATURE_EH_FUNCLETS
-/*
-    Last chance for the runtime support to do fixups in the context
-    before execution continues inside a filter, catch handler, or finally
-*/
 virtual
 void FixContext(ContextType     ctxType,
                 EHContext      *ctx,
@@ -640,6 +636,7 @@ void FixContext(ContextType     ctxType,
                 size_t       ** ppShadowSP,             // OUT
                 size_t       ** ppEndRegion)            // OUT
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
 }
 #endif // !FEATURE_EH_FUNCLETS
@@ -657,41 +654,27 @@ TADDR GetAmbientSP(PREGDISPLAY     pContext,
                    DWORD           nestingLevel,
                    CodeManState   *pState)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return NULL;
 }
 #endif // TARGET_X86
 
-/*
-    Get the number of bytes used for stack parameters.
-    This is currently only used on x86.
-*/
 virtual
 ULONG32 GetStackParameterSize(EECodeInfo* pCodeInfo)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return 0;
 }
 
-/*
-    Unwind the current stack frame, i.e. update the virtual register
-    set in pContext. This will be similar to the state after the function
-    returns back to caller (IP points to after the call, Frame and Stack
-    pointer has been reset, callee-saved registers restored
-    (if UpdateAllRegs), callee-UNsaved registers are trashed)
-    Returns success of operation.
-*/
 virtual
 bool UnwindStackFrame(
                 PREGDISPLAY     pContext,
                 EECodeInfo     *pCodeInfo,
                 unsigned        flags,
                 CodeManState   *pState);
 
-/*
-    Is the function currently at a "GC safe point" ?
-    Can call EnumGcRefs() successfully
-*/
 virtual
 bool IsGcSafe(  EECodeInfo     *pCodeInfo,
                 DWORD           dwRelOffset);
@@ -705,13 +688,6 @@ bool HasTailCalls(EECodeInfo *pCodeInfo)
 }
 #endif // TARGET_ARM || TARGET_ARM64 || TARGET_LOONGARCH64 || defined(TARGET_RISCV64)
 
-/*
-    Enumerate all live object references in that function using
-    the virtual register set. Same reference location cannot be enumerated
-    multiple times (but all differenct references pointing to the same
-    object have to be individually enumerated).
-    Returns success of operation.
-*/
 virtual
 bool EnumGcRefs(PREGDISPLAY     pContext,
                 EECodeInfo     *pCodeInfo,
@@ -725,86 +701,65 @@ OBJECTREF GetInstance(
                 PREGDISPLAY     pContext,
                 EECodeInfo *    pCodeInfo);
 
-/*
-    Returns the extra argument passed to shared generic code if it is still alive.
-    Returns NULL in all other cases.
-*/
 virtual
 PTR_VOID GetParamTypeArg(PREGDISPLAY     pContext,
                          EECodeInfo *    pCodeInfo);
 
-// Returns the type of the context parameter (this, methodtable, methoddesc, or none)
 virtual GenericParamContextType GetParamContextType(PREGDISPLAY     pContext,
                                                     EECodeInfo *    pCodeInfo);
 
-/*
-    Returns the offset of the GuardStack cookie if it exists.
-    Returns NULL if there is no cookie.
-*/
 virtual
 void * GetGSCookieAddr(PREGDISPLAY     pContext,
                        EECodeInfo    * pCodeInfo,
                        unsigned        flags,
                        CodeManState  * pState)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return NULL;
 }
 
 
 #ifndef USE_GC_INFO_DECODER
-/*
-  Returns true if the given IP is in the given method's prolog or an epilog.
-*/
 virtual
 bool IsInPrologOrEpilog(
                 DWORD       relOffset,
                 GCInfoToken gcInfoToken,
                 size_t*     prologSize)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return false;
 }
 
-/*
-  Returns true if the given IP is in the synchronized region of the method (valid for synchronized functions only)
-*/
 virtual
 bool IsInSynchronizedRegion(
                 DWORD       relOffset,
                 GCInfoToken gcInfoToken,
                 unsigned    flags)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return false;
 }
 #endif // !USE_GC_INFO_DECODER
 
-/*
-  Returns the size of a given function.
-*/
 virtual
 size_t GetFunctionSize(GCInfoToken gcInfoToken);
 
-/*
-*  Get information necessary for return address hijacking of the method represented by the gcInfoToken.
-*  If it can be hijacked, it sets the returnKind output parameter to the kind of the return value and
-*  returns true.
-*  If hijacking is not possible for some reason, it return false.
-*/
 virtual bool GetReturnAddressHijackInfo(GCInfoToken gcInfoToken X86_ARG(ReturnKind * returnKind))
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return false;
 }
 
 #ifndef USE_GC_INFO_DECODER
-/*
-  Returns the size of the frame (barring localloc)
-*/
+
 virtual
 unsigned int GetFrameSize(GCInfoToken gcInfoToken)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return 0;
 }
@@ -815,6 +770,7 @@ unsigned int GetFrameSize(GCInfoToken gcInfoToken)
 #ifndef FEATURE_EH_FUNCLETS
 virtual const BYTE* GetFinallyReturnAddr(PREGDISPLAY pReg)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return NULL;
 }
@@ -823,6 +779,7 @@ virtual BOOL LeaveFinally(GCInfoToken gcInfoToken,
                           unsigned offset,
                           PCONTEXT pCtx)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
     return FALSE;
 }
@@ -831,15 +788,13 @@ virtual void LeaveCatch(GCInfoToken gcInfoToken,
                          unsigned offset,
                          PCONTEXT pCtx)
 {
+    // Interpreter-TODO: Implement this if needed
     _ASSERTE(FALSE);
 }
 #endif // FEATURE_EH_FUNCLETS
 
 #ifdef FEATURE_REMAP_FUNCTION
-/*
-    Last chance for the runtime support to do fixups in the context
-    before execution continues inside an EnC updated function.
-*/
+
 virtual
 HRESULT FixContextForEnC(PCONTEXT        pCtx,
                             EECodeInfo    * pOldCodeInfo,
@@ -849,6 +804,7 @@ HRESULT FixContextForEnC(PCONTEXT        pCtx,
        const ICorDebugInfo::NativeVarInfo * newMethodVars,
                             SIZE_T          newMethodVarsCount)
 {
+    // Interpreter-TODO: Implement this
     _ASSERTE(FALSE);
     return E_NOTIMPL;
 }

src/coreclr/vm/codeman.cpp

@@ -422,7 +422,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R
         {
             // This cast is justified because only EEJitManager's have the code type above.
             EEJitManager* pJitMgr = (EEJitManager*)(pRS->_pjit);
-            CodeHeader * pHeader = dac_cast<PTR_CodeHeader>(pJitMgr->GetCodeHeaderFromStartAddress(entryPoint));
+            CodeHeader * pHeader = pJitMgr->GetCodeHeaderFromStartAddress(entryPoint);
             for(ULONG i = 0; i < pHeader->GetNumberOfUnwindInfos(); i++)
                 RemoveFromUnwindInfoTable(&pRS->_pUnwindInfoTable, pRS->_range.RangeStart(), pRS->_range.RangeStart() + pHeader->GetUnwindInfo(i)->BeginAddress);
         }
@@ -461,7 +461,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R
                 {
                     // This cast is justified because only EEJitManager's have the code type above.
                     EEJitManager* pJitMgr = (EEJitManager*)(pRS->_pjit);
-                    CodeHeader * pHeader = dac_cast<PTR_CodeHeader>(pJitMgr->GetCodeHeaderFromStartAddress(methodEntry));
+                    CodeHeader * pHeader = pJitMgr->GetCodeHeaderFromStartAddress(methodEntry);
                     int unwindInfoCount = pHeader->GetNumberOfUnwindInfos();
                     for(int i = 0; i < unwindInfoCount; i++)
                         AddToUnwindInfoTable(&pRS->_pUnwindInfoTable, pHeader->GetUnwindInfo(i), pRS->_range.RangeStart(), pRS->_range.RangeEndOpen());
@@ -519,8 +519,7 @@ void UnwindInfoTable::AddToUnwindInfoTable(UnwindInfoTable** unwindInfoPtr, PT_R
 Setters of EECodeGenManager::m_CodeHeapCritSec
 -----------------------------------------------
 allocCode
-allocGCInfo
-allocEHInfo
+allocFromJitMetaHeap
 allocJumpStubBlock
 ResolveEHClause
 RemoveJitData
@@ -2916,7 +2915,6 @@ void EECodeGenManager::allocCode(MethodDesc* pMD, size_t blockSize, size_t reser
 #endif
         _ASSERTE(reserveForJumpStubs == 0);
         requestInfo.SetInterpreted();
-        requestInfo.SetDynamicDomain();
         realHeaderSize = sizeof(InterpreterRealCodeHeader);
     }
     else
@@ -3198,33 +3196,6 @@ LoaderHeap *EECodeGenManager::GetJitMetaHeap(MethodDesc *pMD)
     return pAllocator->GetLowFrequencyHeap();
 }
 
-void* EECodeGenManager::allocEHInfoRaw(MethodDesc* pMD, DWORD blockSize, size_t * pAllocationSize)
-{
-    CONTRACTL {
-        THROWS;
-        GC_NOTRIGGER;
-    } CONTRACTL_END;
-
-    void * mem = NULL;
-
-    // sadly for light code gen I need the check in here. We should change GetJitMetaHeap
-    if (pMD->IsLCGMethod())
-    {
-        CrstHolder ch(&m_CodeHeapCritSec);
-        mem = (void*)pMD->AsDynamicMethodDesc()->GetResolver()->GetJitMetaHeap()->New(blockSize);
-    }
-    else
-    {
-        mem = (void*)GetJitMetaHeap(pMD)->AllocMem(S_SIZE_T(blockSize));
-    }
-    _ASSERTE(mem);   // AllocMem throws if there's not enough memory
-
-    * pAllocationSize = blockSize; // Store the allocation size so we can backout later.
-
-    return(mem);
-}
-
-
 JumpStubBlockHeader *  EEJitManager::allocJumpStubBlock(MethodDesc* pMD, DWORD numJumps,
                                                         BYTE * loAddr, BYTE * hiAddr,
                                                         LoaderAllocator *pLoaderAllocator,
@@ -3326,53 +3297,27 @@ void * EEJitManager::allocCodeFragmentBlock(size_t blockSize, unsigned alignment
     RETURN((void *)mem);
 }
 
-template<typename TCodeHeader>
-BYTE* EECodeGenManager::allocGCInfo(TCodeHeader* pCodeHeader, DWORD blockSize, size_t * pAllocationSize)
+BYTE* EECodeGenManager::allocFromJitMetaHeap(MethodDesc* pMD, DWORD blockSize, size_t * pAllocationSize)
 {
     CONTRACTL {
         THROWS;
         GC_NOTRIGGER;
     } CONTRACTL_END;
 
-    MethodDesc* pMD = pCodeHeader->GetMethodDesc();
-    // sadly for light code gen I need the check in here. We should change GetJitMetaHeap
+    BYTE *pMem = NULL;
     if (pMD->IsLCGMethod())
     {
         CrstHolder ch(&m_CodeHeapCritSec);
-        pCodeHeader->SetGCInfo((BYTE*)(void*)pMD->AsDynamicMethodDesc()->GetResolver()->GetJitMetaHeap()->New(blockSize));
+        pMem = (BYTE*)(void*)pMD->AsDynamicMethodDesc()->GetResolver()->GetJitMetaHeap()->New(blockSize);
     }
     else
     {
-        pCodeHeader->SetGCInfo((BYTE*) (void*)GetJitMetaHeap(pMD)->AllocMem(S_SIZE_T(blockSize)));
+        pMem = (BYTE*) (void*)GetJitMetaHeap(pMD)->AllocMem(S_SIZE_T(blockSize));
     }
-    _ASSERTE(pCodeHeader->GetGCInfo()); // AllocMem throws if there's not enough memory
-
-    * pAllocationSize = blockSize;  // Store the allocation size so we can backout later.
-
-    return(pCodeHeader->GetGCInfo());
-}
-
-template<typename TCodeHeader>
-EE_ILEXCEPTION* EECodeGenManager::allocEHInfo(TCodeHeader* pCodeHeader, unsigned numClauses, size_t * pAllocationSize)
-{
-    CONTRACTL {
-        THROWS;
-        GC_NOTRIGGER;
-    } CONTRACTL_END;
-
-    // Note - pCodeHeader->phdrJitEHInfo - sizeof(size_t) contains the number of EH clauses
-
-    DWORD temp =  EE_ILEXCEPTION::Size(numClauses);
-    DWORD blockSize = 0;
-    if (!ClrSafeInt<DWORD>::addition(temp, sizeof(size_t), blockSize))
-        COMPlusThrowOM();
 
-    BYTE *EHInfo = (BYTE*)allocEHInfoRaw(pCodeHeader->GetMethodDesc(), blockSize, pAllocationSize);
+    *pAllocationSize = blockSize;  // Store the allocation size so we can backout later.
 
-    pCodeHeader->SetEHInfo((EE_ILEXCEPTION*) (EHInfo + sizeof(size_t)));
-    pCodeHeader->GetEHInfo()->Init(numClauses);
-    *((size_t *)EHInfo) = numClauses;
-    return(pCodeHeader->GetEHInfo());
+    return pMem;
 }
 
 template<typename TCodeHeader>
@@ -3469,21 +3414,10 @@ void EECodeGenManager::RemoveJitData(TCodeHeader * pCHdr, size_t GCinfo_len, siz
 // being in the header file. The bodies call methods of multiple data types that cannot be reasonably defined
 // before the codeman.h is included.
 
-template
-BYTE* EECodeGenManager::allocGCInfo<CodeHeader>(CodeHeader* pCodeHeader, DWORD blockSize, size_t * pAllocationSize);
-
-template
-EE_ILEXCEPTION* EECodeGenManager::allocEHInfo<CodeHeader>(CodeHeader* pCodeHeader, unsigned numClauses, size_t * pAllocationSize);
-
 template
 void EECodeGenManager::RemoveJitData<CodeHeader>(CodeHeader * pCHdr, size_t GCinfo_len, size_t EHinfo_len);
 
 #ifdef FEATURE_INTERPRETER
-template
-BYTE* EECodeGenManager::allocGCInfo<InterpreterCodeHeader>(InterpreterCodeHeader* pCodeHeader, DWORD blockSize, size_t * pAllocationSize);
-
-template
-EE_ILEXCEPTION* EECodeGenManager::allocEHInfo<InterpreterCodeHeader>(InterpreterCodeHeader* pCodeHeader, unsigned numClauses, size_t * pAllocationSize);
 
 template
 void EECodeGenManager::RemoveJitData<InterpreterCodeHeader>(InterpreterCodeHeader * pCHdr, size_t GCinfo_len, size_t EHinfo_len);