Put HasNativeCodeReJITAware into GetFunctionAddress (#90049)

* Remove calls to GetFunctionAddress with Null address and clarify method names

src/coreclr/debug/daccess/dacimpl.h

@@ -1253,17 +1253,6 @@ class ClrDataAccess
         /* [out] */ union STUB_BUF* outBuffer,
         /* [out] */ ULONG32* outFlags);
 
-    DebuggerJitInfo* GetDebuggerJitInfo(MethodDesc* methodDesc,
-                                        TADDR addr)
-    {
-        if (g_pDebugger)
-        {
-            return g_pDebugger->GetJitInfo(methodDesc, (PBYTE)addr, NULL);
-        }
-
-        return NULL;
-    }
-
     HRESULT GetMethodExtents(MethodDesc* methodDesc,
                              METH_EXTENTS** extents);
     HRESULT GetMethodVarInfo(MethodDesc* methodDesc,

src/coreclr/debug/daccess/task.cpp

@@ -5225,7 +5225,7 @@ EnumMethodInstances::Next(ClrDataAccess* dac,
         }
     }
 
-    if (!m_methodIter.Current()->HasNativeCodeReJITAware())
+    if (!m_methodIter.Current()->HasNativeCodeAnyVersion())
     {
         goto NextMethod;
     }
@@ -5243,7 +5243,7 @@ EnumMethodInstances::CdStart(MethodDesc* methodDesc,
                              CLRDATA_ENUM* handle)
 {
     if (!methodDesc->HasClassOrMethodInstantiation() &&
-        !methodDesc->HasNativeCodeReJITAware())
+        !(methodDesc->HasNativeCodeAnyVersion()))
     {
         *handle = 0;
         return S_FALSE;

src/coreclr/debug/di/breakpoint.cpp

@@ -211,11 +211,13 @@ HRESULT CordbFunctionBreakpoint::Activate(BOOL fActivate)
         if (codeIsIL)
         {
             pEvent->BreakpointData.nativeCodeMethodDescToken = pEvent->BreakpointData.nativeCodeMethodDescToken.NullPtr();
+            pEvent->BreakpointData.codeStartAddress = 0;
         }
         else
         {
             pEvent->BreakpointData.nativeCodeMethodDescToken =
                 (m_code.GetValue()->AsNativeCode())->GetVMNativeCodeMethodDescToken().ToLsPtr();
+            pEvent->BreakpointData.codeStartAddress = (m_code.GetValue()->AsNativeCode())->GetAddress();
         }
 
         // Note: we're sending a two-way event, so it blocks here

src/coreclr/debug/ee/controller.cpp

@@ -1247,26 +1247,8 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch,
             startAddr = (CORDB_ADDRESS_TYPE *) CORDB_ADDRESS_TO_PTR(patch->GetDJI()->m_addrOfCode);
             _ASSERTE(startAddr != NULL);
         }
-        if (startAddr == NULL)
-        {
-            // Should not be trying to place patches on MethodDecs's for stubs.
-            // These stubs will never get jitted.
-            CONSISTENCY_CHECK_MSGF(!pMD->IsWrapperStub(), ("Can't place patch at stub md %p, %s::%s",
-                                   pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName));
-
-            startAddr = (CORDB_ADDRESS_TYPE *)g_pEEInterface->GetFunctionAddress(pMD);
-            //
-            // Code is not available yet to patch.  The prestub should
-            // notify us when it is executed.
-            //
-            if (startAddr == NULL)
-            {
-                LOG((LF_CORDB, LL_INFO10000,
-                    "DC::BP: Patch at 0x%zx not bindable yet.\n", patch->offset));
-
-                return false;
-            }
-        }
+        //We should never be calling this function with both a NULL startAddr and a DJI that doesn't have code.
+        _ASSERTE(startAddr != NULL);
     }
 
     _ASSERTE(!g_pEEInterface->IsStub((const BYTE *)startAddr));
@@ -8656,7 +8638,7 @@ bool DebuggerFuncEvalComplete::SendEvent(Thread *thread, bool fIpChanged)
 // DebuggerEnCBreakpoint constructor - creates and activates a new EnC breakpoint
 //
 // Arguments:
-//    offset        - native offset in the function to place the patch
+//    offset        - IL offset in the function to place the patch
 //    jitInfo       - identifies the function in which the breakpoint is being placed
 //    fTriggerType  - breakpoint type: either REMAP_PENDING or REMAP_COMPLETE
 //    pAppDomain    - the breakpoint applies to the specified AppDomain only

src/coreclr/debug/ee/debugger.cpp

@@ -2841,6 +2841,8 @@ HRESULT Debugger::GetILToNativeMapping(PCODE pNativeCodeStartAddress, ULONG32 cM
     }
     CONTRACTL_END;
 
+    _ASSERTE(pNativeCodeStartAddress != NULL);
+
 #ifdef PROFILING_SUPPORTED
     // At this point, we're pulling in the debugger.
     if (!HasLazyData())
@@ -3007,6 +3009,7 @@ HRESULT Debugger::GetILToNativeMappingIntoArrays(
     _ASSERTE(pcMap != NULL);
     _ASSERTE(prguiILOffset != NULL);
     _ASSERTE(prguiNativeOffset != NULL);
+    _ASSERTE(pNativeCodeStartAddress != NULL);
 
     // Any caller of GetILToNativeMappingIntoArrays had better call
     // InitializeLazyDataIfNecessary first!
@@ -5411,28 +5414,6 @@ void Debugger::ReleaseAllRuntimeThreads(AppDomain *pAppDomain)
     g_pEEInterface->ResumeFromDebug(pAppDomain);
 }
 
-// Given a method, get's its EnC version number. 1 if the method is not EnCed.
-// Note that MethodDescs are reused between versions so this will give us
-// the most recent EnC number.
-int Debugger::GetMethodEncNumber(MethodDesc * pMethod)
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_NOTRIGGER;
-    }
-    CONTRACTL_END;
-
-    DebuggerJitInfo * dji = GetLatestJitInfoFromMethodDesc(pMethod);
-    if (dji == NULL)
-    {
-        // If there's no DJI, couldn't have been EnCed.
-        return 1;
-    }
-    return (int) dji->m_encVersion;
-}
-
-
 bool Debugger::IsJMCMethod(Module* pModule, mdMethodDef tkMethod)
 {
     CONTRACTL
@@ -6219,25 +6200,6 @@ void Debugger::LockAndSendEnCRemapCompleteEvent(MethodDesc *pMD)
     Thread *thread = g_pEEInterface->GetThread();
     // Note that the debugger lock is reentrant, so we may or may not hold it already.
     SENDIPCEVENT_BEGIN(this, thread);
-
-    EX_TRY
-    {
-        // Ensure the DJI for the latest version of this method has been pre-created.
-        // It's not clear whether this is necessary or not, but it shouldn't hurt since
-        // we're going to need to create it anyway since we'll be debugging inside it.
-        DebuggerJitInfo *dji = g_pDebugger->GetLatestJitInfoFromMethodDesc(pMD);
-        (void)dji; //prevent "unused variable" error from GCC
-        _ASSERTE( dji != NULL );
-    }
-    EX_CATCH
-    {
-        // GetLatestJitInfo could throw on OOM, but the debugger isn't resiliant to OOM.
-        // I'm not aware of any other legitimate reason why it may throw, so we'll ASSERT
-        // if it fails.
-        _ASSERTE(!"Unexpected exception from Debugger::GetLatestJitInfoFromMethodDesc on EnC remap complete");
-    }
-    EX_END_CATCH(RethrowTerminalExceptions);
-
     // Send an EnC remap complete event to the Right Side.
     DebuggerIPCEvent* ipce = m_pRCThread->GetIPCEventSendBuffer();
     InitIPCEvent(ipce,
@@ -7865,6 +7827,7 @@ void Debugger::FirstChanceManagedExceptionCatcherFound(Thread *pThread,
     // Implements DebugInterface
     // Call by EE/exception. Must be on managed thread
     _ASSERTE(GetThreadNULLOk() != NULL);
+    _ASSERTE(pMethodAddr != NULL);
 
     // Quick check.
     if (!CORDebuggerAttached())
@@ -10498,7 +10461,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
             DebuggerJitInfo * pDJI =  NULL;
             if ((pMethodDesc != NULL) && (pDMI != NULL))
             {
-                pDJI = pDMI->FindOrCreateInitAndAddJitInfo(pMethodDesc, NULL /* startAddr */);
+                pDJI = pDMI->FindOrCreateInitAndAddJitInfo(pMethodDesc, PINSTRToPCODE(dac_cast<TADDR>(pEvent->BreakpointData.codeStartAddress)));
             }
 
             {
@@ -12625,7 +12588,7 @@ DWORD Debugger::GetThreadIdHelper(Thread *pThread)
 // does not own the memory provided via vars outparameter.
 //-----------------------------------------------------------------------------
 void Debugger::GetVarInfo(MethodDesc *       fd,   // [IN] method of interest
-                    void *DebuggerVersionToken,    // [IN] which edit version
+                    CORDB_ADDRESS nativeCodeAddress,    // [IN] which edit version
                     SIZE_T *           cVars,      // [OUT] size of 'vars'
                     const ICorDebugInfo::NativeVarInfo **vars     // [OUT] map telling where local vars are stored
                     )
@@ -12637,7 +12600,7 @@ void Debugger::GetVarInfo(MethodDesc *       fd,   // [IN] method of interest
     }
     CONTRACTL_END;
 
-    DebuggerJitInfo * ji = (DebuggerJitInfo *)DebuggerVersionToken;
+    DebuggerJitInfo * ji = g_pDebugger->GetJitInfo(fd, (const BYTE *)nativeCodeAddress);
 
     // If we didn't supply a DJI, then we're asking for the most recent version.
     if (ji == NULL)
@@ -12961,6 +12924,11 @@ HRESULT Debugger::UpdateFunction(MethodDesc* pMD, SIZE_T encVersion)
 
     // For each offset in the IL->Native map, set a new EnC breakpoint on the
     // ones that we know could be remap points.
+
+    // Depending on which DJI was picked, the code might compute different IL offsets. The JIT may not guarantee it produces
+    // the same set of sequence points for every generic instantiation.
+    // Inside ENCSequencePointHelper there is logic that skips IL offsets that map to the same native offset.
+    // Its possible that one version of the code maps two IL offsets to the same native offset but another version of the code maps them to different offsets.
     PTR_DebuggerILToNativeMap seqMap = pJitInfo->GetSequenceMap();
     for (unsigned int i = 0; i < pJitInfo->GetSequenceMapCount(); i++)
     {

src/coreclr/debug/ee/debugger.h

@@ -1933,8 +1933,6 @@ class Debugger : public DebugInterface
 
     bool IsJMCMethod(Module* pModule, mdMethodDef tkMethod);
 
-    int GetMethodEncNumber(MethodDesc * pMethod);
-
 
     bool FirstChanceManagedException(Thread *pThread, SIZE_T currentIP, SIZE_T currentSP);
 
@@ -1980,7 +1978,7 @@ class Debugger : public DebugInterface
 #endif // EnC_SUPPORTED
 
     void GetVarInfo(MethodDesc *       fd,         // [IN] method of interest
-                    void *DebuggerVersionToken,    // [IN] which edit version
+                    CORDB_ADDRESS nativeCodeAddress,    // [IN] which edit version
                     SIZE_T *           cVars,      // [OUT] size of 'vars'
                     const ICorDebugInfo::NativeVarInfo **vars     // [OUT] map telling where local vars are stored
                     );

src/coreclr/debug/ee/functioninfo.cpp

@@ -1565,9 +1565,7 @@ DebuggerJitInfo *DebuggerMethodInfo::FindOrCreateInitAndAddJitInfo(MethodDesc* f
         GC_NOTRIGGER;
     }
     CONTRACTL_END;
-
     _ASSERTE(fd != NULL);
-
     // The debugger doesn't track Lightweight-codegen methods b/c they have no metadata.
     if (fd->IsDynamicMethod())
     {
@@ -1576,16 +1574,8 @@ DebuggerJitInfo *DebuggerMethodInfo::FindOrCreateInitAndAddJitInfo(MethodDesc* f
 
     if (startAddr == NULL)
     {
-        // This will grab the start address for the current code version.
         startAddr = g_pEEInterface->GetFunctionAddress(fd);
-        if (startAddr == NULL)
-        {
-            startAddr = fd->GetNativeCodeReJITAware();
-            if (startAddr == NULL)
-            {
-                return NULL;
-            }
-        }
+        _ASSERTE(startAddr != NULL);
     }
     else
     {

src/coreclr/debug/inc/dbgipcevents.h

@@ -2011,6 +2011,7 @@ struct MSLAYOUT DebuggerIPCEvent
             SIZE_T       offset;
             SIZE_T       encVersion;
             LSPTR_METHODDESC  nativeCodeMethodDescToken; // points to the MethodDesc if !isIL
+            CORDB_ADDRESS codeStartAddress;
         } BreakpointData;
 
         struct MSLAYOUT

src/coreclr/vm/dbginterface.h

@@ -203,7 +203,7 @@ class DebugInterface
 
     // Get debugger variable information for a specific version of a method
     virtual     void GetVarInfo(MethodDesc *       fd,         // [IN] method of interest
-                                void *DebuggerVersionToken,    // [IN] which edit version
+                                CORDB_ADDRESS nativeCodeAddress,    // [IN] which edit version
                                 SIZE_T *           cVars,      // [OUT] size of 'vars'
                                 const ICorDebugInfo::NativeVarInfo **vars     // [OUT] map telling where local vars are stored
                                 ) = 0;
@@ -262,11 +262,6 @@ class DebugInterface
 
     virtual bool IsJMCMethod(Module* pModule, mdMethodDef tkMethod) = 0;
 
-    // Given a method, get's its EnC version number. 1 if the method is not EnCed.
-    // Note that MethodDescs are reused between versions so this will give us
-    // the most recent EnC number.
-    virtual int GetMethodEncNumber(MethodDesc * pMethod) = 0;
-
     virtual void SendLogSwitchSetting (int iLevel,
                                        int iReason,
                                        _In_z_ LPCWSTR pLogSwitchName,

src/coreclr/vm/eedbginterfaceimpl.cpp

@@ -630,7 +630,6 @@ PCODE EEDbgInterfaceImpl::GetFunctionAddress(MethodDesc *pFD)
         SUPPORTS_DAC;
     }
     CONTRACTL_END;
-
     return pFD->GetNativeCode();
 }
 

src/coreclr/vm/encee.cpp

@@ -806,8 +806,8 @@ NOINLINE void EditAndContinueModule::FixContextAndResume(
     // Get the var info which the codemanager will use for updating
     // enregistered variables correctly, or variables whose lifetimes differ
     // at the update point
-    g_pDebugInterface->GetVarInfo(pMD, oldDebuggerFuncHandle, &oldVarInfoCount, &pOldVarInfo);
-    g_pDebugInterface->GetVarInfo(pMD, NULL,                  &newVarInfoCount, &pNewVarInfo);
+    g_pDebugInterface->GetVarInfo(pMD, oldCodeInfo.GetCodeAddress(), &oldVarInfoCount, &pOldVarInfo);
+    g_pDebugInterface->GetVarInfo(pMD, newCodeInfo.GetCodeAddress(),                  &newVarInfoCount, &pNewVarInfo);
 
 #ifdef TARGET_X86
     // save the frame pointer as FixContextForEnC might step on it.

src/coreclr/vm/method.cpp

@@ -913,7 +913,6 @@ PCODE MethodDesc::GetNativeCode()
     WRAPPER_NO_CONTRACT;
     SUPPORTS_DAC;
     _ASSERTE(!IsDefaultInterfaceMethod() || HasNativeCodeSlot());
-
     if (HasNativeCodeSlot())
     {
         // When profiler is enabled, profiler may ask to rejit a code even though we
@@ -935,7 +934,7 @@ PCODE MethodDesc::GetNativeCode()
     return GetStableEntryPoint();
 }
 
-PCODE MethodDesc::GetNativeCodeReJITAware()
+PCODE MethodDesc::GetNativeCodeAnyVersion()
 {
     WRAPPER_NO_CONTRACT;
     SUPPORTS_DAC;
@@ -946,19 +945,23 @@ PCODE MethodDesc::GetNativeCodeReJITAware()
         return pDefaultCode;
     }
 
+    else
     {
         CodeVersionManager *pCodeVersionManager = GetCodeVersionManager();
         CodeVersionManager::LockHolder codeVersioningLockHolder;
-        ILCodeVersion ilVersion = pCodeVersionManager->GetActiveILCodeVersion(PTR_MethodDesc(this));
-        if (!ilVersion.IsDefaultVersion())
+        ILCodeVersionCollection ilVersionCollection = pCodeVersionManager->GetILCodeVersions(PTR_MethodDesc(this));
+        for (ILCodeVersionIterator curIL = ilVersionCollection.Begin(), endIL = ilVersionCollection.End(); curIL != endIL; curIL++)
         {
-            NativeCodeVersion activeNativeCodeVersion = ilVersion.GetActiveNativeCodeVersion(PTR_MethodDesc(this));
-            if (!activeNativeCodeVersion.IsNull())
+            NativeCodeVersionCollection nativeCollection = curIL->GetNativeCodeVersions(PTR_MethodDesc(this));
+            for (NativeCodeVersionIterator curNative = nativeCollection.Begin(), endNative = nativeCollection.End(); curNative != endNative; curNative++)
             {
-                return activeNativeCodeVersion.GetNativeCode();
+                PCODE native = curNative->GetNativeCode();
+                if(native != NULL)
+                {
+                    return native;
+                }
             }
         }
-
         return NULL;
     }
 }

src/coreclr/vm/method.hpp

@@ -1373,11 +1373,11 @@ class MethodDesc
     }
 
     // Perf warning: takes the CodeVersionManagerLock on every call
-    BOOL HasNativeCodeReJITAware()
+    BOOL HasNativeCodeAnyVersion()
     {
         LIMITED_METHOD_DAC_CONTRACT;
 
-        return GetNativeCodeReJITAware() != NULL;
+        return GetNativeCodeAnyVersion() != NULL;
     }
 
     BOOL SetNativeCodeInterlocked(PCODE addr, PCODE pExpected = NULL);
@@ -1437,9 +1437,9 @@ class MethodDesc
     PCODE GetNativeCode();
 
     // Returns GetNativeCode() if it exists, but also checks to see if there
-    // is a non-default IL code version and returns that.
+    // is a non-default code version that is populated with a code body and returns that.
     // Perf warning: takes the CodeVersionManagerLock on every call
-    PCODE GetNativeCodeReJITAware();
+    PCODE GetNativeCodeAnyVersion();
 
 #if defined(FEATURE_JIT_PITCHING)
     bool IsPitchable();