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

Commit 1cebf4d

Browse files
authored
Allow ILCodeVersion to fallback to default IL (#18502)
* Allow ILCodeVersion to fallback to default IL For compat with profilers that used our APIs in unexpected ways we can allow the ILCodeVersion to fallback to the default IL code when no IL was explicitly given. * Fix incorrect usage of ILCodeVersion::AsNode (issue #18602) When the debugger is querying the active rejit IL for an IL method that has not been rejitted it incorrectly creates a VMPTR_ILCodeVersionNode for a code version that shouldn't have one.
1 parent 5b4773e commit 1cebf4d

File tree

2 files changed

+35
-14
lines changed

2 files changed

+35
-14
lines changed

src/debug/daccess/dacdbiimpl.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7169,7 +7169,7 @@ HRESULT DacDbiInterfaceImpl::GetActiveRejitILCodeVersionNode(VMPTR_Module vmModu
71697169
// manager's active IL version hasn't yet asked the profiler for the IL body to use, in which case we want to filter it
71707170
// out from the return in this method.
71717171
ILCodeVersion activeILVersion = pCodeVersionManager->GetActiveILCodeVersion(pModule, methodTk);
7172-
if (activeILVersion.IsNull() || activeILVersion.GetRejitState() != ILCodeVersion::kStateActive)
7172+
if (activeILVersion.IsNull() || activeILVersion.IsDefaultVersion() || activeILVersion.GetRejitState() != ILCodeVersion::kStateActive)
71737173
{
71747174
pVmILCodeVersionNode->SetDacTargetPtr(0);
71757175
}
@@ -7249,11 +7249,11 @@ HRESULT DacDbiInterfaceImpl::GetILCodeVersionNodeData(VMPTR_ILCodeVersionNode vm
72497249
{
72507250
DD_ENTER_MAY_THROW;
72517251
#ifdef FEATURE_REJIT
7252-
ILCodeVersionNode* pILCodeVersionNode = vmILCodeVersionNode.GetDacPtr();
7253-
pData->m_state = pILCodeVersionNode->GetRejitState();
7254-
pData->m_pbIL = PTR_TO_CORDB_ADDRESS(dac_cast<ULONG_PTR>(pILCodeVersionNode->GetIL()));
7255-
pData->m_dwCodegenFlags = pILCodeVersionNode->GetJitFlags();
7256-
const InstrumentedILOffsetMapping* pMapping = pILCodeVersionNode->GetInstrumentedILMap();
7252+
ILCodeVersion ilCode(vmILCodeVersionNode.GetDacPtr());
7253+
pData->m_state = ilCode.GetRejitState();
7254+
pData->m_pbIL = PTR_TO_CORDB_ADDRESS(dac_cast<ULONG_PTR>(ilCode.GetIL()));
7255+
pData->m_dwCodegenFlags = ilCode.GetJitFlags();
7256+
const InstrumentedILOffsetMapping* pMapping = ilCode.GetInstrumentedILMap();
72577257
if (pMapping)
72587258
{
72597259
pData->m_cInstrumentedMapEntries = (ULONG)pMapping->GetCount();

src/vm/codeversion.cpp

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -773,23 +773,38 @@ PTR_COR_ILMETHOD ILCodeVersion::GetIL() const
773773
}
774774
CONTRACTL_END
775775

776+
PTR_COR_ILMETHOD pIL = NULL;
776777
if (m_storageKind == StorageKind::Explicit)
777778
{
778-
return AsNode()->GetIL();
779+
pIL = AsNode()->GetIL();
779780
}
780-
else
781+
782+
// For the default code version we always fetch the globally stored default IL for a method
783+
//
784+
// In the non-default code version we assume NULL is the equivalent of explicitly requesting to
785+
// re-use the default IL. Ideally there would be no reason to create a new version that re-uses
786+
// the default IL (just use the default code version for that) but we do it here for compat. We've
787+
// got some profilers that use ReJIT to create a new code version and then instead of calling
788+
// ICorProfilerFunctionControl::SetILFunctionBody they call ICorProfilerInfo::SetILFunctionBody.
789+
// This mutates the default IL so that it is now correct for their new code version. Of course this
790+
// also overwrote the previous default IL so now the default code version GetIL() is out of sync
791+
// with the jitted code. In the majority of cases we never re-read the IL after the initial
792+
// jitting so this issue goes unnoticed.
793+
//
794+
// If changing the default IL after it is in use becomes more problematic in the future we would
795+
// need to add enforcement that prevents profilers from using ICorProfilerInfo::SetILFunctionBody
796+
// that way + coordinate with them because it is a breaking change for any profiler currently doing it.
797+
if(pIL == NULL)
781798
{
782799
PTR_Module pModule = GetModule();
783800
PTR_MethodDesc pMethodDesc = dac_cast<PTR_MethodDesc>(pModule->LookupMethodDef(GetMethodDef()));
784-
if (pMethodDesc == NULL)
801+
if (pMethodDesc != NULL)
785802
{
786-
return NULL;
787-
}
788-
else
789-
{
790-
return dac_cast<PTR_COR_ILMETHOD>(pMethodDesc->GetILHeader(TRUE));
803+
pIL = dac_cast<PTR_COR_ILMETHOD>(pMethodDesc->GetILHeader(TRUE));
791804
}
792805
}
806+
807+
return pIL;
793808
}
794809

795810
PTR_COR_ILMETHOD ILCodeVersion::GetILNoThrow() const
@@ -925,13 +940,19 @@ HRESULT ILCodeVersion::SetActiveNativeCodeVersion(NativeCodeVersion activeNative
925940
ILCodeVersionNode* ILCodeVersion::AsNode()
926941
{
927942
LIMITED_METHOD_CONTRACT;
943+
//This is dangerous - NativeCodeVersion coerces non-explicit versions to NULL but ILCodeVersion assumes the caller
944+
//will never invoke AsNode() on a non-explicit node. Asserting for now as a minimal fix, but we should revisit this.
945+
_ASSERTE(m_storageKind == StorageKind::Explicit);
928946
return m_pVersionNode;
929947
}
930948
#endif //DACCESS_COMPILE
931949

932950
PTR_ILCodeVersionNode ILCodeVersion::AsNode() const
933951
{
934952
LIMITED_METHOD_DAC_CONTRACT;
953+
//This is dangerous - NativeCodeVersion coerces non-explicit versions to NULL but ILCodeVersion assumes the caller
954+
//will never invoke AsNode() on a non-explicit node. Asserting for now as a minimal fix, but we should revisit this.
955+
_ASSERTE(m_storageKind == StorageKind::Explicit);
935956
return m_pVersionNode;
936957
}
937958

0 commit comments

Comments
 (0)