Delete dead code and fix issues found by static analysis tools (dotne…

…t#26604)

src/debug/daccess/daccess.cpp

@@ -859,7 +859,10 @@ SplitName::FindType(IMDInternalImport* mdInternal)
 
     ULONG32 length;
     WCHAR   wszName[MAX_CLASS_NAME];
-    ConvertUtf8(m_typeName, MAX_CLASS_NAME, &length, wszName);
+    if (ConvertUtf8(m_typeName, MAX_CLASS_NAME, &length, wszName) != S_OK)
+    {
+        return false;
+    }
 
     WCHAR *pHead;
 

src/debug/di/divalue.cpp

@@ -520,7 +520,7 @@ CordbGenericValue::CordbGenericValue(CordbAppDomain *              pAppdomain,
     _ASSERTE(pType->m_elementType < ELEMENT_TYPE_MAX);
 
     // We can fill in the size now for generic values.
-    ULONG32 size;
+    ULONG32 size = 0;
     HRESULT hr;
     hr = pType->GetUnboxedObjectSize(&size);
     _ASSERTE (!FAILED(hr));
@@ -551,7 +551,7 @@ CordbGenericValue::CordbGenericValue(CordbType * pType)
       m_pValueHome(NULL)
 {
     // The only purpose of a literal value is to hold a RS literal value.
-    ULONG32 size;
+    ULONG32 size = 0;
     HRESULT hr;
     hr = pType->GetUnboxedObjectSize(&size);
     _ASSERTE (!FAILED(hr));
@@ -3413,7 +3413,7 @@ HRESULT CordbBoxValue::GetObject(ICorDebugObjectValue **ppObject)
     FAIL_IF_NEUTERED(this);
     ATT_REQUIRE_STOPPED_MAY_FAIL(GetProcess());
 
-    ULONG32 size;
+    ULONG32 size = 0;
     m_type->GetUnboxedObjectSize(&size);
 
     HRESULT hr = S_OK;

src/debug/ee/debugger.h

@@ -876,7 +876,6 @@ class DebuggerRCThread
 
 
     friend class Debugger;
-    HRESULT VerifySecurityOnRSCreatedEvents(HANDLE sse, HANDLE lsea, HANDLE lser);
     Debugger*                       m_debugger;
 
     // IPC_TARGET_* define default targets - if we ever want to do

src/debug/ee/rcthread.cpp

@@ -467,171 +467,6 @@ HRESULT DebuggerRCThread::Init(void)
     return S_OK;
 }
 
-#ifndef FEATURE_PAL
-
-// This function is used to verify the security descriptor on an event
-// matches our expectation to prevent attack. This should be called when
-// we opened an event by name and assumed that the RS creates the event.
-// That means the event's dacl should match our default policy - current user
-// and admin. It can be narrower. By default, the DACL looks like the debugger
-// process user, debuggee user, and admin.
-//
-HRESULT DebuggerRCThread::VerifySecurityOnRSCreatedEvents(
-    HANDLE  sse,
-    HANDLE  lsea,
-    HANDLE  lser)
-{
-
-    CONTRACTL
-    {
-        NOTHROW; 
-        GC_NOTRIGGER;
-    }
-    CONTRACTL_END;
-
-    STRESS_LOG0(LF_CORDB,LL_INFO1000,"DRCT::VerifySecurityOnRSCreatedEvents\n");
-
-    if (lsea == NULL || lser == NULL)
-    {
-        // no valid handle, does not need to verify.
-        // The caller will close the handles
-        return E_FAIL;
-    }
-
-    HRESULT     hr = S_OK;
-
-    SIZE_T         i;
-    ACCESS_ALLOWED_ACE *pAllowAceSSE  = NULL;
-    ACCESS_ALLOWED_ACE *pAllowAceLSEA = NULL;
-    ACCESS_ALLOWED_ACE *pAllowAceLSER = NULL;
-
-
-    EX_TRY
-    {
-        // Get security descriptors for the handles.
-        Win32SecurityDescriptor sdSSE;
-        sdSSE.InitFromHandle(sse);
-
-        Win32SecurityDescriptor sdLSEA;
-        sdLSEA.InitFromHandle(lsea);
-
-        Win32SecurityDescriptor sdLSER;
-        sdLSER.InitFromHandle(lser);
-
-
-
-
-        // Make sure all 3 have the same creator
-        // We've already verifed in CreateSetupSyncEvent that the SSE's owner is in the DACL.
-        if (!Sid::Equals(sdSSE.GetOwner(), sdLSEA.GetOwner()) ||
-            !Sid::Equals(sdSSE.GetOwner(), sdLSER.GetOwner()))
-        {
-            // Not equal! return now with failure code.
-            STRESS_LOG1(LF_CORDB,LL_INFO1000,"DRCT::VSORSCE failed on EqualSid - 0x%08x\n", hr);
-            ThrowHR(E_FAIL);
-        }
-
-        // DACL_SECURITY_INFORMATION
-        // Now verify the DACL. It should be only two of them at most. One of them is the
-        // target process SID.
-        Dacl daclSSE = sdSSE.GetDacl();
-        Dacl daclLSEA = sdLSEA.GetDacl();
-        Dacl daclLSER = sdLSER.GetDacl();
-
-
-        // Now all of these three ACL should be alike. There should be at most two of entries
-        // there. One if the debugger process's SID and one if debuggee sid.
-        if ((daclSSE.GetAceCount() != 1) && (daclSSE.GetAceCount() != 2))
-        {
-            ThrowHR(E_FAIL);
-        }
-
-
-        // All of the ace count should equal for all events.
-        if ((daclSSE.GetAceCount() != daclLSEA.GetAceCount()) || 
-            (daclSSE.GetAceCount() != daclLSER.GetAceCount()))
-        {
-            ThrowHR(E_FAIL);
-        }
-
-        // Now check the ACE inside.These should be all equal
-        for (i = 0; i < daclSSE.GetAceCount(); i++)
-        {
-            ACE_HEADER *pAce;
-
-            // Get the ace from the SSE
-            pAce = daclSSE.GetAce(i);
-            if (pAce->AceType != ACCESS_ALLOWED_ACE_TYPE)
-            {
-                ThrowHR(E_FAIL);
-            }
-            pAllowAceSSE = (ACCESS_ALLOWED_ACE*)pAce;
-
-            // Get the ace from LSEA
-            pAce = daclLSEA.GetAce(i);        
-            if (pAce->AceType != ACCESS_ALLOWED_ACE_TYPE)
-            {
-                ThrowHR(E_FAIL);
-            }
-            pAllowAceLSEA = (ACCESS_ALLOWED_ACE*)pAce;
-
-            // This is the SID
-            // We can call EqualSid on this pAllowAce->SidStart
-            if (EqualSid((PSID)&(pAllowAceSSE->SidStart), (PSID)&(pAllowAceLSEA->SidStart)) == FALSE)
-            {
-                // ACE not equal. Fail out.
-                ThrowHR(E_FAIL);
-            }
-
-            // Get the ace from LSER
-            pAce = daclLSER.GetAce(i);        
-            if (pAce->AceType != ACCESS_ALLOWED_ACE_TYPE)
-            {
-                ThrowHR(E_FAIL);
-            }
-            pAllowAceLSER = (ACCESS_ALLOWED_ACE*)pAce;
-
-            if (EqualSid((PSID)&(pAllowAceSSE->SidStart), (PSID)&(pAllowAceLSER->SidStart)) == FALSE)
-            {
-                // ACE not equal. Fail out.
-                ThrowHR(E_FAIL);
-            }
-        } // end for loop.
-
-
-        // The last ACE should be target process. That is it should be
-        // our process's sid!
-        //
-        if (pAllowAceLSER == NULL)
-        {
-            ThrowHR(E_FAIL);; // fail if we don't have the ACE.
-        }
-        {
-            SidBuffer sbCurrentProcess;
-            sbCurrentProcess.InitFromProcess(GetCurrentProcessId());
-            if (!Sid::Equals(sbCurrentProcess.GetSid(), (PSID)&(pAllowAceLSER->SidStart)))
-            {
-                ThrowHR(E_FAIL);
-            }
-        }
-    }
-    EX_CATCH    
-    {
-        // If we threw an exception, then the verification failed.
-        hr = E_FAIL;
-    }
-    EX_END_CATCH(RethrowTerminalExceptions);
-
-    if (FAILED(hr))
-    {
-        STRESS_LOG1(LF_CORDB,LL_INFO1000,"DRCT::VSORSCE failed with - 0x%08x\n", hr);
-    }
-
-    return hr;
-}
-
-#endif // FEATURE_PAL
-
 //---------------------------------------------------------------------------------------
 //
 // Setup the Runtime Offsets struct.

src/inc/utilcode.h

@@ -1366,7 +1366,7 @@ class CPUGroupInfo
     static BOOL GetLogicalProcessorInformationEx(LOGICAL_PROCESSOR_RELATIONSHIP relationship,
 		   SYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX *slpiex, PDWORD count); 
     static BOOL SetThreadGroupAffinity(HANDLE h,
-		    GROUP_AFFINITY *groupAffinity, GROUP_AFFINITY *previousGroupAffinity);
+		    const GROUP_AFFINITY *groupAffinity, GROUP_AFFINITY *previousGroupAffinity);
     static BOOL GetThreadGroupAffinity(HANDLE h, GROUP_AFFINITY *groupAffinity);
     static BOOL GetSystemTimes(FILETIME *idleTime, FILETIME *kernelTime, FILETIME *userTime);
     static void ChooseCPUGroupAffinity(GROUP_AFFINITY *gf);

src/md/enc/rwutil.cpp

@@ -458,7 +458,7 @@ HRESULT MDTOKENMAP::Init(
             }
             else
             {   // It has tokens, so we may see a token for every row.
-                pITables->GetTableInfo(ixTbl, 0, &cRows, 0,0,0);
+                IfFailGo(pITables->GetTableInfo(ixTbl, 0, &cRows, 0,0,0));
                 // Safe: cTotal += cRows
                 if (!ClrSafeInt<ULONG>::addition(cTotal, cRows, cTotal))
                 {

src/utilcode/securitywrapper.cpp

@@ -110,7 +110,7 @@ HRESULT GetSidFromProcessWorker(DWORD dwProcessId, SidType sidType, PSID *ppSid)
     TOKEN_USER                  *pTokUser = NULL;
     HANDLE                      hProc = INVALID_HANDLE_VALUE;
     HANDLE                      hToken = INVALID_HANDLE_VALUE;
-    DWORD                       dwRetLength;
+    DWORD                       dwRetLength = 0;
     LPVOID                      pvTokenInfo = NULL;
     TOKEN_INFORMATION_CLASS     tokenInfoClass;
     PSID                        pSidFromTokenInfo = NULL;

src/utilcode/util.cpp

@@ -824,7 +824,7 @@ BYTE * ClrVirtualAllocWithinRange(const BYTE *pMinAddr,
 }
 
 /*static*/ BOOL CPUGroupInfo::SetThreadGroupAffinity(HANDLE h, 
-                        GROUP_AFFINITY *groupAffinity, GROUP_AFFINITY *previousGroupAffinity)
+                        const GROUP_AFFINITY *groupAffinity, GROUP_AFFINITY *previousGroupAffinity)
 {
     LIMITED_METHOD_CONTRACT;
     return ::SetThreadGroupAffinity(h, groupAffinity, previousGroupAffinity);

src/vm/compile.cpp

@@ -408,7 +408,7 @@ HRESULT CEECompileInfo::LoadTypeRefWinRT(
             {
                 LPCSTR psznamespace;
                 LPCSTR pszname;
-                pAssemblyImport->GetNameOfTypeRef(ref, &psznamespace, &pszname);
+                IfFailThrow(pAssemblyImport->GetNameOfTypeRef(ref, &psznamespace, &pszname));
                 AssemblySpec spec;
                 spec.InitializeSpec(tkResolutionScope, pAssemblyImport, NULL);
                 spec.SetWindowsRuntimeType(psznamespace, pszname);

src/vm/profilinghelper.cpp

@@ -284,13 +284,9 @@ void CurrentProfilerStatus::Set(ProfilerStatus newProfStatus)
 //---------------------------------------------------------------------------------------
 // ProfilingAPIUtility members
 
-
 // See code:#LoadUnloadCallbackSynchronization.
 CRITSEC_COOKIE ProfilingAPIUtility::s_csStatus = NULL;
 
-
-SidBuffer * ProfilingAPIUtility::s_pSidBuffer = NULL;
-
 // ----------------------------------------------------------------------------
 // ProfilingAPIUtility::AppendSupplementaryInformation
 // 
@@ -1464,14 +1460,7 @@ void ProfilingAPIUtility::TerminateProfiling()
             g_profControlBlock.pProfInterface.Store(NULL);
         }
 
-        // NOTE: Intentionally not deleting s_pSidBuffer. Doing so can cause annoying races
-        // with other threads that lazily create and initialize it when needed. (Example:
-        // it's used to fill out the "User" field of profiler event log entries.) Keeping
-        // s_pSidBuffer around after a profiler detaches and before a new one attaches
-        // consumes a bit more memory unnecessarily, but it'll get paged out if another
-        // profiler doesn't attach.
-
-        // NOTE: Similarly, intentionally not destroying / NULLing s_csStatus. If
+        // NOTE: Intentionally not destroying / NULLing s_csStatus. If
         // s_csStatus is already initialized, we can reuse it each time we do another
         // attach / detach, so no need to destroy it.
 
@@ -1494,67 +1483,4 @@ void ProfilingAPIUtility::TerminateProfiling()
     }
 }
 
-#ifndef FEATURE_PAL
-
-// ----------------------------------------------------------------------------
-// ProfilingAPIUtility::GetCurrentProcessUserSid
-// 
-// Description:
-//    Generates a SID of the current user from the current process's token. SID is
-//    returned in an [out] param, and is also cached for future use. The SID is used for
-//    two purposes: event log entries (for filling out the User field) and the ACL used
-//    on the globally named pipe object for attaching profilers.
-//    
-// Arguments:
-//    * ppsid - [out] Generated (or cached) SID
-//        
-// Return Value:
-//    HRESULT indicating success or failure.
-//    
-
-// static
-HRESULT ProfilingAPIUtility::GetCurrentProcessUserSid(PSID * ppsid)
-{
-    CONTRACTL
-    {
-        NOTHROW;
-        GC_NOTRIGGER;
-        MODE_ANY;
-    }
-    CONTRACTL_END;
-
-    if (s_pSidBuffer == NULL)
-    {
-        HRESULT hr;
-        NewHolder<SidBuffer> pSidBuffer(new (nothrow) SidBuffer);
-        if (pSidBuffer == NULL)
-        {
-            return E_OUTOFMEMORY;
-        }
-
-        // This gets the SID of the user from the process token
-        hr = pSidBuffer->InitFromProcessUserNoThrow(GetCurrentProcessId());
-        if (FAILED(hr))
-        {
-            return hr;
-        }
-
-        if (FastInterlockCompareExchangePointer(
-            &s_pSidBuffer, 
-            pSidBuffer.GetValue(), 
-            NULL) == NULL)
-        {
-            // Lifetime successfully transferred to s_pSidBuffer, so don't delete it here
-            pSidBuffer.SuppressRelease();
-        }
-    }
-
-    _ASSERTE(s_pSidBuffer != NULL);
-    _ASSERTE(s_pSidBuffer->GetSid().RawSid() != NULL);
-    *ppsid = s_pSidBuffer->GetSid().RawSid();
-    return S_OK;
-}
-
-#endif // !FEATURE_PAL
-
 #endif // PROFILING_SUPPORTED

src/vm/profilinghelper.h

@@ -37,8 +37,6 @@ enum ProfAPIFaultFlags
 };
 #endif // _DEBUG
 
-class SidBuffer;
-
 //---------------------------------------------------------------------------------------
 // Static-only class to coordinate initialization of the various profiling API
 // structures, plus other utility stuff.
@@ -73,9 +71,6 @@ class ProfilingAPIUtility
     static void LogProfInfo(int iStringResourceID, ...);
     static void LogNoInterfaceError(REFIID iidRequested, LPCWSTR wszClsid);
     INDEBUG(static BOOL ShouldInjectProfAPIFault(ProfAPIFaultFlags faultFlag);)
-#ifndef FEATURE_PAL
-    static HRESULT GetCurrentProcessUserSid(PSID * ppsid);
-#endif // !FEATURE_PAL
 
 #ifdef FEATURE_PROFAPI_ATTACH_DETACH
     // ----------------------------------------------------------------------------
@@ -129,10 +124,6 @@ class ProfilingAPIUtility
         kAttachLoad,
     };
 
-    // Allocated lazily the first time it's needed, and then remains allocated until the
-    // process exits.
-    static SidBuffer * s_pSidBuffer;
-
     // See code:ProfilingAPIUtility::InitializeProfiling#LoadUnloadCallbackSynchronization
     static CRITSEC_COOKIE s_csStatus;