From 0af808c8977f16c2c9342c24bb4be9068a6ba5b2 Mon Sep 17 00:00:00 2001 From: AR-May <67507805+AR-May@users.noreply.github.com> Date: Fri, 15 Jan 2021 16:09:43 +0300 Subject: [PATCH] Remove unnecessary work and refactor CompareAssemblyIdentity (#5973) Fixes #3930. We remove the unused native call of CompareAssemblyIdentity (fusion.dll) and keep the custom managed implementation that duplicates it. Fusion implementation seems to be faster than its managed replacement, but it is only available for Windows. We, however, not sure that we run this code often enough for the difference to be noticeable. Current changes get us to a consistent implementation everywhere and are also a perf improvement since the unused native call won't happen. * Remove unnecessary work and refactor CompareAssemblyIdentity (#3930) --- src/Directory.BeforeCommon.targets | 1 - .../AssemblyDependency/ReferenceTable.cs | 118 ++++++++++- src/Tasks/NativeMethods.cs | 194 ------------------ 3 files changed, 114 insertions(+), 199 deletions(-) diff --git a/src/Directory.BeforeCommon.targets b/src/Directory.BeforeCommon.targets index 8e63a94f93d..3741fdc7d47 100644 --- a/src/Directory.BeforeCommon.targets +++ b/src/Directory.BeforeCommon.targets @@ -42,7 +42,6 @@ $(DefineConstants);FEATURE_ENCODING_DEFAULT $(DefineConstants);FEATURE_ENVIRONMENT_SYSTEMDIRECTORY $(DefineConstants);FEATURE_FILE_TRACKER - $(DefineConstants);FEATURE_FUSION_COMPAREASSEMBLYIDENTITY $(DefineConstants);FEATURE_GAC $(DefineConstants);FEATURE_GET_COMMANDLINE $(DefineConstants);FEATURE_HANDLE_SAFEWAITHANDLE diff --git a/src/Tasks/AssemblyDependency/ReferenceTable.cs b/src/Tasks/AssemblyDependency/ReferenceTable.cs index a842843a021..405f8662611 100644 --- a/src/Tasks/AssemblyDependency/ReferenceTable.cs +++ b/src/Tasks/AssemblyDependency/ReferenceTable.cs @@ -2236,6 +2236,118 @@ Dictionary> baseNameToReferences } } + // TODO: Verify correctness of this implementation and extend to more cases. + // Should be consistent with CompareAssemblyIdentity from Fusion API: + // The result should be TRUE if one (or more) of the following conditions is true: + // a) The assembly identities are equivalent. For strongly-named assemblies this means full match on (name, version, pkt, culture); for simply-named assemblies this means a match on (name, culture) + // b) The assemblies being compared are FX assemblies (even if the version numbers are not the same, these will compare as equivalent by way of unification) + // c) The assemblies are not FX assemblies but are equivalent because fUnified1 and/or fUnified2 were set. + // The fUnified flag is used to indicate that all versions up to the version number of the strongly-named assembly are considered equivalent to itself. + // For example, if assemblyIdentity1 is "foo, version=5.0.0.0, culture=neutral, publicKeyToken=...." and fUnified1==TRUE, then this means to treat all versions of the assembly in the range 0.0.0.0-5.0.0.0 to be equivalent to "foo, version=5.0.0.0, culture=neutral, publicKeyToken=...". + // If assemblyIdentity2 is the same as assemblyIdentity1, except has a lower version number (e.g.version range 0.0.0.0-5.0.0.0), then the function will return that the identities are equivalent. + // If assemblyIdentity2 is the same as assemblyIdentity1, but has a greater version number than 5.0.0.0 then the two identities will only be equivalent if fUnified2 is set. + /// + /// Compares two assembly identities to determine whether or not they are equivalent. + /// + /// Textual identity of the first assembly to be compared. + /// Flag to indicate user-specified unification for assemblyIdentity1. + /// Textual identity of the second assembly to be compared. + /// Flag to indicate user-specified unification for assemblyIdentity2. + /// + /// Boolean indicating whether the identities are equivalent. + /// + private static bool AreAssembliesEquivalent( + string assemblyIdentity1, + bool fUnified1, + string assemblyIdentity2, + bool fUnified2) + { + AssemblyName an1 = new AssemblyName(assemblyIdentity1); + AssemblyName an2 = new AssemblyName(assemblyIdentity2); + + if (RefMatchesDef(an1, an2)) + { + return true; + } + + if (!an1.Name.Equals(an2.Name, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + var versionCompare = an1.Version.CompareTo(an2.Version); + + if ((versionCompare < 0 && fUnified2) || (versionCompare > 0 && fUnified1)) + { + return true; + } + + if (versionCompare == 0) + { + return true; + } + + return false; + } + + // Based on coreclr baseassemblyspec.cpp (https://github.com/dotnet/coreclr/blob/4cf8a6b082d9bb1789facd996d8265d3908757b2/src/vm/baseassemblyspec.cpp#L330) + private static bool RefMatchesDef(AssemblyName @ref, AssemblyName def) + { + if (IsStrongNamed(@ref)) + { + return IsStrongNamed(def) && CompareRefToDef(@ref, def); + } + else + { + return @ref.Name.Equals(def.Name, StringComparison.OrdinalIgnoreCase); + } + } + + // Based on coreclr baseassemblyspec.inl (https://github.com/dotnet/coreclr/blob/32f0f9721afb584b4a14d69135bea7ddc129f755/src/vm/baseassemblyspec.inl#L679-L683) + private static bool IsStrongNamed(AssemblyName assembly) + { + var refPkt = assembly.GetPublicKeyToken(); + return refPkt != null && refPkt.Length != 0; + } + + // Based on https://github.com/dotnet/coreclr/blob/4cf8a6b082d9bb1789facd996d8265d3908757b2/src/vm/baseassemblyspec.cpp#L241 + private static bool CompareRefToDef(AssemblyName @ref, AssemblyName def) + { + if (!@ref.Name.Equals(def.Name, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + byte[] rpkt = @ref.GetPublicKeyToken(); + byte[] dpkt = def.GetPublicKeyToken(); + + if (rpkt.Length != dpkt.Length) + { + return false; + } + + for (int i = 0; i < rpkt.Length; i++) + { + if (rpkt[i] != dpkt[i]) + { + return false; + } + } + + if (@ref.Version != def.Version) + { + return false; + } + + if (@ref.CultureName != null && + @ref.CultureName != def.CultureName) + { + return false; + } + + return true; + } + /// /// Given two references along with their fusion names, resolve the filename conflict that they /// would have if both assemblies need to be copied to the same directory. @@ -2279,14 +2391,12 @@ private static int ResolveAssemblyNameConflict(AssemblyNameReference assemblyRef bool rightConflictLegacyUnified = !isNonUnified && assemblyReference1.reference.IsPrimary; // This is ok here because even if the method says two versions are equivalent the algorithm below will still pick the highest version. - NativeMethods.CompareAssemblyIdentity + bool equivalent = AreAssembliesEquivalent ( leftConflictFusionName, leftConflictLegacyUnified, rightConflictFusionName, - rightConflictLegacyUnified, - out bool equivalent, - out _ + rightConflictLegacyUnified ); Version leftConflictVersion = assemblyReference0.assemblyName.Version; diff --git a/src/Tasks/NativeMethods.cs b/src/Tasks/NativeMethods.cs index a4e4fd164a2..174dfe25f83 100644 --- a/src/Tasks/NativeMethods.cs +++ b/src/Tasks/NativeMethods.cs @@ -1052,200 +1052,6 @@ internal static extern int CreateAssemblyNameObject( internal static extern int GetCachePath(AssemblyCacheFlags cacheFlags, StringBuilder cachePath, ref int pcchPath); #endif - /*------------------------------------------------------------------------------ - CompareAssemblyIdentity - The Fusion API to compare two assembly identities to determine whether or not they are equivalent is now available. This new API is exported from mscorwks.dll, which you can access via mscoree's GetRealProcAddress. The function prototype is defined in fusion.h as follows: - - STDAPI CompareAssemblyIdentity(LPCWSTR pwzAssemblyIdentity1, - BOOL fUnified1, - LPCWSTR pwzAssemblyIdentity2, - BOOL fUnified2, - BOOL *pfEquivalent, - AssemblyComparisonResult *pResult); - -typedef enum _tagAssemblyComparisonResult -{ - ACR_Unknown, // Unknown - ACR_EquivalentFullMatch, // all fields match - ACR_EquivalentWeakNamed, // match based on weak-name, version numbers ignored - ACR_EquivalentFXUnified, // match based on FX-unification of version numbers - ACR_EquivalentUnified, // match based on legacy-unification of version numbers - ACR_NonEquivalentVersion, // all fields match except version field - ACR_NonEquivalent, // no match - - ACR_EquivalentPartialMatch, - ACR_EquivalentPartialWeakNamed, - ACR_EquivalentPartialUnified, - ACR_EquivalentPartialFXUnified, - ACR_NonEquivalentPartialVersion -} AssemblyComparisonResult; - - Parameters: - [in] LPCWSTR pwzAssemblyIdentity1 : Textual identity of the first assembly to be compared - [in] BOOL fUnified1 : Flag to indicate user-specified unification for pwzAssemblyIdentity1 (see below) - [in] LPCWSTR pwzAssemblyIdentity2 : Textual identity of the second assembly to be compared - [in] BOOL fUnified2 : Flag to inidcate user-specified unification for pwzAssemblyIdentity2 (see below) - [out] BOOL *pfEquivalent : Boolean indicating whether the identities are equivalent - [out] AssemblyComparisonResult *pResult : Contains detailed information about the comparison result - - This API will check whether or not pwzAssemblyIdentity1 and pwzAssemblyIdentity2 are equivalent. Both of these identities must be full-specified (name, version, pkt, culture). The pfEquivalent parameter will be set to TRUE if one (or more) of the following conditions is true: - - a) The assembly identities are equivalent. For strongly-named assemblies this means full match on (name, version, pkt, culture); for simply-named assemblies this means a match on (name, culture) - - b) The assemblies being compared are FX assemblies (even if the version numbers are not the same, these will compare as equivalent by way of unification) - - c) The assemblies are not FX assemblies but are equivalent because fUnified1 and/or fUnified2 were set. - - The fUnified flag is used to indicate that all versions up to the version number of the strongly-named assembly are considered equivalent to itself. For example, if pwzAssemblyIdentity1 is "foo, version=5.0.0.0, culture=neutral, publicKeyToken=...." and fUnified1==TRUE, then this means to treat all versions of the assembly in the range 0.0.0.0-5.0.0.0 to be equivalent to "foo, version=5.0.0.0, culture=neutral, publicKeyToken=...". If pwzAssemblyIdentity2 is the same as pwzAssemblyIdentity1, except has a lower version number (e.g. version range 0.0.0.0-5.0.0.0), then the API will return that the identities are equivalent. If pwzAssemblyIdentity2 is the same as pwzAssemblyIdentity1, but has a greater version number than 5.0.0.0 then the two identities will only be equivalent if fUnified2 is set. - - The AssemblyComparisonResult gives you information about why the identities compared as equal or not equal. The description of the meaning of each ACR_* return value is described in the declaration above. - ------------------------------------------------------------------------------*/ - [DllImport("fusion.dll", CharSet = CharSet.Unicode, EntryPoint = "CompareAssemblyIdentity")] - internal static extern int CompareAssemblyIdentityWindows - ( - string pwzAssemblyIdentity1, - [MarshalAs(UnmanagedType.Bool)] bool fUnified1, - string pwzAssemblyIdentity2, - [MarshalAs(UnmanagedType.Bool)] bool fUnified2, - [MarshalAs(UnmanagedType.Bool)] out bool pfEquivalent, - out AssemblyComparisonResult pResult - ); - - // TODO: Verify correctness of this implementation and - // extend to more cases. - internal static void CompareAssemblyIdentity( - string assemblyIdentity1, - bool fUnified1, - string assemblyIdentity2, - bool fUnified2, - out bool pfEquivalent, - out AssemblyComparisonResult pResult) - { -#if FEATURE_FUSION_COMPAREASSEMBLYIDENTITY - if (NativeMethodsShared.IsWindows) - { - CompareAssemblyIdentityWindows( - assemblyIdentity1, - fUnified1, - assemblyIdentity2, - fUnified2, - out pfEquivalent, - out pResult); - } -#endif - - AssemblyName an1 = new AssemblyName(assemblyIdentity1); - AssemblyName an2 = new AssemblyName(assemblyIdentity2); - - //pfEquivalent = AssemblyName.ReferenceMatchesDefinition(an1, an2); - pfEquivalent = RefMatchesDef(an1, an2); - if (pfEquivalent) - { - pResult = AssemblyComparisonResult.ACR_EquivalentFullMatch; - return; - } - - if (!an1.Name.Equals(an2.Name, StringComparison.OrdinalIgnoreCase)) - { - pResult = AssemblyComparisonResult.ACR_NonEquivalent; - pfEquivalent = false; - return; - } - - var versionCompare = an1.Version.CompareTo(an2.Version); - - if ((versionCompare < 0 && fUnified2) || (versionCompare > 0 && fUnified1)) - { - pResult = AssemblyComparisonResult.ACR_NonEquivalentVersion; - pfEquivalent = true; - return; - } - - if (versionCompare == 0) - { - pResult = AssemblyComparisonResult.ACR_EquivalentFullMatch; - pfEquivalent = true; - return; - } - - pResult = pfEquivalent ? AssemblyComparisonResult.ACR_EquivalentFullMatch : AssemblyComparisonResult.ACR_NonEquivalent; - } - - // Based on coreclr baseassemblyspec.cpp (https://github.com/dotnet/coreclr/blob/4cf8a6b082d9bb1789facd996d8265d3908757b2/src/vm/baseassemblyspec.cpp#L330) - private static bool RefMatchesDef(AssemblyName @ref, AssemblyName def) - { - if (IsStrongNamed(@ref)) - { - return IsStrongNamed(def) && CompareRefToDef(@ref, def); - } - else - { - return @ref.Name.Equals(def.Name, StringComparison.OrdinalIgnoreCase); - } - } - - // Based on coreclr baseassemblyspec.inl (https://github.com/dotnet/coreclr/blob/32f0f9721afb584b4a14d69135bea7ddc129f755/src/vm/baseassemblyspec.inl#L679-L683) - private static bool IsStrongNamed(AssemblyName assembly) - { - var refPkt = assembly.GetPublicKeyToken(); - return refPkt != null && refPkt.Length != 0; - } - - // Based on https://github.com/dotnet/coreclr/blob/4cf8a6b082d9bb1789facd996d8265d3908757b2/src/vm/baseassemblyspec.cpp#L241 - private static bool CompareRefToDef(AssemblyName @ref, AssemblyName def) - { - if (!@ref.Name.Equals(def.Name, StringComparison.OrdinalIgnoreCase)) - { - return false; - } - - byte[] rpkt = @ref.GetPublicKeyToken(); - byte[] dpkt = def.GetPublicKeyToken(); - - if (rpkt.Length != dpkt.Length) - { - return false; - } - - for (int i = 0; i < rpkt.Length; i++) - { - if (rpkt[i] != dpkt[i]) - { - return false; - } - } - - if (@ref.Version != def.Version) - { - return false; - } - - if (@ref.CultureName != null && - @ref.CultureName != def.CultureName) - { - return false; - } - - return true; - } - - internal enum AssemblyComparisonResult - { - ACR_Unknown, // Unknown - ACR_EquivalentFullMatch, // all fields match - ACR_EquivalentWeakNamed, // match based on weak-name, version numbers ignored - ACR_EquivalentFXUnified, // match based on FX-unification of version numbers - ACR_EquivalentUnified, // match based on legacy-unification of version numbers - ACR_NonEquivalentVersion, // all fields match except version field - ACR_NonEquivalent, // no match - - ACR_EquivalentPartialMatch, - ACR_EquivalentPartialWeakNamed, - ACR_EquivalentPartialUnified, - ACR_EquivalentPartialFXUnified, - ACR_NonEquivalentPartialVersion - } - //------------------------------------------------------------------------------ // PFXImportCertStore //------------------------------------------------------------------------------