From a9db69382e48b6c0c6ce9c612d295c3b3a3b0351 Mon Sep 17 00:00:00 2001 From: Michael Voorhees Date: Fri, 17 Apr 2026 14:27:21 -0400 Subject: [PATCH] Fix UnreachableBody with only ref to assembly. When an unreachable body contains a reference to another assembly, and there are no other references to that assembly, the linker would retain the reference but delete the assembly. This was happening because the SweepStep was running before CodeRewriterStep. Which meant that when SweepStep.SweepAssemblyReferences iterated all of the types references in the assembly it would visit type references to method bodies that were going to be stubbed. By moving the CodeRewriterStep ahead of the sweep step we ensure that the SweepStep is visiting methods as they will look in the end. --- .../src/linker/Linker.Steps/MarkStep.cs | 2 +- src/tools/illink/src/linker/Linker/Driver.cs | 2 +- .../UnreachableBodyTests.g.cs | 6 +++ .../Dependencies/OtherAssembly2.cs | 9 ++++ .../OnlyReferenceInUnreachableBody.cs | 42 +++++++++++++++++++ 5 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBody/Dependencies/OtherAssembly2.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBody/OnlyReferenceInUnreachableBody.cs diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 6e8b5176787542..977bcdebaf49e5 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -3167,7 +3167,7 @@ void MarkMethodCollection(IList methods, in DependencyInfo rea var methodAction = Annotations.GetAction(method); if (methodAction is MethodAction.ConvertToStub) { - // CodeRewriterStep runs after sweeping, and may request the stubbed value for any preserved method + // CodeRewriterStep may request the stubbed value for any preserved method // with the action ConvertToStub. Ensure we have precomputed any stub value that may be needed by // CodeRewriterStep. This ensures sweeping doesn't change the stub value (which can be determined by // FeatureGuardAttribute or FeatureSwitchDefinitionAttribute that might have been removed). diff --git a/src/tools/illink/src/linker/Linker/Driver.cs b/src/tools/illink/src/linker/Linker/Driver.cs index 194bd0f1e2d862..3e86f26048f863 100644 --- a/src/tools/illink/src/linker/Linker/Driver.cs +++ b/src/tools/illink/src/linker/Linker/Driver.cs @@ -1620,9 +1620,9 @@ static Pipeline GetStandardPipeline() p.AppendStep(new ValidateVirtualMethodAnnotationsStep()); p.AppendStep(new ProcessWarningsStep()); p.AppendStep(new OutputWarningSuppressions()); + p.AppendStep(new CodeRewriterStep()); p.AppendStep(new SweepStep()); p.AppendStep(new CheckSuppressionsDispatcher()); - p.AppendStep(new CodeRewriterStep()); p.AppendStep(new CleanStep()); p.AppendStep(new RegenerateGuidStep()); p.AppendStep(new OutputStep()); diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBodyTests.g.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBodyTests.g.cs index a34af627fda627..564e2a45eadc20 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBodyTests.g.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/UnreachableBodyTests.g.cs @@ -117,6 +117,12 @@ public Task NotWorthConvertingReturnTrue() return RunTest(allowMissingWarnings: true); } + [Fact] + public Task OnlyReferenceInUnreachableBody() + { + return RunTest(allowMissingWarnings: true); + } + [Fact] public Task OverrideOfAbstractAndInterfaceMethodCalledFromLocal() { diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBody/Dependencies/OtherAssembly2.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBody/Dependencies/OtherAssembly2.cs new file mode 100644 index 00000000000000..47545bab185b22 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBody/Dependencies/OtherAssembly2.cs @@ -0,0 +1,9 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Mono.Linker.Tests.Cases.UnreachableBody.Dependencies; + +public class OtherAssembly2 +{ + public static int Field; +} diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBody/OnlyReferenceInUnreachableBody.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBody/OnlyReferenceInUnreachableBody.cs new file mode 100644 index 00000000000000..8f0125083aa030 --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/UnreachableBody/OnlyReferenceInUnreachableBody.cs @@ -0,0 +1,42 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; +using Mono.Linker.Tests.Cases.UnreachableBody.Dependencies; + +namespace Mono.Linker.Tests.Cases.UnreachableBody; + +/// +/// This test was added to fix an issue with how SweepStep.SweepAssemblyReferences and CodeRewriterStep interact. +/// CodeRewriterStep must run before SweepStep to ensure that SweepStep.SweepAssemblyReferences doesn't see TypeReferences in method bodies that +/// are going to be removed by CodeRewriterStep. +/// +[SetupCompileBefore("other.dll", new[] { typeof(OtherAssembly2) })] +[RemovedAssemblyReference("test", "other")] +[RemovedAssembly("other.dll")] +[SetupLinkerArgument("--enable-opt", "unreachablebodies")] +public class OnlyReferenceInUnreachableBody +{ + public static void Main() + { + UsedToMarkMethod(null); + } + + [Kept] + static void UsedToMarkMethod(Foo f) + { + f.Method(); + } + + [Kept] + class Foo + { + [Kept] + [ExpectBodyModified] + public void Method() + { + OtherAssembly2.Field = 1; + } + } +}