Remove internal binder flags (dotnet/coreclr#27476)

Removed flags:
* Dynamic bind flags were only used for one assert - which is obviously always true (otherwise it would fail)
* Ignore dynamic bind flag was never used.
* Bind cache failures flags was on the other hand always used - so hardcode it directly
* Change the other flags to simple booleans as it makes the code easier to read (and there are now only 2 such flags). Also renamed:
  * cache rerun -> skip failure caching
  * ignore ref/def match -> skip version compatibility check

Minor additional cleanup

Commit migrated from https://github.com/dotnet/coreclr/commit/7c5a46c27f2120528b13e7db78521134531e5edc

diff --git a/src/coreclr/src/binder/assemblybinder.cpp b/src/coreclr/src/binder/assemblybinder.cpp
index bd4aebd..647f166 100644 (file)
--- a/src/coreclr/src/binder/assemblybinder.cpp
+++ b/src/coreclr/src/binder/assemblybinder.cpp
@@ -486,7 +486,8 @@ namespace BINDER_SPACE
 
                 IF_FAIL_GO(BindByName(pApplicationContext,
                                       pAssemblyName,
-                                      BIND_CACHE_FAILURES,
+                                      false, // skipFailureCaching
+                                      false, // skipVersionCompatibilityCheck
                                       excludeAppPaths,
                                       &bindResult));
             }
@@ -650,7 +651,8 @@ namespace BINDER_SPACE
     /* static */
     HRESULT AssemblyBinder::BindByName(ApplicationContext *pApplicationContext,
                                        AssemblyName       *pAssemblyName,
-                                       DWORD               dwBindFlags,
+                                       bool                skipFailureCaching,
+                                       bool                skipVersionCompatibilityCheck,
                                        bool                excludeAppPaths,
                                        BindResult         *pBindResult)
     {
@@ -665,7 +667,7 @@ namespace BINDER_SPACE
         hr = pApplicationContext->GetFailureCache()->Lookup(assemblyDisplayName);
         if (FAILED(hr))
         {
-            if ((hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)) && RerunBind(dwBindFlags))
+            if ((hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND)) && skipFailureCaching)
             {
                 // Ignore pre-existing transient bind error (re-bind will succeed)
                 pApplicationContext->GetFailureCache()->Remove(assemblyDisplayName);
@@ -688,7 +690,7 @@ namespace BINDER_SPACE
 
         IF_FAIL_GO(BindLocked(pApplicationContext,
                               pAssemblyName,
-                              dwBindFlags,
+                              skipVersionCompatibilityCheck,
                               excludeAppPaths,
                               pBindResult));
 
@@ -699,9 +701,9 @@ namespace BINDER_SPACE
         }
 
     Exit:
-        if (FAILED(hr) && CacheBindFailures(dwBindFlags))
+        if (FAILED(hr))
         {
-            if (RerunBind(dwBindFlags))
+            if (skipFailureCaching)
             {
                 if (hr != HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND))
                 {
@@ -770,7 +772,7 @@ namespace BINDER_SPACE
         {
             IF_FAIL_GO(BindLocked(pApplicationContext,
                                   pAssemblyName,
-                                  0 /*  Do not IgnoreDynamicBinds */,
+                                  false, // skipVersionCompatibilityCheck
                                   excludeAppPaths,
                                   &lockedBindResult));
             if (lockedBindResult.HaveResult())
@@ -781,7 +783,6 @@ namespace BINDER_SPACE
         }
 
         hr = S_OK;
-        pAssembly->SetIsDynamicBind(TRUE);
         pBindResult->SetResult(pAssembly);
 
     Exit:
@@ -801,27 +802,20 @@ namespace BINDER_SPACE
     /* static */
     HRESULT AssemblyBinder::BindLocked(ApplicationContext *pApplicationContext,
                                        AssemblyName       *pAssemblyName,
-                                       DWORD               dwBindFlags,
+                                       bool                skipVersionCompatibilityCheck,
                                        bool                excludeAppPaths,
                                        BindResult         *pBindResult)
     {
         HRESULT hr = S_OK;
         BINDER_LOG_ENTER(W("AssemblyBinder::BindLocked"));
         
-        BOOL fIgnoreDynamicBinds = IgnoreDynamicBinds(dwBindFlags);
-        
 #ifndef CROSSGEN_COMPILE
         ContextEntry *pContextEntry = NULL;
         IF_FAIL_GO(FindInExecutionContext(pApplicationContext, pAssemblyName, &pContextEntry));
         if (pContextEntry != NULL)
         {
-            if (fIgnoreDynamicBinds && pContextEntry->GetIsDynamicBind())
-            {
-                // Dynamic binds need to be always considered a failure for binding closures
-                IF_FAIL_GO(FUSION_E_APP_DOMAIN_LOCKED);
-            }
 #if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
-            else if (IgnoreRefDefMatch(dwBindFlags))
+            if (skipVersionCompatibilityCheck)
             {
                 // Skip RefDef matching if we have been asked to.
             }
@@ -1369,31 +1363,25 @@ namespace BINDER_SPACE
         HRESULT hr = S_OK;
         BINDER_LOG_ENTER(W("AssemblyBinder::Register"));
 
-        if (!pBindResult->GetIsContextBound())
-        {
-            pApplicationContext->IncrementVersion();
+        _ASSERTE(!pBindResult->GetIsContextBound());
 
-            // Register the bindResult in the ExecutionContext only if we dont have it already.
-            // This method is invoked under a lock (by its caller), so we are thread safe.
-            ContextEntry *pContextEntry = NULL;
-            hr = FindInExecutionContext(pApplicationContext, pBindResult->GetAssemblyName(), &pContextEntry);
-            if (hr == S_OK)
-            {
-                if (pContextEntry == NULL)
-                {
-                    ExecutionContext *pExecutionContext = pApplicationContext->GetExecutionContext();
-                    IF_FAIL_GO(pExecutionContext->Register(pBindResult));
-                }
-                else
-                {
-                    // The dynamic binds are compiled in CoreCLR, but they are not supported. They are only reachable by internal API Assembly.Load(byte[]) that nobody should be calling.
-                    // This code path does not handle dynamic binds correctly (and is not expected to). We do not expect to come here for dynamic binds.
+        pApplicationContext->IncrementVersion();
 
-                    _ASSERTE(!pContextEntry->GetIsDynamicBind());
-                        
-                    // Update the BindResult with the contents of the ContextEntry we found
-                    pBindResult->SetResult(pContextEntry);
-                }
+        // Register the bindResult in the ExecutionContext only if we dont have it already.
+        // This method is invoked under a lock (by its caller), so we are thread safe.
+        ContextEntry *pContextEntry = NULL;
+        hr = FindInExecutionContext(pApplicationContext, pBindResult->GetAssemblyName(), &pContextEntry);
+        if (hr == S_OK)
+        {
+            if (pContextEntry == NULL)
+            {
+                ExecutionContext *pExecutionContext = pApplicationContext->GetExecutionContext();
+                IF_FAIL_GO(pExecutionContext->Register(pBindResult));
+            }
+            else
+            {
+                // Update the BindResult with the contents of the ContextEntry we found
+                pBindResult->SetResult(pContextEntry);
             }
         }
 
@@ -1543,11 +1531,14 @@ Retry:
         CRITSEC_Holder contextLock(pApplicationContext->GetCriticalSectionCookie());
         
         // Attempt uncached bind and register stream if possible
+        // We skip version compatibility check - so assemblies with same simple name will be reported
+        // as a successfull bind. Below we compare MVIDs in that case instead (which is a more precise equality check).
         hr = BindByName(pApplicationContext,
-                               pAssemblyName,
-                               BIND_CACHE_FAILURES|BIND_CACHE_RERUN_BIND|BIND_IGNORE_REFDEF_MATCH,
-                               false, // excludeAppPaths
-                               &bindResult);
+                        pAssemblyName,
+                        true,  // skipFailureCaching
+                        true,  // skipVersionCompatibilityCheck
+                        false, // excludeAppPaths
+                        &bindResult);
         
         if (hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND))
         {
@@ -1556,7 +1547,6 @@ Retry:
                                            pPEImage,
                                            NULL,
                                            &bindResult));
-
         }
         else if (hr == S_OK)
         {

diff --git a/src/coreclr/src/binder/clrprivbindercoreclr.cpp b/src/coreclr/src/binder/clrprivbindercoreclr.cpp
index 05dee08..93a33fb 100644 (file)
--- a/src/coreclr/src/binder/clrprivbindercoreclr.cpp
+++ b/src/coreclr/src/binder/clrprivbindercoreclr.cpp
@@ -138,8 +138,6 @@ HRESULT CLRPrivBinderCoreCLR::BindUsingPEImage( /* in */ PEImage *pPEImage,
             IF_FAIL_GO(HRESULT_FROM_WIN32(ERROR_BAD_FORMAT));
         }
         
-        // Ensure we are not being asked to bind to a TPA assembly
-        //
         // Easy out for mscorlib
         if (pAssemblyName->IsMscorlib())
         {
@@ -147,8 +145,10 @@ HRESULT CLRPrivBinderCoreCLR::BindUsingPEImage( /* in */ PEImage *pPEImage,
         }
 
         {
+            // Ensure we are not being asked to bind to a TPA assembly
+            //
             SString& simpleName = pAssemblyName->GetSimpleName();
-            SimpleNameToFileNameMap * tpaMap = GetAppContext()->GetTpaList();
+            SimpleNameToFileNameMap* tpaMap = GetAppContext()->GetTpaList();
             if (tpaMap->LookupPtr(simpleName.GetUnicode()) != NULL)
             {
                 // The simple name of the assembly being requested to be bound was found in the TPA list.
@@ -159,18 +159,18 @@ HRESULT CLRPrivBinderCoreCLR::BindUsingPEImage( /* in */ PEImage *pPEImage,
                     if (pCoreCLRFoundAssembly->GetIsInGAC())
                     {
                         *ppAssembly = pCoreCLRFoundAssembly.Extract();
-                        goto Exit;                        
+                        goto Exit;
                     }
                 }
             }
+        }
             
-            hr = AssemblyBinder::BindUsingPEImage(&m_appContext, pAssemblyName, pPEImage, PeKind, pIMetaDataAssemblyImport, &pCoreCLRFoundAssembly);
-            if (hr == S_OK)
-            {
-                _ASSERTE(pCoreCLRFoundAssembly != NULL);
-                pCoreCLRFoundAssembly->SetBinder(this);
-                *ppAssembly = pCoreCLRFoundAssembly.Extract();
-            }
+        hr = AssemblyBinder::BindUsingPEImage(&m_appContext, pAssemblyName, pPEImage, PeKind, pIMetaDataAssemblyImport, &pCoreCLRFoundAssembly);
+        if (hr == S_OK)
+        {
+            _ASSERTE(pCoreCLRFoundAssembly != NULL);
+            pCoreCLRFoundAssembly->SetBinder(this);
+            *ppAssembly = pCoreCLRFoundAssembly.Extract();
         }
 Exit:;        
     }

diff --git a/src/coreclr/src/binder/inc/assembly.hpp b/src/coreclr/src/binder/inc/assembly.hpp
index 9d189bc..6d6c3e4 100644 (file)
--- a/src/coreclr/src/binder/inc/assembly.hpp
+++ b/src/coreclr/src/binder/inc/assembly.hpp
@@ -104,8 +104,6 @@ namespace BINDER_SPACE
 
         inline AssemblyName *GetAssemblyName(BOOL fAddRef = FALSE);
         inline BOOL GetIsInGAC();
-        inline BOOL GetIsDynamicBind();
-        inline void SetIsDynamicBind(BOOL fIsDynamicBind);
         inline BOOL GetIsByteArray();
         inline void SetIsByteArray(BOOL fIsByteArray);
         inline BOOL GetIsSharable();
@@ -121,10 +119,10 @@ namespace BINDER_SPACE
         static PEKIND GetSystemArchitecture();
         static BOOL IsValidArchitecture(PEKIND kArchitecture);
 
-               inline ICLRPrivBinder* GetBinder()
-               {
-                       return m_pBinder;
-               }
+        inline ICLRPrivBinder* GetBinder()
+        {
+            return m_pBinder;
+        }
 
 #ifndef CROSSGEN_COMPILE
     protected:
@@ -134,7 +132,7 @@ namespace BINDER_SPACE
         {
             FLAG_NONE = 0x00,
             FLAG_IS_IN_GAC = 0x02,
-            FLAG_IS_DYNAMIC_BIND = 0x04,
+            //FLAG_IS_DYNAMIC_BIND = 0x04,
             FLAG_IS_BYTE_ARRAY = 0x08,
             FLAG_IS_SHARABLE = 0x10
         };

diff --git a/src/coreclr/src/binder/inc/assembly.inl b/src/coreclr/src/binder/inc/assembly.inl
index 2a3646a..a011c26 100644 (file)
--- a/src/coreclr/src/binder/inc/assembly.inl
+++ b/src/coreclr/src/binder/inc/assembly.inl
@@ -100,23 +100,6 @@ void Assembly::SetIsInGAC(BOOL fIsInGAC)
     }
 }
 
-BOOL Assembly::GetIsDynamicBind()
-{
-    return ((m_dwAssemblyFlags & FLAG_IS_DYNAMIC_BIND) != 0);
-}
-
-void Assembly::SetIsDynamicBind(BOOL fIsDynamicBind)
-{
-    if (fIsDynamicBind)
-    {
-        m_dwAssemblyFlags |= FLAG_IS_DYNAMIC_BIND;
-    }
-    else
-    {
-        m_dwAssemblyFlags &= ~FLAG_IS_DYNAMIC_BIND;
-    }
-}
-
 BOOL Assembly::GetIsByteArray()
 {
     return ((m_dwAssemblyFlags & FLAG_IS_BYTE_ARRAY) != 0);

diff --git a/src/coreclr/src/binder/inc/assemblybinder.hpp b/src/coreclr/src/binder/inc/assemblybinder.hpp
index 216c3ea..0faccb2 100644 (file)
--- a/src/coreclr/src/binder/inc/assemblybinder.hpp
+++ b/src/coreclr/src/binder/inc/assemblybinder.hpp
@@ -81,44 +81,11 @@ namespace BINDER_SPACE
                                  
         static HRESULT TranslatePEToArchitectureType(DWORD  *pdwPAFlags, PEKIND *PeKind);
         
-    protected:
-        enum
-        {
-            BIND_NONE = 0x00,
-            BIND_CACHE_FAILURES = 0x01,
-            BIND_CACHE_RERUN_BIND = 0x02,
-            BIND_IGNORE_DYNAMIC_BINDS = 0x04
-#if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
-            ,
-            BIND_IGNORE_REFDEF_MATCH = 0x8
-#endif // !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
-        };
-
-        static BOOL IgnoreDynamicBinds(DWORD dwBindFlags)
-        {
-            return ((dwBindFlags & BIND_IGNORE_DYNAMIC_BINDS) != 0);
-        }
-
-        static BOOL CacheBindFailures(DWORD dwBindFlags)
-        {
-            return ((dwBindFlags & BIND_CACHE_FAILURES) != 0);
-        }
-
-        static BOOL RerunBind(DWORD dwBindFlags)
-        {
-            return ((dwBindFlags & BIND_CACHE_RERUN_BIND) != 0);
-        }
-        
-#if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
-        static BOOL IgnoreRefDefMatch(DWORD dwBindFlags)
-        {
-            return ((dwBindFlags & BIND_IGNORE_REFDEF_MATCH) != 0);
-        }
-#endif // !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
-        
+    private:
         static HRESULT BindByName(/* in */  ApplicationContext *pApplicationContext,
                                   /* in */  AssemblyName       *pAssemblyName,
-                                  /* in */  DWORD               dwBindFlags,
+                                  /* in */  bool                skipFailureCaching,
+                                  /* in */  bool                skipVersionCompatibilityCheck,
                                   /* in */  bool                excludeAppPaths,
                                   /* out */ BindResult         *pBindResult);
 
@@ -134,7 +101,7 @@ namespace BINDER_SPACE
 
         static HRESULT BindLocked(/* in */  ApplicationContext *pApplicationContext,
                                   /* in */  AssemblyName       *pAssemblyName,
-                                  /* in */  DWORD               dwBindFlags,
+                                  /* in */  bool                skipVersionCompatibilityCheck,
                                   /* in */  bool                excludeAppPaths,
                                   /* out */ BindResult         *pBindResult);
 

diff --git a/src/coreclr/src/binder/inc/bindresult.hpp b/src/coreclr/src/binder/inc/bindresult.hpp
index c0d3659..7393864 100644 (file)
--- a/src/coreclr/src/binder/inc/bindresult.hpp
+++ b/src/coreclr/src/binder/inc/bindresult.hpp
@@ -29,8 +29,6 @@ namespace BINDER_SPACE
         inline IUnknown *GetAssembly(BOOL fAddRef = FALSE);
         inline Assembly *GetAsAssembly(BOOL fAddRef = FALSE);
 
-        inline BOOL GetIsDynamicBind();
-        inline void SetIsDynamicBind(BOOL fIsDynamicBind);
         inline BOOL GetIsInGAC();
         inline void SetIsInGAC(BOOL fIsInGAC);
         inline BOOL GetIsContextBound();

diff --git a/src/coreclr/src/binder/inc/bindresult.inl b/src/coreclr/src/binder/inc/bindresult.inl
index 2609b97..7529389 100644 (file)
--- a/src/coreclr/src/binder/inc/bindresult.inl
+++ b/src/coreclr/src/binder/inc/bindresult.inl
@@ -61,23 +61,6 @@ Assembly *BindResult::GetAsAssembly(BOOL fAddRef /* = FALSE */)
     return static_cast<Assembly *>(GetAssembly(fAddRef));
 }
 
-BOOL BindResult::GetIsDynamicBind()
-{
-    return ((m_dwResultFlags & ContextEntry::RESULT_FLAG_IS_DYNAMIC_BIND) != 0);
-}
-
-void BindResult::SetIsDynamicBind(BOOL fIsDynamicBind)
-{
-    if (fIsDynamicBind)
-    {
-        m_dwResultFlags |= ContextEntry::RESULT_FLAG_IS_DYNAMIC_BIND;
-    }
-    else
-    {
-        m_dwResultFlags &= ~ContextEntry::RESULT_FLAG_IS_DYNAMIC_BIND;
-    }
-}
-
 BOOL BindResult::GetIsInGAC()
 {
     return ((m_dwResultFlags & ContextEntry::RESULT_FLAG_IS_IN_GAC) != 0);
@@ -150,7 +133,6 @@ void BindResult::SetResult(ContextEntry *pContextEntry, BOOL fIsContextBound /*
 {
     _ASSERTE(pContextEntry != NULL);
 
-    SetIsDynamicBind(pContextEntry->GetIsDynamicBind());
     SetIsInGAC(pContextEntry->GetIsInGAC());
     SetIsContextBound(fIsContextBound);
     SetIsSharable(pContextEntry->GetIsSharable());
@@ -163,7 +145,6 @@ void BindResult::SetResult(Assembly *pAssembly)
 {
     _ASSERTE(pAssembly != NULL);
 
-    SetIsDynamicBind(pAssembly->GetIsDynamicBind());
     SetIsInGAC(pAssembly->GetIsInGAC());
     SetIsSharable(pAssembly->GetIsSharable());
     SAFE_RELEASE(m_pAssemblyName);

diff --git a/src/coreclr/src/binder/inc/contextentry.hpp b/src/coreclr/src/binder/inc/contextentry.hpp
index d35f57b..3a8b9b2 100644 (file)
--- a/src/coreclr/src/binder/inc/contextentry.hpp
+++ b/src/coreclr/src/binder/inc/contextentry.hpp
@@ -26,7 +26,7 @@ namespace BINDER_SPACE
         typedef enum
         {
             RESULT_FLAG_NONE             = 0x00,
-            RESULT_FLAG_IS_DYNAMIC_BIND  = 0x01,
+            //RESULT_FLAG_IS_DYNAMIC_BIND  = 0x01,
             RESULT_FLAG_IS_IN_GAC        = 0x02,
             //RESULT_FLAG_FROM_MANIFEST    = 0x04,
             RESULT_FLAG_CONTEXT_BOUND    = 0x08,
@@ -45,23 +45,6 @@ namespace BINDER_SPACE
             SAFE_RELEASE(m_pIUnknownAssembly);
         }
         
-        BOOL GetIsDynamicBind()
-        {
-            return ((m_dwResultFlags & RESULT_FLAG_IS_DYNAMIC_BIND) != 0);
-        }
-
-        void SetIsDynamicBind(BOOL fIsDynamicBind)
-        {
-            if (fIsDynamicBind)
-            {
-                m_dwResultFlags |= RESULT_FLAG_IS_DYNAMIC_BIND;
-            }
-            else
-            {
-                m_dwResultFlags &= ~RESULT_FLAG_IS_DYNAMIC_BIND;
-            }
-        }
-
         BOOL GetIsInGAC()
         {
             return ((m_dwResultFlags & RESULT_FLAG_IS_IN_GAC) != 0);

diff --git a/src/coreclr/src/binder/inc/loadcontext.inl b/src/coreclr/src/binder/inc/loadcontext.inl
index c584f8d..4fafa61 100644 (file)
--- a/src/coreclr/src/binder/inc/loadcontext.inl
+++ b/src/coreclr/src/binder/inc/loadcontext.inl
@@ -70,7 +70,6 @@ HRESULT LoadContext<dwIncludeFlags>::Register(BindResult *pBindResult)
 
     SAFE_NEW(pContextEntry, ContextEntry);
 
-    pContextEntry->SetIsDynamicBind(pBindResult->GetIsDynamicBind());
     pContextEntry->SetIsInGAC(pBindResult->GetIsInGAC());
     pContextEntry->SetIsSharable(pBindResult->GetIsSharable());
     pContextEntry->SetAssemblyName(pBindResult->GetAssemblyName(), TRUE /* fAddRef */);

diff --git a/src/coreclr/src/vm/appdomain.cpp b/src/coreclr/src/vm/appdomain.cpp
index ba59930..7d530db 100644 (file)
--- a/src/coreclr/src/vm/appdomain.cpp
+++ b/src/coreclr/src/vm/appdomain.cpp
@@ -6620,8 +6620,6 @@ HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadContextToB
                 COMPlusThrowHR(COR_E_INVALIDOPERATION, IDS_HOST_ASSEMBLY_RESOLVER_DYNAMICALLY_EMITTED_ASSEMBLIES_UNSUPPORTED, name);
             }
                 
-            // Is the assembly already bound using a binding context that will be incompatible?
-            // An example is attempting to consume an assembly bound to WinRT binder.
             pResolvedAssembly = pLoadedPEAssembly->GetHostAssembly();
         }
             
@@ -6630,6 +6628,8 @@ HRESULT RuntimeInvokeHostAssemblyResolver(INT_PTR pManagedAssemblyLoadContextToB
             _ASSERTE(pResolvedAssembly != NULL);
 
 #ifdef FEATURE_COMINTEROP
+            // Is the assembly already bound using a binding context that will be incompatible?
+            // An example is attempting to consume an assembly bound to WinRT binder.
             if (AreSameBinderInstance(pResolvedAssembly, GetAppDomain()->GetWinRtBinder()))
             {
                 // It is invalid to return an assembly bound to an incompatible binder