Parse custom attribute value blobs for dependencies, rewrite type name strings in ILTrim, track corrupted attribute blobs, and simplify rewrite type handling

[Copilot]

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

Follows the same pattern as CustomAttributeBasedDependencyAlgorithm in ILCompiler. All dependency helper methods accept a DependencyList parameter and use .Add() directly, avoiding yield return and IEnumerable overhead. GetStaticDependencies creates and returns the populated DependencyList. The unnecessary customAttribute.Constructor.IsNil guard has been removed since custom attributes always have a constructor. Dependency blob parsing and named argument handling are inlined directly into GetStaticDependencies rather than delegated to standalone methods. Constructor resolution uses TryGetMethod (backed by GetObject(handle, NotFoundBehavior.ReturnNull)) 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.

Additionally, dependency decode failures now set an instance _isCorrupted flag so rewrite can avoid re-processing known-bad blobs.

Blob rewriting (WriteInternal)

RewriteCustomAttributeBlob no longer uses customAttribute.DecodeValue for rewriting. Instead, it 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).

This avoids losing declared named-argument type information (especially object/tagged object cases), preserves malformed/edge metadata behavior by falling back to the original blob on decode/format failures, and still normalizes type names after trimming/type-forwarder changes (for example, rewriting System.Object, System.Runtime to System.Object, System.Private.CoreLib via CustomAttributeTypeNameFormatter).

Rewrite now takes advantage of corruption tracking:

• early-outs to copying the original blob when _isCorrupted is set
• simplifies rewrite-time branching by relying on dependency-time validation for valid-path assumptions
• retains fallback behavior for malformed/unresolvable cases during rewrite

Latest follow-up feedback updates also:

• switched named-argument/value dispatch in rewrite parsing to switch-based control flow
• removed redundant invalid-sentinel handling in the validated-blob path
• fixed enum fixed-argument typing by using type.UnderlyingType in fixed-argument type-code inference
• removed dead enum/fallback branches in fixed-argument type-code inference
• removed premature enum rewrite optimization (always writes the rewritten serialized enum type name)
• removed CustomAttributeTypeProvider indirection from rewrite helpers by using direct EcmaModule lookup for serialized type names and underlying enum primitive mapping logic
• converted rewrite blob helper methods to instance methods so _module is no longer threaded through helper parameters
• changed primitive serialization-type mapping to a switch expression
• updated grouped primitive serialization cases to use C# or patterns
• combined _isCorrupted and constructor resolution into a single early-out guard in rewrite
• simplified serialized string rewrite to direct read/rewrite/write flow and removed the premature byte-copy optimization for serialized strings

Code style

• Methods use cascading if/else if/else instead of early return statements.
• EncodeElementType switches on type.Category and handles IsEnum and System.Type in the default case, rather than checking them before the switch.
• Exception handling uses catch (Exception ex) when (ex is TypeSystemException or BadImageFormatException) to collapse multiple catch blocks.
• Rewrite parser type dispatch uses switch statements per reviewer guidance.
• Removed inaccurate decode-failure commentary and removed an unnecessary type is null check in dependency argument handling.

Test results

25 previously-expected-failing tests now pass and have been removed from ILTrimExpectedFailures.txt (including Attributes.GenericAttributes).

Validation run for this follow-up update:

• ./build.sh clr+libs -rc release
• dotnet build src/coreclr/tools/ILTrim.Core/ILTrim.Core.csproj
• dotnet test src/coreclr/tools/ILTrim.Tests/ILTrim.Tests.csproj --filter "FullyQualifiedName~AttributesTests"

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @dotnet/illink
See info in area-owners.md if you want to be subscribed.

[MichalStrehovsky]

@copilot

[MichalStrehovsky]

@copilot

[MichalStrehovsky]

@copilot

[MichalStrehovsky]

Do not pass _module to the various helpers here. Make the helpers instance methods instead.

[Copilot]

Done in d4b6451. I converted the rewrite helpers to instance methods and removed _module from helper parameter lists.

[MichalStrehovsky]

Suggested change

	case SerializationTypeCode.Boolean:
	case SerializationTypeCode.Byte:
	case SerializationTypeCode.SByte:
	case SerializationTypeCode.Boolean or SerializationTypeCode.Byte or SerializationTypeCode.SByte:

Also same below.

[Copilot]

Applied in d4b6451. I switched grouped primitive cases to or patterns in CopySerializedValue (including the lower grouped cases).

[MichalStrehovsky]

@copilot

[MichalStrehovsky]

This comment doesn't seem true.

[Copilot]

Updated in ef95ab1. I removed that inaccurate comment and replaced it with a concise decode-failure comment.

[MichalStrehovsky]

When can type be null?

[Copilot]

Fixed in ef95ab1. type cannot be null there, so I removed the null check.

[MichalStrehovsky]

Combine with the subsequent if:

Suggested change

	if (_isCorrupted)
	if (_isCorrupted || _module.TryGetMethod(customAttribute.Constructor) is not MethodDesc constructor)

[Copilot]

Done in ef95ab1. I combined the checks into a single early-out using _isCorrupted || _module.TryGetMethod(...) is not MethodDesc constructor.

[MichalStrehovsky]

Since we know the blob is valid, we don't need error handling.

The extra optimization is also premature.

Suggested change

	int startOffset = valueReader.Offset;
	string? original = valueReader.ReadSerializedString();
	int endOffset = valueReader.Offset;
	if (rewriteTypeName && original is not null)
	{
	TypeDesc resolved = _module.GetTypeByCustomAttributeTypeName(original);
	if (resolved is not null)
	{
	string rewritten = formatter.FormatName(resolved, true);
	if (rewritten != original)
	{
	blobBuilder.WriteSerializedString(rewritten);
	return;
	}
	}
	}
	blobBuilder.WriteBytes(originalBlob, startOffset, endOffset - startOffset);
	string? s = valueReader.ReadSerializedString();
	if (rewriteTypeName)
	{
	TypeDesc resolved = _module.GetTypeByCustomAttributeTypeName(s);
	s = formatter.FormatName(resolved, true);
	}
	blobBuilder.WriteSerializedString(s);

[Copilot]

Applied in ef95ab1. CopySerializedString now directly reads/writes serialized strings and rewrites type names without the previous byte-copy optimization path.

[MichalStrehovsky]

Suggested change

	TypeDesc underlyingType = type.UnderlyingType;
	if (underlyingType.IsPrimitive)
	{
	valueTypeCode = GetPrimitiveSerializationTypeCode(underlyingType);
	if (type.IsPrimitive || type.IsEnum)
	{
	valueTypeCode = GetPrimitiveSerializationTypeCode(type.UnderlyingType);

[Copilot]

Updated in ef95ab1. GetFixedArgumentTypeCodes now handles primitives/enums as if (type.IsPrimitive || type.IsEnum) and maps via type.UnderlyingType.