Fix reported result for incompatible version for default ALC (#1083)

FindInLoadContext and ApplicationAssemblies stages were reporting name
mismatch instead of incompatible version

diff --git a/src/coreclr/src/binder/assemblybinder.cpp b/src/coreclr/src/binder/assemblybinder.cpp
index 48c9ff0..9ac5d96 100644 (file)
--- a/src/coreclr/src/binder/assemblybinder.cpp
+++ b/src/coreclr/src/binder/assemblybinder.cpp
@@ -51,83 +51,66 @@ namespace BINDER_SPACE
         //
         // This defines the assembly equivalence relation
         //
-        HRESULT IsValidAssemblyVersion(/* in */ AssemblyName *pRequestedName,
-                                       /* in */ AssemblyName *pFoundName,
-                                       /* in */ ApplicationContext *pApplicationContext)
+        bool IsCompatibleAssemblyVersion(/* in */ AssemblyName *pRequestedName,
+                                         /* in */ AssemblyName *pFoundName)
         {
-            HRESULT hr = S_OK;
             AssemblyVersion *pRequestedVersion = pRequestedName->GetVersion();
             AssemblyVersion *pFoundVersion = pFoundName->GetVersion();
 
-            do
+            if (!pRequestedVersion->HasMajor())
             {
-                if (!pRequestedVersion->HasMajor())
-                {
-                    // An unspecified requested version component matches any value for the same component in the found version,
-                    // regardless of lesser-order version components
-                    break;
-                }
-                if (!pFoundVersion->HasMajor() || pRequestedVersion->GetMajor() > pFoundVersion->GetMajor())
-                {
-                    // - A specific requested version component does not match an unspecified value for the same component in
-                    //   the found version, regardless of lesser-order version components
-                    // - Or, the requested version is greater than the found version
-                    hr = FUSION_E_APP_DOMAIN_LOCKED;
-                    break;
-                }
-                if (pRequestedVersion->GetMajor() < pFoundVersion->GetMajor())
-                {
-                    // The requested version is less than the found version
-                    break;
-                }
-
-                if (!pRequestedVersion->HasMinor())
-                {
-                    break;
-                }
-                if (!pFoundVersion->HasMinor() || pRequestedVersion->GetMinor() > pFoundVersion->GetMinor())
-                {
-                    hr = FUSION_E_APP_DOMAIN_LOCKED;
-                    break;
-                }
-                if (pRequestedVersion->GetMinor() < pFoundVersion->GetMinor())
-                {
-                    break;
-                }
-
-                if (!pRequestedVersion->HasBuild())
-                {
-                    break;
-                }
-                if (!pFoundVersion->HasBuild() || pRequestedVersion->GetBuild() > pFoundVersion->GetBuild())
-                {
-                    hr = FUSION_E_APP_DOMAIN_LOCKED;
-                    break;
-                }
-                if (pRequestedVersion->GetBuild() < pFoundVersion->GetBuild())
-                {
-                    break;
-                }
+                // An unspecified requested version component matches any value for the same component in the found version,
+                // regardless of lesser-order version components
+                return true;
+            }
+            if (!pFoundVersion->HasMajor() || pRequestedVersion->GetMajor() > pFoundVersion->GetMajor())
+            {
+                // - A specific requested version component does not match an unspecified value for the same component in
+                //   the found version, regardless of lesser-order version components
+                // - Or, the requested version is greater than the found version
+                return false;
+            }
+            if (pRequestedVersion->GetMajor() < pFoundVersion->GetMajor())
+            {
+                // The requested version is less than the found version
+                return true;
+            }
 
-                if (!pRequestedVersion->HasRevision())
-                {
-                    break;
-                }
-                if (!pFoundVersion->HasRevision() || pRequestedVersion->GetRevision() > pFoundVersion->GetRevision())
-                {
-                    hr = FUSION_E_APP_DOMAIN_LOCKED;
-                    break;
-                }
-            } while (false);
+            if (!pRequestedVersion->HasMinor())
+            {
+                return true;
+            }
+            if (!pFoundVersion->HasMinor() || pRequestedVersion->GetMinor() > pFoundVersion->GetMinor())
+            {
+                return false;
+            }
+            if (pRequestedVersion->GetMinor() < pFoundVersion->GetMinor())
+            {
+                return true;
+            }
 
-            if (pApplicationContext->IsTpaListProvided() && hr == FUSION_E_APP_DOMAIN_LOCKED)
+            if (!pRequestedVersion->HasBuild())
+            {
+                return true;
+            }
+            if (!pFoundVersion->HasBuild() || pRequestedVersion->GetBuild() > pFoundVersion->GetBuild())
             {
-                // For our new binding models, use a more descriptive error code than APP_DOMAIN_LOCKED for bind
-                // failures.
-                hr = FUSION_E_REF_DEF_MISMATCH;
+                return false;
+            }
+            if (pRequestedVersion->GetBuild() < pFoundVersion->GetBuild())
+            {
+                return true;
             }
 
-            return hr;
+            if (!pRequestedVersion->HasRevision())
+            {
+                return true;
+            }
+            if (!pFoundVersion->HasRevision() || pRequestedVersion->GetRevision() > pFoundVersion->GetRevision())
+            {
+                return false;
+            }
+            return true;
         }
 
         HRESULT URLToFullPath(PathString &assemblyPath)
@@ -613,6 +596,7 @@ namespace BINDER_SPACE
     {
         HRESULT hr = S_OK;
 
+        bool isTpaListProvided = pApplicationContext->IsTpaListProvided();
 #ifndef CROSSGEN_COMPILE
         ContextEntry *pContextEntry = NULL;
         hr = FindInExecutionContext(pApplicationContext, pAssemblyName, &pContextEntry);
@@ -624,26 +608,29 @@ namespace BINDER_SPACE
         IF_FAIL_GO(hr);
         if (pContextEntry != NULL)
         {
-#if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
-            if (skipVersionCompatibilityCheck)
+            if (!skipVersionCompatibilityCheck)
             {
-                // Skip RefDef matching if we have been asked to.
+                // Can't give higher version than already bound
+                bool isCompatible = IsCompatibleAssemblyVersion(pAssemblyName, pContextEntry->GetAssemblyName());
+                hr = isCompatible ? S_OK : FUSION_E_APP_DOMAIN_LOCKED;
+                pBindResult->SetAttemptResult(hr, pContextEntry);
+
+                // TPA binder returns FUSION_E_REF_DEF_MISMATCH for incompatible version
+                if (hr == FUSION_E_APP_DOMAIN_LOCKED && isTpaListProvided)
+                    hr = FUSION_E_REF_DEF_MISMATCH;
             }
-#endif // !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE)
             else
             {
-                // Can't give higher version than already bound
-                hr = IsValidAssemblyVersion(pAssemblyName, pContextEntry->GetAssemblyName(), pApplicationContext);
+                pBindResult->SetAttemptResult(hr, pContextEntry);
             }
 
-            pBindResult->SetAttemptResult(hr, pContextEntry);
             IF_FAIL_GO(hr);
 
             pBindResult->SetResult(pContextEntry);
         }
         else
 #endif // !CROSSGEN_COMPILE
-        if (pApplicationContext->IsTpaListProvided())
+        if (isTpaListProvided)
         {
             // BindByTpaList handles setting attempt results on the bind result
             hr = BindByTpaList(pApplicationContext,
@@ -652,8 +639,13 @@ namespace BINDER_SPACE
                                      pBindResult);
             if (SUCCEEDED(hr) && pBindResult->HaveResult())
             {
-                hr = IsValidAssemblyVersion(pAssemblyName, pBindResult->GetAssemblyName(), pApplicationContext);
+                bool isCompatible = IsCompatibleAssemblyVersion(pAssemblyName, pBindResult->GetAssemblyName());
+                hr = isCompatible ? S_OK : FUSION_E_APP_DOMAIN_LOCKED;
                 pBindResult->SetAttemptResult(hr, pBindResult->GetAsAssembly());
+
+                // TPA binder returns FUSION_E_REF_DEF_MISMATCH for incompatible version
+                if (hr == FUSION_E_APP_DOMAIN_LOCKED && isTpaListProvided)
+                    hr = FUSION_E_REF_DEF_MISMATCH;
             }
 
             if (FAILED(hr))

diff --git a/src/coreclr/tests/src/Loader/binding/tracing/BinderTracingTest.ResolutionFlow.cs b/src/coreclr/tests/src/Loader/binding/tracing/BinderTracingTest.ResolutionFlow.cs
index c0b2e2f..32c32c2 100644 (file)
--- a/src/coreclr/tests/src/Loader/binding/tracing/BinderTracingTest.ResolutionFlow.cs
+++ b/src/coreclr/tests/src/Loader/binding/tracing/BinderTracingTest.ResolutionFlow.cs
@@ -75,7 +75,7 @@ namespace BinderTracingTests
         //   ResolutionAttempted : AssemblyLoadContextResolvingEvent    (CustomALC)     [AssemblyNotFound]
         //   ResolutionAttempted : AppDomainAssemblyResolveEvent        (CustomALC)     [AssemblyNotFound]
         [BinderTest(isolate: true, testSetup: nameof(LoadSubdirectoryAssembly_InstanceALC))]
-        public static BindOperation FindInLoadContext_IncompatibleVersion()
+        public static BindOperation FindInLoadContext_CustomALC_IncompatibleVersion()
         {
             var assemblyName = new AssemblyName($"{SubdirectoryAssemblyName}, Version=4.3.2.1");
             Assert.Throws<FileNotFoundException>(() => alcInstance.LoadFromAssemblyName(assemblyName));
@@ -99,6 +99,31 @@ namespace BinderTracingTests
             };
         }
 
+        // Incompatible version in load context:
+        //   ResolutionAttempted : FindInLoadContext                    (DefaultALC)    [IncompatibleVersion]
+        //   ResolutionAttempted : AssemblyLoadContextResolvingEvent    (DefaultALC)    [AssemblyNotFound]
+        //   ResolutionAttempted : AppDomainAssemblyResolveEvent        (DefaultALC)    [AssemblyNotFound]
+        [BinderTest(isolate: true, testSetup: nameof(UseDependentAssembly))]
+        public static BindOperation FindInLoadContext_DefaultALC_IncompatibleVersion()
+        {
+            var assemblyName = new AssemblyName($"{DependentAssemblyName}, Version=4.3.2.1");
+            Assert.Throws<FileNotFoundException>(() => AssemblyLoadContext.Default.LoadFromAssemblyName(assemblyName));
+
+            return new BindOperation()
+            {
+                AssemblyName = assemblyName,
+                AssemblyLoadContext = DefaultALC,
+                Success = false,
+                Cached = false,
+                ResolutionAttempts = new List<ResolutionAttempt>()
+                {
+                    GetResolutionAttempt(assemblyName, ResolutionStage.FindInLoadContext, AssemblyLoadContext.Default, ResolutionResult.IncompatibleVersion, UseDependentAssembly()),
+                    GetResolutionAttempt(assemblyName, ResolutionStage.AssemblyLoadContextResolvingEvent, AssemblyLoadContext.Default, ResolutionResult.AssemblyNotFound),
+                    GetResolutionAttempt(assemblyName, ResolutionStage.AppDomainAssemblyResolveEvent, AssemblyLoadContext.Default, ResolutionResult.AssemblyNotFound)
+                }
+            };
+        }
+
         // Successful load through application assemblies search:
         //   ResolutionAttempted : FindInLoadContext        (DefaultALC)    [AssemblyNotFound]
         //   ResolutionAttempted : ApplicationAssemblies    (DefaultALC)    [Success]
@@ -124,6 +149,34 @@ namespace BinderTracingTests
             };
         }
 
+        // Incompatible version in load context:
+        //   ResolutionAttempted : FindInLoadContext                    (DefaultALC)    [AssemblyNotFound]
+        //   ResolutionAttempted : ApplicationAssemblies                (DefaultALC)    [IncompatibleVersion]
+        //   ResolutionAttempted : AssemblyLoadContextResolvingEvent    (DefaultALC)    [AssemblyNotFound]
+        //   ResolutionAttempted : AppDomainAssemblyResolveEvent        (DefaultALC)    [AssemblyNotFound]
+        [BinderTest(isolate: true)]
+        public static BindOperation ApplicationAssemblies_IncompatibleVersion()
+        {
+            var assemblyName = new AssemblyName($"{DependentAssemblyName}, Version=4.3.2.1");
+            Assert.Throws<FileNotFoundException>(() => AssemblyLoadContext.Default.LoadFromAssemblyName(assemblyName));
+
+            string assemblyPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), $"{DependentAssemblyName}.dll");
+            return new BindOperation()
+            {
+                AssemblyName = assemblyName,
+                AssemblyLoadContext = DefaultALC,
+                Success = false,
+                Cached = false,
+                ResolutionAttempts = new List<ResolutionAttempt>()
+                {
+                    GetResolutionAttempt(assemblyName, ResolutionStage.FindInLoadContext, AssemblyLoadContext.Default, ResolutionResult.AssemblyNotFound),
+                    GetResolutionAttempt(assemblyName, ResolutionStage.ApplicationAssemblies, AssemblyLoadContext.Default, ResolutionResult.IncompatibleVersion, new AssemblyName($"{DependentAssemblyName}, Version=1.0.0.0"), assemblyPath),
+                    GetResolutionAttempt(assemblyName, ResolutionStage.AssemblyLoadContextResolvingEvent, AssemblyLoadContext.Default, ResolutionResult.AssemblyNotFound),
+                    GetResolutionAttempt(assemblyName, ResolutionStage.AppDomainAssemblyResolveEvent, AssemblyLoadContext.Default, ResolutionResult.AssemblyNotFound)
+                }
+            };
+        }
+
         // Mismatched assembly name from platform assemblies:
         //   ResolutionAttempted : FindInLoadContext                    (DefaultALC)    [AssemblyNotFound]
         //   ResolutionAttempted : ApplicationAssemblies                (DefaultALC)    [MismatchedAssemblyName]