Skip to content

Commit

Permalink
Preserve all InternalsVisibleTo attributes in ILLink Trimmer (#98910)
Browse files Browse the repository at this point in the history
Removes previous special handling to only keep IVT attributes for
assemblies that were resolvable and marked at link time. This behaviour
could cause a noticeable difference in behaviour at runtime without emitting
any trim analysis warnings while providing no mechanism to retain the
attributes. The removal of these attributes provides only a neglible
size reduction.

Fixes #92582
  • Loading branch information
Xtansia authored Apr 15, 2024
1 parent af05e76 commit a4f1e74
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 56 deletions.
49 changes: 5 additions & 44 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ protected LinkContext Context {
protected Queue<(MethodDefinition, DependencyInfo, MessageOrigin)> _methods;
protected Dictionary<MethodDefinition, MarkScopeStack.Scope> _virtual_methods;
protected Queue<AttributeProviderPair> _assemblyLevelAttributes;
readonly List<AttributeProviderPair> _ivt_attributes;
protected Queue<(AttributeProviderPair, DependencyInfo, MarkScopeStack.Scope)> _lateMarkedAttributes;
protected List<(TypeDefinition, MarkScopeStack.Scope)> _typesWithInterfaces;
protected HashSet<AssemblyDefinition> _dynamicInterfaceCastableImplementationTypesDiscovered;
Expand Down Expand Up @@ -222,7 +221,6 @@ public MarkStep ()
_methods = new Queue<(MethodDefinition, DependencyInfo, MessageOrigin)> ();
_virtual_methods = new Dictionary<MethodDefinition, MarkScopeStack.Scope> ();
_assemblyLevelAttributes = new Queue<AttributeProviderPair> ();
_ivt_attributes = new List<AttributeProviderPair> ();
_lateMarkedAttributes = new Queue<(AttributeProviderPair, DependencyInfo, MarkScopeStack.Scope)> ();
_typesWithInterfaces = new List<(TypeDefinition, MarkScopeStack.Scope)> ();
_dynamicInterfaceCastableImplementationTypesDiscovered = new HashSet<AssemblyDefinition> ();
Expand Down Expand Up @@ -288,42 +286,6 @@ protected virtual void Complete ()
}
}

bool ProcessInternalsVisibleAttributes ()
{
bool marked_any = false;
foreach (var attr in _ivt_attributes) {

var provider = attr.Provider;
Debug.Assert (attr.Provider is ModuleDefinition or AssemblyDefinition);
var assembly = (provider is ModuleDefinition module) ? module.Assembly : provider as AssemblyDefinition;

using var assemblyScope = ScopeStack.PushLocalScope (new MessageOrigin (assembly));

if (!Annotations.IsMarked (attr.Attribute) && IsInternalsVisibleAttributeAssemblyMarked (attr.Attribute)) {
MarkCustomAttribute (attr.Attribute, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, attr.Provider));
marked_any = true;
}
}

return marked_any;

bool IsInternalsVisibleAttributeAssemblyMarked (CustomAttribute ca)
{
System.Reflection.AssemblyName an;
try {
an = new System.Reflection.AssemblyName ((string) ca.ConstructorArguments[0].Value);
} catch {
return false;
}

var assembly = Context.GetLoadedAssembly (an.Name!);
if (assembly == null)
return false;

return Annotations.IsMarked (assembly.MainModule);
}
}

static bool TypeIsDynamicInterfaceCastableImplementation (TypeDefinition type)
{
if (!type.IsInterface || !type.HasInterfaces || !type.HasCustomAttributes)
Expand Down Expand Up @@ -416,8 +378,7 @@ void Process ()
ProcessMarkedPending () ||
ProcessLazyAttributes () ||
ProcessLateMarkedAttributes () ||
MarkFullyPreservedAssemblies () ||
ProcessInternalsVisibleAttributes ()) ;
MarkFullyPreservedAssemblies ()) ;

ProcessPendingTypeChecks ();
}
Expand Down Expand Up @@ -1169,6 +1130,9 @@ protected virtual bool ShouldMarkCustomAttribute (CustomAttribute ca, ICustomAtt
case "System.Runtime.InteropServices.InterfaceTypeAttribute":
case "System.Runtime.InteropServices.GuidAttribute":
return true;
// May be implicitly used by the runtime
case "System.Runtime.CompilerServices.InternalsVisibleToAttribute":
return true;
}

TypeDefinition? type = Context.Resolve (attr_type);
Expand Down Expand Up @@ -1546,10 +1510,7 @@ bool ProcessLazyAttributes ()
if (IsAttributeRemoved (customAttribute, resolved.DeclaringType) && Annotations.GetAction (CustomAttributeSource.GetAssemblyFromCustomAttributeProvider (assemblyLevelAttribute.Provider)) == AssemblyAction.Link)
continue;

if (customAttribute.AttributeType.IsTypeOf ("System.Runtime.CompilerServices", "InternalsVisibleToAttribute") && !Annotations.IsMarked (customAttribute)) {
_ivt_attributes.Add (assemblyLevelAttribute);
continue;
} else if (!ShouldMarkTopLevelCustomAttribute (assemblyLevelAttribute, resolved)) {
if (!ShouldMarkTopLevelCustomAttribute (assemblyLevelAttribute, resolved)) {
skippedItems.Add (assemblyLevelAttribute);
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ private static IEnumerable<string> GetTestDependencies (string testCaseDir, Synt
switch (attributeName) {
case "SetupCompileBefore": {
var arrayExpression = args["#1"];
if (arrayExpression is not (ArrayCreationExpressionSyntax or ImplicitArrayCreationExpressionSyntax))
if (arrayExpression is not (ArrayCreationExpressionSyntax or ImplicitArrayCreationExpressionSyntax or CollectionExpressionSyntax))
throw new InvalidOperationException ();
foreach (var sourceFile in args["#1"].DescendantNodes ().OfType<LiteralExpressionSyntax> ())
yield return Path.Combine (testCaseDir, LinkerTestBase.GetStringFromExpression (sourceFile));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ public Task IVTUnused ()
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task IVTUnusedKeptWhenKeepingUsedAttributesOnly ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task IVTUsed ()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#if IVT
[assembly: System.Runtime.CompilerServices.InternalsVisibleTo ("missing")]
[assembly: System.Runtime.CompilerServices.InternalsVisibleTo ("test-with-key, PublicKey=00240000")]
#endif

namespace Mono.Linker.Tests.Cases.Attributes.Dependencies;

public class IVTUnusedLib;
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
using System.Runtime.CompilerServices;
using Mono.Linker.Tests.Cases.Attributes.Dependencies;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Metadata;

#if IVT
[assembly: InternalsVisibleTo ("missing")]
[assembly: InternalsVisibleTo ("test-with-key, PublicKey=00240000")]
namespace Mono.Linker.Tests.Cases.Attributes;

#endif

namespace Mono.Linker.Tests.Cases.Attributes
[SetupCompileBefore ("lib.dll", ["Dependencies/IVTUnused_Lib.cs"], defines: ["IVT"])]
[KeptAssembly ("lib.dll")]
[KeptTypeInAssembly ("lib.dll", typeof(IVTUnusedLib))]
[KeptAttributeInAssembly ("lib.dll", typeof (InternalsVisibleToAttribute))]
class IVTUnused
{
[Define ("IVT")]
class IVTUnused
static void Main ()
{
static void Main ()
{
}
_ = new IVTUnusedLib ();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using System.Runtime.CompilerServices;
using Mono.Linker.Tests.Cases.Attributes.Dependencies;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Metadata;

namespace Mono.Linker.Tests.Cases.Attributes;

[SetupCompileBefore ("lib.dll", ["Dependencies/IVTUnused_Lib.cs"], defines: ["IVT"])]
[KeptAssembly ("lib.dll")]
[KeptTypeInAssembly ("lib.dll", typeof(IVTUnusedLib))]
[KeptAttributeInAssembly ("lib.dll", typeof (InternalsVisibleToAttribute))]

[SetupLinkerArgument("--used-attrs-only", "true")]
class IVTUnusedKeptWhenKeepingUsedAttributesOnly
{
static void Main ()
{
_ = new IVTUnusedLib ();
}
}

0 comments on commit a4f1e74

Please sign in to comment.