Allow ILCodeVersion to fallback to default IL (dotnet/coreclr#18322)
authorNoah Falk <noahfalk@users.noreply.github.com>
Thu, 7 Jun 2018 19:53:08 +0000 (12:53 -0700)
committerGitHub <noreply@github.com>
Thu, 7 Jun 2018 19:53:08 +0000 (12:53 -0700)
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.

Commit migrated from https://github.com/dotnet/coreclr/commit/435a69a9aca037f348a09e00dffef8a0888682a7

src/coreclr/src/debug/daccess/dacdbiimpl.cpp
src/coreclr/src/vm/codeversion.cpp

index a85cfa9..e5cc45b 100644 (file)
@@ -7249,11 +7249,11 @@ HRESULT DacDbiInterfaceImpl::GetILCodeVersionNodeData(VMPTR_ILCodeVersionNode vm
 {
     DD_ENTER_MAY_THROW;
 #ifdef FEATURE_REJIT
-    ILCodeVersionNode* pILCodeVersionNode = vmILCodeVersionNode.GetDacPtr();
-    pData->m_state = pILCodeVersionNode->GetRejitState();
-    pData->m_pbIL = PTR_TO_CORDB_ADDRESS(dac_cast<ULONG_PTR>(pILCodeVersionNode->GetIL()));
-    pData->m_dwCodegenFlags = pILCodeVersionNode->GetJitFlags();
-    const InstrumentedILOffsetMapping* pMapping = pILCodeVersionNode->GetInstrumentedILMap();
+    ILCodeVersion ilCode(vmILCodeVersionNode.GetDacPtr());
+    pData->m_state = ilCode.GetRejitState();
+    pData->m_pbIL = PTR_TO_CORDB_ADDRESS(dac_cast<ULONG_PTR>(ilCode.GetIL()));
+    pData->m_dwCodegenFlags = ilCode.GetJitFlags();
+    const InstrumentedILOffsetMapping* pMapping = ilCode.GetInstrumentedILMap();
     if (pMapping)
     {
         pData->m_cInstrumentedMapEntries = (ULONG)pMapping->GetCount();
index 7406d19..cf756e1 100644 (file)
@@ -773,23 +773,38 @@ PTR_COR_ILMETHOD ILCodeVersion::GetIL() const
     }
     CONTRACTL_END
 
+    PTR_COR_ILMETHOD pIL = NULL;
     if (m_storageKind == StorageKind::Explicit)
     {
-        return AsNode()->GetIL();
+        pIL = AsNode()->GetIL();
     }
-    else
+    
+    // For the default code version we always fetch the globally stored default IL for a method
+    //
+    // In the non-default code version we assume NULL is the equivalent of explicitly requesting to
+    // re-use the default IL. Ideally there would be no reason to create a new version that re-uses
+    // the default IL (just use the default code version for that) but we do it here for compat. We've 
+    // got some profilers that use ReJIT to create a new code version and then instead of calling
+    // ICorProfilerFunctionControl::SetILFunctionBody they call ICorProfilerInfo::SetILFunctionBody. 
+    // This mutates the default IL so that it is now correct for their new code version. Of course this
+    // also overwrote the previous default IL so now the default code version GetIL() is out of sync
+    // with the jitted code. In the majority of cases we never re-read the IL after the initial
+    // jitting so this issue goes unnoticed.
+    //
+    // If changing the default IL after it is in use becomes more problematic in the future we would
+    // need to add enforcement that prevents profilers from using ICorProfilerInfo::SetILFunctionBody
+    // that way + coordinate with them because it is a breaking change for any profiler currently doing it.
+    if(pIL == NULL)
     {
         PTR_Module pModule = GetModule();
         PTR_MethodDesc pMethodDesc = dac_cast<PTR_MethodDesc>(pModule->LookupMethodDef(GetMethodDef()));
-        if (pMethodDesc == NULL)
+        if (pMethodDesc != NULL)
         {
-            return NULL;
-        }
-        else
-        {
-            return dac_cast<PTR_COR_ILMETHOD>(pMethodDesc->GetILHeader(TRUE));
+            pIL = dac_cast<PTR_COR_ILMETHOD>(pMethodDesc->GetILHeader(TRUE));
         }
     }
+
+    return pIL;
 }
 
 PTR_COR_ILMETHOD ILCodeVersion::GetILNoThrow() const