Remove some indirection when raising AssemblyResolve event (dotnet#142)

* Remove some indirection when raising AssemblyResolve event * Delete unused code
temfordump · Nov 21, 2019 · 5cd7170 · 5cd7170
1 parent 7b408c1
commit 5cd7170
Show file tree

Hide file tree

Showing 9 changed files with 46 additions and 199 deletions.
diff --git a/src/coreclr/src/vm/appdomain.cpp b/src/coreclr/src/vm/appdomain.cpp
@@ -4735,14 +4735,13 @@ BOOL AppDomain::PostBindResolveAssembly(AssemblySpec  *pPrePolicySpec,
     BOOL fFailure = TRUE;
     *ppFailedSpec = pPrePolicySpec;
 
-
     PEAssemblyHolder result;
 
     if ((EEFileLoadException::GetFileLoadKind(hrBindResult) == kFileNotFoundException) ||
         (hrBindResult == FUSION_E_REF_DEF_MISMATCH) ||
         (hrBindResult == FUSION_E_INVALID_NAME))
     {
-        result = TryResolveAssembly(*ppFailedSpec);
+        result = TryResolveAssemblyUsingEvent(*ppFailedSpec);
 
         if (result != NULL && pPrePolicySpec->CanUseWithBindingCache() && result->CanUseWithBindingCache())
         {
@@ -4969,7 +4968,7 @@ EndTry2:;
     {
         HRESULT hrBindResult = S_OK;
         PEAssemblyHolder result;
-        
+
         bool isCached = false;
         EX_TRY
         {
@@ -5151,22 +5150,30 @@ EndTry2:;
 
 
 
-PEAssembly *AppDomain::TryResolveAssembly(AssemblySpec *pSpec)
+PEAssembly *AppDomain::TryResolveAssemblyUsingEvent(AssemblySpec *pSpec)
 {
     STATIC_CONTRACT_THROWS;
     STATIC_CONTRACT_GC_TRIGGERS;
     STATIC_CONTRACT_MODE_ANY;
 
-    PEAssembly *result = NULL;
+    // No assembly resolve on codebase binds
+    if (pSpec->GetName() == nullptr)
+        return nullptr;
 
+    PEAssembly *result = nullptr;
     EX_TRY
     {
-        result = pSpec->ResolveAssemblyFile(this);
+        Assembly *pAssembly = RaiseAssemblyResolveEvent(pSpec);
+        if (pAssembly != nullptr)
+        {
+            PEAssembly *pFile = pAssembly->GetManifestFile();
+            pFile->AddRef();
+            result = pFile;
+        }
     }
     EX_HOOK
     {
         Exception *pEx = GET_EXCEPTION();
-
         if (!pEx->IsTransient())
         {
             AddExceptionToCache(pSpec, pEx);
@@ -5448,21 +5455,6 @@ void AppDomain::NotifyDebuggerUnload()
 }
 #endif // DEBUGGING_SUPPORTED
 
-void AppDomain::SetSystemAssemblyLoadEventSent(BOOL fFlag)
-{
-    LIMITED_METHOD_CONTRACT;
-    if (fFlag == TRUE)
-        m_dwFlags |= LOAD_SYSTEM_ASSEMBLY_EVENT_SENT;
-    else
-        m_dwFlags &= ~LOAD_SYSTEM_ASSEMBLY_EVENT_SENT;
-}
-
-BOOL AppDomain::WasSystemAssemblyLoadEventSent(void)
-{
-    LIMITED_METHOD_CONTRACT;
-    return ((m_dwFlags & LOAD_SYSTEM_ASSEMBLY_EVENT_SENT) == 0) ? FALSE : TRUE;
-}
-
 #ifndef CROSSGEN_COMPILE
 
 #ifdef FEATURE_COMINTEROP

diff --git a/src/coreclr/src/vm/appdomain.hpp b/src/coreclr/src/vm/appdomain.hpp
@@ -2111,9 +2111,6 @@ class AppDomain : public BaseDomain
         IAssemblyName *    pAssemblyName,
         PEAssembly **      ppAssembly) DAC_EMPTY_RET(S_OK);
 
-
-    PEAssembly *TryResolveAssembly(AssemblySpec *pSpec);
-
     //****************************************************************************************
     //
     //****************************************************************************************
@@ -2122,8 +2119,6 @@ class AppDomain : public BaseDomain
     // in a lazy fashion so executables do not take the perf hit unless the load other
     // assemblies
 #ifndef DACCESS_COMPILE
-    void OnAssemblyLoad(Assembly *assem);
-    void OnAssemblyLoadUnlocked(Assembly *assem);
     static BOOL OnUnhandledException(OBJECTREF *pThrowable, BOOL isTerminating = TRUE);
 
 #endif
@@ -2139,9 +2134,6 @@ class AppDomain : public BaseDomain
     void NotifyDebuggerUnload();
 #endif // DEBUGGING_SUPPORTED
 
-    void SetSystemAssemblyLoadEventSent (BOOL fFlag);
-    BOOL WasSystemAssemblyLoadEventSent (void);
-
 #ifndef DACCESS_COMPILE
     OBJECTREF* AllocateStaticFieldObjRefPtrs(int nRequested, OBJECTREF** ppLazyAllocate = NULL)
     {
@@ -2485,7 +2477,8 @@ class AppDomain : public BaseDomain
     DispIDCache* SetupRefDispIDCache();
 #endif // FEATURE_COMINTEROP
 
-protected:
+private:
+    PEAssembly *TryResolveAssemblyUsingEvent(AssemblySpec *pSpec);
     BOOL PostBindResolveAssembly(AssemblySpec  *pPrePolicySpec,
                                  AssemblySpec  *pPostPolicySpec,
                                  HRESULT        hrBindResult,
@@ -2496,7 +2489,6 @@ class AppDomain : public BaseDomain
     void ReleaseRCWs(LPVOID pCtxCookie);
     void DetachRCWs();
 
-protected:
 #endif // FEATURE_COMINTEROP
 
 private:
@@ -2679,7 +2671,6 @@ class AppDomain : public BaseDomain
 
     enum {
         CONTEXT_INITIALIZED =               0x0001,
-        LOAD_SYSTEM_ASSEMBLY_EVENT_SENT =   0x0040,
         COMPILATION_DOMAIN =                0x0400, // Are we ngenning?
         IGNORE_UNHANDLED_EXCEPTIONS =      0x10000, // AppDomain was created using the APPDOMAIN_IGNORE_UNHANDLED_EXCEPTIONS flag
     };
@@ -2927,9 +2918,6 @@ class SystemDomain : public BaseDomain
         return AppDomain::GetCurrentDomain();
     }
 
-    // Notification when an assembly is loaded into the system domain
-    void OnAssemblyLoad(Assembly *assem);
-
     //****************************************************************************************
     //
     // Global Static to get the one and only system domain

diff --git a/src/coreclr/src/vm/assemblyname.cpp b/src/coreclr/src/vm/assemblyname.cpp
@@ -77,36 +77,6 @@ FCIMPL1(Object*, AssemblyNameNative::GetFileInformation, StringObject* filenameU
 }
 FCIMPLEND
 
-FCIMPL1(Object*, AssemblyNameNative::ToString, Object* refThisUNSAFE)
-{
-    FCALL_CONTRACT;
-
-    OBJECTREF pObj          = NULL;
-    ASSEMBLYNAMEREF pThis   = (ASSEMBLYNAMEREF) (OBJECTREF) refThisUNSAFE;
-    HELPER_METHOD_FRAME_BEGIN_RET_1(pThis);
-
-    if (pThis == NULL)
-        COMPlusThrow(kNullReferenceException, W("NullReference_This"));
-
-    ACQUIRE_STACKING_ALLOCATOR(pStackingAllocator);
-
-    AssemblySpec spec;
-    spec.InitializeSpec(pStackingAllocator, (ASSEMBLYNAMEREF*) &pThis, FALSE);
-
-    StackSString name;
-    spec.GetFileOrDisplayName(ASM_DISPLAYF_VERSION |
-                              ASM_DISPLAYF_CULTURE |
-                              ASM_DISPLAYF_PUBLIC_KEY_TOKEN,
-                              name);
-
-    pObj = (OBJECTREF) StringObject::NewString(name);
-
-    HELPER_METHOD_FRAME_END();
-    return OBJECTREFToObject(pObj);
-}
-FCIMPLEND
-
-
 FCIMPL1(Object*, AssemblyNameNative::GetPublicKeyToken, Object* refThisUNSAFE)
 {
     FCALL_CONTRACT;

diff --git a/src/coreclr/src/vm/assemblyname.hpp b/src/coreclr/src/vm/assemblyname.hpp
@@ -20,9 +20,7 @@ class AssemblyNameNative
 {
 public:
     static FCDECL1(Object*, GetFileInformation, StringObject* filenameUNSAFE);
-    static FCDECL1(Object*, ToString, Object* refThisUNSAFE);
     static FCDECL1(Object*, GetPublicKeyToken, Object* refThisUNSAFE);
-    static FCDECL1(Object*, EscapeCodeBase, StringObject* filenameUNSAFE);
     static FCDECL1(void, Init, Object * refThisUNSAFE);
 };
 

diff --git a/src/coreclr/src/vm/assemblyspec.cpp b/src/coreclr/src/vm/assemblyspec.cpp
@@ -652,22 +652,6 @@ void AssemblySpec::SetCodeBase(StackingAllocator* alloc, STRINGREF *pCodeBase)
 
 #endif // CROSSGEN_COMPILE
 
-
-void AssemblySpec::MatchRetargetedPublicKeys(Assembly *pAssembly)
-{
-    CONTRACTL
-    {
-        INSTANCE_CHECK;
-        THROWS;
-        GC_TRIGGERS;
-        MODE_ANY;
-        PRECONDITION(CheckPointer(pAssembly));
-    }
-    CONTRACTL_END;
-    ThrowHR(FUSION_E_REF_DEF_MISMATCH);
-}
-
-
 // Check if the supplied assembly's public key matches up with the one in the Spec, if any
 // Throws an appropriate exception in case of a mismatch
 void AssemblySpec::MatchPublicKeys(Assembly *pAssembly)
@@ -680,72 +664,45 @@ void AssemblySpec::MatchPublicKeys(Assembly *pAssembly)
         MODE_ANY;
     }
     CONTRACTL_END;
-    // Check that the public keys are the same as in the AR.
-    if (IsStrongNamed()) {
-
-        const void *pbPublicKey;
-        DWORD cbPublicKey;
-        pbPublicKey = pAssembly->GetPublicKey(&cbPublicKey);
-        if (cbPublicKey == 0)
-            ThrowHR(FUSION_E_PRIVATE_ASM_DISALLOWED);
-
-        if (m_dwFlags & afPublicKey) {
-            if ((m_cbPublicKeyOrToken != cbPublicKey) ||
-                memcmp(m_pbPublicKeyOrToken, pbPublicKey, m_cbPublicKeyOrToken))
-                return MatchRetargetedPublicKeys(pAssembly);
-        }
 
-        // Ref has a token
-        else {
-            StrongNameBufferHolder<BYTE> pbStrongNameToken;
-            DWORD cbStrongNameToken;
-
-            if (!StrongNameTokenFromPublicKey((BYTE*) pbPublicKey,
-                                              cbPublicKey,
-                                              &pbStrongNameToken,
-                                              &cbStrongNameToken))
-                ThrowHR(StrongNameErrorInfo());
-            if ((m_cbPublicKeyOrToken != cbStrongNameToken) ||
-                memcmp(m_pbPublicKeyOrToken,
-                       pbStrongNameToken,
-                       cbStrongNameToken)) {
-                return MatchRetargetedPublicKeys(pAssembly);
-            }
-        }
-    }
-}
+    // Check that the public keys are the same as in the AR.
+    if (!IsStrongNamed())
+        return;
 
+    const void *pbPublicKey;
+    DWORD cbPublicKey;
+    pbPublicKey = pAssembly->GetPublicKey(&cbPublicKey);
+    if (cbPublicKey == 0)
+        ThrowHR(FUSION_E_PRIVATE_ASM_DISALLOWED);
 
-PEAssembly *AssemblySpec::ResolveAssemblyFile(AppDomain *pDomain)
-{
-    CONTRACT(PEAssembly *)
+    if (IsAfPublicKey(m_dwFlags))
     {
-        INSTANCE_CHECK;
-        THROWS;
-        GC_TRIGGERS;
-        MODE_ANY;
-        POSTCONDITION(CheckPointer(RETVAL, NULL_OK));
-        INJECT_FAULT(COMPlusThrowOM(););
+        if ((m_cbPublicKeyOrToken != cbPublicKey) ||
+            memcmp(m_pbPublicKeyOrToken, pbPublicKey, m_cbPublicKeyOrToken))
+        {
+            ThrowHR(FUSION_E_REF_DEF_MISMATCH);
+        }
     }
-    CONTRACT_END;
-
-    // No assembly resolve on codebase binds
-    if (GetName() == NULL)
-        RETURN NULL;
-
-    Assembly *pAssembly = pDomain->RaiseAssemblyResolveEvent(this);
+    else
+    {
+        // Ref has a token
+        StrongNameBufferHolder<BYTE> pbStrongNameToken;
+        DWORD cbStrongNameToken;
 
-    if (pAssembly != NULL) {
-        PEAssembly *pFile = pAssembly->GetManifestFile();
-        pFile->AddRef();
+        if (!StrongNameTokenFromPublicKey((BYTE*) pbPublicKey,
+                                            cbPublicKey,
+                                            &pbStrongNameToken,
+                                            &cbStrongNameToken))
+            ThrowHR(StrongNameErrorInfo());
 
-        RETURN pFile;
+        if ((m_cbPublicKeyOrToken != cbStrongNameToken) ||
+            memcmp(m_pbPublicKeyOrToken, pbStrongNameToken, cbStrongNameToken))
+        {
+            ThrowHR(FUSION_E_REF_DEF_MISMATCH);
+        }
     }
-
-    RETURN NULL;
 }
 
-
 Assembly *AssemblySpec::LoadAssembly(FileLoadLevel targetLevel, BOOL fThrowOnFileNotFound)
 {
     CONTRACTL

diff --git a/src/coreclr/src/vm/assemblyspec.hpp b/src/coreclr/src/vm/assemblyspec.hpp
@@ -224,12 +224,8 @@ class AssemblySpec  : public BaseAssemblySpec
     // Load an assembly based on an explicit path
     static Assembly *LoadAssembly(LPCWSTR pFilePath);
 
-
-  private:
-    void MatchRetargetedPublicKeys(Assembly *pAssembly);
   public:
     void MatchPublicKeys(Assembly *pAssembly);
-    PEAssembly *ResolveAssemblyFile(AppDomain *pAppDomain);
 
     AppDomain *GetAppDomain()
     {

diff --git a/src/coreclr/src/vm/baseassemblyspec.h b/src/coreclr/src/vm/baseassemblyspec.h
@@ -172,49 +172,10 @@ class BaseAssemblySpec
     BOOL CompareEx(BaseAssemblySpec *pSpec, DWORD dwCompareFlags = ASC_Default);
     static int CompareStrings(LPCUTF8 string1, LPCUTF8 string2);
     static BOOL RefMatchesDef(const BaseAssemblySpec* pRef, const BaseAssemblySpec* pDef);
-    static BOOL VerifyBindingString(LPCWSTR pwStr);
 
     void GetFileOrDisplayName(DWORD flags, SString &result) const;
     void GetDisplayName(DWORD flags, SString &result) const;
 
-    inline void GetPublicKey(
-        PBYTE * ppbPublicKey,
-        DWORD * pcbPublicKey) const
-    {
-        LIMITED_METHOD_CONTRACT;
-        PRECONDITION(HasPublicKey());
-        if (ppbPublicKey != nullptr)
-        {
-            *ppbPublicKey = m_pbPublicKeyOrToken;
-        }
-        if (pcbPublicKey != nullptr)
-        {
-            *pcbPublicKey = m_cbPublicKeyOrToken;
-        }
-    }
-
-    inline void GetPublicKeyToken(
-        PBYTE * ppbPublicKeyToken,
-        DWORD * pcbPublicKeyToken) const
-    {
-        LIMITED_METHOD_CONTRACT;
-        PRECONDITION(HasPublicKeyToken());
-        if (ppbPublicKeyToken != nullptr)
-        {
-            *ppbPublicKeyToken = m_pbPublicKeyOrToken;
-        }
-        if (pcbPublicKeyToken != nullptr)
-        {
-            *pcbPublicKeyToken = m_cbPublicKeyOrToken;
-        }
-    }
-
-    inline BOOL IsRetargetable() const
-    {
-        LIMITED_METHOD_CONTRACT;
-        return IsAfRetargetable(m_dwFlags);
-    }
-
 protected:
     static BOOL CompareRefToDef(const BaseAssemblySpec *pRef, const BaseAssemblySpec *pDef);