Skip to content

Commit

Permalink
Fix generic dataflow for generic virtual methods (#80009)
Browse files Browse the repository at this point in the history
Generic virtual method analysis happens on top of canonical forms, so we would miss the fact that `IFoo.SomeMethod<SomeType>` was called and only see `IFoo.SomeMethod<__Canon>`.
  • Loading branch information
MichalStrehovsky authored Dec 28, 2022
1 parent 4131a83 commit 8c58fc2
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
{
dependencies ??= new DependencyList();
dependencies.Add(factory.GVMDependencies(_targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), "GVM dependencies for runtime method handle");

// GVM analysis happens on canonical forms, but this is potentially injecting new genericness
// into the system. Ensure reflection analysis can still see this.
if (_targetMethod.IsAbstract)
factory.MetadataManager.GetDependenciesDueToMethodCodePresence(ref dependencies, factory, _targetMethod, methodIL: null);
}

factory.MetadataManager.GetDependenciesDueToLdToken(ref dependencies, factory, _targetMethod);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,9 +567,9 @@ protected override void GetDependenciesDueToMethodCodePresenceInternal(ref Depen

Debug.Assert(methodIL != null || method.IsAbstract || method.IsPInvoke || method.IsInternalCall);

if (methodIL != null && scanReflection)
if (scanReflection)
{
if (FlowAnnotations.RequiresDataflowAnalysis(method))
if (methodIL != null && FlowAnnotations.RequiresDataflowAnalysis(method))
{
AddDataflowDependency(ref dependencies, factory, methodIL, "Method has annotated parameters");
}
Expand Down
26 changes: 26 additions & 0 deletions src/tests/nativeaot/SmokeTests/TrimmingBehaviors/Dataflow.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,13 @@ public static void AlsoKeptMethod() { }
private static void RemovedMethod() { }
}

class Type4WithPublicKept
{
public static void KeptMethod() { }
public static void AlsoKeptMethod() { }
private static void RemovedMethod() { }
}

struct Struct1WithPublicKept
{
public static void KeptMethod() { }
Expand Down Expand Up @@ -276,6 +283,21 @@ public static void Keep<U>()
}
}

static IKeepPublicThroughGvm s_keepPublicThroughGvm = new KeepPublicThroughGvm();

interface IKeepPublicThroughGvm
{
void Keep<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>();
}

class KeepPublicThroughGvm : IKeepPublicThroughGvm
{
public void Keep<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] T>()
{
Assert.NotNull(typeof(T).GetMethod("KeptMethod", BindingFlags.Public | BindingFlags.Static));
}
}

public static void Run()
{
new KeepsNonPublic();
Expand All @@ -293,6 +315,10 @@ public static void Run()
KeepsPublic<Type3WithPublicKept>.Keep<object>();
Assert.Equal(2, typeof(Type3WithPublicKept).CountMethods());
Assert.Equal(2, typeof(Type3WithPublicKept).CountPublicMethods());

s_keepPublicThroughGvm.Keep<Type4WithPublicKept>();
Assert.Equal(2, typeof(Type4WithPublicKept).CountMethods());
Assert.Equal(2, typeof(Type4WithPublicKept).CountPublicMethods());
}
}

Expand Down

0 comments on commit 8c58fc2

Please sign in to comment.