diff options
author | Vitek Karas <10670590+vitek-karas@users.noreply.github.com> | 2022-05-06 01:02:08 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-05-06 01:02:08 +0300 |
commit | 00e9a154efeb6369b1345bdafeebb686c0163841 (patch) | |
tree | d7b682777487d8ebefbb83fcad05c7cd6c75c9f7 | |
parent | 2e91f8916748e794570fab82f7cf549ffde66aeb (diff) |
Fix NullReferenceException when sweeping unused static interface (#2783)
* Fix NullReferenceException when sweeping unused static interface
The case here is if the static interface itself is actually used, but the method on it is not and the implementation of that method is directly referecned. In that case we should remove the iface method, but keep everything else.
This can cause (depends on order) a NRE in the sweep step since the removed iface method is still in the list of overrides for the implementation method.
* Formatting
-rw-r--r-- | src/linker/Linker.Steps/SweepStep.cs | 8 | ||||
-rw-r--r-- | test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs | 49 |
2 files changed, 44 insertions, 13 deletions
diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs index 60506e466..8e8b09cd8 100644 --- a/src/linker/Linker.Steps/SweepStep.cs +++ b/src/linker/Linker.Steps/SweepStep.cs @@ -472,7 +472,13 @@ namespace Mono.Linker.Steps void SweepOverrides (MethodDefinition method) { for (int i = 0; i < method.Overrides.Count;) { - if (Context.Resolve (method.Overrides[i]) is MethodDefinition ov && ShouldRemove (ov) && !IgnoreScope (ov.DeclaringType.Scope)) + // We can't rely on the context resolution cache anymore, since it may remember methods which are already removed + // So call the direct Resolve here and avoid the cache. + // It can still happen that the Resolve will return a removed method (this happens if the Overrides collection stores actual + // MethodDefinition and not MethodReference, in which case the Resolve is just "return this" and doesn't check the existence of the method + // in any way). In such case the Declaring type of the method is null. And since it has been removed the override pointing + // to it should be removed as well. + if (method.Overrides[i].Resolve () is MethodDefinition ov && ShouldRemove (ov) && (ov.DeclaringType == null || !IgnoreScope (ov.DeclaringType.Scope))) method.Overrides.RemoveAt (i); else i++; diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs index 9b766b42a..de7f5a537 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs @@ -20,9 +20,8 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces t = typeof (UninstantiatedClassWithImplicitlyImplementedInterface); t = typeof (UninstantiatedPublicClassWithPrivateInterface); t = typeof (ImplementsUsedStaticInterface.InterfaceMethodUnused); - t = typeof (ImplementsUnusedStaticInterface.InterfaceMethodUnused); - ImplementsUnusedStaticInterface.InterfaceMethodUsedThroughImplementation.InternalStaticInterfaceMethodUsedThroughImplementation (); + ImplementsUnusedStaticInterface.Test (); ; GenericMethodThatCallsInternalStaticInterfaceMethod <ImplementsUsedStaticInterface.InterfaceMethodUsedThroughInterface> (); // Use all public interfaces - they're marked as public only to denote them as "used" @@ -66,21 +65,53 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces } } + [Kept] internal class ImplementsUnusedStaticInterface { [Kept] - internal class InterfaceMethodUsedThroughImplementation : IStaticInterfaceUnused + // The interface methods themselves are not used, but the implementation of these methods is + internal interface IStaticInterfaceMethodUnused + { + static abstract void InterfaceUsedMethodNot (); + } + + // The interface methods themselves are not used, but the implementation of these methods is + internal interface IStaticInterfaceUnused + { + static abstract void InterfaceAndMethodNoUsed (); + } + + [Kept] + internal class InterfaceMethodUsedThroughImplementation : IStaticInterfaceMethodUnused, IStaticInterfaceUnused { [Kept] + [RemovedOverride (typeof (IStaticInterfaceMethodUnused))] + public static void InterfaceUsedMethodNot () { } + + [Kept] [RemovedOverride (typeof (IStaticInterfaceUnused))] - public static void InternalStaticInterfaceMethodUsedThroughImplementation () { } + public static void InterfaceAndMethodNoUsed () { } } [Kept] - internal class InterfaceMethodUnused : IStaticInterfaceUnused + [KeptInterface (typeof (IStaticInterfaceMethodUnused))] + internal class InterfaceMethodUnused : IStaticInterfaceMethodUnused, IStaticInterfaceUnused { - public static void InternalStaticInterfaceMethodUsedThroughImplementation () { } + public static void InterfaceUsedMethodNot () { } + + public static void InterfaceAndMethodNoUsed () { } + } + + [Kept] + public static void Test () + { + InterfaceMethodUsedThroughImplementation.InterfaceUsedMethodNot (); + InterfaceMethodUsedThroughImplementation.InterfaceAndMethodNoUsed (); + + Type t; + t = typeof (IStaticInterfaceMethodUnused); + t = typeof (InterfaceMethodUnused); } } @@ -270,12 +301,6 @@ namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces static abstract void ExplicitImplementationInternalStaticInterfaceMethod (); } - // The interface methods themselves are not used, but the implementation of these methods is - internal interface IStaticInterfaceUnused - { - static abstract void InternalStaticInterfaceMethodUsedThroughImplementation (); - } - // The interface methods themselves are used through the interface [Kept] internal interface IStaticInterfaceUsed |