Skip to content

Commit

Permalink
Apply feedbacks, fix RVA alignment
Browse files Browse the repository at this point in the history
  • Loading branch information
@buyaa-n
buyaa-n committed Jan 11, 2024
1 parent f0bcf1e commit 1be0fb3
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 85 deletions.
4 changes: 2 additions & 2 deletions ...ries/System.Reflection.Emit/src/System/Reflection/Emit/GenericTypeParameterBuilderImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ internal GenericTypeParameterBuilderImpl(string name, int genParamPosition, Type
_type = typeBuilder;
}

public GenericTypeParameterBuilderImpl(string name, int genParamPosition, MethodBuilderImpl methodBuilder)
public GenericTypeParameterBuilderImpl(string name, int genParamPosition, MethodBuilderImpl methodBuilder, TypeBuilderImpl typeBuilder)
{
_name = name;
_genParamPosition = genParamPosition;
_methodBuilder = methodBuilder;
_type = (TypeBuilder)methodBuilder.DeclaringType!;
_type = typeBuilder;
}

protected override void SetBaseTypeConstraintCore([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type? baseTypeConstraint)
Expand Down
8 changes: 4 additions & 4 deletions src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/MethodBuilderImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ internal MethodBuilderImpl(string name, MethodAttributes attributes, CallingConv
_initLocals = true;
}

internal void CreateDllData(string dllName, string entryName, CallingConvention nativeCallConv, CharSet nativeCharSet)
internal void CreateDllImportData(string dllName, string entryName, CallingConvention nativeCallConv, CharSet nativeCharSet)
{
_dllImportData = DllImportData.CreateDllImportData(dllName, entryName, nativeCallConv, nativeCharSet);
_dllImportData = DllImportData.Create(dllName, entryName, nativeCallConv, nativeCharSet);
}

internal int ParameterCount => _parameterTypes == null ? 0 : _parameterTypes.Length;
Expand Down Expand Up @@ -122,7 +122,7 @@ protected override GenericTypeParameterBuilder[] DefineGenericParametersCore(par
{
string name = names[i];
ArgumentNullException.ThrowIfNull(names, nameof(names));
typeParameters[i] = new GenericTypeParameterBuilderImpl(name, i, this);
typeParameters[i] = new GenericTypeParameterBuilderImpl(name, i, this, _declaringType);
}

return _typeParameters = typeParameters;
Expand Down Expand Up @@ -182,7 +182,7 @@ protected override void SetCustomAttributeCore(ConstructorInfo con, ReadOnlySpan
return;
case "System.Runtime.InteropServices.DllImportAttribute":
{
_dllImportData = DllImportData.CreateDllImportData(CustomAttributeInfo.DecodeCustomAttribute(con, binaryAttribute), out var preserveSig);
_dllImportData = DllImportData.Create(CustomAttributeInfo.DecodeCustomAttribute(con, binaryAttribute), out var preserveSig);
_attributes |= MethodAttributes.PinvokeImpl;
_canBeRuntimeImpl = true;
if (preserveSig)
Expand Down
15 changes: 9 additions & 6 deletions src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ internal void AppendMetadata(MethodBodyStreamEncoder methodBodyEncoder, BlobBuil
// All generic parameters for all types and methods should be written in specific order
List<GenericTypeParameterBuilderImpl> genericParams = new();
PopulateTokensForTypesAndItsMembers();

// Add global members
WriteFields(_globalTypeBuilder, fieldDataBuilder);
WriteMethods(_globalTypeBuilder._methodDefinitions, genericParams, methodBodyEncoder);
Expand Down Expand Up @@ -449,6 +450,7 @@ private void WriteFields(TypeBuilderImpl typeBuilder, BlobBuilder fieldDataBuild
{
_metadataBuilder.AddFieldRelativeVirtualAddress(handle, fieldDataBuilder.Count);
fieldDataBuilder.WriteBytes(field._rvaData!);
fieldDataBuilder.Align(8);
}
}
}
Expand Down Expand Up @@ -516,18 +518,19 @@ private MethodSpecificationHandle AddMethodSpecification(EntityHandle methodHand
method: methodHandle,
instantiation: _metadataBuilder.GetOrAddBlob(MetadataSignatureHelper.GetMethodSpecificationSignature(genericArgs, this)));

private EntityHandle GetMemberReferenceHandle(MemberInfo member)
private EntityHandle GetMemberReferenceHandle(MemberInfo memberInfo)
{
if (!_memberReferences.TryGetValue(member, out var memberHandle))
if (!_memberReferences.TryGetValue(memberInfo, out var memberHandle))
{
MemberInfo member = GetOriginalMemberIfConstructedType(memberInfo);
switch (member)
{
case FieldInfo field:
memberHandle = AddMemberReference(field.Name, GetTypeHandle(field.DeclaringType!),
memberHandle = AddMemberReference(field.Name, GetTypeHandle(memberInfo.DeclaringType!),
MetadataSignatureHelper.GetFieldSignature(field.FieldType, field.GetRequiredCustomModifiers(), field.GetOptionalCustomModifiers(), this));
break;
case ConstructorInfo ctor:
memberHandle = AddMemberReference(ctor.Name, GetTypeHandle(ctor.DeclaringType!), MetadataSignatureHelper.GetConstructorSignature(ctor.GetParameters(), this));
memberHandle = AddMemberReference(ctor.Name, GetTypeHandle(memberInfo.DeclaringType!), MetadataSignatureHelper.GetConstructorSignature(ctor.GetParameters(), this));
break;
case MethodInfo method:
if (method.IsConstructedGenericMethod)
Expand All @@ -536,15 +539,15 @@ private EntityHandle GetMemberReferenceHandle(MemberInfo member)
}
else
{
memberHandle = AddMemberReference(method.Name, GetTypeHandle(method.DeclaringType!), GetMethodSignature(method, null));
memberHandle = AddMemberReference(method.Name, GetTypeHandle(memberInfo.DeclaringType!), GetMethodSignature(method, null));
}
break;
default:
throw new NotSupportedException();

}

_memberReferences.Add(member, memberHandle);
_memberReferences.Add(memberInfo, memberHandle);
}

return memberHandle;
Expand Down
4 changes: 2 additions & 2 deletions ...libraries/System.Reflection.Emit/src/System/Reflection/Emit/PseudoCustomAttributesData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal DllImportData(string moduleName, string? entryPoint, MethodImportAttrib

public MethodImportAttributes Flags => _flags;

internal static DllImportData CreateDllImportData(CustomAttributeInfo attr, out bool preserveSig)
internal static DllImportData Create(CustomAttributeInfo attr, out bool preserveSig)
{
string? moduleName = (string?)attr._ctorArgs[0];
if (string.IsNullOrEmpty(moduleName))
Expand Down Expand Up @@ -93,7 +93,7 @@ internal static DllImportData CreateDllImportData(CustomAttributeInfo attr, out
return new DllImportData(moduleName, entryPoint, importAttributes);
}

internal static DllImportData CreateDllImportData(string moduleName, string entryName, CallingConvention nativeCallConv, CharSet nativeCharSet)
internal static DllImportData Create(string moduleName, string entryName, CallingConvention nativeCallConv, CharSet nativeCharSet)
{
if (string.IsNullOrEmpty(moduleName))
{
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/TypeBuilderImpl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ protected override MethodBuilder DefinePInvokeMethodCore(string name, string dll
attributes |= MethodAttributes.PinvokeImpl;
MethodBuilderImpl method = new MethodBuilderImpl(name, attributes, callingConvention, returnType, returnTypeRequiredCustomModifiers,
returnTypeOptionalCustomModifiers, parameterTypes, parameterTypeRequiredCustomModifiers, parameterTypeOptionalCustomModifiers, _module, this);
method.CreateDllData(dllName, entryName, nativeCallConv, nativeCharSet);
method.CreateDllImportData(dllName, entryName, nativeCallConv, nativeCharSet);

if (_methodDefinitions.Contains(method))
{
Expand Down
12 changes: 9 additions & 3 deletions ...System.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveModuleBuilderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public void DefineInitializedData_EnsureAlignmentIsMinimumNeededForUseOfCreateSp
{
AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderAndSaveMethod(new AssemblyName("MyAssembly"), out MethodInfo saveMethod);
ModuleBuilder module = ab.DefineDynamicModule("MyModule");
TypeBuilder tb = module.DefineType("MyType", TypeAttributes.Public);
// Create static field data in a variety of orders that requires the runtime to actively apply alignment
// RuntimeHelpers.CreateSpan requires data to be naturally aligned within the "PE" file. At this time CreateSpan only
// requires alignments up to 8 bytes.
Expand All @@ -204,11 +204,13 @@ public void DefineInitializedData_EnsureAlignmentIsMinimumNeededForUseOfCreateSp
byte[] field4Byte_2_data = new byte[] { 5, 6, 7, 8 };
byte[] field8Byte_2_data = new byte[] { 9, 10, 11, 12, 13, 14, 15, 16 };
FieldBuilder field4Byte_1 = module.DefineInitializedData("Field4Bytes_1", field4Byte_1_data, FieldAttributes.Public);
FieldBuilder tbField4Byte_1 = tb.DefineInitializedData("Field4Bytes_1", field4Byte_1_data, FieldAttributes.Public);
FieldBuilder field8Byte_1 = module.DefineInitializedData("Field8Bytes_1", field8Byte_1_data, FieldAttributes.Public);
FieldBuilder field4Byte_2 = module.DefineInitializedData("Field4Bytes_2", field4Byte_2_data, FieldAttributes.Public);
FieldBuilder field8Byte_2 = module.DefineInitializedData("Field8Bytes_2", field8Byte_2_data, FieldAttributes.Public);
FieldBuilder tbField8Byte_2 = tb.DefineInitializedData("Field8Bytes_2", field8Byte_2_data, FieldAttributes.Public);
module.CreateGlobalFunctions();
tb.CreateType();
Assert.Null(field4Byte_1.DeclaringType);
Assert.Null(field8Byte_1.DeclaringType);
Assert.Null(field4Byte_2.DeclaringType);
Expand All @@ -217,9 +219,11 @@ public void DefineInitializedData_EnsureAlignmentIsMinimumNeededForUseOfCreateSp
var checkTypeBuilder = module.DefineType("CheckType", TypeAttributes.Public);
CreateLoadAddressMethod("LoadAddress1", field1Byte);
CreateLoadAddressMethod("LoadAddress4_1", field4Byte_1);
CreateLoadAddressMethod("LoadAddress4_3", tbField4Byte_1);
CreateLoadAddressMethod("LoadAddress4_2", field4Byte_2);
CreateLoadAddressMethod("LoadAddress8_1", field8Byte_1);
CreateLoadAddressMethod("LoadAddress8_2", field8Byte_2);
CreateLoadAddressMethod("LoadAddress8_3", tbField8Byte_2);
void CreateLoadAddressMethod(string name, FieldBuilder fieldBuilder)
{
Expand All @@ -236,9 +240,11 @@ void CreateLoadAddressMethod(string name, FieldBuilder fieldBuilder)
Type checkType = assemblyFromDisk.GetType("CheckType");
CheckMethod("LoadAddress4_1", 4, field4Byte_1_data);
CheckMethod("LoadAddress4_3", 4, field4Byte_1_data);
CheckMethod("LoadAddress4_2", 4, field4Byte_2_data);
CheckMethod("LoadAddress8_1", 8, field8Byte_1_data);
CheckMethod("LoadAddress8_2", 8, field8Byte_2_data);
CheckMethod("LoadAddress8_3", 8, field8Byte_2_data);
void CheckMethod(string name, int minAlignmentRequired, byte[] dataToVerify)
{
Expand All @@ -249,7 +255,7 @@ void CheckMethod(string name, int minAlignmentRequired, byte[] dataToVerify)
{
Assert.Equal(dataToVerify[i], Marshal.ReadByte(address + (nint)i));
}
//Assert.Equal(name + "_0" + "_" + address.ToString(), name + "_" + (address % minAlignmentRequired).ToString() + "_" + address.ToString());
Assert.Equal(name + "_0" + "_" + address.ToString(), name + "_" + (address % minAlignmentRequired).ToString() + "_" + address.ToString());
}
}
Expand Down
66 changes: 0 additions & 66 deletions ...stem.Reflection.Emit/tests/PersistableAssemblyBuilder/AssemblySaveTypeBuilderAPIsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,71 +691,5 @@ public void DefineTypeInitializer()
return RemoteExecutor.SuccessExitCode;
}).Dispose();
}

[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public void DefineInitializedData_EnsureAlignmentIsMinimumNeededForUseOfCreateSpan()
{
RemoteExecutor.Invoke(() =>
{
using (TempFile file = TempFile.Create())
{
AssemblyBuilder ab = AssemblySaveTools.PopulateAssemblyBuilderTypeBuilderAndSaveMethod(out TypeBuilder tb, out MethodInfo saveMethod);
// Create static field data in a variety of orders that requires the runtime to actively apply alignment
// RuntimeHelpers.CreateSpan requires data to be naturally aligned within the "PE" file. At this time CreateSpan only
// requires alignments up to 8 bytes.
FieldBuilder field1Byte = tb.DefineInitializedData("Field1Byte", new byte[] { 1 }, FieldAttributes.Public);
byte[] field4Byte_1_data = [1, 2, 3, 4];
byte[] field8Byte_1_data = [1, 2, 3, 4, 5, 6, 7, 8];
byte[] field4Byte_2_data = [5, 6, 7, 8];
byte[] field8Byte_2_data = [9, 10, 11, 12, 13, 14, 15, 16];
FieldBuilder field4Byte_1 = tb.DefineInitializedData("Field4Bytes_1", field4Byte_1_data, FieldAttributes.Public);
FieldBuilder field8Byte_1 = tb.DefineInitializedData("Field8Bytes_1", field8Byte_1_data, FieldAttributes.Public);
FieldBuilder field4Byte_2 = tb.DefineInitializedData("Field4Bytes_2", field4Byte_2_data, FieldAttributes.Public);
FieldBuilder field8Byte_2 = tb.DefineInitializedData("Field8Bytes_2", field8Byte_2_data, FieldAttributes.Public);
tb.CreateType();
var checkTypeBuilder = ab.GetDynamicModule("MyModule").DefineType("CheckType", TypeAttributes.Public);
CreateLoadAddressMethod("LoadAddress1", field1Byte);
CreateLoadAddressMethod("LoadAddress4_1", field4Byte_1);
CreateLoadAddressMethod("LoadAddress4_2", field4Byte_2);
CreateLoadAddressMethod("LoadAddress8_1", field8Byte_1);
CreateLoadAddressMethod("LoadAddress8_2", field8Byte_2);
void CreateLoadAddressMethod(string name, FieldBuilder fieldBuilder)
{
var loadAddressMethod = checkTypeBuilder.DefineMethod(name, MethodAttributes.Public | MethodAttributes.Static, typeof(IntPtr), null);
var methodIL = loadAddressMethod.GetILGenerator();
methodIL.Emit(OpCodes.Ldsflda, fieldBuilder);
methodIL.Emit(OpCodes.Ret);
}
checkTypeBuilder.CreateType();
saveMethod.Invoke(ab, [file.Path]);
Assembly assemblyFromDisk = Assembly.LoadFile(file.Path);
Type checkType = assemblyFromDisk.GetType("CheckType");
CheckMethod("LoadAddress4_1", 4, field4Byte_1_data);
CheckMethod("LoadAddress4_2", 4, field4Byte_2_data);
CheckMethod("LoadAddress8_1", 8, field8Byte_1_data);
CheckMethod("LoadAddress8_2", 8, field8Byte_2_data);
void CheckMethod(string name, int minAlignmentRequired, byte[] dataToVerify)
{
var methodToCall = checkType.GetMethod(name);
nint address = (nint)methodToCall.Invoke(null, null);
for (int i = 0; i < dataToVerify.Length; i++)
{
Assert.Equal(dataToVerify[i], Marshal.ReadByte(address + (nint)i));
}
//Assert.Equal(name + "_0" + "_" + address.ToString(), name + "_" + (address % minAlignmentRequired).ToString() + "_" + address.ToString());
}
}
return RemoteExecutor.SuccessExitCode;
}).Dispose();
}
}
}
2 changes: 1 addition & 1 deletion ...System.Reflection.Metadata/src/System/Reflection/PortableExecutable/ManagedTextSection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ private int ComputeOffsetToMetadata()

public int ComputeSizeOfTextSection()
{
// Debug.Assert(MappedFieldDataSize % MappedFieldDataAlignment == 0);
Debug.Assert(MappedFieldDataSize % MappedFieldDataAlignment == 0);
return CalculateOffsetToMappedFieldDataStream() + MappedFieldDataSize;
}

Expand Down

0 comments on commit 1be0fb3

Please sign in to comment.