From 686ba4948188c7631bb3c31daf9f2d3d1a6acd3b Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Fri, 29 Jan 2021 17:08:14 +0100 Subject: [PATCH 1/5] Test for Ref.Emit with IgnoresAccessChecksToAttribute --- src/tests/reflection/RefEmit/Dependency.cs | 10 ++ .../reflection/RefEmit/Dependency.csproj | 8 ++ ...noresAccessChecksToAttributeIsRespected.cs | 130 ++++++++++++++++++ ...sAccessChecksToAttributeIsRespected.csproj | 12 ++ 4 files changed, 160 insertions(+) create mode 100644 src/tests/reflection/RefEmit/Dependency.cs create mode 100644 src/tests/reflection/RefEmit/Dependency.csproj create mode 100644 src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.cs create mode 100644 src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.csproj diff --git a/src/tests/reflection/RefEmit/Dependency.cs b/src/tests/reflection/RefEmit/Dependency.cs new file mode 100644 index 00000000000000..440bd6fad986ff --- /dev/null +++ b/src/tests/reflection/RefEmit/Dependency.cs @@ -0,0 +1,10 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; + +[assembly: InternalsVisibleTo("EmittingIgnoresAccessChecksToAttributeIsRespected")] + +internal class BaseClass2 +{ +} diff --git a/src/tests/reflection/RefEmit/Dependency.csproj b/src/tests/reflection/RefEmit/Dependency.csproj new file mode 100644 index 00000000000000..809291c86bd6b1 --- /dev/null +++ b/src/tests/reflection/RefEmit/Dependency.csproj @@ -0,0 +1,8 @@ + + + Library + + + + + diff --git a/src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.cs b/src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.cs new file mode 100644 index 00000000000000..f083336fdc1d94 --- /dev/null +++ b/src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.cs @@ -0,0 +1,130 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Linq; +using System.Reflection; +using System.Reflection.Emit; + +class BaseClass1 { } + +class Test +{ + + public static int Main() + { + AssemblyBuilder assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName("testassembly"), AssemblyBuilderAccess.Run); + ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule("testmodule"); + ConstructorInfo ignoreAccessChecksToAttributeCtor = DefineIgnoresAccessChecksToAttribute(moduleBuilder); + + { + Type type = typeof(BaseClass1); + AddInstanceOfIgnoresAccessChecksToAttribute(assemblyBuilder, ignoreAccessChecksToAttributeCtor, type.Assembly); + TypeBuilder typeBuilder = moduleBuilder.DefineType("DerivedTypeFor" + type.Name, TypeAttributes.Public, type); + typeBuilder.CreateType(); + } + + { + Type type = typeof(BaseClass2); + AddInstanceOfIgnoresAccessChecksToAttribute(assemblyBuilder, ignoreAccessChecksToAttributeCtor, type.Assembly); + TypeBuilder typeBuilder = moduleBuilder.DefineType("DerivedTypeFor" + type.Name, TypeAttributes.Public, type); + typeBuilder.CreateType(); + } + Console.WriteLine("PASS"); + return 100; + } + + static void AddInstanceOfIgnoresAccessChecksToAttribute(AssemblyBuilder assemblyBuilder, ConstructorInfo ignoreAccessChecksToAttributeCtor, Assembly assembly) + { + // Add this assembly level attribute: + // [assembly: System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute(assemblyName)] + ConstructorInfo attributeConstructor = ignoreAccessChecksToAttributeCtor; + CustomAttributeBuilder customAttributeBuilder = + new CustomAttributeBuilder(attributeConstructor, new object[] { assembly.GetName().Name }); + assemblyBuilder.SetCustomAttribute(customAttributeBuilder); + } + + static ConstructorInfo DefineIgnoresAccessChecksToAttribute(ModuleBuilder mb) + { + TypeBuilder attributeTypeBuilder = + mb.DefineType("System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute", + TypeAttributes.Public | TypeAttributes.Class, + typeof(Attribute)); + + // Create backing field as: + // private string assemblyName; + FieldBuilder assemblyNameField = + attributeTypeBuilder.DefineField("assemblyName", typeof(string), FieldAttributes.Private); + + // Create ctor as: + // public IgnoresAccessChecksToAttribute(string) + ConstructorBuilder constructorBuilder = attributeTypeBuilder.DefineConstructor(MethodAttributes.Public, + CallingConventions.HasThis, + new Type[] { assemblyNameField.FieldType }); + + ILGenerator il = constructorBuilder.GetILGenerator(); + + // Create ctor body as: + // this.assemblyName = {ctor parameter 0} + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Ldarg, 1); + il.Emit(OpCodes.Stfld, assemblyNameField); + + // return + il.Emit(OpCodes.Ret); + + // Define property as: + // public string AssemblyName {get { return this.assemblyName; } } + PropertyBuilder propertyBuilder = attributeTypeBuilder.DefineProperty( + "AssemblyName", + PropertyAttributes.None, + CallingConventions.HasThis, + returnType: typeof(string), + parameterTypes: null); + + MethodBuilder getterMethodBuilder = attributeTypeBuilder.DefineMethod( + "get_AssemblyName", + MethodAttributes.Public, + CallingConventions.HasThis, + returnType: typeof(string), + parameterTypes: null); + propertyBuilder.SetGetMethod(getterMethodBuilder); + + // Generate body: + // return this.assemblyName; + il = getterMethodBuilder.GetILGenerator(); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Ldfld, assemblyNameField); + il.Emit(OpCodes.Ret); + + // Generate the AttributeUsage attribute for this attribute type: + // [AttributeUsage(AttributeTargets.Assembly, AllowMultiple = true)] + TypeInfo attributeUsageTypeInfo = typeof(AttributeUsageAttribute).GetTypeInfo(); + + // Find the ctor that takes only AttributeTargets + ConstructorInfo attributeUsageConstructorInfo = + attributeUsageTypeInfo.DeclaredConstructors + .Single(c => c.GetParameters().Length == 1 && + c.GetParameters()[0].ParameterType == typeof(AttributeTargets)); + + // Find the property to set AllowMultiple + PropertyInfo allowMultipleProperty = + attributeUsageTypeInfo.DeclaredProperties + .Single(f => string.Equals(f.Name, "AllowMultiple")); + + // Create a builder to construct the instance via the ctor and property + CustomAttributeBuilder customAttributeBuilder = + new CustomAttributeBuilder(attributeUsageConstructorInfo, + new object[] { AttributeTargets.Assembly }, + new PropertyInfo[] { allowMultipleProperty }, + new object[] { true }); + + // Attach this attribute instance to the newly defined attribute type + attributeTypeBuilder.SetCustomAttribute(customAttributeBuilder); + + // Make the TypeInfo real so the constructor can be used. + return attributeTypeBuilder.CreateTypeInfo()!.DeclaredConstructors.Single(); + } +} + + diff --git a/src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.csproj b/src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.csproj new file mode 100644 index 00000000000000..dba1aca4a21317 --- /dev/null +++ b/src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.csproj @@ -0,0 +1,12 @@ + + + Exe + + + + + + + + + From 2c0a3d7d7d34bed336cdd5e865730f1a92b9cd9c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 2 Feb 2021 12:50:43 -0800 Subject: [PATCH 2/5] Add ability to update the friend assembly set of an assembly with Reflection Emit - Add routine/lock for updating friend assemblies - Unify all assembly handling custom attribute updates in the runtime - This adjusts the handling of the DebuggableAttribute, which in the past would have worked even if it was applied to anything via the CustomAttributeBuilder apis --- .../System/Reflection/Emit/AssemblyBuilder.cs | 3 +- .../Reflection/Emit/CustomAttributeBuilder.cs | 3 +- .../System/Reflection/Emit/EventBuilder.cs | 2 +- .../System/Reflection/Emit/FieldBuilder.cs | 2 +- .../System/Reflection/Emit/MethodBuilder.cs | 2 +- .../System/Reflection/Emit/ModuleBuilder.cs | 2 +- .../Reflection/Emit/ParameterBuilder.cs | 2 +- .../System/Reflection/Emit/PropertyBuilder.cs | 2 +- .../src/System/Reflection/Emit/TypeBuilder.cs | 10 +- src/coreclr/inc/corhdr.h | 14 +- src/coreclr/vm/assembly.cpp | 120 ++++++++++++++---- src/coreclr/vm/assembly.hpp | 25 +++- src/coreclr/vm/ceemain.cpp | 2 + src/coreclr/vm/comdynamic.cpp | 110 ++++++++++------ src/coreclr/vm/comdynamic.h | 2 +- 15 files changed, 216 insertions(+), 85 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs index 3d4709bc717c26..47db6a5a7ca49f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs @@ -595,8 +595,7 @@ private void SetCustomAttributeNoLock(ConstructorInfo con, byte[] binaryAttribut AssemblyBuilderData.AssemblyDefToken, _manifestModuleBuilder.GetConstructorToken(con), binaryAttribute, - false, - typeof(DebuggableAttribute) == con.DeclaringType); + false); } /// diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs index 586e1e55929b6e..7639827989c48d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs @@ -533,8 +533,7 @@ internal void CreateCustomAttribute(ModuleBuilder mod, int tkOwner) /// internal void CreateCustomAttribute(ModuleBuilder mod, int tkOwner, int tkAttrib, bool toDisk) { - TypeBuilder.DefineCustomAttribute(mod, tkOwner, tkAttrib, m_blob, toDisk, - typeof(System.Diagnostics.DebuggableAttribute) == m_con.DeclaringType); + TypeBuilder.DefineCustomAttribute(mod, tkOwner, tkAttrib, m_blob, toDisk); } } } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EventBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EventBuilder.cs index a66718182a8316..e1021e89db9dfe 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EventBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EventBuilder.cs @@ -97,7 +97,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) m_evToken, m_module.GetConstructorToken(con), binaryAttribute, - false, false); + false); } // Use this function if client wishes to build CustomAttribute using CustomAttributeBuilder diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs index c136a2c69368ea..4c1355196c5762 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs @@ -172,7 +172,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) m_typeBuilder.ThrowIfCreated(); TypeBuilder.DefineCustomAttribute(module, - m_fieldTok, module.GetConstructorToken(con), binaryAttribute, false, false); + m_fieldTok, module.GetConstructorToken(con), binaryAttribute, false); } public void SetCustomAttribute(CustomAttributeBuilder customBuilder) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs index 0807751b2999b5..4d356d338a9ae7 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs @@ -776,7 +776,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) TypeBuilder.DefineCustomAttribute(m_module, MetadataToken, ((ModuleBuilder)m_module).GetConstructorToken(con), binaryAttribute, - false, false); + false); if (IsKnownCA(con)) ParseCA(con); diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs index 1481ba0f6f499d..96df6854cc8c01 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs @@ -1559,7 +1559,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) 1, // This is hard coding the module token to 1 GetConstructorToken(con), binaryAttribute, - false, false); + false); } public void SetCustomAttribute(CustomAttributeBuilder customBuilder) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs index 661bb8eddf6d3e..a9f2350c9823b4 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs @@ -34,7 +34,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) _token, ((ModuleBuilder)_methodBuilder.GetModule()).GetConstructorToken(con), binaryAttribute, - false, false); + false); } // Use this function if client wishes to build CustomAttribute using CustomAttributeBuilder diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs index 4151db895be04d..36f36925751b23 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs @@ -118,7 +118,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) m_tkProperty, m_moduleBuilder.GetConstructorToken(con), binaryAttribute, - false, false); + false); } // Use this function if client wishes to build CustomAttribute using CustomAttributeBuilder diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs index 3a2a7747e28e52..9e8708c8e87937 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs @@ -51,7 +51,7 @@ public void Bake(ModuleBuilder module, int token) { Debug.Assert(m_con != null); DefineCustomAttribute(module, token, module.GetConstructorToken(m_con), - m_binaryAttribute, false, false); + m_binaryAttribute, false); } else { @@ -178,10 +178,10 @@ private static extern void SetMethodIL(QCallModule module, int tk, bool isInitLo [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] private static extern void DefineCustomAttribute(QCallModule module, int tkAssociate, int tkConstructor, - byte[]? attr, int attrLength, bool toDisk, bool updateCompilerFlags); + byte[]? attr, int attrLength, bool toDisk); internal static void DefineCustomAttribute(ModuleBuilder module, int tkAssociate, int tkConstructor, - byte[]? attr, bool toDisk, bool updateCompilerFlags) + byte[]? attr, bool toDisk) { byte[]? localAttr = null; @@ -192,7 +192,7 @@ internal static void DefineCustomAttribute(ModuleBuilder module, int tkAssociate } DefineCustomAttribute(new QCallModule(ref module), tkAssociate, tkConstructor, - localAttr, (localAttr != null) ? localAttr.Length : 0, toDisk, updateCompilerFlags); + localAttr, (localAttr != null) ? localAttr.Length : 0, toDisk); } [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] @@ -2159,7 +2159,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) throw new ArgumentNullException(nameof(binaryAttribute)); DefineCustomAttribute(m_module, m_tdType, ((ModuleBuilder)m_module).GetConstructorToken(con), - binaryAttribute, false, false); + binaryAttribute, false); } public void SetCustomAttribute(CustomAttributeBuilder customBuilder) diff --git a/src/coreclr/inc/corhdr.h b/src/coreclr/inc/corhdr.h index 75f91be3ed0c93..7187191a635fa1 100644 --- a/src/coreclr/inc/corhdr.h +++ b/src/coreclr/inc/corhdr.h @@ -1796,11 +1796,15 @@ typedef enum CorAttributeTargets #define FORWARD_INTEROP_STUB_METHOD_TYPE "System.Runtime.InteropServices.ManagedToNativeComInteropStubAttribute" #define FRIEND_ASSEMBLY_TYPE_W W("System.Runtime.CompilerServices.InternalsVisibleToAttribute") -#define FRIEND_ASSEMBLY_TYPE "System.Runtime.CompilerServices.InternalsVisibleToAttribute" +#define FRIEND_ASSEMBLY_TYPE "System.Runtime.CompilerServices.InternalsVisibleToAttribute" +#define FRIEND_ASSEMBLY_TYPE_NAMESPACE "System.Runtime.CompilerServices" +#define FRIEND_ASSEMBLY_TYPE_NAME "InternalsVisibleToAttribute" #define FRIEND_ASSEMBLY_SIG {IMAGE_CEE_CS_CALLCONV_DEFAULT_HASTHIS, 2, ELEMENT_TYPE_VOID, ELEMENT_TYPE_STRING, ELEMENT_TYPE_BOOLEAN} #define SUBJECT_ASSEMBLY_TYPE_W W("System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute") -#define SUBJECT_ASSEMBLY_TYPE "System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute" +#define SUBJECT_ASSEMBLY_TYPE "System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute" +#define SUBJECT_ASSEMBLY_TYPE_NAMESPACE "System.Runtime.CompilerServices" +#define SUBJECT_ASSEMBLY_TYPE_NAME "IgnoresAccessChecksToAttribute" #define SUBJECT_ASSEMBLY_SIG {IMAGE_CEE_CS_CALLCONV_DEFAULT_HASTHIS, 1, ELEMENT_TYPE_VOID, ELEMENT_TYPE_STRING} #define DISABLED_PRIVATE_REFLECTION_TYPE_W W("System.Runtime.CompilerServices.DisablePrivateReflectionAttribute") @@ -1818,6 +1822,12 @@ typedef enum CorAttributeTargets #define NONVERSIONABLE_TYPE_W W("System.Runtime.Versioning.NonVersionableAttribute") #define NONVERSIONABLE_TYPE "System.Runtime.Versioning.NonVersionableAttribute" +#define DEBUGGABLE_ATTRIBUTE_TYPE_W W("System.Diagnostics.DebuggableAttribute") +#define DEBUGGABLE_ATTRIBUTE_TYPE "System.Diagnostics.DebuggableAttribute" +#define DEBUGGABLE_ATTRIBUTE_TYPE_NAMESPACE "System.Diagnostics" +#define DEBUGGABLE_ATTRIBUTE_TYPE_NAME "DebuggableAttribute" + + // Keep in sync with CompilationRelaxations.cs typedef enum CompilationRelaxationsEnum { diff --git a/src/coreclr/vm/assembly.cpp b/src/coreclr/vm/assembly.cpp index 0e61a2ef6fe173..4c34f4469a4db5 100644 --- a/src/coreclr/vm/assembly.cpp +++ b/src/coreclr/vm/assembly.cpp @@ -94,7 +94,12 @@ enum ReasonForNotSharing ReasonForNotSharing_ClosureComparisonFailed = 0x8, }; -#define NO_FRIEND_ASSEMBLIES_MARKER ((FriendAssemblyDescriptor *)S_FALSE) +static CrstStatic g_friendAssembliesCrst; + +void Assembly::Initialize() +{ + g_friendAssembliesCrst.Init(CrstLeafLock); +} //---------------------------------------------------------------------------------------------- // The ctor's job is to initialize the Assembly enough so that the dtor can safely run. @@ -255,8 +260,8 @@ Assembly::~Assembly() Terminate(); - if (m_pFriendAssemblyDescriptor != NULL && m_pFriendAssemblyDescriptor != NO_FRIEND_ASSEMBLIES_MARKER) - delete m_pFriendAssemblyDescriptor; + if (m_pFriendAssemblyDescriptor != NULL) + m_pFriendAssemblyDescriptor->Release(); if (m_pManifestFile) { @@ -1234,25 +1239,79 @@ void Assembly::CacheFriendAssemblyInfo() if (m_pFriendAssemblyDescriptor == NULL) { - FriendAssemblyDescriptor *pFriendAssemblies = FriendAssemblyDescriptor::CreateFriendAssemblyDescriptor(this->GetManifestFile()); - if (pFriendAssemblies == NULL) + ReleaseHolder pFriendAssemblies = FriendAssemblyDescriptor::CreateFriendAssemblyDescriptor(this->GetManifestFile()); + _ASSERTE(pFriendAssemblies != NULL); + + CrstHolder friendDescriptorLock(&g_friendAssembliesCrst); + + if (m_pFriendAssemblyDescriptor == NULL) { - pFriendAssemblies = NO_FRIEND_ASSEMBLIES_MARKER; + m_pFriendAssemblyDescriptor = pFriendAssemblies.Extract(); + pFriendAssemblies.SuppressRelease(); } + } +} // void Assembly::CacheFriendAssemblyInfo() - void *pvPreviousDescriptor = InterlockedCompareExchangeT(&m_pFriendAssemblyDescriptor, - pFriendAssemblies, - NULL); +void Assembly::UpdateCachedFriendAssemblyInfo() +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + INJECT_FAULT(COMPlusThrowOM();); + } + CONTRACTL_END - if (pvPreviousDescriptor != NULL && pFriendAssemblies != NO_FRIEND_ASSEMBLIES_MARKER) + ReleaseHolder pOldFriendAssemblyDescriptor; + + { + CrstHolder friendDescriptorLock(&g_friendAssembliesCrst); + if (m_pFriendAssemblyDescriptor != NULL) { - if (pFriendAssemblies != NO_FRIEND_ASSEMBLIES_MARKER) + m_pFriendAssemblyDescriptor->AddRef(); + pOldFriendAssemblyDescriptor = m_pFriendAssemblyDescriptor; + } + } + + while (true) + { + ReleaseHolder pFriendAssemblies = FriendAssemblyDescriptor::CreateFriendAssemblyDescriptor(this->GetManifestFile()); + FriendAssemblyDescriptor* pFriendAssemblyDescriptorNextLoop = NULL; + + { + CrstHolder friendDescriptorLock(&g_friendAssembliesCrst); + + if (m_pFriendAssemblyDescriptor == pOldFriendAssemblyDescriptor) + { + if (m_pFriendAssemblyDescriptor != NULL) + m_pFriendAssemblyDescriptor->Release(); + + m_pFriendAssemblyDescriptor = pFriendAssemblies.Extract(); + pFriendAssemblies.SuppressRelease(); + return; + } + else { - delete pFriendAssemblies; + m_pFriendAssemblyDescriptor->AddRef(); + pFriendAssemblyDescriptorNextLoop = m_pFriendAssemblyDescriptor; } } + + // Initialize this here to avoid calling Release on the previous value of pOldFriendAssemblyDescriptor while holding the lock + pOldFriendAssemblyDescriptor = pFriendAssemblyDescriptorNextLoop; } -} // void Assembly::CacheFriendAssemblyInfo() +} + +ReleaseHolder Assembly::GetFriendAssemblyInfo(bool *pfNoFriendAssemblies) +{ + CacheFriendAssemblyInfo(); + + CrstHolder friendDescriptorLock(&g_friendAssembliesCrst); + m_pFriendAssemblyDescriptor->AddRef(); + ReleaseHolder friendAssemblyDescriptor(m_pFriendAssemblyDescriptor); + + return friendAssemblyDescriptor; +} //***************************************************************************** // Is the given assembly a friend of this assembly? @@ -1260,42 +1319,45 @@ bool Assembly::GrantsFriendAccessTo(Assembly *pAccessingAssembly, FieldDesc *pFD { WRAPPER_NO_CONTRACT; - CacheFriendAssemblyInfo(); + bool noFriendAssemblies = false; + ReleaseHolder friendAssemblyInfo = GetFriendAssemblyInfo(&noFriendAssemblies); - if (m_pFriendAssemblyDescriptor == NO_FRIEND_ASSEMBLIES_MARKER) + if (noFriendAssemblies) { return false; } - return m_pFriendAssemblyDescriptor->GrantsFriendAccessTo(pAccessingAssembly, pFD); + return friendAssemblyInfo->GrantsFriendAccessTo(pAccessingAssembly, pFD); } bool Assembly::GrantsFriendAccessTo(Assembly *pAccessingAssembly, MethodDesc *pMD) { WRAPPER_NO_CONTRACT; - CacheFriendAssemblyInfo(); + bool noFriendAssemblies = false; + ReleaseHolder friendAssemblyInfo = GetFriendAssemblyInfo(&noFriendAssemblies); - if (m_pFriendAssemblyDescriptor == NO_FRIEND_ASSEMBLIES_MARKER) + if (noFriendAssemblies) { return false; } - return m_pFriendAssemblyDescriptor->GrantsFriendAccessTo(pAccessingAssembly, pMD); + return friendAssemblyInfo->GrantsFriendAccessTo(pAccessingAssembly, pMD); } bool Assembly::GrantsFriendAccessTo(Assembly *pAccessingAssembly, MethodTable *pMT) { WRAPPER_NO_CONTRACT; - CacheFriendAssemblyInfo(); + bool noFriendAssemblies = false; + ReleaseHolder friendAssemblyInfo = GetFriendAssemblyInfo(&noFriendAssemblies); - if (m_pFriendAssemblyDescriptor == NO_FRIEND_ASSEMBLIES_MARKER) + if (noFriendAssemblies) { return false; } - return m_pFriendAssemblyDescriptor->GrantsFriendAccessTo(pAccessingAssembly, pMT); + return friendAssemblyInfo->GrantsFriendAccessTo(pAccessingAssembly, pMT); } bool Assembly::IgnoresAccessChecksTo(Assembly *pAccessedAssembly) @@ -1308,19 +1370,21 @@ bool Assembly::IgnoresAccessChecksTo(Assembly *pAccessedAssembly) } CONTRACTL_END; - CacheFriendAssemblyInfo(); + bool noFriendAssemblies = false; + ReleaseHolder friendAssemblyInfo = GetFriendAssemblyInfo(&noFriendAssemblies); - if (m_pFriendAssemblyDescriptor == NO_FRIEND_ASSEMBLIES_MARKER) + if (noFriendAssemblies) { return false; } + if (pAccessedAssembly->IsDisabledPrivateReflection()) { return false; } - return m_pFriendAssemblyDescriptor->IgnoresAccessChecksTo(pAccessedAssembly); + return friendAssemblyInfo->IgnoresAccessChecksTo(pAccessedAssembly); } @@ -2270,11 +2334,11 @@ FriendAssemblyDescriptor::~FriendAssemblyDescriptor() // pAssembly - assembly to get friend assembly information for // // Return Value: -// A friend assembly descriptor if the assembly declares any friend assemblies, otherwise NULL +// A friend assembly descriptor if the assembly declares any friend assemblies // // static -FriendAssemblyDescriptor *FriendAssemblyDescriptor::CreateFriendAssemblyDescriptor(PEAssembly *pAssembly) +ReleaseHolder FriendAssemblyDescriptor::CreateFriendAssemblyDescriptor(PEAssembly *pAssembly) { CONTRACTL { @@ -2284,7 +2348,7 @@ FriendAssemblyDescriptor *FriendAssemblyDescriptor::CreateFriendAssemblyDescript } CONTRACTL_END - NewHolder pFriendAssemblies = new FriendAssemblyDescriptor; + ReleaseHolder pFriendAssemblies = new FriendAssemblyDescriptor; // We're going to do this twice, once for InternalsVisibleTo and once for IgnoresAccessChecks ReleaseHolder pImport(pAssembly->GetMDImportWithRef()); diff --git a/src/coreclr/vm/assembly.hpp b/src/coreclr/vm/assembly.hpp index 4b594c46dc3744..082bffb471d510 100644 --- a/src/coreclr/vm/assembly.hpp +++ b/src/coreclr/vm/assembly.hpp @@ -86,6 +86,7 @@ class Assembly void Terminate( BOOL signalProfiler = TRUE ); static Assembly *Create(BaseDomain *pDomain, PEAssembly *pFile, DebuggerAssemblyControlFlags debuggerFlags, BOOL fIsCollectible, AllocMemTracker *pamTracker, LoaderAllocator *pLoaderAllocator); + static void Initialize(); BOOL IsSystem() { WRAPPER_NO_CONTRACT; return m_pManifestFile->IsSystem(); } @@ -557,6 +558,12 @@ class Assembly void CacheManifestFiles(); void CacheFriendAssemblyInfo(); +#ifndef DACCESS_COMPILE + ReleaseHolder GetFriendAssemblyInfo(bool *pfNoFriendAssemblies); +#endif +public: + void UpdateCachedFriendAssemblyInfo(); +private: PTR_BaseDomain m_pDomain; // Parent Domain @@ -608,14 +615,14 @@ typedef Assembly::ModuleIterator ModuleIterator; // FriendSecurityDescriptor contains information on which assemblies are friends of an assembly, as well as // which individual internals are visible to those friend assemblies. // - class FriendAssemblyDescriptor { + friend class Assembly; public: ~FriendAssemblyDescriptor(); static - FriendAssemblyDescriptor *CreateFriendAssemblyDescriptor(PEAssembly *pAssembly); + ReleaseHolder CreateFriendAssemblyDescriptor(PEAssembly *pAssembly); //--------------------------------------------------------------------------------------- // @@ -653,12 +660,26 @@ class FriendAssemblyDescriptor return IsAssemblyOnList(pAccessedAssembly, m_subjectAssemblies); } + void AddRef() + { + InterlockedIncrement(&m_refCount); + } + + void Release() + { + if (InterlockedDecrement(&m_refCount) == 0) + { + delete this; + } + } + private: typedef AssemblySpec FriendAssemblyName_t; typedef NewHolder FriendAssemblyNameHolder; ArrayList m_alFullAccessFriendAssemblies; // Friend assemblies which have access to all internals ArrayList m_subjectAssemblies; // Subject assemblies which we will not perform access checks against + LONG m_refCount = 1; FriendAssemblyDescriptor(); diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 0d24f8cfd3764b..fb7f2e95a781e7 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -992,6 +992,8 @@ void EEStartupHelper() #endif // CROSSGEN_COMPILE + Assembly::Initialize(); + #if defined(HOST_OSX) && defined(HOST_ARM64) PAL_JITWriteEnable(true); #endif // defined(HOST_OSX) && defined(HOST_ARM64) diff --git a/src/coreclr/vm/comdynamic.cpp b/src/coreclr/vm/comdynamic.cpp index acfcbe9d6523ba..f6332dfaab9d80 100644 --- a/src/coreclr/vm/comdynamic.cpp +++ b/src/coreclr/vm/comdynamic.cpp @@ -818,52 +818,29 @@ void QCALLTYPE COMDynamicWrite::SetClassLayout(QCall::ModuleHandle pModule, INT3 END_QCALL; } -void QCALLTYPE COMDynamicWrite::DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob, BOOL toDisk, BOOL updateCompilerFlags) +// Helper function for COMDynamicWrite::DefineCustomAttribute +void UpdateRuntimeStateForAssemblyCustomAttribute(Module* pModule, mdToken tkCustomAttribute, mdToken token, mdToken conTok, LPCBYTE pBlob, INT32 cbBlob) { - QCALL_CONTRACT; - - BEGIN_QCALL; + WRAPPER_NO_CONTRACT; - RefClassWriter* pRCW = pModule->GetReflectionModule()->GetClassWriter(); - _ASSERTE(pRCW); + LPCUTF8 szNamespace; + LPCUTF8 szName; - HRESULT hr; - mdCustomAttribute retToken; - - if (toDisk && pRCW->GetOnDiskEmitter()) - { - hr = pRCW->GetOnDiskEmitter()->DefineCustomAttribute( - token, - conTok, - pBlob, - cbBlob, - &retToken); - } - else + HRESULT hr = pModule->GetMDImport()->GetNameOfCustomAttribute(tkCustomAttribute, &szNamespace, &szName); + if (FAILED(hr)) { - hr = pRCW->GetEmitter()->DefineCustomAttribute( - token, - conTok, - pBlob, - cbBlob, - &retToken); + // If the type name cannot be acquired, then this isn't an interesting CustomAttribute + return; } - if (FAILED(hr)) + if (szNamespace == NULL || szName == NULL) { - // See if the metadata engine gave us any error information. - SafeComHolderPreemp pIErrInfo; - BSTRHolder bstrMessage; - if (SafeGetErrorInfo(&pIErrInfo) == S_OK) - { - if (SUCCEEDED(pIErrInfo->GetDescription(&bstrMessage)) && bstrMessage != NULL) - COMPlusThrow(kArgumentException, IDS_EE_INVALID_CA_EX, bstrMessage); - } - - COMPlusThrow(kArgumentException, IDS_EE_INVALID_CA); + // If either of the namespace or name are NULL, then this isn't an interesting attribute + return; } - if (updateCompilerFlags) + // Debuggable attribute processing + if ((strcmp(szNamespace, DEBUGGABLE_ATTRIBUTE_TYPE_NAMESPACE) == 0) && (strcmp(szName, DEBUGGABLE_ATTRIBUTE_TYPE_NAME) == 0)) { DWORD flags = 0; DWORD mask = ~(DACF_OBSOLETE_TRACK_JIT_INFO | DACF_IGNORE_PDBS | DACF_ALLOW_JIT_OPTS) & DACF_CONTROL_FLAGS_MASK; @@ -910,5 +887,64 @@ void QCALLTYPE COMDynamicWrite::DefineCustomAttribute(QCall::ModuleHandle pModul } } + // Debuggable attribute processing + if (((strcmp(szNamespace, FRIEND_ASSEMBLY_TYPE_NAMESPACE) == 0) && (strcmp(szName, FRIEND_ASSEMBLY_TYPE_NAME) == 0)) || + ((strcmp(szNamespace, SUBJECT_ASSEMBLY_TYPE_NAMESPACE) == 0) && (strcmp(szName, SUBJECT_ASSEMBLY_TYPE_NAME) == 0))) + { + Assembly* pAssembly = pModule->GetAssembly(); + pAssembly->UpdateCachedFriendAssemblyInfo(); + } +} + +void QCALLTYPE COMDynamicWrite::DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob, BOOL toDisk) +{ + QCALL_CONTRACT; + + BEGIN_QCALL; + + RefClassWriter* pRCW = pModule->GetReflectionModule()->GetClassWriter(); + _ASSERTE(pRCW); + + HRESULT hr; + mdCustomAttribute retToken; + + if (toDisk && pRCW->GetOnDiskEmitter()) + { + hr = pRCW->GetOnDiskEmitter()->DefineCustomAttribute( + token, + conTok, + pBlob, + cbBlob, + &retToken); + } + else + { + hr = pRCW->GetEmitter()->DefineCustomAttribute( + token, + conTok, + pBlob, + cbBlob, + &retToken); + } + + if (FAILED(hr)) + { + // See if the metadata engine gave us any error information. + SafeComHolderPreemp pIErrInfo; + BSTRHolder bstrMessage; + if (SafeGetErrorInfo(&pIErrInfo) == S_OK) + { + if (SUCCEEDED(pIErrInfo->GetDescription(&bstrMessage)) && bstrMessage != NULL) + COMPlusThrow(kArgumentException, IDS_EE_INVALID_CA_EX, bstrMessage); + } + + COMPlusThrow(kArgumentException, IDS_EE_INVALID_CA); + } + + if (token == TokenFromRid(1, mdtAssembly)) + { + UpdateRuntimeStateForAssemblyCustomAttribute(pModule, retToken, token, conTok, pBlob, cbBlob); + } + END_QCALL; } diff --git a/src/coreclr/vm/comdynamic.h b/src/coreclr/vm/comdynamic.h index 364aa21c9bee88..6a4b35f5f9c154 100644 --- a/src/coreclr/vm/comdynamic.h +++ b/src/coreclr/vm/comdynamic.h @@ -114,7 +114,7 @@ class COMDynamicWrite // Set a custom attribute static - void QCALLTYPE DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob, BOOL toDisk, BOOL updateCompilerFlags); + void QCALLTYPE DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob, BOOL toDisk); // functions to set ParamInfo static From 9a85c3598f4002ef520afa337d35ae03b5c7e7b6 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 2 Feb 2021 14:11:58 -0800 Subject: [PATCH 3/5] Address feedback --- src/coreclr/dlls/mscorrc/mscorrc.rc | 1 - src/coreclr/dlls/mscorrc/resource.h | 1 - src/coreclr/vm/assembly.cpp | 5 +---- src/coreclr/vm/comdynamic.cpp | 11 +---------- src/coreclr/vm/domainfile.cpp | 14 ++------------ 5 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/coreclr/dlls/mscorrc/mscorrc.rc b/src/coreclr/dlls/mscorrc/mscorrc.rc index 360442044ca439..e53950ff4b31b8 100644 --- a/src/coreclr/dlls/mscorrc/mscorrc.rc +++ b/src/coreclr/dlls/mscorrc/mscorrc.rc @@ -502,7 +502,6 @@ BEGIN IDS_EE_VARARG_NOT_SUPPORTED "Vararg calling convention not supported." IDS_EE_INVALID_CA "Invalid custom attribute provided." - IDS_EE_INVALID_CA_EX "Invalid custom attribute provided: '%1'" IDS_EE_THREADSTART_STATE "Thread is running or terminated; it cannot restart." IDS_EE_THREAD_CANNOT_GET "Unable to retrieve thread information." diff --git a/src/coreclr/dlls/mscorrc/resource.h b/src/coreclr/dlls/mscorrc/resource.h index 4763f7d075c8f9..834a0475f7aa30 100644 --- a/src/coreclr/dlls/mscorrc/resource.h +++ b/src/coreclr/dlls/mscorrc/resource.h @@ -244,7 +244,6 @@ #define IDS_EE_VARARG_NOT_SUPPORTED 0x1a0f #define IDS_EE_INVALID_CA 0x1a10 -#define IDS_EE_INVALID_CA_EX 0x1a11 #define IDS_EE_THREADSTART_STATE 0x1a12 diff --git a/src/coreclr/vm/assembly.cpp b/src/coreclr/vm/assembly.cpp index 4c34f4469a4db5..0ba2578e8023ee 100644 --- a/src/coreclr/vm/assembly.cpp +++ b/src/coreclr/vm/assembly.cpp @@ -1247,7 +1247,6 @@ void Assembly::CacheFriendAssemblyInfo() if (m_pFriendAssemblyDescriptor == NULL) { m_pFriendAssemblyDescriptor = pFriendAssemblies.Extract(); - pFriendAssemblies.SuppressRelease(); } } } // void Assembly::CacheFriendAssemblyInfo() @@ -1287,7 +1286,6 @@ void Assembly::UpdateCachedFriendAssemblyInfo() m_pFriendAssemblyDescriptor->Release(); m_pFriendAssemblyDescriptor = pFriendAssemblies.Extract(); - pFriendAssemblies.SuppressRelease(); return; } else @@ -2437,8 +2435,7 @@ ReleaseHolder FriendAssemblyDescriptor::CreateFriendAs } } - pFriendAssemblies.SuppressRelease(); - return pFriendAssemblies.Extract(); + return pFriendAssemblies; } //--------------------------------------------------------------------------------------- diff --git a/src/coreclr/vm/comdynamic.cpp b/src/coreclr/vm/comdynamic.cpp index f6332dfaab9d80..12fc8771919917 100644 --- a/src/coreclr/vm/comdynamic.cpp +++ b/src/coreclr/vm/comdynamic.cpp @@ -887,7 +887,7 @@ void UpdateRuntimeStateForAssemblyCustomAttribute(Module* pModule, mdToken tkCus } } - // Debuggable attribute processing + // InternalsVisibleTo and IgnoresAccessChecksTo attribute processing if (((strcmp(szNamespace, FRIEND_ASSEMBLY_TYPE_NAMESPACE) == 0) && (strcmp(szName, FRIEND_ASSEMBLY_TYPE_NAME) == 0)) || ((strcmp(szNamespace, SUBJECT_ASSEMBLY_TYPE_NAMESPACE) == 0) && (strcmp(szName, SUBJECT_ASSEMBLY_TYPE_NAME) == 0))) { @@ -929,15 +929,6 @@ void QCALLTYPE COMDynamicWrite::DefineCustomAttribute(QCall::ModuleHandle pModul if (FAILED(hr)) { - // See if the metadata engine gave us any error information. - SafeComHolderPreemp pIErrInfo; - BSTRHolder bstrMessage; - if (SafeGetErrorInfo(&pIErrInfo) == S_OK) - { - if (SUCCEEDED(pIErrInfo->GetDescription(&bstrMessage)) && bstrMessage != NULL) - COMPlusThrow(kArgumentException, IDS_EE_INVALID_CA_EX, bstrMessage); - } - COMPlusThrow(kArgumentException, IDS_EE_INVALID_CA); } diff --git a/src/coreclr/vm/domainfile.cpp b/src/coreclr/vm/domainfile.cpp index 0d25af99d8fbc1..944d5268f1c4ee 100644 --- a/src/coreclr/vm/domainfile.cpp +++ b/src/coreclr/vm/domainfile.cpp @@ -1838,14 +1838,6 @@ BOOL DomainAssembly::CheckZapDependencyIdentities(PEImage *pNativeImage) } #endif // FEATURE_PREJIT - - - - -// @todo Find a better place for these -#define DE_CUSTOM_VALUE_NAMESPACE "System.Diagnostics" -#define DE_DEBUGGABLE_ATTRIBUTE_NAME "DebuggableAttribute" - // @todo .INI file is a temporary workaround for Beta 1 #define DE_INI_FILE_SECTION_NAME W(".NET Framework Debugging Control") #define DE_INI_FILE_KEY_TRACK_INFO W("GenerateTrackingInfo") @@ -2080,9 +2072,7 @@ HRESULT DomainAssembly::GetDebuggingCustomAttributes(DWORD *pdwFlags) mdAssembly asTK = TokenFromRid(mdtAssembly, 1); hr = mdImport->GetCustomAttributeByName(asTK, - DE_CUSTOM_VALUE_NAMESPACE - NAMESPACE_SEPARATOR_STR - DE_DEBUGGABLE_ATTRIBUTE_NAME, + DEBUGGABLE_ATTRIBUTE_TYPE, (const void**)&blob, &size); @@ -2131,7 +2121,7 @@ HRESULT DomainAssembly::GetDebuggingCustomAttributes(DWORD *pdwFlags) } LOG((LF_CORDB, LL_INFO10, "Assembly %S: has %s=%d,%d bits = 0x%x\n", GetDebugName(), - DE_DEBUGGABLE_ATTRIBUTE_NAME, + DEBUGGABLE_ATTRIBUTE_TYPE_NAME, blob[2], blob[3], *pdwFlags)); } } From 32a33ee127b5a0114cc2d248e3bbf54931dc6327 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 2 Feb 2021 14:53:56 -0800 Subject: [PATCH 4/5] Remove todisk feature from custom attribute logic as it is unreachable --- .../System/Reflection/Emit/AssemblyBuilder.cs | 3 +-- .../Reflection/Emit/CustomAttributeBuilder.cs | 13 +--------- .../System/Reflection/Emit/EventBuilder.cs | 3 +-- .../System/Reflection/Emit/FieldBuilder.cs | 2 +- .../System/Reflection/Emit/MethodBuilder.cs | 3 +-- .../System/Reflection/Emit/ModuleBuilder.cs | 3 +-- .../Reflection/Emit/ParameterBuilder.cs | 3 +-- .../System/Reflection/Emit/PropertyBuilder.cs | 3 +-- .../src/System/Reflection/Emit/TypeBuilder.cs | 10 +++---- src/coreclr/vm/comdynamic.cpp | 26 +++++-------------- src/coreclr/vm/comdynamic.h | 2 +- src/coreclr/vm/reflectclasswriter.cpp | 5 ---- src/coreclr/vm/reflectclasswriter.h | 25 ------------------ 13 files changed, 21 insertions(+), 80 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs index 47db6a5a7ca49f..a2b88b63e31688 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/AssemblyBuilder.cs @@ -594,8 +594,7 @@ private void SetCustomAttributeNoLock(ConstructorInfo con, byte[] binaryAttribut _manifestModuleBuilder, // pass in the in-memory assembly module AssemblyBuilderData.AssemblyDefToken, _manifestModuleBuilder.GetConstructorToken(con), - binaryAttribute, - false); + binaryAttribute); } /// diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs index 7639827989c48d..1038d9ba86504a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs @@ -519,21 +519,10 @@ private static void EmitValue(BinaryWriter writer, Type type, object? value) } } - - - // return the byte interpretation of the custom attribute internal void CreateCustomAttribute(ModuleBuilder mod, int tkOwner) { - CreateCustomAttribute(mod, tkOwner, mod.GetConstructorToken(m_con), false); - } - - /// - /// Call this function with toDisk=1, after on disk module has been snapped. - /// - internal void CreateCustomAttribute(ModuleBuilder mod, int tkOwner, int tkAttrib, bool toDisk) - { - TypeBuilder.DefineCustomAttribute(mod, tkOwner, tkAttrib, m_blob, toDisk); + TypeBuilder.DefineCustomAttribute(mod, tkOwner, mod.GetConstructorToken(m_con), m_blob); } } } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EventBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EventBuilder.cs index e1021e89db9dfe..be0ed200842094 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EventBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EventBuilder.cs @@ -96,8 +96,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) m_module, m_evToken, m_module.GetConstructorToken(con), - binaryAttribute, - false); + binaryAttribute); } // Use this function if client wishes to build CustomAttribute using CustomAttributeBuilder diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs index 4c1355196c5762..e4c35c9dbbc034 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/FieldBuilder.cs @@ -172,7 +172,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) m_typeBuilder.ThrowIfCreated(); TypeBuilder.DefineCustomAttribute(module, - m_fieldTok, module.GetConstructorToken(con), binaryAttribute, false); + m_fieldTok, module.GetConstructorToken(con), binaryAttribute); } public void SetCustomAttribute(CustomAttributeBuilder customBuilder) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs index 4d356d338a9ae7..db05015f6a664f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs @@ -775,8 +775,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) TypeBuilder.DefineCustomAttribute(m_module, MetadataToken, ((ModuleBuilder)m_module).GetConstructorToken(con), - binaryAttribute, - false); + binaryAttribute); if (IsKnownCA(con)) ParseCA(con); diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs index 96df6854cc8c01..7989225a585018 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ModuleBuilder.cs @@ -1558,8 +1558,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) this, 1, // This is hard coding the module token to 1 GetConstructorToken(con), - binaryAttribute, - false); + binaryAttribute); } public void SetCustomAttribute(CustomAttributeBuilder customBuilder) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs index a9f2350c9823b4..79379c535874c7 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ParameterBuilder.cs @@ -33,8 +33,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) _methodBuilder.GetModuleBuilder(), _token, ((ModuleBuilder)_methodBuilder.GetModule()).GetConstructorToken(con), - binaryAttribute, - false); + binaryAttribute); } // Use this function if client wishes to build CustomAttribute using CustomAttributeBuilder diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs index 36f36925751b23..ae6e704b2a5c87 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/PropertyBuilder.cs @@ -117,8 +117,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) m_moduleBuilder, m_tkProperty, m_moduleBuilder.GetConstructorToken(con), - binaryAttribute, - false); + binaryAttribute); } // Use this function if client wishes to build CustomAttribute using CustomAttributeBuilder diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs index 9e8708c8e87937..ebc6048638fa54 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs @@ -51,7 +51,7 @@ public void Bake(ModuleBuilder module, int token) { Debug.Assert(m_con != null); DefineCustomAttribute(module, token, module.GetConstructorToken(m_con), - m_binaryAttribute, false); + m_binaryAttribute); } else { @@ -178,10 +178,10 @@ private static extern void SetMethodIL(QCallModule module, int tk, bool isInitLo [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] private static extern void DefineCustomAttribute(QCallModule module, int tkAssociate, int tkConstructor, - byte[]? attr, int attrLength, bool toDisk); + byte[]? attr, int attrLength); internal static void DefineCustomAttribute(ModuleBuilder module, int tkAssociate, int tkConstructor, - byte[]? attr, bool toDisk) + byte[]? attr) { byte[]? localAttr = null; @@ -192,7 +192,7 @@ internal static void DefineCustomAttribute(ModuleBuilder module, int tkAssociate } DefineCustomAttribute(new QCallModule(ref module), tkAssociate, tkConstructor, - localAttr, (localAttr != null) ? localAttr.Length : 0, toDisk); + localAttr, (localAttr != null) ? localAttr.Length : 0); } [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] @@ -2159,7 +2159,7 @@ public void SetCustomAttribute(ConstructorInfo con, byte[] binaryAttribute) throw new ArgumentNullException(nameof(binaryAttribute)); DefineCustomAttribute(m_module, m_tdType, ((ModuleBuilder)m_module).GetConstructorToken(con), - binaryAttribute, false); + binaryAttribute); } public void SetCustomAttribute(CustomAttributeBuilder customBuilder) diff --git a/src/coreclr/vm/comdynamic.cpp b/src/coreclr/vm/comdynamic.cpp index 12fc8771919917..892e7498624cc2 100644 --- a/src/coreclr/vm/comdynamic.cpp +++ b/src/coreclr/vm/comdynamic.cpp @@ -896,7 +896,7 @@ void UpdateRuntimeStateForAssemblyCustomAttribute(Module* pModule, mdToken tkCus } } -void QCALLTYPE COMDynamicWrite::DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob, BOOL toDisk) +void QCALLTYPE COMDynamicWrite::DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob) { QCALL_CONTRACT; @@ -908,24 +908,12 @@ void QCALLTYPE COMDynamicWrite::DefineCustomAttribute(QCall::ModuleHandle pModul HRESULT hr; mdCustomAttribute retToken; - if (toDisk && pRCW->GetOnDiskEmitter()) - { - hr = pRCW->GetOnDiskEmitter()->DefineCustomAttribute( - token, - conTok, - pBlob, - cbBlob, - &retToken); - } - else - { - hr = pRCW->GetEmitter()->DefineCustomAttribute( - token, - conTok, - pBlob, - cbBlob, - &retToken); - } + hr = pRCW->GetEmitter()->DefineCustomAttribute( + token, + conTok, + pBlob, + cbBlob, + &retToken); if (FAILED(hr)) { diff --git a/src/coreclr/vm/comdynamic.h b/src/coreclr/vm/comdynamic.h index 6a4b35f5f9c154..12185eae38da20 100644 --- a/src/coreclr/vm/comdynamic.h +++ b/src/coreclr/vm/comdynamic.h @@ -114,7 +114,7 @@ class COMDynamicWrite // Set a custom attribute static - void QCALLTYPE DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob, BOOL toDisk); + void QCALLTYPE DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob); // functions to set ParamInfo static diff --git a/src/coreclr/vm/reflectclasswriter.cpp b/src/coreclr/vm/reflectclasswriter.cpp index 3b8749908e4eba..40688a9d629acb 100644 --- a/src/coreclr/vm/reflectclasswriter.cpp +++ b/src/coreclr/vm/reflectclasswriter.cpp @@ -113,9 +113,4 @@ RefClassWriter::~RefClassWriter() m_pCeeGen->Release(); m_pCeeGen = NULL; } - - if (m_pOnDiskEmitter) { - m_pOnDiskEmitter->Release(); - m_pOnDiskEmitter = NULL; - } } diff --git a/src/coreclr/vm/reflectclasswriter.h b/src/coreclr/vm/reflectclasswriter.h index 858feaf9eb61f3..574eb78c8faf69 100644 --- a/src/coreclr/vm/reflectclasswriter.h +++ b/src/coreclr/vm/reflectclasswriter.h @@ -23,12 +23,10 @@ class RefClassWriter { IMetaDataEmitHelper* m_pEmitHelper; ULONG m_ulResourceSize; mdFile m_tkFile; - IMetaDataEmit* m_pOnDiskEmitter; public: RefClassWriter() { LIMITED_METHOD_CONTRACT; - m_pOnDiskEmitter = NULL; } HRESULT Init(ICeeGen *pCeeGen, IUnknown *pUnk, LPCWSTR szName); @@ -68,29 +66,6 @@ class RefClassWriter { return m_ceeFile; } - IMetaDataEmit* GetOnDiskEmitter() { - LIMITED_METHOD_CONTRACT; - return m_pOnDiskEmitter; - } - - void SetOnDiskEmitter(IMetaDataEmit *pOnDiskEmitter) { - CONTRACTL { - NOTHROW; - GC_TRIGGERS; - // we know that the com implementation is ours so we use mode-any to simplify - // having to switch mode - MODE_ANY; - FORBID_FAULT; - } - CONTRACTL_END; - if (pOnDiskEmitter) - pOnDiskEmitter->AddRef(); - if (m_pOnDiskEmitter) - m_pOnDiskEmitter->Release(); - m_pOnDiskEmitter = pOnDiskEmitter; - } - - ~RefClassWriter(); }; From 6ba0567cdcf601a5b4a991f29506f206feb4d7b0 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 2 Feb 2021 18:06:06 -0800 Subject: [PATCH 5/5] In addition to cleaning up old code... one should clean up new code --- src/coreclr/vm/assembly.cpp | 43 +++++-------------------------------- src/coreclr/vm/assembly.hpp | 2 +- 2 files changed, 6 insertions(+), 39 deletions(-) diff --git a/src/coreclr/vm/assembly.cpp b/src/coreclr/vm/assembly.cpp index 0ba2578e8023ee..1f899a07db4522 100644 --- a/src/coreclr/vm/assembly.cpp +++ b/src/coreclr/vm/assembly.cpp @@ -1300,7 +1300,7 @@ void Assembly::UpdateCachedFriendAssemblyInfo() } } -ReleaseHolder Assembly::GetFriendAssemblyInfo(bool *pfNoFriendAssemblies) +ReleaseHolder Assembly::GetFriendAssemblyInfo() { CacheFriendAssemblyInfo(); @@ -1317,45 +1317,21 @@ bool Assembly::GrantsFriendAccessTo(Assembly *pAccessingAssembly, FieldDesc *pFD { WRAPPER_NO_CONTRACT; - bool noFriendAssemblies = false; - ReleaseHolder friendAssemblyInfo = GetFriendAssemblyInfo(&noFriendAssemblies); - - if (noFriendAssemblies) - { - return false; - } - - return friendAssemblyInfo->GrantsFriendAccessTo(pAccessingAssembly, pFD); + return GetFriendAssemblyInfo()->GrantsFriendAccessTo(pAccessingAssembly, pFD); } bool Assembly::GrantsFriendAccessTo(Assembly *pAccessingAssembly, MethodDesc *pMD) { WRAPPER_NO_CONTRACT; - bool noFriendAssemblies = false; - ReleaseHolder friendAssemblyInfo = GetFriendAssemblyInfo(&noFriendAssemblies); - - if (noFriendAssemblies) - { - return false; - } - - return friendAssemblyInfo->GrantsFriendAccessTo(pAccessingAssembly, pMD); + return GetFriendAssemblyInfo()->GrantsFriendAccessTo(pAccessingAssembly, pMD); } bool Assembly::GrantsFriendAccessTo(Assembly *pAccessingAssembly, MethodTable *pMT) { WRAPPER_NO_CONTRACT; - bool noFriendAssemblies = false; - ReleaseHolder friendAssemblyInfo = GetFriendAssemblyInfo(&noFriendAssemblies); - - if (noFriendAssemblies) - { - return false; - } - - return friendAssemblyInfo->GrantsFriendAccessTo(pAccessingAssembly, pMT); + return GetFriendAssemblyInfo()->GrantsFriendAccessTo(pAccessingAssembly, pMT); } bool Assembly::IgnoresAccessChecksTo(Assembly *pAccessedAssembly) @@ -1368,21 +1344,12 @@ bool Assembly::IgnoresAccessChecksTo(Assembly *pAccessedAssembly) } CONTRACTL_END; - bool noFriendAssemblies = false; - ReleaseHolder friendAssemblyInfo = GetFriendAssemblyInfo(&noFriendAssemblies); - - if (noFriendAssemblies) - { - return false; - } - - if (pAccessedAssembly->IsDisabledPrivateReflection()) { return false; } - return friendAssemblyInfo->IgnoresAccessChecksTo(pAccessedAssembly); + return GetFriendAssemblyInfo()->IgnoresAccessChecksTo(pAccessedAssembly); } diff --git a/src/coreclr/vm/assembly.hpp b/src/coreclr/vm/assembly.hpp index 082bffb471d510..5ef0a6598b976d 100644 --- a/src/coreclr/vm/assembly.hpp +++ b/src/coreclr/vm/assembly.hpp @@ -559,7 +559,7 @@ class Assembly void CacheFriendAssemblyInfo(); #ifndef DACCESS_COMPILE - ReleaseHolder GetFriendAssemblyInfo(bool *pfNoFriendAssemblies); + ReleaseHolder GetFriendAssemblyInfo(); #endif public: void UpdateCachedFriendAssemblyInfo();