Fix NullableAttribute illink test failures

[sbomer]

When we started building with preview 7 in
5549f72, NullableAttribute in these testcases started to use the attribute definition from the framework, instead of generating it into the code. This broke the --used-attrs-only optimization because skip assemblies (the default for the framework in these testcases) are treated as if all types in them are kept, for the purposes of the --used-attrs-only optimization. (The optimization removes attribute instances unless the attribute type is preserved for some other reason).

It's not clear what the intended behavior of --used-attrs-only is for skip assemblies, and the discussion in
dotnet/linker#952 indicates that it's considered experimental, so this fixes the failures by using the link action. This represents a more realistic scenario since skip is mainly used in testing to avoid linking the framework in every testcase.

An alternative fix is to change

runtime/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs

Lines 842 to 843 in 05dfb7e

	bool providerInLinkedAssembly = Annotations.GetAction (CustomAttributeSource.GetAssemblyFromCustomAttributeProvider (provider)) == AssemblyAction.Link;
	bool markOnUse = Context.KeepUsedAttributeTypesOnly && providerInLinkedAssembly;

to also set markOnUse for skip assemblies, but I think such a change would require more discussion of what skip is supposed to mean.

Fixes #90431

[ghost]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

When we started building with preview 7 in
5549f72, NullableAttribute in these testcases started to use the attribute definition from the framework, instead of generating it into the code. This broke the --used-attrs-only optimization because skip assemblies (the default for the framework in these testcases) are treated as if all types in them are kept, for the purposes of the --used-attrs-only optimization. (The optimization removes attribute instances unless the attribute type is preserved for some other reason).

It's not clear what the intended behavior of --used-attrs-only is for skip assemblies, and the discussion in
dotnet/linker#952 indicates that it's considered experimental, so this fixes the failures by using the link action. This represents a more realistic scenario since skip is mainly used in testing to avoid linking the framework in every testcase.

An alternative fix is to change

runtime/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs

Lines 842 to 843 in 05dfb7e

	bool providerInLinkedAssembly = Annotations.GetAction (CustomAttributeSource.GetAssemblyFromCustomAttributeProvider (provider)) == AssemblyAction.Link;
	bool markOnUse = Context.KeepUsedAttributeTypesOnly && providerInLinkedAssembly;

to also set markOnUse for skip assemblies, but I think such a change would require more discussion of what skip is supposed to mean.

Fixes #90431

Author:	sbomer
Assignees:	-
Labels:	area-Tools-ILLink
Milestone:	-

[vitek-karas]

I agree with basically everything in the PR description - so the change looks good. Thanks!

[carlossanlop]

This is hitting the RC1 branch. I'll backport it.

[carlossanlop]

/backport to release/8.0-rc1

[github-actions]

Started backporting to release/8.0-rc1: https://github.com/dotnet/runtime/actions/runs/5882378435

[carlossanlop]

I'm not entirely sure if it's the exact same failure, but it looks very similar. @sbomer can you confirm it's the same we're seeing here? #90665

[sbomer]

Yup, this is the same failure. Backporting should fix it.