Remove helper method frame from Profiler hooks (#107152)

Remove helper method frame and update the CONTRACTs to suit the new behavior. This WILL change the profiler behavior in that a set of apis in the Profiler api surface will not fail immediately on any attempt to use them. However, the set of apis which change are I believe not safe to call within a ProfilerEnter/Leave hook as an actual triggered GC will cause potential memory corruption if the arguments/return value of the function have any GC pointers.

src/coreclr/debug/daccess/dacdbiimpl.cpp

@@ -649,39 +649,7 @@ void DacDbiInterfaceImpl::GetAppDomainFullName(
     AppDomain * pAppDomain = vmAppDomain.GetDacPtr();
 
     // Get the AppDomain name from the VM without changing anything
-    // We might be able to simplify this, eg. by returning an SString.
-    bool fIsUtf8;
-    PVOID pRawName = pAppDomain->GetFriendlyNameNoSet(&fIsUtf8);
-
-    if (!pRawName)
-    {
-        ThrowHR(E_NOINTERFACE);
-    }
-
-    HRESULT hrStatus = S_OK;
-    if (fIsUtf8)
-    {
-        // we have to allocate a temporary string
-        // we could avoid this by adding a version of IStringHolder::AssignCopy that takes a UTF8 string
-        // We should also probably check to see when fIsUtf8 is ever true (it looks like it should normally be false).
-        ULONG32 dwNameLen = 0;
-        hrStatus = ConvertUtf8((LPCUTF8)pRawName, 0, &dwNameLen, NULL);
-        if (SUCCEEDED( hrStatus ))
-        {
-            NewArrayHolder<WCHAR> pwszName(new WCHAR[dwNameLen]);
-            hrStatus = ConvertUtf8((LPCUTF8)pRawName, dwNameLen, &dwNameLen, pwszName );
-            IfFailThrow(hrStatus);
-
-            hrStatus =  pStrName->AssignCopy(pwszName);
-        }
-    }
-    else
-    {
-        hrStatus =  pStrName->AssignCopy(static_cast<PCWSTR>(pRawName));
-    }
-
-    // Very important that this either sets pStrName or Throws.
-    IfFailThrow(hrStatus);
+    IfFailThrow(pStrName->AssignCopy(pAppDomain->GetFriendlyName()));
 }
 
 //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/coreclr/debug/daccess/request.cpp

@@ -3040,24 +3040,39 @@ ClrDataAccess::GetAppDomainName(CLRDATA_ADDRESS addr, unsigned int count, _Inout
         // SystemDomain doesn't have this field.
         if (pNeeded)
             *pNeeded = 1;
-        if (name)
+        if (name && count > 0)
             name[0] = 0;
     }
     else
     {
         PTR_AppDomain pAppDomain = PTR_AppDomain(TO_TADDR(addr));
-        if (!pAppDomain->m_friendlyName.IsEmpty())
+
+        size_t countAsSizeT = count;
+        if (pAppDomain->m_friendlyName.IsValid())
         {
-            if (!pAppDomain->m_friendlyName.DacGetUnicode(count, name, pNeeded))
+            LPCWSTR friendlyName = (LPCWSTR)pAppDomain->m_friendlyName;
+            size_t friendlyNameLen = u16_strlen(friendlyName);
+
+            if (pNeeded)
             {
-                hr =  E_FAIL;
+                *pNeeded = (unsigned int)(friendlyNameLen + 1);
+            }
+
+            if (name && count > 0)
+            {
+                if (countAsSizeT > (friendlyNameLen + 1))
+                {
+                    countAsSizeT = friendlyNameLen + 1;
+                }
+                memcpy(name, friendlyName, countAsSizeT * sizeof(WCHAR));
+                name[countAsSizeT - 1] = 0;
             }
         }
         else
         {
             if (pNeeded)
                 *pNeeded = 1;
-            if (name)
+            if (name && count > 0)
                 name[0] = 0;
 
             hr = S_OK;

src/coreclr/debug/daccess/task.cpp

@@ -713,30 +713,21 @@ ClrDataAppDomain::GetName(
 
     EX_TRY
     {
-        bool isUtf8;
-        PVOID rawName = m_appDomain->GetFriendlyNameNoSet(&isUtf8);
+        LPCWSTR rawName = m_appDomain->GetFriendlyName();
         if (rawName)
         {
-            if (isUtf8)
-            {
-                status = ConvertUtf8((LPCUTF8)rawName,
-                                     bufLen, nameLen, name);
-            }
-            else
+            status = StringCchCopy(name, bufLen, rawName) == S_OK ?
+                S_OK : S_FALSE;
+            if (nameLen)
             {
-                status = StringCchCopy(name, bufLen, (PCWSTR)rawName) == S_OK ?
-                    S_OK : S_FALSE;
-                if (nameLen)
+                size_t cchName = u16_strlen(rawName) + 1;
+                if (FitsIn<ULONG32>(cchName))
                 {
-                    size_t cchName = u16_strlen((PCWSTR)rawName) + 1;
-                    if (FitsIn<ULONG32>(cchName))
-                    {
-                        *nameLen = (ULONG32) cchName;
-                    }
-                    else
-                    {
-                        status = COR_E_OVERFLOW;
-                    }
+                    *nameLen = (ULONG32) cchName;
+                }
+                else
+                {
+                    status = COR_E_OVERFLOW;
                 }
             }
         }

src/coreclr/debug/ee/debugger.cpp

@@ -2832,7 +2832,7 @@ HRESULT Debugger::GetILToNativeMapping(PCODE pNativeCodeStartAddress, ULONG32 cM
     CONTRACTL
     {
         THROWS;
-        GC_TRIGGERS_FROM_GETJITINFO;
+        GC_NOTRIGGER;
     }
     CONTRACTL_END;
 

src/coreclr/debug/ee/functioninfo.cpp

@@ -926,8 +926,6 @@ void DebuggerJitInfo::LazyInitBounds()
         LOG((LF_CORDB,LL_EVERYTHING, "DJI::LazyInitBounds: this=%p GetBoundariesAndVars success=%s\n",
             this, (fSuccess ? "true" : "false")));
 
-        // SetBoundaries uses the CodeVersionManager, need to take it now for lock ordering reasons
-        CodeVersionManager::LockHolder codeVersioningLockHolder;
         Debugger::DebuggerDataLockHolder debuggerDataLockHolder(g_pDebugger);
 
         if (!m_fAttemptInit)
@@ -1053,15 +1051,22 @@ void DebuggerJitInfo::SetBoundaries(ULONG32 cMap, ICorDebugInfo::OffsetMapping *
     // Pick a unique initial value (-10) so that the 1st doesn't accidentally match.
     int ilPrevOld = -10;
 
-    _ASSERTE(CodeVersionManager::IsLockOwnedByCurrentThread());
-
     InstrumentedILOffsetMapping mapping;
 
     ILCodeVersion ilVersion = m_nativeCodeVersion.GetILCodeVersion();
     if (!ilVersion.IsDefaultVersion())
     {
         // Did the current rejit provide a map?
-        const InstrumentedILOffsetMapping *pReJitMap = ilVersion.GetInstrumentedILMap();
+        const InstrumentedILOffsetMapping *pReJitMap = NULL;
+        if (ilVersion.GetRejitState() == ILCodeVersion::kStateActive)
+        {
+            pReJitMap = ilVersion.GetInstrumentedILMap();
+        }
+        else
+        {
+            _ASSERTE(!"Unexpected rejit state, should be active as there exists a native code version for this IL code version");
+        }
+
         if (pReJitMap != NULL)
         {
             mapping = *pReJitMap;
@@ -1607,7 +1612,6 @@ DebuggerJitInfo *DebuggerMethodInfo::FindOrCreateInitAndAddJitInfo(MethodDesc* f
     if (fd->IsVersionable())
     {
         CodeVersionManager *pCodeVersionManager = fd->GetCodeVersionManager();
-        CodeVersionManager::LockHolder codeVersioningLockHolder;
         nativeCodeVersion = pCodeVersionManager->GetNativeCodeVersion(fd, startAddr);
         if (nativeCodeVersion.IsNull())
         {

src/coreclr/inc/daccess.h

@@ -2469,6 +2469,12 @@ typedef DPTR(T_RUNTIME_FUNCTION) PTR_RUNTIME_FUNCTION;
 #endif
 #endif
 
+#ifdef DACCESS_COMPILE
+#define DAC_IGNORE(x)
+#else
+#define DAC_IGNORE(x) x
+#endif // DACCESS_COMPILE
+
 //----------------------------------------------------------------------------
 //
 // A PCODE is a valid PC/IP value -- a pointer to an instruction, possibly including some processor mode bits.

src/coreclr/vm/appdomain.cpp

@@ -1610,6 +1610,7 @@ AppDomain::AppDomain()
     , m_hndMissing{NULL}
 #endif //FEATURE_COMINTEROP
     , m_pDelayedLoaderAllocatorUnloadList{NULL}
+    , m_friendlyName{NULL}
     , m_pRootAssembly{NULL}
     , m_dwFlags{0}
     , m_cRef{1}
@@ -2803,7 +2804,7 @@ DomainAssembly * AppDomain::FindAssembly(PEAssembly * pPEAssembly, FindAssemblyO
     return NULL;
 }
 
-void AppDomain::SetFriendlyName(LPCWSTR pwzFriendlyName, BOOL fDebuggerCares/*=TRUE*/)
+void AppDomain::SetFriendlyName(LPCWSTR pwzFriendlyName)
 {
     CONTRACTL
     {
@@ -2821,72 +2822,48 @@ void AppDomain::SetFriendlyName(LPCWSTR pwzFriendlyName, BOOL fDebuggerCares/*=T
     if (pwzFriendlyName)
         tmpFriendlyName.Set(pwzFriendlyName);
     else
-    {
-        // If there is an assembly, try to get the name from it.
-        // If no assembly, but if it's the DefaultDomain, then give it a name
-
-        if (m_pRootAssembly)
-        {
-            tmpFriendlyName.SetUTF8(m_pRootAssembly->GetSimpleName());
-
-            SString::Iterator i = tmpFriendlyName.End();
-            if (tmpFriendlyName.FindBack(i, '.'))
-                tmpFriendlyName.Truncate(i);
-        }
-        else
-        {
-            tmpFriendlyName.Set(DEFAULT_DOMAIN_FRIENDLY_NAME);
-        }
-    }
+        tmpFriendlyName.Set(DEFAULT_DOMAIN_FRIENDLY_NAME);
 
     tmpFriendlyName.Normalize();
 
+    // This happens at most twice in a process, so don't worry about freeing the old one.
+    LPWSTR newFriendlyName = new WCHAR[tmpFriendlyName.GetCount() + 1];
+    u16_strcpy_s(newFriendlyName, tmpFriendlyName.GetCount() + 1, tmpFriendlyName.GetUnicode());
 
-    m_friendlyName = tmpFriendlyName;
-    m_friendlyName.Normalize();
+    m_friendlyName = newFriendlyName;
 
     if(g_pDebugInterface)
     {
         // update the name in the IPC publishing block
         if (SUCCEEDED(g_pDebugInterface->UpdateAppDomainEntryInIPC(this)))
         {
             // inform the attached debugger that the name of this appdomain has changed.
-            if (IsDebuggerAttached() && fDebuggerCares)
+            if (IsDebuggerAttached())
                 g_pDebugInterface->NameChangeEvent(this, NULL);
         }
     }
 }
+#endif // !DACCESS_COMPILE
 
-LPCWSTR AppDomain::GetFriendlyName(BOOL fDebuggerCares/*=TRUE*/)
+LPCWSTR AppDomain::GetFriendlyName()
 {
     CONTRACT (LPCWSTR)
     {
-        THROWS;
-        if (GetThreadNULLOk()) {GC_TRIGGERS;} else {DISABLED(GC_NOTRIGGER);}
+        NOTHROW;
+        GC_NOTRIGGER;
         MODE_ANY;
         POSTCONDITION(CheckPointer(RETVAL, NULL_OK));
         INJECT_FAULT(COMPlusThrowOM(););
     }
     CONTRACT_END;
 
-    if (m_friendlyName.IsEmpty())
-        SetFriendlyName(NULL, fDebuggerCares);
+    if (m_friendlyName == NULL)
+        RETURN DEFAULT_DOMAIN_FRIENDLY_NAME;
 
-    RETURN m_friendlyName;
+    RETURN (LPCWSTR)m_friendlyName;
 }
 
-LPCWSTR AppDomain::GetFriendlyNameForLogging()
-{
-    CONTRACT(LPCWSTR)
-    {
-        NOTHROW;
-        GC_NOTRIGGER;
-        MODE_ANY;
-        POSTCONDITION(CheckPointer(RETVAL,NULL_OK));
-    }
-    CONTRACT_END;
-    RETURN (m_friendlyName.IsEmpty() ?W(""):(LPCWSTR)m_friendlyName);
-}
+#ifndef DACCESS_COMPILE
 
 LPCWSTR AppDomain::GetFriendlyNameForDebugger()
 {
@@ -2900,7 +2877,7 @@ LPCWSTR AppDomain::GetFriendlyNameForDebugger()
     CONTRACT_END;
 
 
-    if (m_friendlyName.IsEmpty())
+    if (m_friendlyName == NULL)
     {
         BOOL fSuccess = FALSE;
 
@@ -2928,36 +2905,6 @@ LPCWSTR AppDomain::GetFriendlyNameForDebugger()
 
 #endif // !DACCESS_COMPILE
 
-#ifdef DACCESS_COMPILE
-
-PVOID AppDomain::GetFriendlyNameNoSet(bool* isUtf8)
-{
-    SUPPORTS_DAC;
-
-    if (!m_friendlyName.IsEmpty())
-    {
-        *isUtf8 = false;
-        return m_friendlyName.DacGetRawContent();
-    }
-    else if (m_pRootAssembly)
-    {
-        *isUtf8 = true;
-        return (PVOID)m_pRootAssembly->GetSimpleName();
-    }
-    else if (dac_cast<TADDR>(this) ==
-             dac_cast<TADDR>(SystemDomain::System()->DefaultDomain()))
-    {
-        *isUtf8 = false;
-        return (PVOID)DEFAULT_DOMAIN_FRIENDLY_NAME;
-    }
-    else
-    {
-        return NULL;
-    }
-}
-
-#endif // DACCESS_COMPILE
-
 #ifndef DACCESS_COMPILE
 
 BOOL AppDomain::AddFileToCache(AssemblySpec* pSpec, PEAssembly * pPEAssembly)
@@ -4480,7 +4427,7 @@ AppDomain::EnumMemoryRegions(CLRDataEnumMemoryFlags flags, bool enumThis)
     // We don't need AppDomain name in triage dumps.
     if (flags != CLRDATA_ENUM_MEM_TRIAGE)
     {
-        m_friendlyName.EnumMemoryRegions(flags);
+        m_friendlyName.EnumMem();
     }
 
     if (flags == CLRDATA_ENUM_MEM_HEAP2)

src/coreclr/vm/appdomain.hpp

@@ -1207,13 +1207,9 @@ class AppDomain : public BaseDomain
     ULONG Release(void) DAC_EMPTY_RET(0);
 
     //****************************************************************************************
-    LPCWSTR GetFriendlyName(BOOL fDebuggerCares = TRUE);
+    LPCWSTR GetFriendlyName();
     LPCWSTR GetFriendlyNameForDebugger();
-    LPCWSTR GetFriendlyNameForLogging();
-#ifdef DACCESS_COMPILE
-    PVOID GetFriendlyNameNoSet(bool* isUtf8);
-#endif
-    void SetFriendlyName(LPCWSTR pwzFriendlyName, BOOL fDebuggerCares = TRUE);
+    void SetFriendlyName(LPCWSTR pwzFriendlyName);
 
     PEAssembly * BindAssemblySpec(
         AssemblySpec *pSpec,
@@ -1522,7 +1518,7 @@ class AppDomain : public BaseDomain
     SPTR_DECL(AppDomain, m_pTheAppDomain);
 
 private:
-    SString         m_friendlyName;
+    PTR_CWSTR       m_friendlyName;
     PTR_Assembly    m_pRootAssembly;
 
     // General purpose flags.

src/coreclr/vm/ceeload.inl

@@ -285,7 +285,6 @@ inline PTR_ILCodeVersioningState Module::LookupILCodeVersioningState(mdMethodDef
     }
     CONTRACTL_END
 
-    _ASSERTE(CodeVersionManager::IsLockOwnedByCurrentThread());
     _ASSERTE(TypeFromToken(token) == mdtMethodDef);
     return m_ILCodeVersioningStateMap.GetElement(RidFromToken(token));
 }