Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put HasNativeCodeReJITAware into GetFunctionAddress #90049

Merged
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
11 changes: 0 additions & 11 deletions src/coreclr/debug/daccess/dacimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
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()->HasNativeCodeReJITAware())
if (!m_methodIter.Current()->HasNativeCodeAnyVersion())
{
goto NextMethod;
}
Expand All @@ -5243,7 +5243,7 @@ EnumMethodInstances::CdStart(MethodDesc* methodDesc,
CLRDATA_ENUM* handle)
{
if (!methodDesc->HasClassOrMethodInstantiation() &&
!methodDesc->HasNativeCodeReJITAware())
!(methodDesc->HasNativeCodeAnyVersion()))
{
*handle = 0;
return S_FALSE;
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/debug/di/breakpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
mikelle-rogers marked this conversation as resolved.
Show resolved Hide resolved
}

// Note: we're sending a two-way event, so it blocks here
Expand Down
24 changes: 3 additions & 21 deletions src/coreclr/debug/ee/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -8655,7 +8637,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
Expand Down
56 changes: 12 additions & 44 deletions src/coreclr/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2832,6 +2832,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())
Expand Down Expand Up @@ -2998,6 +3000,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!
Expand Down Expand Up @@ -5402,28 +5405,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
Expand Down Expand Up @@ -6210,25 +6191,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,
Expand Down Expand Up @@ -7856,6 +7818,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())
Expand Down Expand Up @@ -10489,7 +10452,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)));
}

{
Expand Down Expand Up @@ -12616,7 +12579,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
)
Expand All @@ -12628,7 +12591,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)
Expand Down Expand Up @@ -12952,6 +12915,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++)
{
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/debug/ee/debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
);
Expand Down
12 changes: 1 addition & 11 deletions src/coreclr/debug/ee/functioninfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
{
Expand All @@ -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
{
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/debug/inc/dbgipcevents.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 1 addition & 6 deletions 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
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;
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/eedbginterfaceimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,6 @@ 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 @@ -804,8 +804,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.
Expand Down
19 changes: 11 additions & 8 deletions src/coreclr/vm/method.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -935,7 +934,7 @@ PCODE MethodDesc::GetNativeCode()
return GetStableEntryPoint();
}

PCODE MethodDesc::GetNativeCodeReJITAware()
PCODE MethodDesc::GetNativeCodeAnyVersion()
{
WRAPPER_NO_CONTRACT;
SUPPORTS_DAC;
Expand All @@ -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));
Copy link
Member

Choose a reason for hiding this comment

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

Before you were getting the current IL version's address. Is there any guarantee of what order you get this in? Does it return in order of IL version and descend until a native body is found? For things like deoptimization, is that what we want?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no guarantee of order, however, the work has been done to look through everything that calls this code, and make adjustments to allow this behavior to be safe. GetNativeCodeAnyVersion is only used in HasNativeCodeAnyVersion, which is used in EnumMethodInstances::Next and CdStart. It is only used to see whether or not there is a native code body. The reason it was changed is with the addition of deoptimization, there is a possibility that the user asks the code to be deoptimized before it has a default and active version (before the method is jitted). Thus, the code could have a native code version even though it did not have an active nor default native code version. @davmason @noahfalk

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;
}
}
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 HasNativeCodeReJITAware()
BOOL HasNativeCodeAnyVersion()
{
LIMITED_METHOD_DAC_CONTRACT;

return GetNativeCodeReJITAware() != NULL;
return GetNativeCodeAnyVersion() != 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 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();
Expand Down