From a36bc61442d89d0b5c58b0b14e7bd3bde218f24d Mon Sep 17 00:00:00 2001 From: Steve MacLean Date: Sun, 21 Apr 2019 18:25:04 -0400 Subject: [PATCH] Fix AssemblyName cache hash and key (#24138) * Add ContextualReflection LoadWithPartialName case * Remove unnecessary MethodImplOptions.NoInlining * Remove m_assembly warning * Fix AssemblyName hash function * AssemblyNative::Load fix stackMark usage Do not use the stackMark if (ptrLoadContextBinder != NULL) * Temporarily disable DefaultContextOverrideTPA Test is failing due to a logic error. Fix is pending in https://github.com/dotnet/corefx/pull/37071 --- .../src/System/Reflection/RuntimeAssembly.cs | 2 ++ src/binder/assemblyname.cpp | 9 +++++++++ src/vm/assemblynative.cpp | 10 +++++----- tests/CoreFX/CoreFX.issues.json | 14 ++++++++++++++ .../Loader/ContextualReflection/ContextualReflection.cs | 4 +++- .../ContextualReflection/ContextualReflectionDependency.cs | 1 - 6 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs b/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs index 16dc8ec..a97fddc 100644 --- a/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs +++ b/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs @@ -25,7 +25,9 @@ namespace System.Reflection private event ModuleResolveEventHandler _ModuleResolve; private string? m_fullname; private object? m_syncRoot; // Used to keep collectible types alive and as the syncroot for reflection.emit +#pragma warning disable 169 private IntPtr m_assembly; // slack for ptr datum on unmanaged side +#pragma warning restore 169 #endregion diff --git a/src/binder/assemblyname.cpp b/src/binder/assemblyname.cpp index 616869b..bc97814 100644 --- a/src/binder/assemblyname.cpp +++ b/src/binder/assemblyname.cpp @@ -486,6 +486,15 @@ Exit: { dwUseIdentityFlags &= ~AssemblyIdentity::IDENTITY_FLAG_CONTENT_TYPE; } + if ((dwIncludeFlags & INCLUDE_PUBLIC_KEY_TOKEN) == 0) + { + dwUseIdentityFlags &= ~AssemblyIdentity::IDENTITY_FLAG_PUBLIC_KEY; + dwUseIdentityFlags &= ~AssemblyIdentity::IDENTITY_FLAG_PUBLIC_KEY_TOKEN; + } + if ((dwIncludeFlags & EXCLUDE_CULTURE) != 0) + { + dwUseIdentityFlags &= ~AssemblyIdentity::IDENTITY_FLAG_CULTURE; + } dwHash ^= static_cast(HashCaseInsensitive(GetSimpleName())); dwHash = _rotl(dwHash, 4); diff --git a/src/vm/assemblynative.cpp b/src/vm/assemblynative.cpp index aa83093..cc95151 100644 --- a/src/vm/assemblynative.cpp +++ b/src/vm/assemblynative.cpp @@ -78,18 +78,18 @@ FCIMPL6(Object*, AssemblyNative::Load, AssemblyNameBaseObject* assemblyNameUNSAF else { // Compute parent assembly - if (gc.requestingAssembly == NULL) + if (gc.requestingAssembly != NULL) { - pRefAssembly = SystemDomain::GetCallersAssembly(stackMark); + pRefAssembly = gc.requestingAssembly->GetAssembly(); } - else + else if (ptrLoadContextBinder == NULL) { - pRefAssembly = gc.requestingAssembly->GetAssembly(); + pRefAssembly = SystemDomain::GetCallersAssembly(stackMark); } if (pRefAssembly) { - pParentAssembly= pRefAssembly->GetDomainAssembly(); + pParentAssembly = pRefAssembly->GetDomainAssembly(); } } diff --git a/tests/CoreFX/CoreFX.issues.json b/tests/CoreFX/CoreFX.issues.json index b0eb61c..e8428f5 100644 --- a/tests/CoreFX/CoreFX.issues.json +++ b/tests/CoreFX/CoreFX.issues.json @@ -1150,6 +1150,20 @@ } }, { + "name": "System.Runtime.Loader.DefaultContext.Tests", + "enabled": true, + "exclusions": { + "namespaces": null, + "classes": null, + "methods": [ + { + "name" : "System.Runtime.Loader.Tests.DefaultLoadContextTests.LoadInDefaultContext", + "reason" : "Waiting for https://github.com/dotnet/corefx/pull/37071" + } + ] + } + }, + { "name": "System.Runtime.Loader.Tests", "enabled": true, "exclusions": { diff --git a/tests/src/Loader/ContextualReflection/ContextualReflection.cs b/tests/src/Loader/ContextualReflection/ContextualReflection.cs index 1612c14..2c1d081 100644 --- a/tests/src/Loader/ContextualReflection/ContextualReflection.cs +++ b/tests/src/Loader/ContextualReflection/ContextualReflection.cs @@ -321,6 +321,9 @@ namespace ContextualReflectionTest { TestAssemblyLoad(isolated, (string assemblyName) => Assembly.Load(assemblyName)); TestAssemblyLoad(isolated, (string assemblyName) => Assembly.Load(new AssemblyName(assemblyName))); +#pragma warning disable 618 + TestAssemblyLoad(isolated, (string assemblyName) => Assembly.LoadWithPartialName(assemblyName)); +#pragma warning restore 618 } void TestAssemblyLoad(bool isolated, Func assemblyLoad) @@ -748,7 +751,6 @@ namespace ContextualReflectionTest TestMockAssemblyThrows(); } - [MethodImplAttribute(MethodImplOptions.NoInlining)] public void RunTestsIsolated() { VerifyIsolationAlc(); diff --git a/tests/src/Loader/ContextualReflection/ContextualReflectionDependency.cs b/tests/src/Loader/ContextualReflection/ContextualReflectionDependency.cs index f498db4..933194a 100644 --- a/tests/src/Loader/ContextualReflection/ContextualReflectionDependency.cs +++ b/tests/src/Loader/ContextualReflection/ContextualReflectionDependency.cs @@ -16,7 +16,6 @@ namespace ContextualReflectionTest Assembly alcAssembly { get; } Type alcProgramType { get; } IProgram alcProgramInstance { get; } - [MethodImplAttribute(MethodImplOptions.NoInlining)] void RunTestsIsolated(); } -- 2.7.4