Remove MethodDesc::GetSig. (#109211)

This API is inherently unsafe because it doesn't provide a length. All uses are replaced with MethodDesc::GetSigPointer or MethodDesc::GetSigParser.
dotnet · Oct 25, 2024 · 661fd5e · 661fd5e
1 parent f1ef9bb
commit 661fd5e
Show file tree

Hide file tree

Showing 12 changed files with 43 additions and 83 deletions.
diff --git a/src/coreclr/debug/daccess/daccess.cpp b/src/coreclr/debug/daccess/daccess.cpp
@@ -5897,8 +5897,8 @@ ClrDataAccess::RawGetMethodName(
     return E_NOINTERFACE;
 
 NameFromMethodDesc:
-    if (methodDesc->GetClassification() == mcDynamic &&
-        !methodDesc->GetSig())
+    if (methodDesc->GetClassification() == mcDynamic
+        && methodDesc->GetSigParser().IsNull())
     {
         return FormatCLRStubName(
             NULL,

diff --git a/src/coreclr/inc/utilcode.h b/src/coreclr/inc/utilcode.h
@@ -2887,7 +2887,6 @@ class MethodNamesListBase
 
     MethodName     *pNames;         // List of names
 
-    bool IsInList(LPCUTF8 methodName, LPCUTF8 className, int numArgs);
 
 public:
     void Init()
@@ -2896,7 +2895,7 @@ class MethodNamesListBase
         pNames = 0;
     }
 
-    void Init(_In_ _In_z_ LPWSTR list)
+    void Init(_In_z_ LPWSTR list)
     {
         WRAPPER_NO_CONTRACT;
         pNames = 0;
@@ -2905,9 +2904,9 @@ class MethodNamesListBase
 
     void Destroy();
 
-    void Insert(_In_ _In_z_ LPWSTR list);
+    void Insert(_In_z_ LPWSTR list);
 
-    bool IsInList(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGNATURE sig = NULL);
+    bool IsInList(LPCUTF8 methodName, LPCUTF8 className, int numArgs = -1);
     bool IsInList(LPCUTF8 methodName, LPCUTF8 className, CORINFO_SIG_INFO* pSigInfo);
     bool IsEmpty()
     {
@@ -3011,7 +3010,7 @@ class ConfigMethodSet
         return m_list.IsEmpty();
     }
 
-    bool contains(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGNATURE sig = NULL);
+    bool contains(LPCUTF8 methodName, LPCUTF8 className, int argCount = -1);
     bool contains(LPCUTF8 methodName, LPCUTF8 className, CORINFO_SIG_INFO* pSigInfo);
 
     inline void ensureInit(const CLRConfig::ConfigStringInfo & info)

diff --git a/src/coreclr/utilcode/util.cpp b/src/coreclr/utilcode/util.cpp
@@ -1166,7 +1166,7 @@ void ConfigMethodSet::init(const CLRConfig::ConfigStringInfo & info)
 }
 
 /**************************************************************************/
-bool ConfigMethodSet::contains(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGNATURE sig)
+bool ConfigMethodSet::contains(LPCUTF8 methodName, LPCUTF8 className, int argCount)
 {
     CONTRACTL
     {
@@ -1178,7 +1178,7 @@ bool ConfigMethodSet::contains(LPCUTF8 methodName, LPCUTF8 className, PCCOR_SIGN
 
     if (m_list.IsEmpty())
         return false;
-    return(m_list.IsInList(methodName, className, sig));
+    return(m_list.IsInList(methodName, className, argCount));
 }
 
 /**************************************************************************/
@@ -1511,25 +1511,6 @@ void MethodNamesListBase::Destroy()
     }
 }
 
-/**************************************************************/
-bool MethodNamesListBase::IsInList(LPCUTF8 methName, LPCUTF8 clsName, PCCOR_SIGNATURE sig)
-{
-    CONTRACTL
-    {
-        NOTHROW;
-    }
-    CONTRACTL_END;
-
-    int numArgs = -1;
-    if (sig != NULL)
-    {
-        sig++;      // Skip calling convention
-        numArgs = CorSigUncompressData(sig);
-    }
-
-    return IsInList(methName, clsName, numArgs);
-}
-
 /**************************************************************/
 bool MethodNamesListBase::IsInList(LPCUTF8 methName, LPCUTF8 clsName, CORINFO_SIG_INFO* pSigInfo)
 {

diff --git a/src/coreclr/vm/comdelegate.cpp b/src/coreclr/vm/comdelegate.cpp
@@ -1971,11 +1971,12 @@ Stub* COMDelegate::GetInvokeMethodStub(EEImplMethodDesc* pMD)
     {
         // Validate the invoke method, which at the moment just means checking the calling convention
 
-        if (*pMD->GetSig() != (IMAGE_CEE_CS_CALLCONV_HASTHIS | IMAGE_CEE_CS_CALLCONV_DEFAULT))
-            COMPlusThrow(kInvalidProgramException);
-
         MetaSig sig(pMD);
 
+        BYTE callConv = sig.GetCallingConventionInfo();
+        if (callConv != (IMAGE_CEE_CS_CALLCONV_HASTHIS | IMAGE_CEE_CS_CALLCONV_DEFAULT))
+            COMPlusThrow(kInvalidProgramException);
+
         BOOL fReturnVal = !sig.IsReturnTypeVoid();
 
         SigTypeContext emptyContext;

diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp
@@ -408,7 +408,7 @@ class ILStubState : public StubState
 
         {
             // Convert to a module independent signature
-            SigPointer sigPtr(pStubMD->GetSig());
+            SigPointer sigPtr = pStubMD->GetSigPointer();
             sigPtr.ConvertToInternalSignature(pStubMD->GetModule(), NULL, &sigBuilder, /* bSkipCustomModifier */ FALSE);
         }
 
@@ -469,7 +469,7 @@ class ILStubState : public StubState
     {
         SigBuilder sigBuilder;
         _ASSERTE(pStubMD->IsNoMetadata());
-        SigPointer sigPtr(pStubMD->GetSig());
+        SigPointer sigPtr = pStubMD->GetSigPointer();
         sigPtr.ConvertToInternalSignature(pStubMD->GetModule(), NULL, &sigBuilder, /* bSkipCustomModifier */ FALSE);
 
         //
@@ -3163,15 +3163,15 @@ HRESULT NDirect::HasNAT_LAttribute(IMDInternalImport *pInternalImport, mdToken t
 /*static*/
 BOOL NDirect::MarshalingRequired(
     _In_opt_ MethodDesc* pMD,
-    _In_opt_ PCCOR_SIGNATURE pSig,
+    _In_opt_ SigPointer sigPointer,
     _In_opt_ Module* pModule,
     _In_opt_ SigTypeContext* pTypeContext,
     _In_ bool unmanagedCallersOnlyRequiresMarshalling)
 {
     CONTRACTL
     {
         STANDARD_VM_CHECK;
-        PRECONDITION(pMD != NULL || (pSig != NULL && pModule != NULL));
+        PRECONDITION(pMD != NULL || (!sigPointer.IsNull() && pModule != NULL));
     }
     CONTRACTL_END;
 
@@ -3227,19 +3227,17 @@ BOOL NDirect::MarshalingRequired(
         callConv = sigInfo.GetCallConv();
     }
 
-    if (pSig == NULL)
+    if (sigPointer.IsNull())
     {
         PREFIX_ASSUME(pMD != NULL);
 
-        pSig = pMD->GetSig();
+        sigPointer = pMD->GetSigPointer();
         pModule = pMD->GetModule();
     }
 
     // Check to make certain that the signature only contains types that marshal trivially
-    SigPointer ptr(pSig);
-    IfFailThrow(ptr.GetCallingConvInfo(NULL));
     uint32_t numArgs;
-    IfFailThrow(ptr.GetData(&numArgs));
+    IfFailThrow(sigPointer.SkipMethodHeaderSignature(&numArgs, false /* skipReturnType */));
     numArgs++;   // +1 for return type
 
     // We'll need to parse parameter native types
@@ -3264,7 +3262,7 @@ BOOL NDirect::MarshalingRequired(
 
     for (ULONG i = 0; i < numArgs; i++)
     {
-        SigPointer arg = ptr;
+        SigPointer arg = sigPointer;
         CorElementType type;
         IfFailThrow(arg.PeekElemType(&type));
 
@@ -3392,7 +3390,7 @@ BOOL NDirect::MarshalingRequired(
             }
         }
 
-        IfFailThrow(ptr.SkipExactlyOne());
+        IfFailThrow(sigPointer.SkipExactlyOne());
     }
 
     if (!FitsInU2(dwStackSize))
@@ -5021,7 +5019,7 @@ namespace
                                     {
                                         // For generic calli, we only support blittable types
                                         if (SF_IsCALLIStub(dwStubFlags)
-                                            && NDirect::MarshalingRequired(NULL, pStubMD->GetSig(), pSigDesc->m_pModule, &pSigDesc->m_typeContext))
+                                            && NDirect::MarshalingRequired(NULL, pStubMD->GetSigPointer(), pSigDesc->m_pModule, &pSigDesc->m_typeContext))
                                         {
                                             COMPlusThrow(kMarshalDirectiveException, IDS_EE_BADMARSHAL_GENERICS_RESTRICTION);
                                         }

diff --git a/src/coreclr/vm/dllimport.h b/src/coreclr/vm/dllimport.h
@@ -101,7 +101,7 @@ class NDirect
     // is compiling a method containing a P/Invoke that is being considered for inlining.
     static BOOL MarshalingRequired(
         _In_opt_ MethodDesc* pMD,
-        _In_opt_ PCCOR_SIGNATURE pSig = NULL,
+        _In_opt_ SigPointer sigPointer = {},
         _In_opt_ Module* pModule = NULL,
         _In_opt_ SigTypeContext* pTypeContext = NULL,
         _In_ bool unmanagedCallersOnlyRequiresMarshalling = true);

diff --git a/src/coreclr/vm/eeconfig.cpp b/src/coreclr/vm/eeconfig.cpp
@@ -681,7 +681,7 @@ HRESULT EEConfig::sync()
 
         tieredCompilation_CallCountingDelayMs =
             Configuration::GetKnobDWORDValue(W("System.Runtime.TieredCompilation.CallCountingDelayMs"), CLRConfig::EXTERNAL_TC_CallCountingDelayMs);
-        
+
         bool hasSingleProcessor = GetCurrentProcessCpuCount() == 1;
         if (hasSingleProcessor)
         {
@@ -847,25 +847,24 @@ bool EEConfig::IsInMethList(MethodNamesList* list, MethodDesc* pMD)
     } CONTRACTL_END;
 
     if (list == 0)
-        return(false);
-    else
-    {
-        DefineFullyQualifiedNameForClass();
+        return false;
 
-        LPCUTF8 name = pMD->GetName();
-        if (name == NULL)
-        {
-            return false;
-        }
-        LPCUTF8 className = GetFullyQualifiedNameForClass(pMD->GetMethodTable());
-        if (className == NULL)
-        {
-            return false;
-        }
-        PCCOR_SIGNATURE sig = pMD->GetSig();
+    DefineFullyQualifiedNameForClass();
 
-        return list->IsInList(name, className, sig);
-    }
+    LPCUTF8 name = pMD->GetName();
+    if (name == NULL)
+        return false;
+
+    LPCUTF8 className = GetFullyQualifiedNameForClass(pMD->GetMethodTable());
+    if (className == NULL)
+        return false;
+
+    SigPointer sig = pMD->GetSigPointer();
+    uint32_t argCount = 0;
+    if (FAILED(sig.SkipMethodHeaderSignature(&argCount)))
+        return false;
+
+    return list->IsInList(name, className, (int32_t)argCount);
 }
 
 // Ownership of the string buffer passes to ParseTypeList

diff --git a/src/coreclr/vm/genericdict.h b/src/coreclr/vm/genericdict.h
@@ -70,9 +70,6 @@ enum DictionaryEntrySignatureSource : BYTE
 class DictionaryEntryLayout
 {
 public:
-    DictionaryEntryLayout(PTR_VOID signature)
-    { LIMITED_METHOD_CONTRACT; m_signature = signature; }
-
     DictionaryEntryKind GetKind();
 
     PTR_VOID m_signature;

diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp
@@ -9849,7 +9849,7 @@ bool CEEInfo::pInvokeMarshalingRequired(CORINFO_METHOD_HANDLE method, CORINFO_SI
         GetTypeContext(&callSiteSig->sigInst, &typeContext);
         result = NDirect::MarshalingRequired(
                     NULL,
-                    callSiteSig->pSig,
+                    SigPointer{ callSiteSig->pSig, callSiteSig->cbSig },
                     GetModule(callSiteSig->scope),
                     &typeContext);
     }

diff --git a/src/coreclr/vm/method.cpp b/src/coreclr/vm/method.cpp
@@ -474,19 +474,6 @@ void MethodDesc::GetSigFromMetadata(IMDInternalImport * importer,
 }
 
 //*******************************************************************************
-PCCOR_SIGNATURE MethodDesc::GetSig()
-{
-    WRAPPER_NO_CONTRACT;
-
-    PCCOR_SIGNATURE pSig;
-    DWORD           cSig;
-
-    GetSig(&pSig, &cSig);
-
-    PREFIX_ASSUME(pSig != NULL);
-
-    return pSig;
-}
 
 Signature MethodDesc::GetSignature()
 {

diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp
@@ -820,8 +820,6 @@ class MethodDesc
         return IsEEImpl() || IsArray() || IsNoMetadata();
     }
 
-    PCCOR_SIGNATURE GetSig();
-
     void GetSig(PCCOR_SIGNATURE *ppSig, DWORD *pcSig);
     SigParser GetSigParser();
 

diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp
@@ -5415,7 +5415,7 @@ void MetaSig::EnsureSigValueTypesLoaded(MethodDesc *pMD)
     // The signature format is approximately:
     // CallingConvention   NumberOfArguments    ReturnType   Arg1  ...
     // There is also a blob length at pSig-1.
-    SigPointer ptr(pMD->GetSig());
+    SigPointer ptr = pMD->GetSigPointer();
 
     // Skip over calling convention.
     IfFailThrowBF(ptr.GetCallingConv(NULL), BFA_BAD_SIGNATURE, pModule);
@@ -5454,7 +5454,7 @@ void MetaSig::CheckSigTypesCanBeLoaded(MethodDesc * pMD)
     // The signature format is approximately:
     // CallingConvention   NumberOfArguments    ReturnType   Arg1  ...
     // There is also a blob length at pSig-1.
-    SigPointer ptr(pMD->GetSig());
+    SigPointer ptr = pMD->GetSigPointer();
 
     // Skip over calling convention.
     IfFailThrowBF(ptr.GetCallingConv(NULL), BFA_BAD_SIGNATURE, pModule);