Skip to content

Commit

Permalink
Add ability to dump ILLink traces for all tests (#102409)
Browse files Browse the repository at this point in the history
This extends the `--dump-dependencies` option to take an optional
argument that specifies an assembly name. If none is specified,
the behavior is unchanged (trace will include everything),
otherwise the trace will only include members from the mentioned
assembly.

This also adds the ability to run ILLink tests in two new modes:

- `/p:GenerateExpectedDependencyTraces=true` will dump
  dependencies for all testcases to a location alongside the test
  source files (in a Dependencies subfolder).

- `/p:ValidateDependencyTraces=true` will look for the expected
  dependency traces in the source locations, dump dependencies
  for all testcases into the test output directory, and validate
  them against the expected dependency traces.

I couldn't think of a great way to automate this validation
without checking in a bunch of XML files or introducing more
infrastructure to save the traces corresponding to a commit, so
for now the intended use is for local validation.
  • Loading branch information
sbomer authored May 22, 2024
1 parent 3608eee commit acb7155
Show file tree
Hide file tree
Showing 20 changed files with 153 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class TestCaseLinkerOptions
public bool StripDescriptors;
public bool StripSubstitutions;
public bool StripLinkAttributes;
public bool DumpDependencies;

public bool IlcFrameworkCompilation;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public virtual TestCaseLinkerOptions GetLinkerOptions (NPath inputPath)
StripDescriptors = GetOptionAttributeValue (nameof (StripDescriptorsAttribute), true),
StripSubstitutions = GetOptionAttributeValue (nameof (StripSubstitutionsAttribute), true),
StripLinkAttributes = GetOptionAttributeValue (nameof (StripLinkAttributesAttribute), true),
DumpDependencies = GetOptionAttribute (nameof (DumpDependenciesAttribute)),
IlcFrameworkCompilation = _testCaseTypeDefinition.HasAttribute (nameof (SetupIlcWholeProgramAnalysisAttribute)),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ private static T GetResultOfTaskThatMakesAssertions<T> (Task<T> task)
protected partial TrimmingCustomizations? CustomizeTrimming (TrimmingDriver linker, TestCaseMetadataProvider metadataProvider)
=> null;

protected partial void AddDumpDependenciesOptions (TestCaseLinkerOptions caseDefinedOptions, ManagedCompilationResult compilationResult, TrimmingArgumentBuilder builder, TestCaseMetadataProvider metadataProvider)
{
}

static partial void AddOutputDirectory (TestCaseSandbox sandbox, ManagedCompilationResult compilationResult, TrimmingArgumentBuilder builder)
{
builder.AddOutputDirectory (sandbox.OutputDirectory.Combine (compilationResult.InputAssemblyPath.FileNameWithoutExtension + ".obj"));
Expand Down
47 changes: 31 additions & 16 deletions src/tools/illink/src/linker/Linker/DependencyRecorderHelper.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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 System.Diagnostics;
using Mono.Cecil;


Expand Down Expand Up @@ -48,16 +49,28 @@ public static string TokenString (LinkContext context, object? o)
return "Other:" + o;
}

static bool WillAssemblyBeModified (LinkContext context, AssemblyDefinition assembly)
static bool ShouldRecordAssembly (LinkContext context, AssemblyDefinition assembly)
{
switch (context.Annotations.GetAction (assembly)) {
case AssemblyAction.Link:
case AssemblyAction.AddBypassNGen:
case AssemblyAction.AddBypassNGenUsed:
return true;
default:
return false;
Debug.Assert (context.EnableReducedTracing || context.TraceAssembly != null);

if (context.TraceAssembly != null && !context.TraceAssembly.Contains (assembly.Name.Name))
return false; // We were asked to only trace a specific set of assemblies and this is not one of them

// We were either asked to trace in general, or to trace this assembly in particular.

// Note: with reduced tracing, we may still not trace an assembly if it's not linked.
if (context.EnableReducedTracing) {
switch (context.Annotations.GetAction (assembly)) {
case AssemblyAction.Link:
case AssemblyAction.AddBypassNGen:
case AssemblyAction.AddBypassNGenUsed:
return true;
default:
return false;
}
}

return true;
}

public static bool ShouldRecord (LinkContext context, object? source, object target)
Expand Down Expand Up @@ -85,14 +98,16 @@ public static bool ShouldRecord (LinkContext context, object? source, object tar

public static bool ShouldRecord (LinkContext context, object? o)
{
if (!context.EnableReducedTracing)
// If tracing a specific set of assemblies (TraceAssembly != null),
// this takes precedence over the EnableReducedTracing setting.
if (!context.EnableReducedTracing && context.TraceAssembly == null)
return true;

if (o is TypeDefinition t)
return WillAssemblyBeModified (context, t.Module.Assembly);
return ShouldRecordAssembly (context, t.Module.Assembly);

if (o is IMemberDefinition m)
return WillAssemblyBeModified (context, m.DeclaringType.Module.Assembly);
return ShouldRecordAssembly (context, m.DeclaringType.Module.Assembly);

if (o is TypeReference typeRef) {
var resolved = context.TryResolve (typeRef);
Expand All @@ -101,7 +116,7 @@ public static bool ShouldRecord (LinkContext context, object? o)
if (resolved == null)
return true;

return WillAssemblyBeModified (context, resolved.Module.Assembly);
return ShouldRecordAssembly (context, resolved.Module.Assembly);
}

if (o is MemberReference mRef) {
Expand All @@ -111,18 +126,18 @@ public static bool ShouldRecord (LinkContext context, object? o)
if (resolved == null)
return true;

return WillAssemblyBeModified (context, resolved.DeclaringType.Module.Assembly);
return ShouldRecordAssembly (context, resolved.DeclaringType.Module.Assembly);
}

if (o is ModuleDefinition module)
return WillAssemblyBeModified (context, module.Assembly);
return ShouldRecordAssembly (context, module.Assembly);

if (o is AssemblyDefinition assembly)
return WillAssemblyBeModified (context, assembly);
return ShouldRecordAssembly (context, assembly);

if (o is ParameterDefinition parameter) {
if (parameter.Method is MethodDefinition parameterMethodDefinition)
return WillAssemblyBeModified (context, parameterMethodDefinition.DeclaringType.Module.Assembly);
return ShouldRecordAssembly (context, parameterMethodDefinition.DeclaringType.Module.Assembly);
}

return true;
Expand Down
19 changes: 16 additions & 3 deletions src/tools/illink/src/linker/Linker/Driver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,22 @@ protected int SetupContext (ILogger? customLogger = null)

continue;

case "--dump-dependencies":
dumpDependencies = true;
continue;
case "--dump-dependencies": {
dumpDependencies = true;

string? assemblyName = GetNextStringValue ();
if (assemblyName != null) {
if (!IsValidAssemblyName (assemblyName)) {
context.LogError (null, DiagnosticId.InvalidAssemblyName, assemblyName);
return -1;
}

context.TraceAssembly ??= new HashSet<string> ();
context.TraceAssembly.Add (assemblyName);
}

continue;
}

case "--dependencies-file-format":
if (!GetStringParam (token, out var dependenciesFileFormat))
Expand Down
2 changes: 2 additions & 0 deletions src/tools/illink/src/linker/Linker/LinkContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ internal TypeNameResolver TypeNameResolver {

public Tracer Tracer { get; private set; }

internal HashSet<string>? TraceAssembly { get; set; }

public EmbeddedXmlInfo EmbeddedXmlInfo { get; private set; }

public CodeOptimizationsSettings Optimizations { get; set; }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// 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 System;

namespace Mono.Linker.Tests.Cases.Expectations.Metadata
{
[AttributeUsage (AttributeTargets.Class, AllowMultiple = true)]
public class DumpDependenciesAttribute : BaseMetadataAttribute
{
public DumpDependenciesAttribute ()
{
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Metadata;
using Mono.Linker.Tests.Cases.Libraries.Dependencies;

#if RootAllLibrary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

namespace Mono.Linker.Tests.Cases.Tracing.Individual
{

[SetupLinkerArgument ("--dump-dependencies")]
[DumpDependencies]
[SetupLinkerArgument ("--dependencies-file-format", "Dgml")]
[SetupLinkerArgument ("--dependencies-file", "linker-dependencies.dgml")]
public class CanDumpDependenciesToUncompressedDgml
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

namespace Mono.Linker.Tests.Cases.Tracing.Individual
{

[SetupLinkerArgument ("--dump-dependencies")]
[DumpDependencies]
[SetupLinkerArgument ("--dependencies-file", "linker-dependencies.xml")]
public class CanDumpDependenciesToUncompressedXml
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

namespace Mono.Linker.Tests.Cases.Tracing.Individual
{

[SetupLinkerArgument ("--dump-dependencies")]
[DumpDependencies]
public class CanEnableDependenciesDump
{
public static void Main ()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

namespace Mono.Linker.Tests.Cases.Tracing.Individual
{

[SetupLinkerArgument ("--dump-dependencies")]
[DumpDependencies]
[SetupLinkerArgument ("--reduced-tracing", "true")]
// Avoid excessive output from core assemblies
[SetupLinkerTrimMode ("skip")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@
<RuntimeHostConfigurationOption Include="Mono.Linker.Tests.CecilPackageVersion">
<Value>$(MicrosoftDotNetCecilVersion)</Value>
</RuntimeHostConfigurationOption>

<RuntimeHostConfigurationOption Include="GenerateExpectedDependencyTraces" Value="true"
Condition="'$(GenerateExpectedDependencyTraces)' == 'true'" />
<RuntimeHostConfigurationOption Include="ValidateDependencyTraces" Value="true"
Condition="'$(ValidateDependencyTraces)' == 'true'" />
</ItemGroup>

<Import Project="..\Trimming.Tests.Shared\Trimming.Tests.Shared.projitems" Label="Shared" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
using Microsoft.VisualStudio.TestPlatform.ObjectModel;
using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Metadata;
using Mono.Linker.Tests.Extensions;
using Mono.Linker.Tests.TestCases;
using Mono.Linker.Tests.TestCasesRunner.ILVerification;
using NUnit.Framework;
using WellKnownType = ILLink.Shared.TypeSystemProxy.WellKnownType;
Expand Down Expand Up @@ -102,6 +104,8 @@ public virtual void Check (TrimmedTestCaseResult linkResult)
CreateAssemblyChecker (original, linked, linkResult).Verify ();
}
CreateILChecker ().Check(linkResult, original);

VerifyExpectedDependencyTrace (linkResult.MetadataProvider, linkResult.OutputAssemblyPath);
}

VerifyLinkingOfOtherAssemblies (original);
Expand Down Expand Up @@ -1051,6 +1055,22 @@ static string DependencyToString (TestDependencyRecorder.Dependency dependency)
}
}

void VerifyExpectedDependencyTrace (TestCaseMetadataProvider testCaseMetadata, NPath outputAssemblyPath)
{
if (!AppContext.TryGetSwitch ("ValidateDependencyTraces", out var validateDependencyTraces) || !validateDependencyTraces)
return;

var expectedTracePath = testCaseMetadata.GetExpectedDependencyTrace ();
Assert.IsTrue (expectedTracePath.FileExists (), $"Expected dependency trace file '{expectedTracePath}' does not exist.");

// linker-dependencies.xml in same dir as output
var tracePath = outputAssemblyPath.Parent.Combine ("linker-dependencies.xml");
Assert.IsTrue (tracePath.FileExists (), $"Dependency trace file '{tracePath}' does not exist.");

Assert.That (File.ReadAllLines (tracePath), Is.EquivalentTo (
File.ReadAllLines (expectedTracePath)));
}

void VerifyExpectedInstructionSequenceOnMemberInAssembly (CustomAttribute inAssemblyAttribute, TypeDefinition linkedType)
{
var originalType = GetOriginalTypeFromInAssemblyAttribute (inAssemblyAttribute);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class TestCaseLinkerOptions
public bool StripDescriptors;
public bool StripSubstitutions;
public bool StripLinkAttributes;
public bool DumpDependencies;

public List<KeyValuePair<string, string[]>> AdditionalArguments = new List<KeyValuePair<string, string[]>> ();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public virtual TestCaseLinkerOptions GetLinkerOptions (NPath inputPath)
StripDescriptors = GetOptionAttributeValue (nameof (StripDescriptorsAttribute), true),
StripSubstitutions = GetOptionAttributeValue (nameof (StripSubstitutionsAttribute), true),
StripLinkAttributes = GetOptionAttributeValue (nameof (StripLinkAttributesAttribute), true),
DumpDependencies = GetOptionAttribute (nameof (DumpDependenciesAttribute)),
};

foreach (var assemblyAction in _testCaseTypeDefinition.CustomAttributes.Where (attr => attr.AttributeType.Name == nameof (SetupLinkerActionAttribute))) {
Expand Down Expand Up @@ -186,5 +187,18 @@ public virtual bool LinkAll ()
return _testCaseTypeDefinition.CustomAttributes
.FirstOrDefault (attr => attr.AttributeType.Name == nameof (SetupLinkerLinkAllAttribute)) != null;
}

public virtual NPath GetExpectedDependencyTrace ()
{
var traceFileName = _testCase.SourceFile
.ChangeExtension ("linker-dependencies.xml")
.RelativeTo (_testCase.TestSuiteDirectory)
.ToString ()
.Replace (Path.DirectorySeparatorChar, '.');
var testName = _testCase.SourceFile.FileNameWithoutExtension;
return _testCase.TestSuiteDirectory
.Combine("Dependencies")
.Combine(traceFileName);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ private static T GetResultOfTaskThatMakesAssertions<T> (Task<T> task)
return customizations;
}

protected partial void AddDumpDependenciesOptions (TestCaseLinkerOptions caseDefinedOptions, ManagedCompilationResult compilationResult, TrimmingArgumentBuilder builder, TestCaseMetadataProvider metadataProvider)
{
if (!caseDefinedOptions.DumpDependencies) {
// The testcase didn't specify [DumpDependencies]
// Dump dependencies only for the test assembly.
builder.AddAdditionalArgument ("--dump-dependencies", [compilationResult.InputAssemblyPath.FileNameWithoutExtension]);

if (AppContext.TryGetSwitch ("GenerateExpectedDependencyTraces", out var generateExpectedDependencyTraces) && generateExpectedDependencyTraces) {
// If running with GenerateExpectedDependencyTrace=true, generate the traces directly into the expected src directory,
// (only for tests which did not especify [DumpDependencies] explicitly).
var expectedTracePath = metadataProvider.GetExpectedDependencyTrace ();
expectedTracePath.Parent.EnsureDirectoryExists ();
builder.AddAdditionalArgument ("--dependencies-file", [expectedTracePath]);
}
}
}

static partial void AddOutputDirectory (TestCaseSandbox sandbox, ManagedCompilationResult compilationResult, TrimmingArgumentBuilder builder)
{
builder.AddOutputDirectory (sandbox.OutputDirectory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ public virtual void AddStripLinkAttributes (bool stripLinkAttributes)
}
}

public virtual void AddDumpDependencies ()
{
Append ("--dump-dependencies");
}

public virtual void AddSubstitutions (string file)
{
Append ("--substitutions");
Expand Down Expand Up @@ -225,6 +230,10 @@ public virtual void ProcessOptions (TestCaseLinkerOptions options)

AddStripDescriptors (options.StripDescriptors);

// The testcase specified [DumpDependencies] so just do that.
if (options.DumpDependencies)
AddDumpDependencies ();

AddStripSubstitutions (options.StripSubstitutions);

AddStripLinkAttributes (options.StripLinkAttributes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ protected BaseMetadataProvider (TestCase testCase, AssemblyDefinition fullTestCa
return defaultValue;
}

protected bool GetOptionAttribute (string attributeName)
{
var attribute = _testCaseTypeDefinition.CustomAttributes.FirstOrDefault (attr => attr.AttributeType.Name == attributeName);
return attribute != null;
}

protected NPath MakeSourceTreeFilePathAbsolute (string value)
{
return _testCase.SourceFile.Parent.Combine (value);
Expand Down
4 changes: 4 additions & 0 deletions src/tools/illink/test/Trimming.Tests.Shared/TestRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,15 @@ protected virtual void AddTrimmingOptions (TestCaseSandbox sandbox, ManagedCompi

builder.ProcessOptions (caseDefinedOptions);

AddDumpDependenciesOptions (caseDefinedOptions, compilationResult, builder, metadataProvider);

builder.ProcessTestInputAssembly (compilationResult.InputAssemblyPath);
}

protected partial TrimmingCustomizations? CustomizeTrimming (TrimmingDriver linker, TestCaseMetadataProvider metadataProvider);

protected partial void AddDumpDependenciesOptions (TestCaseLinkerOptions caseDefinedOptions, ManagedCompilationResult compilationResult, TrimmingArgumentBuilder builder, TestCaseMetadataProvider metadataProvider);

static partial void AddOutputDirectory (TestCaseSandbox sandbox, ManagedCompilationResult compilationResult, TrimmingArgumentBuilder builder);

static partial void AddInputReference (NPath inputReference, TrimmingArgumentBuilder builder);
Expand Down

0 comments on commit acb7155

Please sign in to comment.