From b4864a45f51222d62bcfdf83a8ddcf8567cc742d Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Tue, 7 Sep 2021 02:29:00 -0700 Subject: [PATCH 1/2] Add a test for #2260 This uncovers a null ref in sweepstep. The test is order dependent: - Assembly (librarywithnonemtpy) has a virtual method which referes to type from second library (library). This processes the type reference. - The type is removed from library - The virtual method is re-processed again -> hits a type def which does not belong to any assembly/scope For this to happen the virtual method must be processed after the type has been removed. This also requires the method to be meant for removal later on (the body will be rewritten to throw), since otherwise the type would have been marked. This also requires the library with the removed type to be kept (so something else in it must be kept). --- ...OverrideOfAbstractIsKeptNonEmptyLibrary.cs | 21 ++++++++++++++++ ...stractIsKeptNonEmptyLibraryWithNonEmpty.cs | 23 ++++++++++++++++++ .../OverrideOfAbstractIsKeptNonEmpty.cs | 24 +++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/Dependencies/OverrideOfAbstractIsKeptNonEmptyLibrary.cs create mode 100644 test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/Dependencies/OverrideOfAbstractIsKeptNonEmptyLibraryWithNonEmpty.cs diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/Dependencies/OverrideOfAbstractIsKeptNonEmptyLibrary.cs b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/Dependencies/OverrideOfAbstractIsKeptNonEmptyLibrary.cs new file mode 100644 index 000000000000..cfa09483d87b --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/Dependencies/OverrideOfAbstractIsKeptNonEmptyLibrary.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Mono.Linker.Tests.Cases.Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.Dependencies +{ + public class OverrideOfAbstractIsKeptNonEmpty_UnusedType + { + } + + public abstract class OverrideOfAbstractIsKeptNonEmpty_BaseType + { + public abstract void Method (); + } +} diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/Dependencies/OverrideOfAbstractIsKeptNonEmptyLibraryWithNonEmpty.cs b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/Dependencies/OverrideOfAbstractIsKeptNonEmptyLibraryWithNonEmpty.cs new file mode 100644 index 000000000000..e7a10c647622 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/Dependencies/OverrideOfAbstractIsKeptNonEmptyLibraryWithNonEmpty.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Mono.Linker.Tests.Cases.Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval.Dependencies +{ + public class OverrideOfAbstractIsKeptNonEmptyLibraryWithNonEmpty : + OverrideOfAbstractIsKeptNonEmpty_BaseType + { + Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType _field; + + public override void Method () + { + _field = null; + } + } +} diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs index b2a3d5f2b048..fe5c42066ed7 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs @@ -4,12 +4,27 @@ namespace Mono.Linker.Tests.Cases.Inheritance.AbstractClasses.NoKeptCtor.OverrideRemoval { [SetupLinkerArgument ("--enable-opt", "unreachablebodies")] + + // The order is important - since the bug this uncovers depends on order of processing of assemblies in the sweep step + [SetupCompileBefore ("library.dll", new[] { "Dependencies/OverrideOfAbstractIsKeptNonEmptyLibrary.cs" })] + [SetupCompileBefore ( + "librarywithnonempty.dll", + new[] { "Dependencies/OverrideOfAbstractIsKeptNonEmptyLibraryWithNonEmpty.cs" }, + new[] { "library.dll" })] + + [KeptAssembly ("library.dll")] + [KeptAssembly ("librarywithnonempty.dll")] + [RemovedTypeInAssembly("library.dll", typeof(Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType))] + public class OverrideOfAbstractIsKeptNonEmpty { public static void Main () { Base b = HelperToMarkFooAndRequireBase (); b.Method (); + + Dependencies.OverrideOfAbstractIsKeptNonEmpty_BaseType c = HelperToMarkLibraryAndRequireItsBase (); + c.Method (); } [Kept] @@ -18,6 +33,12 @@ static Foo HelperToMarkFooAndRequireBase () return null; } + [Kept] + static Dependencies.OverrideOfAbstractIsKeptNonEmptyLibraryWithNonEmpty HelperToMarkLibraryAndRequireItsBase () + { + return null; + } + [Kept] abstract class Base { @@ -41,11 +62,14 @@ abstract class Base3 : Base2 [KeptBaseType (typeof (Base3))] class Foo : Base3 { + Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType _field; + [Kept] [ExpectBodyModified] public override void Method () { Other (); + _field = null; } static void Other () From 183e3913402b0107998553fe5484c7b157ad4446 Mon Sep 17 00:00:00 2001 From: vitek-karas Date: Tue, 7 Sep 2021 03:00:07 -0700 Subject: [PATCH 2/2] Disable the test so that it doesn't break everybody --- .../OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs index fe5c42066ed7..593465da536c 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.AbstractClasses/NoKeptCtor/OverrideRemoval/OverrideOfAbstractIsKeptNonEmpty.cs @@ -14,7 +14,10 @@ namespace Mono.Linker.Tests.Cases.Inheritance.AbstractClasses.NoKeptCtor.Overrid [KeptAssembly ("library.dll")] [KeptAssembly ("librarywithnonempty.dll")] - [RemovedTypeInAssembly("library.dll", typeof(Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType))] + + // Uncomment after this bug is fixed + // https://github.com/mono/linker/issues/2260 + //[RemovedTypeInAssembly ("library.dll", typeof (Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType))] public class OverrideOfAbstractIsKeptNonEmpty { @@ -25,6 +28,10 @@ public static void Main () Dependencies.OverrideOfAbstractIsKeptNonEmpty_BaseType c = HelperToMarkLibraryAndRequireItsBase (); c.Method (); + + // Remove after this bug is fixed + // https://github.com/mono/linker/issues/2260 + new Dependencies.OverrideOfAbstractIsKeptNonEmpty_UnusedType (); } [Kept]