Skip to content

Commit

Permalink
Revert "Put HasNativeCodeReJITAware into GetFunctionAddress (#90049)" (
Browse files Browse the repository at this point in the history
…#90696)

This reverts commit eacb32e.
Need to investigate changes because they caused a test failure.
  • Loading branch information
mikelle-rogers authored Aug 17, 2023
1 parent 702d02c commit 149a289
Show file tree
Hide file tree
Showing 13 changed files with 113 additions and 40 deletions.
11 changes: 11 additions & 0 deletions src/coreclr/debug/daccess/dacimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1253,6 +1253,17 @@ 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,
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/debug/daccess/task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5225,7 +5225,7 @@ EnumMethodInstances::Next(ClrDataAccess* dac,
}
}

if (!m_methodIter.Current()->HasNativeCodeAnyVersion())
if (!m_methodIter.Current()->HasNativeCodeReJITAware())
{
goto NextMethod;
}
Expand All @@ -5243,7 +5243,7 @@ EnumMethodInstances::CdStart(MethodDesc* methodDesc,
CLRDATA_ENUM* handle)
{
if (!methodDesc->HasClassOrMethodInstantiation() &&
!(methodDesc->HasNativeCodeAnyVersion()))
!methodDesc->HasNativeCodeReJITAware())
{
*handle = 0;
return S_FALSE;
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/debug/di/breakpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,11 @@ 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
Expand Down
24 changes: 21 additions & 3 deletions src/coreclr/debug/ee/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1247,8 +1247,26 @@ bool DebuggerController::BindPatch(DebuggerControllerPatch *patch,
startAddr = (CORDB_ADDRESS_TYPE *) CORDB_ADDRESS_TO_PTR(patch->GetDJI()->m_addrOfCode);
_ASSERTE(startAddr != NULL);
}
//We should never be calling this function with both a NULL startAddr and a DJI that doesn't have code.
_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;
}
}
}

_ASSERTE(!g_pEEInterface->IsStub((const BYTE *)startAddr));
Expand Down Expand Up @@ -8638,7 +8656,7 @@ bool DebuggerFuncEvalComplete::SendEvent(Thread *thread, bool fIpChanged)
// DebuggerEnCBreakpoint constructor - creates and activates a new EnC breakpoint
//
// Arguments:
// offset - IL offset in the function to place the patch
// offset - native 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
Expand Down
56 changes: 44 additions & 12 deletions src/coreclr/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2841,8 +2841,6 @@ 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())
Expand Down Expand Up @@ -3009,7 +3007,6 @@ HRESULT Debugger::GetILToNativeMappingIntoArrays(
_ASSERTE(pcMap != NULL);
_ASSERTE(prguiILOffset != NULL);
_ASSERTE(prguiNativeOffset != NULL);
_ASSERTE(pNativeCodeStartAddress != NULL);

// Any caller of GetILToNativeMappingIntoArrays had better call
// InitializeLazyDataIfNecessary first!
Expand Down Expand Up @@ -5414,6 +5411,28 @@ 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
Expand Down Expand Up @@ -6200,6 +6219,25 @@ 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,
Expand Down Expand Up @@ -7827,7 +7865,6 @@ 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())
Expand Down Expand Up @@ -10461,7 +10498,7 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
DebuggerJitInfo * pDJI = NULL;
if ((pMethodDesc != NULL) && (pDMI != NULL))
{
pDJI = pDMI->FindOrCreateInitAndAddJitInfo(pMethodDesc, PINSTRToPCODE(dac_cast<TADDR>(pEvent->BreakpointData.codeStartAddress)));
pDJI = pDMI->FindOrCreateInitAndAddJitInfo(pMethodDesc, NULL /* startAddr */);
}

{
Expand Down Expand Up @@ -12588,7 +12625,7 @@ DWORD Debugger::GetThreadIdHelper(Thread *pThread)
// does not own the memory provided via vars outparameter.
//-----------------------------------------------------------------------------
void Debugger::GetVarInfo(MethodDesc * fd, // [IN] method of interest
CORDB_ADDRESS nativeCodeAddress, // [IN] which edit version
void *DebuggerVersionToken, // [IN] which edit version
SIZE_T * cVars, // [OUT] size of 'vars'
const ICorDebugInfo::NativeVarInfo **vars // [OUT] map telling where local vars are stored
)
Expand All @@ -12600,7 +12637,7 @@ void Debugger::GetVarInfo(MethodDesc * fd, // [IN] method of interest
}
CONTRACTL_END;

DebuggerJitInfo * ji = g_pDebugger->GetJitInfo(fd, (const BYTE *)nativeCodeAddress);
DebuggerJitInfo * ji = (DebuggerJitInfo *)DebuggerVersionToken;

// If we didn't supply a DJI, then we're asking for the most recent version.
if (ji == NULL)
Expand Down Expand Up @@ -12924,11 +12961,6 @@ 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++)
{
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/debug/ee/debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -1933,6 +1933,8 @@ class Debugger : public DebugInterface

bool IsJMCMethod(Module* pModule, mdMethodDef tkMethod);

int GetMethodEncNumber(MethodDesc * pMethod);


bool FirstChanceManagedException(Thread *pThread, SIZE_T currentIP, SIZE_T currentSP);

Expand Down Expand Up @@ -1978,7 +1980,7 @@ class Debugger : public DebugInterface
#endif // EnC_SUPPORTED

void GetVarInfo(MethodDesc * fd, // [IN] method of interest
CORDB_ADDRESS nativeCodeAddress, // [IN] which edit version
void *DebuggerVersionToken, // [IN] which edit version
SIZE_T * cVars, // [OUT] size of 'vars'
const ICorDebugInfo::NativeVarInfo **vars // [OUT] map telling where local vars are stored
);
Expand Down
12 changes: 11 additions & 1 deletion src/coreclr/debug/ee/functioninfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1565,7 +1565,9 @@ 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())
{
Expand All @@ -1574,8 +1576,16 @@ DebuggerJitInfo *DebuggerMethodInfo::FindOrCreateInitAndAddJitInfo(MethodDesc* f

if (startAddr == NULL)
{
// This will grab the start address for the current code version.
startAddr = g_pEEInterface->GetFunctionAddress(fd);
_ASSERTE(startAddr != NULL);
if (startAddr == NULL)
{
startAddr = fd->GetNativeCodeReJITAware();
if (startAddr == NULL)
{
return NULL;
}
}
}
else
{
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/debug/inc/dbgipcevents.h
Original file line number Diff line number Diff line change
Expand Up @@ -2011,7 +2011,6 @@ struct MSLAYOUT DebuggerIPCEvent
SIZE_T offset;
SIZE_T encVersion;
LSPTR_METHODDESC nativeCodeMethodDescToken; // points to the MethodDesc if !isIL
CORDB_ADDRESS codeStartAddress;
} BreakpointData;

struct MSLAYOUT
Expand Down
7 changes: 6 additions & 1 deletion src/coreclr/vm/dbginterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
CORDB_ADDRESS nativeCodeAddress, // [IN] which edit version
void *DebuggerVersionToken, // [IN] which edit version
SIZE_T * cVars, // [OUT] size of 'vars'
const ICorDebugInfo::NativeVarInfo **vars // [OUT] map telling where local vars are stored
) = 0;
Expand Down Expand Up @@ -262,6 +262,11 @@ 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,
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/eedbginterfaceimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,7 @@ PCODE EEDbgInterfaceImpl::GetFunctionAddress(MethodDesc *pFD)
SUPPORTS_DAC;
}
CONTRACTL_END;

return pFD->GetNativeCode();
}

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/encee.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, oldCodeInfo.GetCodeAddress(), &oldVarInfoCount, &pOldVarInfo);
g_pDebugInterface->GetVarInfo(pMD, newCodeInfo.GetCodeAddress(), &newVarInfoCount, &pNewVarInfo);
g_pDebugInterface->GetVarInfo(pMD, oldDebuggerFuncHandle, &oldVarInfoCount, &pOldVarInfo);
g_pDebugInterface->GetVarInfo(pMD, NULL, &newVarInfoCount, &pNewVarInfo);

#ifdef TARGET_X86
// save the frame pointer as FixContextForEnC might step on it.
Expand Down
19 changes: 8 additions & 11 deletions src/coreclr/vm/method.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,7 @@ 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
Expand All @@ -934,7 +935,7 @@ PCODE MethodDesc::GetNativeCode()
return GetStableEntryPoint();
}

PCODE MethodDesc::GetNativeCodeAnyVersion()
PCODE MethodDesc::GetNativeCodeReJITAware()
{
WRAPPER_NO_CONTRACT;
SUPPORTS_DAC;
Expand All @@ -945,23 +946,19 @@ PCODE MethodDesc::GetNativeCodeAnyVersion()
return pDefaultCode;
}

else
{
CodeVersionManager *pCodeVersionManager = GetCodeVersionManager();
CodeVersionManager::LockHolder codeVersioningLockHolder;
ILCodeVersionCollection ilVersionCollection = pCodeVersionManager->GetILCodeVersions(PTR_MethodDesc(this));
for (ILCodeVersionIterator curIL = ilVersionCollection.Begin(), endIL = ilVersionCollection.End(); curIL != endIL; curIL++)
ILCodeVersion ilVersion = pCodeVersionManager->GetActiveILCodeVersion(PTR_MethodDesc(this));
if (!ilVersion.IsDefaultVersion())
{
NativeCodeVersionCollection nativeCollection = curIL->GetNativeCodeVersions(PTR_MethodDesc(this));
for (NativeCodeVersionIterator curNative = nativeCollection.Begin(), endNative = nativeCollection.End(); curNative != endNative; curNative++)
NativeCodeVersion activeNativeCodeVersion = ilVersion.GetActiveNativeCodeVersion(PTR_MethodDesc(this));
if (!activeNativeCodeVersion.IsNull())
{
PCODE native = curNative->GetNativeCode();
if(native != NULL)
{
return native;
}
return activeNativeCodeVersion.GetNativeCode();
}
}

return NULL;
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/vm/method.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1373,11 +1373,11 @@ class MethodDesc
}

// Perf warning: takes the CodeVersionManagerLock on every call
BOOL HasNativeCodeAnyVersion()
BOOL HasNativeCodeReJITAware()
{
LIMITED_METHOD_DAC_CONTRACT;

return GetNativeCodeAnyVersion() != NULL;
return GetNativeCodeReJITAware() != NULL;
}

BOOL SetNativeCodeInterlocked(PCODE addr, PCODE pExpected = NULL);
Expand Down Expand Up @@ -1437,9 +1437,9 @@ class MethodDesc
PCODE GetNativeCode();

// Returns GetNativeCode() if it exists, but also checks to see if there
// is a non-default code version that is populated with a code body and returns that.
// is a non-default IL code version and returns that.
// Perf warning: takes the CodeVersionManagerLock on every call
PCODE GetNativeCodeAnyVersion();
PCODE GetNativeCodeReJITAware();

#if defined(FEATURE_JIT_PITCHING)
bool IsPitchable();
Expand Down

0 comments on commit 149a289

Please sign in to comment.