Skip to content

Commit bb4ef64

Browse files
Parse custom attribute value blobs for dependencies and rewrite type name strings (#127596)
## Description `CustomAttributeNode` had two TODOs: it wasn't parsing the custom attribute value blob for dependencies, and it wasn't rewriting type name strings that could become stale after trimming removes type forwarders. ### Dependency analysis (`GetStaticDependencies`) Decodes the custom attribute blob via `CustomAttributeTypeProvider` and reports dependencies for: - `typeof()` arguments - Enum types (needed for boxing) - Property setters and fields referenced in named arguments - Non-primitive array element types Constructor resolution uses `TryGetMethod` instead of try/catch, with a null constructor triggering an early return before named argument processing. `GetDependenciesFromPropertySetter` uses `GetTypeDefinition()` to correctly resolve property setters on generic attribute types. Dependency decode failures set an instance `_isCorrupted` flag so rewrite can avoid re-processing known-bad blobs. ### Blob rewriting (`WriteInternal`) `RewriteCustomAttributeBlob` reads the custom attribute blob manually with `BlobReader`, writes into `writeContext.GetSharedBlobBuilder()`, and rewrites only serialized type-name strings where needed (`System.Type` payloads and enum type names). Type-name resolution uses `GetTypeByCustomAttributeTypeName(..., throwIfNotFound: false)` and rewrites only when resolution succeeds, preserving unresolved serialized type names as-is. Rewrite early-outs to copying the original blob when `_isCorrupted` is set or when the constructor cannot be resolved. ### Type safety improvements `NodeFactory.ReflectedType`, `ReflectedMethod`, and `ReflectedField` now return `DependencyNode` directly instead of `object`, since all return paths produce a `DependencyNode` (either a real definition node or `NullDependencyNode.Instance`). This removes the need for `is DependencyNode` pattern checks at all call sites. Similarly, the `value is ImmutableArray<CustomAttributeTypedArgument<TypeDesc>>` pattern check in `GetDependenciesFromCustomAttributeArgument` has been replaced with a hard cast, relying on the `System.Reflection.Metadata` contract that `DecodeValue` always produces `ImmutableArray<...>` for `SzArray`-typed arguments (the `value is null` case is already handled earlier). --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com> Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
1 parent 98fa2f1 commit bb4ef64

3 files changed

Lines changed: 283 additions & 34 deletions

File tree

src/coreclr/tools/ILTrim.Core/DependencyAnalysis/NodeFactory.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ public bool IsModuleTrimmed(EcmaModule module)
301301
NodeCache<MetadataType, ObjectGetTypeCalledNode> _objectGetTypeCalledNodes = new NodeCache<MetadataType, ObjectGetTypeCalledNode>(key
302302
=> new ObjectGetTypeCalledNode(key));
303303

304-
public object ReflectedType(TypeDesc type)
304+
public DependencyNode ReflectedType(TypeDesc type)
305305
{
306306
// TODO: this should be a separate node with more logic
307307

@@ -316,7 +316,7 @@ public object ReflectedType(TypeDesc type)
316316
return TypeDefinition(definition.Module, definition.Handle);
317317
}
318318

319-
public object ReflectedMethod(MethodDesc method)
319+
public DependencyNode ReflectedMethod(MethodDesc method)
320320
{
321321
// TODO: this should be a separate node with more logic
322322
var definition = (EcmaMethod)method.GetTypicalMethodDefinition();
@@ -327,7 +327,7 @@ public object ReflectedMethod(MethodDesc method)
327327
return MethodDefinition(definition.Module, definition.Handle);
328328
}
329329

330-
public object ReflectedField(FieldDesc field)
330+
public DependencyNode ReflectedField(FieldDesc field)
331331
{
332332
// TODO: this should be a separate node with more logic
333333
var definition = (EcmaField)field.GetTypicalFieldDefinition();

src/coreclr/tools/ILTrim.Core/DependencyAnalysis/TokenBased/CustomAttributeNode.cs

Lines changed: 280 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,29 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System;
45
using System.Collections.Generic;
6+
using System.Collections.Immutable;
57
using System.Reflection.Metadata;
8+
using System.Reflection.Metadata.Ecma335;
9+
using System.Text;
610

11+
using Internal.TypeSystem;
712
using Internal.TypeSystem.Ecma;
813

14+
using DependencyNode = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>;
15+
916
namespace ILCompiler.DependencyAnalysis
1017
{
1118
/// <summary>
1219
/// Represents an entry in the Custom Attribute metadata table.
1320
/// </summary>
1421
public sealed class CustomAttributeNode : TokenBasedNode
1522
{
23+
// Set when dependency-time custom attribute decoding fails.
24+
// Rewrite then preserves the original blob for this node.
25+
private bool _isCorrupted;
26+
1627
public CustomAttributeNode(EcmaModule module, CustomAttributeHandle handle)
1728
: base(module, handle)
1829
{
@@ -54,14 +65,121 @@ public static bool IsCustomAttributeForSecurity(EcmaModule module, CustomAttribu
5465

5566
public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory)
5667
{
68+
DependencyList dependencies = new DependencyList();
69+
5770
CustomAttribute customAttribute = _module.MetadataReader.GetCustomAttribute(Handle);
5871

5972
// We decided not to report parent as a dependency because we don't expect custom attributes to be needed outside of their parent references
6073

61-
if (!customAttribute.Constructor.IsNil)
62-
yield return new DependencyListEntry(factory.GetNodeForMethodToken(_module, customAttribute.Constructor), "Custom attribute constructor");
74+
dependencies.Add(factory.GetNodeForMethodToken(_module, customAttribute.Constructor), "Custom attribute constructor");
75+
76+
// Parse the custom attribute value blob and add dependencies from it
77+
CustomAttributeValue<TypeDesc> decodedValue;
78+
try
79+
{
80+
decodedValue = customAttribute.DecodeValue(new CustomAttributeTypeProvider(_module));
81+
}
82+
catch (Exception ex) when (ex is TypeSystemException or BadImageFormatException)
83+
{
84+
// Metadata decode failed.
85+
_isCorrupted = true;
86+
return dependencies;
87+
}
88+
89+
foreach (CustomAttributeTypedArgument<TypeDesc> fixedArg in decodedValue.FixedArguments)
90+
{
91+
GetDependenciesFromCustomAttributeArgument(dependencies, factory, fixedArg.Type, fixedArg.Value);
92+
}
93+
94+
// Resolve the constructor once for all named arguments
95+
MethodDesc constructor = _module.TryGetMethod(customAttribute.Constructor);
96+
if (constructor is null)
97+
return dependencies;
6398

64-
// TODO: Parse custom attribute value and add dependencies from it
99+
foreach (CustomAttributeNamedArgument<TypeDesc> namedArg in decodedValue.NamedArguments)
100+
{
101+
if (namedArg.Kind == CustomAttributeNamedArgumentKind.Property)
102+
GetDependenciesFromPropertySetter(dependencies, factory, constructor.OwningType, namedArg.Name);
103+
else if (namedArg.Kind == CustomAttributeNamedArgumentKind.Field)
104+
GetDependenciesFromField(dependencies, factory, constructor.OwningType, namedArg.Name);
105+
106+
GetDependenciesFromCustomAttributeArgument(dependencies, factory, namedArg.Type, namedArg.Value);
107+
}
108+
109+
return dependencies;
110+
}
111+
112+
private static void GetDependenciesFromCustomAttributeArgument(DependencyList dependencies, NodeFactory factory, TypeDesc type, object value)
113+
{
114+
// Report the type itself (e.g. enum types that need to be kept for boxing)
115+
dependencies.Add(factory.ReflectedType(type), "Custom attribute blob");
116+
117+
if (type.UnderlyingType.IsPrimitive || type.IsString || value is null)
118+
return;
119+
120+
if (type.IsSzArray)
121+
{
122+
TypeDesc elementType = ((ArrayType)type).ElementType;
123+
if (!elementType.UnderlyingType.IsPrimitive && !elementType.IsString)
124+
{
125+
// DecodeValue always produces ImmutableArray<...> for SzArray types.
126+
var arrayElements = (ImmutableArray<CustomAttributeTypedArgument<TypeDesc>>)value;
127+
foreach (CustomAttributeTypedArgument<TypeDesc> element in arrayElements)
128+
{
129+
GetDependenciesFromCustomAttributeArgument(dependencies, factory, element.Type, element.Value);
130+
}
131+
}
132+
}
133+
else if (value is TypeDesc typeofType)
134+
{
135+
// typeof() - the value is a TypeDesc
136+
dependencies.Add(factory.ReflectedType(typeofType), "Custom attribute blob");
137+
}
138+
}
139+
140+
private static void GetDependenciesFromPropertySetter(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, string propertyName)
141+
{
142+
if (attributeType.GetTypeDefinition() is not EcmaType ecmaType)
143+
return;
144+
145+
MetadataReader reader = ecmaType.MetadataReader;
146+
TypeDefinition typeDef = reader.GetTypeDefinition(ecmaType.Handle);
147+
148+
foreach (PropertyDefinitionHandle propDefHandle in typeDef.GetProperties())
149+
{
150+
PropertyDefinition propDef = reader.GetPropertyDefinition(propDefHandle);
151+
if (reader.StringComparer.Equals(propDef.Name, propertyName))
152+
{
153+
PropertyAccessors accessors = propDef.GetAccessors();
154+
if (!accessors.Setter.IsNil)
155+
{
156+
dependencies.Add(factory.ReflectedMethod(ecmaType.Module.GetMethod(accessors.Setter)), "Custom attribute blob");
157+
}
158+
159+
return;
160+
}
161+
}
162+
163+
// Check base type
164+
TypeDesc baseType = attributeType.BaseType;
165+
if (baseType is not null)
166+
GetDependenciesFromPropertySetter(dependencies, factory, baseType, propertyName);
167+
}
168+
169+
private static void GetDependenciesFromField(DependencyList dependencies, NodeFactory factory, TypeDesc attributeType, string fieldName)
170+
{
171+
FieldDesc field = attributeType.GetField(Encoding.UTF8.GetBytes(fieldName));
172+
if (field is not null)
173+
{
174+
dependencies.Add(factory.ReflectedField(field), "Custom attribute blob");
175+
}
176+
else
177+
{
178+
// Check base type
179+
TypeDesc baseType = attributeType.BaseType;
180+
if (baseType is not null)
181+
GetDependenciesFromField(dependencies, factory, baseType, fieldName);
182+
}
65183
}
66184

67185
protected override EntityHandle WriteInternal(ModuleWritingContext writeContext)
@@ -71,12 +189,168 @@ protected override EntityHandle WriteInternal(ModuleWritingContext writeContext)
71189

72190
var builder = writeContext.MetadataBuilder;
73191

74-
// TODO: the value blob might contain references to tokens we need to rewrite
75-
var valueBlob = reader.GetBlobBytes(customAttribute.Value);
192+
// Resolve type name strings in the blob to their definitions.
193+
// Trimming may drop type forwarders, so we re-encode type names
194+
// to point to the assembly where the type is actually defined.
195+
BlobBuilder blobBuilder = writeContext.GetSharedBlobBuilder();
196+
RewriteCustomAttributeBlob(customAttribute, blobBuilder);
76197

77198
return builder.AddCustomAttribute(writeContext.TokenMap.MapToken(customAttribute.Parent),
78199
writeContext.TokenMap.MapToken(customAttribute.Constructor),
79-
builder.GetOrAddBlob(valueBlob));
200+
builder.GetOrAddBlob(blobBuilder));
201+
}
202+
203+
private void RewriteCustomAttributeBlob(CustomAttribute customAttribute, BlobBuilder blobBuilder)
204+
{
205+
if (_isCorrupted || _module.TryGetMethod(customAttribute.Constructor) is not MethodDesc constructor)
206+
{
207+
blobBuilder.WriteBytes(_module.MetadataReader.GetBlobBytes(customAttribute.Value));
208+
return;
209+
}
210+
211+
BlobReader valueReader = _module.MetadataReader.GetBlobReader(customAttribute.Value);
212+
var formatter = new CustomAttributeTypeNameFormatter();
213+
214+
// The dependency walk already decoded the blob successfully unless `_isCorrupted` is set.
215+
blobBuilder.WriteUInt16(valueReader.ReadUInt16());
216+
217+
MethodSignature constructorSig = constructor.Signature;
218+
for (int i = 0; i < constructorSig.Length; i++)
219+
{
220+
GetFixedArgumentTypeCodes(constructorSig[i], out SerializationTypeCode valueTypeCode, out SerializationTypeCode elementValueTypeCode);
221+
CopySerializedValue(ref valueReader, blobBuilder, formatter, valueTypeCode, elementValueTypeCode);
222+
}
223+
224+
ushort namedArgumentCount = valueReader.ReadUInt16();
225+
blobBuilder.WriteUInt16(namedArgumentCount);
226+
227+
for (int i = 0; i < namedArgumentCount; i++)
228+
{
229+
CustomAttributeNamedArgumentKind kind = (CustomAttributeNamedArgumentKind)valueReader.ReadByte();
230+
blobBuilder.WriteByte((byte)kind);
231+
232+
CopyNamedArgumentType(ref valueReader, blobBuilder, formatter, out SerializationTypeCode valueTypeCode, out SerializationTypeCode elementValueTypeCode);
233+
CopySerializedString(ref valueReader, blobBuilder, rewriteTypeName: false, formatter);
234+
CopySerializedValue(ref valueReader, blobBuilder, formatter, valueTypeCode, elementValueTypeCode);
235+
}
236+
}
237+
238+
// We can avoid the intermediate array allocation after https://github.com/dotnet/runtime/issues/85280 (or with unsafe code now)
239+
private static void CopyRawBytes(ref BlobReader reader, BlobBuilder blobBuilder, int byteCount)
240+
=> blobBuilder.WriteBytes(reader.ReadBytes(byteCount));
241+
242+
private TypeDesc? CopySerializedString(ref BlobReader valueReader, BlobBuilder blobBuilder, bool rewriteTypeName, CustomAttributeTypeNameFormatter formatter)
243+
{
244+
string? s = valueReader.ReadSerializedString();
245+
TypeDesc? resolved = null;
246+
247+
if (rewriteTypeName && s is not null)
248+
{
249+
resolved = _module.GetTypeByCustomAttributeTypeName(s);
250+
s = formatter.FormatName(resolved, true);
251+
}
252+
253+
blobBuilder.WriteSerializedString(s);
254+
return resolved;
255+
}
256+
257+
private static void GetFixedArgumentTypeCodes(TypeDesc type, out SerializationTypeCode valueTypeCode, out SerializationTypeCode elementValueTypeCode)
258+
{
259+
elementValueTypeCode = default;
260+
if (type.IsPrimitive || type.IsEnum)
261+
{
262+
valueTypeCode = GetPrimitiveSerializationTypeCode(type.UnderlyingType);
263+
}
264+
else if (type.IsString)
265+
{
266+
valueTypeCode = SerializationTypeCode.String;
267+
}
268+
else if (type.IsObject)
269+
{
270+
valueTypeCode = SerializationTypeCode.TaggedObject;
271+
}
272+
else if (type.IsSzArray)
273+
{
274+
valueTypeCode = SerializationTypeCode.SZArray;
275+
GetFixedArgumentTypeCodes(((ArrayType)type).ElementType, out elementValueTypeCode, out _);
276+
}
277+
else
278+
{
279+
valueTypeCode = SerializationTypeCode.Type;
280+
}
281+
}
282+
283+
private static SerializationTypeCode GetPrimitiveSerializationTypeCode(TypeDesc type)
284+
=> type.Category switch
285+
{
286+
TypeFlags.Boolean => SerializationTypeCode.Boolean,
287+
TypeFlags.Char => SerializationTypeCode.Char,
288+
TypeFlags.SByte => SerializationTypeCode.SByte,
289+
TypeFlags.Byte => SerializationTypeCode.Byte,
290+
TypeFlags.Int16 => SerializationTypeCode.Int16,
291+
TypeFlags.UInt16 => SerializationTypeCode.UInt16,
292+
TypeFlags.Int32 => SerializationTypeCode.Int32,
293+
TypeFlags.UInt32 => SerializationTypeCode.UInt32,
294+
TypeFlags.Int64 => SerializationTypeCode.Int64,
295+
TypeFlags.UInt64 => SerializationTypeCode.UInt64,
296+
TypeFlags.Single => SerializationTypeCode.Single,
297+
TypeFlags.Double => SerializationTypeCode.Double,
298+
_ => throw new BadImageFormatException(),
299+
};
300+
301+
private void CopyNamedArgumentType(ref BlobReader valueReader, BlobBuilder blobBuilder, CustomAttributeTypeNameFormatter formatter, out SerializationTypeCode valueTypeCode, out SerializationTypeCode elementValueTypeCode)
302+
{
303+
valueTypeCode = valueReader.ReadSerializationTypeCode();
304+
elementValueTypeCode = default;
305+
blobBuilder.WriteByte((byte)valueTypeCode);
306+
307+
switch (valueTypeCode)
308+
{
309+
case SerializationTypeCode.SZArray:
310+
CopyNamedArgumentType(ref valueReader, blobBuilder, formatter, out elementValueTypeCode, out _);
311+
break;
312+
case SerializationTypeCode.Enum:
313+
TypeDesc enumType = CopySerializedString(ref valueReader, blobBuilder, rewriteTypeName: true, formatter);
314+
valueTypeCode = GetPrimitiveSerializationTypeCode(enumType.UnderlyingType);
315+
break;
316+
}
317+
}
318+
319+
private void CopySerializedValue(ref BlobReader valueReader, BlobBuilder blobBuilder, CustomAttributeTypeNameFormatter formatter, SerializationTypeCode valueTypeCode, SerializationTypeCode elementValueTypeCode)
320+
{
321+
switch (valueTypeCode)
322+
{
323+
case SerializationTypeCode.Boolean or SerializationTypeCode.Byte or SerializationTypeCode.SByte:
324+
CopyRawBytes(ref valueReader, blobBuilder, 1);
325+
break;
326+
case SerializationTypeCode.Char or SerializationTypeCode.Int16 or SerializationTypeCode.UInt16:
327+
CopyRawBytes(ref valueReader, blobBuilder, 2);
328+
break;
329+
case SerializationTypeCode.Int32 or SerializationTypeCode.UInt32 or SerializationTypeCode.Single:
330+
CopyRawBytes(ref valueReader, blobBuilder, 4);
331+
break;
332+
case SerializationTypeCode.Int64 or SerializationTypeCode.UInt64 or SerializationTypeCode.Double:
333+
CopyRawBytes(ref valueReader, blobBuilder, 8);
334+
break;
335+
case SerializationTypeCode.String:
336+
CopySerializedString(ref valueReader, blobBuilder, rewriteTypeName: false, formatter);
337+
break;
338+
case SerializationTypeCode.Type:
339+
CopySerializedString(ref valueReader, blobBuilder, rewriteTypeName: true, formatter);
340+
break;
341+
case SerializationTypeCode.SZArray:
342+
int elementCount = valueReader.ReadInt32();
343+
blobBuilder.WriteInt32(elementCount);
344+
for (int i = 0; i < elementCount; i++)
345+
{
346+
CopySerializedValue(ref valueReader, blobBuilder, formatter, elementValueTypeCode, default);
347+
}
348+
break;
349+
case SerializationTypeCode.TaggedObject:
350+
CopyNamedArgumentType(ref valueReader, blobBuilder, formatter, out SerializationTypeCode boxedTypeCode, out SerializationTypeCode boxedElementTypeCode);
351+
CopySerializedValue(ref valueReader, blobBuilder, formatter, boxedTypeCode, boxedElementTypeCode);
352+
break;
353+
}
80354
}
81355

82356
public override string ToString()

0 commit comments

Comments
 (0)