Skip to content

Commit 7f9a3f1

Browse files
jkotasjgh07teo-tsirpanis
authored
[release/10.0] PersistedAssemblyBuilder: Fix encoding of custom modifiers. (#123925) (#124537)
Backport of #119935, #121128, #123925 to release/10.0 ## Customer Impact - [X] Customer reported - [ ] Found internally Unable to emit references to methods and fields with signatures that include modopts and modreqs using `PersistedAssemblyBuilder` (e.g. method signatures that use C# `readonly ref` or `in`). ## Regression - [ ] Yes - [X] No This is a bug in PersistedAssemblyBuilder that shipped in .NET 9. Adoption blocker with no workaround. ## Testing Dedicated tests. ## Risk Medium. The change is a combination of 3 different PRs that fixed different aspects of the bug. The size of the delta makes is a medium risk. --------- Co-authored-by: Jonas <35030845+jgh07@users.noreply.github.com> Co-authored-by: Theodore Tsirpanis <teo@tsirpanis.gr>
1 parent ad38f95 commit 7f9a3f1

File tree

14 files changed

+411
-83
lines changed

14 files changed

+411
-83
lines changed

‎src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/FieldOnTypeBuilderInstantiation.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ internal FieldOnTypeBuilderInstantiation(FieldInfo field, TypeBuilderInstantiati
5656
#region Public Abstract\Virtual Members
5757
public override Type[] GetRequiredCustomModifiers() { return _field.GetRequiredCustomModifiers(); }
5858
public override Type[] GetOptionalCustomModifiers() { return _field.GetOptionalCustomModifiers(); }
59+
public override Type GetModifiedFieldType() => _field.GetModifiedFieldType();
5960
public override void SetValueDirect(TypedReference obj, object value)
6061
{
6162
throw new NotImplementedException();

‎src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/MethodOnTypeBuilderInstantiation.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public override MethodInfo MakeGenericMethod(params Type[] typeArgs)
118118

119119
#region Public Abstract\Virtual Members
120120
public override Type ReturnType => _method.ReturnType;
121-
public override ParameterInfo ReturnParameter => throw new NotSupportedException();
121+
public override ParameterInfo ReturnParameter => _method.ReturnParameter;
122122
public override ICustomAttributeProvider ReturnTypeCustomAttributes => throw new NotSupportedException();
123123
public override MethodInfo GetBaseDefinition() { throw new NotSupportedException(); }
124124
#endregion

‎src/libraries/System.Private.CoreLib/src/System/Reflection/ModifiedType.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,10 @@ public override Type[] GetOptionalCustomModifiers()
172172
public override bool IsEnum => _unmodifiedType.IsEnum;
173173
protected override bool IsPrimitiveImpl() => _unmodifiedType.IsPrimitive;
174174
protected override bool IsByRefImpl() => _unmodifiedType.IsByRef;
175+
public override bool IsGenericParameter => _unmodifiedType.IsGenericParameter;
175176
public override bool IsGenericTypeParameter => _unmodifiedType.IsGenericTypeParameter;
176177
public override bool IsGenericMethodParameter => _unmodifiedType.IsGenericMethodParameter;
178+
public override int GenericParameterPosition => _unmodifiedType.GenericParameterPosition;
177179
protected override bool IsPointerImpl() => _unmodifiedType.IsPointer;
178180
protected override bool IsValueTypeImpl() => _unmodifiedType.IsValueType;
179181
protected override bool IsCOMObjectImpl() => _unmodifiedType.IsCOMObject;

‎src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/FieldBuilderImpl.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ public override void SetValue(object? obj, object? val, BindingFlags invokeAttr,
113113

114114
public override Type[] GetOptionalCustomModifiers() => _optionalCustomModifiers ?? Type.EmptyTypes;
115115

116+
public override Type GetModifiedFieldType() => FieldType;
117+
116118
#endregion
117119

118120
#region ICustomAttributeProvider Implementation

‎src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ModuleBuilderImpl.cs‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ internal void AppendMetadata(MethodBodyStreamEncoder methodBodyEncoder, BlobBuil
184184
}
185185

186186
// Now write all generic parameters in order
187-
genericParams.Sort((x, y) => {
187+
genericParams.Sort((x, y) =>
188+
{
188189
int primary = CodedIndex.TypeOrMethodDef(x._parentHandle).CompareTo(CodedIndex.TypeOrMethodDef(y._parentHandle));
189190
if (primary != 0)
190191
return primary;
@@ -690,6 +691,8 @@ internal void WriteCustomAttributes(List<CustomAttributeWrapper>? customAttribut
690691

691692
private EntityHandle GetTypeReferenceOrSpecificationHandle(Type type)
692693
{
694+
type = type.UnderlyingSystemType;
695+
693696
if (!_typeReferences.TryGetValue(type, out var typeHandle))
694697
{
695698
if (type.HasElementType || type.IsGenericParameter ||
@@ -740,7 +743,7 @@ private EntityHandle GetMemberReferenceHandle(MemberInfo memberInfo)
740743
declaringType = declaringType.MakeGenericType(declaringType.GetGenericArguments());
741744
}
742745

743-
Type fieldType = ((FieldInfo)GetOriginalMemberIfConstructedType(field)).FieldType;
746+
Type fieldType = ((FieldInfo)GetOriginalMemberIfConstructedType(field)).GetModifiedFieldType();
744747
memberHandle = AddMemberReference(field.Name, GetTypeHandle(declaringType),
745748
MetadataSignatureHelper.GetFieldSignature(fieldType, field.GetRequiredCustomModifiers(), field.GetOptionalCustomModifiers(), this));
746749

@@ -791,7 +794,7 @@ private EntityHandle GetMethodReference(MethodInfo methodInfo, Type[] optionalPa
791794
}
792795

793796
private BlobBuilder GetMethodSignature(MethodInfo method, Type[]? optionalParameterTypes) =>
794-
MetadataSignatureHelper.GetMethodSignature(this, ParameterTypes(method.GetParameters()), method.ReturnType,
797+
MetadataSignatureHelper.GetMethodSignature(this, MetadataSignatureHelper.GetParameterTypes(method.GetParameters()), method.ReturnParameter.GetModifiedParameterType(),
795798
GetSignatureConvention(method.CallingConvention), method.GetGenericArguments().Length, !method.IsStatic, optionalParameterTypes);
796799

797800
private BlobBuilder GetMethodArrayMethodSignature(ArrayMethod method) => MetadataSignatureHelper.GetMethodSignature(
@@ -829,23 +832,6 @@ private MemberInfo GetOriginalMemberIfConstructedType(MemberInfo memberInfo)
829832
return memberInfo;
830833
}
831834

832-
private static Type[] ParameterTypes(ParameterInfo[] parameterInfos)
833-
{
834-
if (parameterInfos.Length == 0)
835-
{
836-
return Type.EmptyTypes;
837-
}
838-
839-
Type[] parameterTypes = new Type[parameterInfos.Length];
840-
841-
for (int i = 0; i < parameterInfos.Length; i++)
842-
{
843-
parameterTypes[i] = parameterInfos[i].ParameterType;
844-
}
845-
846-
return parameterTypes;
847-
}
848-
849835
private AssemblyReferenceHandle GetAssemblyReference(Assembly assembly)
850836
{
851837
if (!_assemblyReferences.TryGetValue(assembly, out var handle))
@@ -861,7 +847,7 @@ private AssemblyReferenceHandle GetAssemblyReference(Assembly assembly)
861847
}
862848
else
863849
{
864-
publicKeyOrToken = aName.GetPublicKeyToken();
850+
publicKeyOrToken = aName.GetPublicKeyToken();
865851
}
866852
handle = AddAssemblyReference(aName.Name, aName.Version, aName.CultureName, publicKeyOrToken, assemblyFlags);
867853
_assemblyReferences.Add(assembly, handle);

‎src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/ParameterBuilderImpl.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,7 @@ protected override void SetCustomAttributeCore(ConstructorInfo con, ReadOnlySpan
6868
internal sealed class ParameterInfoWrapper : ParameterInfo
6969
{
7070
private readonly ParameterBuilderImpl _pb;
71-
private readonly Type _type
72-
;
71+
private readonly Type _type;
7372
public ParameterInfoWrapper(ParameterBuilderImpl pb, Type type)
7473
{
7574
_pb = pb;
@@ -87,5 +86,7 @@ public ParameterInfoWrapper(ParameterBuilderImpl pb, Type type)
8786
public override bool HasDefaultValue => _pb._defaultValue != DBNull.Value;
8887

8988
public override object? DefaultValue => HasDefaultValue ? _pb._defaultValue : null;
89+
90+
public override Type GetModifiedParameterType() => ParameterType;
9091
}
9192
}

‎src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs‎

Lines changed: 75 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ internal static BlobBuilder GetFieldSignature(Type fieldType, Type[] requiredCus
3030
{
3131
BlobBuilder fieldSignature = new();
3232
FieldTypeEncoder encoder = new BlobEncoder(fieldSignature).Field();
33-
WriteReturnTypeCustomModifiers(encoder.CustomModifiers(), requiredCustomModifiers, optionalCustomModifiers, module);
34-
WriteSignatureForType(encoder.Type(), fieldType, module);
33+
WriteSignatureForType(encoder.Type(), fieldType, module, requiredCustomModifiers, optionalCustomModifiers);
3534

3635
return fieldSignature;
3736
}
@@ -48,7 +47,7 @@ internal static BlobBuilder GetConstructorSignature(ParameterInfo[]? parameters,
4847

4948
retType.Void();
5049

51-
WriteParametersSignature(module, Array.ConvertAll(parameters, p => p.ParameterType), parameterEncoder);
50+
WriteParametersSignature(module, GetParameterTypes(parameters), parameterEncoder);
5251

5352
return constructorSignature;
5453
}
@@ -85,16 +84,8 @@ internal static BlobBuilder GetMethodSignature(ModuleBuilderImpl module, Type[]?
8584
new BlobEncoder(methodSignature).MethodSignature(convention, genParamCount, isInstance).
8685
Parameters(paramsLength, out ReturnTypeEncoder retEncoder, out ParametersEncoder parEncoder);
8786

88-
WriteReturnTypeCustomModifiers(retEncoder.CustomModifiers(), returnTypeRequiredModifiers, returnTypeOptionalModifiers, module);
89-
90-
if (returnType != null && returnType != module.GetTypeFromCoreAssembly(CoreTypeId.Void))
91-
{
92-
WriteSignatureForType(retEncoder.Type(), returnType, module);
93-
}
94-
else
95-
{
96-
retEncoder.Void();
97-
}
87+
returnType ??= module.GetTypeFromCoreAssembly(CoreTypeId.Void);
88+
WriteSignatureForType(retEncoder.Type(), returnType, module, returnTypeRequiredModifiers, returnTypeOptionalModifiers);
9889

9990
WriteParametersSignature(module, parameters, parEncoder, parameterRequiredModifiers, parameterOptionalModifiers);
10091

@@ -106,24 +97,29 @@ internal static BlobBuilder GetMethodSignature(ModuleBuilderImpl module, Type[]?
10697
return methodSignature;
10798
}
10899

109-
private static void WriteReturnTypeCustomModifiers(CustomModifiersEncoder encoder,
110-
Type[]? requiredModifiers, Type[]? optionalModifiers, ModuleBuilderImpl module)
100+
internal static Type[] GetParameterTypes(ParameterInfo[] parameterInfos)
111101
{
112-
if (requiredModifiers != null)
102+
if (parameterInfos.Length == 0)
113103
{
114-
WriteCustomModifiers(encoder, requiredModifiers, isOptional: false, module);
104+
return Type.EmptyTypes;
115105
}
116106

117-
if (optionalModifiers != null)
107+
Type[] parameterTypes = new Type[parameterInfos.Length];
108+
109+
for (int i = 0; i < parameterInfos.Length; i++)
118110
{
119-
WriteCustomModifiers(encoder, optionalModifiers, isOptional: true, module);
111+
parameterTypes[i] = parameterInfos[i].GetModifiedParameterType();
120112
}
113+
114+
return parameterTypes;
121115
}
122116

123117
private static void WriteCustomModifiers(CustomModifiersEncoder encoder, Type[] customModifiers, bool isOptional, ModuleBuilderImpl module)
124118
{
125-
foreach (Type modifier in customModifiers)
119+
// GetOptionalCustomModifiers and GetRequiredCustomModifiers return modifiers in reverse order
120+
for (int i = customModifiers.Length - 1; i >= 0; i--)
126121
{
122+
Type modifier = customModifiers[i];
127123
encoder.AddModifier(module.GetTypeHandle(modifier), isOptional);
128124
}
129125
}
@@ -137,17 +133,10 @@ private static void WriteParametersSignature(ModuleBuilderImpl module, Type[]? p
137133
{
138134
ParameterTypeEncoder encoder = parameterEncoder.AddParameter();
139135

140-
if (requiredModifiers != null && requiredModifiers.Length > i && requiredModifiers[i] != null)
141-
{
142-
WriteCustomModifiers(encoder.CustomModifiers(), requiredModifiers[i], isOptional: false, module);
143-
}
144-
145-
if (optionalModifiers != null && optionalModifiers.Length > i && optionalModifiers[i] != null)
146-
{
147-
WriteCustomModifiers(encoder.CustomModifiers(), optionalModifiers[i], isOptional: true, module);
148-
}
136+
Type[]? modreqs = (requiredModifiers != null && requiredModifiers.Length > i) ? requiredModifiers[i] : null;
137+
Type[]? modopts = (optionalModifiers != null && optionalModifiers.Length > i) ? optionalModifiers[i] : null;
149138

150-
WriteSignatureForType(encoder.Type(), parameters[i], module);
139+
WriteSignatureForType(encoder.Type(), parameters[i], module, modreqs, modopts);
151140
}
152141
}
153142
}
@@ -160,15 +149,16 @@ internal static BlobBuilder GetPropertySignature(PropertyBuilderImpl property, M
160149
PropertySignature(isInstanceProperty: property.CallingConventions.HasFlag(CallingConventions.HasThis)).
161150
Parameters(property.ParameterTypes == null ? 0 : property.ParameterTypes.Length, out ReturnTypeEncoder retType, out ParametersEncoder paramEncoder);
162151

163-
WriteReturnTypeCustomModifiers(retType.CustomModifiers(), property._returnTypeRequiredCustomModifiers, property._returnTypeOptionalCustomModifiers, module);
164-
WriteSignatureForType(retType.Type(), property.PropertyType, module);
152+
WriteSignatureForType(retType.Type(), property.PropertyType, module, property._returnTypeRequiredCustomModifiers, property._returnTypeOptionalCustomModifiers);
165153
WriteParametersSignature(module, property.ParameterTypes, paramEncoder, property._parameterTypeRequiredCustomModifiers, property._parameterTypeOptionalCustomModifiers);
166154

167155
return propertySignature;
168156
}
169157

170-
private static void WriteSignatureForType(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module)
158+
private static void WriteSignatureForType(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module, Type[]? requiredModifiers = null, Type[]? optionalModifiers = null)
171159
{
160+
WriteCustomModifiers(signature.CustomModifiers(), requiredModifiers ?? type.GetRequiredCustomModifiers(), isOptional: false, module);
161+
WriteCustomModifiers(signature.CustomModifiers(), optionalModifiers ?? type.GetOptionalCustomModifiers(), isOptional: true, module);
172162
if (type.IsArray)
173163
{
174164
Type elementType = type.GetElementType()!;
@@ -180,8 +170,8 @@ private static void WriteSignatureForType(SignatureTypeEncoder signature, Type t
180170
else
181171
{
182172
signature.Array(out SignatureTypeEncoder elTypeSignature, out ArrayShapeEncoder arrayEncoder);
183-
WriteSimpleSignature(elTypeSignature, elementType, module);
184-
arrayEncoder.Shape(type.GetArrayRank(), ImmutableArray.Create<int>(), ImmutableArray.Create<int>(new int[rank]));
173+
WriteSignatureForType(elTypeSignature, elementType, module);
174+
arrayEncoder.Shape(type.GetArrayRank(), [], default);
185175
}
186176
}
187177
else if (type.IsPointer)
@@ -223,14 +213,64 @@ private static void WriteSignatureForType(SignatureTypeEncoder signature, Type t
223213
{
224214
signature.GenericTypeParameter(type.GenericParameterPosition);
225215
}
216+
else if (type.IsFunctionPointer)
217+
{
218+
WriteSignatureForFunctionPointerType(signature, type, module);
219+
}
226220
else
227221
{
228222
WriteSimpleSignature(signature, type, module);
229223
}
230224
}
231225

226+
private static void WriteSignatureForFunctionPointerType(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module)
227+
{
228+
SignatureCallingConvention callConv = SignatureCallingConvention.Default;
229+
FunctionPointerAttributes attribs = FunctionPointerAttributes.None;
230+
231+
Type returnType = type.GetFunctionPointerReturnType();
232+
Type[] paramTypes = type.GetFunctionPointerParameterTypes();
233+
234+
if (type.IsUnmanagedFunctionPointer)
235+
{
236+
callConv = SignatureCallingConvention.Unmanaged;
237+
238+
if (type.GetFunctionPointerCallingConventions() is Type[] conventions && conventions.Length == 1)
239+
{
240+
switch (conventions[0].FullName)
241+
{
242+
case "System.Runtime.CompilerServices.CallConvCdecl":
243+
callConv = SignatureCallingConvention.CDecl;
244+
break;
245+
case "System.Runtime.CompilerServices.CallConvStdcall":
246+
callConv = SignatureCallingConvention.StdCall;
247+
break;
248+
case "System.Runtime.CompilerServices.CallConvThiscall":
249+
callConv = SignatureCallingConvention.ThisCall;
250+
break;
251+
case "System.Runtime.CompilerServices.CallConvFastcall":
252+
callConv = SignatureCallingConvention.FastCall;
253+
break;
254+
}
255+
}
256+
}
257+
258+
MethodSignatureEncoder sigEncoder = signature.FunctionPointer(callConv, attribs);
259+
sigEncoder.Parameters(paramTypes.Length, out ReturnTypeEncoder retTypeEncoder, out ParametersEncoder paramsEncoder);
260+
261+
WriteSignatureForType(retTypeEncoder.Type(), returnType, module);
262+
263+
foreach (Type paramType in paramTypes)
264+
{
265+
ParameterTypeEncoder paramEncoder = paramsEncoder.AddParameter();
266+
267+
WriteSignatureForType(paramEncoder.Type(), paramType, module);
268+
}
269+
}
270+
232271
private static void WriteSimpleSignature(SignatureTypeEncoder signature, Type type, ModuleBuilderImpl module)
233272
{
273+
type = type.UnderlyingSystemType;
234274
CoreTypeId? typeId = module.GetTypeIdFromCoreTypes(type);
235275

236276
switch (typeId)

‎src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveILGeneratorTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2637,9 +2637,9 @@ public void ReferenceNestedGenericTypeWithConstructedTypeBuilderParameterInIL()
26372637
TypeBuilder nestedItem = type.DefineNestedType("ItemInfo", TypeAttributes.NestedPublic);
26382638
GenericTypeParameterBuilder itemParam = nestedItem.DefineGenericParameters(genParams)[0];
26392639
TypeBuilder nestedSector = type.DefineNestedType("Sector", TypeAttributes.NestedPublic);
2640-
GenericTypeParameterBuilder nestedParam = nestedSector.DefineGenericParameters(genParams)[0];
2640+
GenericTypeParameterBuilder sectorParam = nestedSector.DefineGenericParameters(genParams)[0];
26412641

2642-
Type nestedOfT = nestedItem.MakeGenericType(nestedParam);
2642+
Type nestedOfT = nestedItem.MakeGenericType(sectorParam);
26432643
Type parent = typeof(HashSet<>).MakeGenericType(nestedOfT);
26442644
nestedSector.SetParent(parent);
26452645

‎src/libraries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderAPIsTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -633,13 +633,13 @@ public void ReturnTypeAndParameterRequiredOptionalCustomModifiers()
633633
Type[] par0RequiredMods = allModMethod.GetParameters()[0].GetRequiredCustomModifiers();
634634
Type[] par0OptionalMods = allModMethod.GetParameters()[0].GetOptionalCustomModifiers();
635635
Assert.Equal(2, returnReqMods.Length);
636-
Assert.Equal(mlc.CoreAssembly.GetType(typeof(short).FullName), returnReqMods[0]);
637-
Assert.Equal(mlc.CoreAssembly.GetType(typeof(int).FullName), returnReqMods[1]);
636+
Assert.Equal(mlc.CoreAssembly.GetType(typeof(int).FullName), returnReqMods[0]);
637+
Assert.Equal(mlc.CoreAssembly.GetType(typeof(short).FullName), returnReqMods[1]);
638638
Assert.Equal(1, returnOptMods.Length);
639639
Assert.Equal(mlc.CoreAssembly.GetType(typeof(Version).FullName), returnOptMods[0]);
640640
Assert.Equal(cmodsReq1.Length, par0RequiredMods.Length);
641-
Assert.Equal(mlc.CoreAssembly.GetType(cmodsReq1[1].FullName), par0RequiredMods[0]);
642-
Assert.Equal(mlc.CoreAssembly.GetType(cmodsReq1[0].FullName), par0RequiredMods[1]);
641+
Assert.Equal(mlc.CoreAssembly.GetType(cmodsReq1[0].FullName), par0RequiredMods[0]);
642+
Assert.Equal(mlc.CoreAssembly.GetType(cmodsReq1[1].FullName), par0RequiredMods[1]);
643643
Assert.Equal(cmodsOpt1.Length, par0OptionalMods.Length);
644644
Assert.Equal(mlc.CoreAssembly.GetType(cmodsOpt1[0].FullName), par0OptionalMods[0]);
645645
Assert.Equal(cmodsReq2.Length, allModMethod.GetParameters()[1].GetRequiredCustomModifiers().Length);

0 commit comments

Comments
 (0)