From 9219ef3debdb97ad8507c24c7c7c9bf2660ecaba Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 11 Jun 2024 16:50:27 +0000 Subject: [PATCH 1/5] Preserve symbol paths when DeterministicSourcePaths is true --- eng/illink.targets | 1 + src/tools/illink/src/ILLink.Tasks/LinkTask.cs | 10 ++ .../build/Microsoft.NET.ILLink.targets | 6 + .../src/linker/Linker.Steps/OutputStep.cs | 3 +- .../src/linker/Linker/CustomSymbolWriter.cs | 117 ++++++++++++++++++ src/tools/illink/src/linker/Linker/Driver.cs | 19 ++- .../illink/src/linker/Linker/LinkContext.cs | 2 + 7 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs diff --git a/eng/illink.targets b/eng/illink.targets index 962e6f49ceb786..e6bdc8f8935910 100644 --- a/eng/illink.targets +++ b/eng/illink.targets @@ -208,6 +208,7 @@ $(ILLinkArgs) --action link $(TargetName) $(ILLinkArgs) -b true + $(ILLinkArgs) --preserve-symbol-paths $(ILLinkArgs) -x "$(ILLinkDescriptorsLibraryBuildXml)" $(ILLinkArgs) --substitutions "$(ILLinkSubstitutionsLibraryBuildXml)" diff --git a/src/tools/illink/src/ILLink.Tasks/LinkTask.cs b/src/tools/illink/src/ILLink.Tasks/LinkTask.cs index 9fd5c536603ee0..ce24b35de9e485 100644 --- a/src/tools/illink/src/ILLink.Tasks/LinkTask.cs +++ b/src/tools/illink/src/ILLink.Tasks/LinkTask.cs @@ -194,6 +194,13 @@ public class ILLink : ToolTask public bool RemoveSymbols { set => _removeSymbols = value; } bool? _removeSymbols; + /// + /// Preserve original path to debug symbols from each assembly's debug header. + /// Maps to '--preserve-symbol-paths' if true. + /// Default if not specified is to write out the full path to the pdb in the debug header. + /// + public bool PreserveSymbolPaths { get; set; } + /// /// Sets the default action for trimmable assemblies. /// Maps to '--trim-mode' @@ -474,6 +481,9 @@ protected override string GenerateResponseFileCommands () if (_removeSymbols == false) args.AppendLine ("-b"); + if (PreserveSymbolPaths) + args.AppendLine ("--preserve-symbol-paths"); + if (CustomSteps != null) { foreach (var customStep in CustomSteps) { args.Append ("--custom-step "); diff --git a/src/tools/illink/src/ILLink.Tasks/build/Microsoft.NET.ILLink.targets b/src/tools/illink/src/ILLink.Tasks/build/Microsoft.NET.ILLink.targets index d6d45f475fc33b..a59527573202ea 100644 --- a/src/tools/illink/src/ILLink.Tasks/build/Microsoft.NET.ILLink.targets +++ b/src/tools/illink/src/ILLink.Tasks/build/Microsoft.NET.ILLink.targets @@ -144,6 +144,7 @@ Copyright (c) .NET Foundation. All rights reserved. TrimMode="$(TrimMode)" DefaultAction="$(_TrimmerDefaultAction)" RemoveSymbols="$(TrimmerRemoveSymbols)" + PreserveSymbolPaths="$(_TrimmerPreserveSymbolPaths)" FeatureSettings="@(_TrimmerFeatureSettings)" CustomData="@(_TrimmerCustomData)" @@ -237,6 +238,11 @@ Copyright (c) .NET Foundation. All rights reserved. false + + <_TrimmerPreserveSymbolPaths Condition=" '$(_TrimmerPreserveSymbolPaths)' == '' and '$(DeterministicSourcePaths)' == 'true' ">true + <_TrimmerPreserveSymbolPaths Condition=" '$(_TrimmerPreserveSymbolPaths)' == '' ">false + + diff --git a/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs b/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs index 2fa3c03f2b396b..97ab92d1559824 100644 --- a/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs @@ -175,7 +175,8 @@ void CloseSymbols (AssemblyDefinition assembly) WriterParameters SaveSymbols (AssemblyDefinition assembly) { var parameters = new WriterParameters { - DeterministicMvid = Context.DeterministicOutput + DeterministicMvid = Context.DeterministicOutput, + SymbolWriterProvider = new CustomSymbolWriterProvider (Context.PreserveSymbolPaths) }; if (!Context.LinkSymbols) diff --git a/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs b/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs new file mode 100644 index 00000000000000..52c85045a71020 --- /dev/null +++ b/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs @@ -0,0 +1,117 @@ +// 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.IO; +using System.Text; + +using Mono.Cecil; +using Mono.Cecil.Cil; + +namespace Mono.Linker +{ + public sealed class CustomSymbolWriterProvider : ISymbolWriterProvider + { + readonly DefaultSymbolWriterProvider _defaultProvider = new DefaultSymbolWriterProvider (); + readonly bool _preserveSymbolPaths; + + public CustomSymbolWriterProvider (bool preserveSymbolPaths) => this._preserveSymbolPaths = preserveSymbolPaths; + + public ISymbolWriter GetSymbolWriter (ModuleDefinition module, string fileName) + => new CustomSymbolWriter (_defaultProvider.GetSymbolWriter (module, fileName), module, _preserveSymbolPaths); + + public ISymbolWriter GetSymbolWriter (ModuleDefinition module, Stream symbolStream) + => new CustomSymbolWriter (_defaultProvider.GetSymbolWriter (module, symbolStream), module, _preserveSymbolPaths); + } + + public sealed class CustomSymbolWriter : ISymbolWriter + { + readonly ISymbolWriter _symbolWriter; + readonly ModuleDefinition _module; + readonly bool _preserveSymbolPaths; + + internal CustomSymbolWriter (ISymbolWriter defaultWriter, ModuleDefinition module, bool preserveSymbolPaths) + { + _symbolWriter = defaultWriter; + _module = module; + _preserveSymbolPaths = preserveSymbolPaths; + } + + public ImageDebugHeader GetDebugHeader () + { + var header = _symbolWriter.GetDebugHeader (); + if (!_preserveSymbolPaths) + return header; + + if (!header.HasEntries) + return header; + + for (int i = 0; i < header.Entries.Length; i++) { + header.Entries [i] = ProcessEntry (header.Entries [i]); + } + + return header; + } + + ImageDebugHeaderEntry ProcessEntry (ImageDebugHeaderEntry entry) + { + if (entry.Directory.Type != ImageDebugType.CodeView) + return entry; + + var reader = new BinaryReader (new MemoryStream (entry.Data)); + var newDataStream = new MemoryStream (); + var writer = new BinaryWriter (newDataStream); + + var sig = reader.ReadUInt32 (); + if (sig != 0x53445352) + return entry; + + writer.Write (sig); + writer.Write (reader.ReadBytes (16)); // MVID + writer.Write (reader.ReadUInt32 ()); // Age + + writer.Write (Encoding.UTF8.GetBytes (GetOriginalPdbPath ())); + writer.Write ((byte) 0); + + var newData = newDataStream.ToArray (); + + var directory = entry.Directory; + directory.SizeOfData = newData.Length; + + return new ImageDebugHeaderEntry (directory, newData); + } + + string GetOriginalPdbPath () + { + if (!_module.HasDebugHeader) + return string.Empty; + + var debugHeader = _module.GetDebugHeader (); + foreach (var entry in debugHeader.Entries) { + if (entry.Directory.Type != ImageDebugType.CodeView) + continue; + + var reader = new BinaryReader (new MemoryStream (entry.Data)); + var sig = reader.ReadUInt32 (); + if (sig != 0x53445352) + return string.Empty; + + var stream = reader.BaseStream; + stream.Seek (16 + 4, SeekOrigin.Current); // MVID and Age + // Pdb path is NUL-terminated path at offset 24. + // https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#codeview-debug-directory-entry-type-2 + return Encoding.UTF8.GetString ( + reader.ReadBytes ((int) (stream.Length - stream.Position - 1))); // remaining length - ending \0 + } + + return string.Empty; + } + + public ISymbolReaderProvider GetReaderProvider () => _symbolWriter.GetReaderProvider (); + + public void Write (MethodDebugInformation info) => _symbolWriter.Write (info); + + public void Write () => _symbolWriter.Write (); + + public void Dispose () => _symbolWriter.Dispose (); + } +} diff --git a/src/tools/illink/src/linker/Linker/Driver.cs b/src/tools/illink/src/linker/Linker/Driver.cs index 149abe748e45e6..5b0c6971a06e9a 100644 --- a/src/tools/illink/src/linker/Linker/Driver.cs +++ b/src/tools/illink/src/linker/Linker/Driver.cs @@ -618,6 +618,12 @@ protected int SetupContext (ILogger? customLogger = null) continue; } + case "--preserve-symbol-paths": + if (!GetBoolParam (token, l => context.PreserveSymbolPaths = l)) + return -1; + + continue; + case "--version": Version (); return 1; @@ -1333,12 +1339,13 @@ static void Usage () Console.WriteLine (); Console.WriteLine ("Options"); - Console.WriteLine (" -d PATH Specify additional directory to search in for assembly references"); - Console.WriteLine (" -reference FILE Specify additional file location used to resolve assembly references"); - Console.WriteLine (" -b Update debug symbols for all modified files. Defaults to false"); - Console.WriteLine (" -out PATH Specify the output directory. Defaults to 'output'"); - Console.WriteLine (" -h Lists all {0} options", _linker); - Console.WriteLine (" @FILE Read response file for more options"); + Console.WriteLine (" -d PATH Specify additional directory to search in for assembly references"); + Console.WriteLine (" -reference FILE Specify additional file location used to resolve assembly references"); + Console.WriteLine (" -b Update debug symbols for all modified files. Defaults to false"); + Console.WriteLine (" --preserve-symbol-paths Preserve debug header paths to pdb files. Defaults to false"); + Console.WriteLine (" -out PATH Specify the output directory. Defaults to 'output'"); + Console.WriteLine (" -h Lists all {0} options", _linker); + Console.WriteLine (" @FILE Read response file for more options"); Console.WriteLine (); Console.WriteLine ("Actions"); diff --git a/src/tools/illink/src/linker/Linker/LinkContext.cs b/src/tools/illink/src/linker/Linker/LinkContext.cs index 9e6ec519d3054e..ce8dd2b0936616 100644 --- a/src/tools/illink/src/linker/Linker/LinkContext.cs +++ b/src/tools/illink/src/linker/Linker/LinkContext.cs @@ -108,6 +108,8 @@ public Pipeline Pipeline { public bool LinkSymbols { get; set; } + public bool PreserveSymbolPaths { get; set; } + public bool KeepComInterfaces { get; set; } public bool KeepMembersForDebugger { get; set; } = true; From ddcba54dfe2b1fd65e3d7f44512e5a186f7be203 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 11 Jun 2024 17:15:45 +0000 Subject: [PATCH 2/5] Fix ApiCompat --- src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs | 4 ++-- src/tools/illink/src/linker/Linker/LinkContext.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs b/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs index 52c85045a71020..5ff0f5d4c625c2 100644 --- a/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs +++ b/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs @@ -9,7 +9,7 @@ namespace Mono.Linker { - public sealed class CustomSymbolWriterProvider : ISymbolWriterProvider + internal sealed class CustomSymbolWriterProvider : ISymbolWriterProvider { readonly DefaultSymbolWriterProvider _defaultProvider = new DefaultSymbolWriterProvider (); readonly bool _preserveSymbolPaths; @@ -23,7 +23,7 @@ public ISymbolWriter GetSymbolWriter (ModuleDefinition module, Stream symbolStre => new CustomSymbolWriter (_defaultProvider.GetSymbolWriter (module, symbolStream), module, _preserveSymbolPaths); } - public sealed class CustomSymbolWriter : ISymbolWriter + internal sealed class CustomSymbolWriter : ISymbolWriter { readonly ISymbolWriter _symbolWriter; readonly ModuleDefinition _module; diff --git a/src/tools/illink/src/linker/Linker/LinkContext.cs b/src/tools/illink/src/linker/Linker/LinkContext.cs index ce8dd2b0936616..41ada974422019 100644 --- a/src/tools/illink/src/linker/Linker/LinkContext.cs +++ b/src/tools/illink/src/linker/Linker/LinkContext.cs @@ -108,7 +108,7 @@ public Pipeline Pipeline { public bool LinkSymbols { get; set; } - public bool PreserveSymbolPaths { get; set; } + internal bool PreserveSymbolPaths { get; set; } public bool KeepComInterfaces { get; set; } From 4d2f66937f1e43e540c39e0a8d0b101e79101823 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 11 Jun 2024 18:25:05 +0000 Subject: [PATCH 3/5] Only set SymbolWriterProvider if writing symbols Otherwise writing a module will try to use the symbol writer provider, but the underlying DefaultSymbolWriterProvider throws when the module has no symbol reader (this is the case when there were no symbols corresponding to the module). --- src/tools/illink/src/linker/Linker.Steps/OutputStep.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs b/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs index 97ab92d1559824..ccd5f42a774aac 100644 --- a/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/OutputStep.cs @@ -175,8 +175,7 @@ void CloseSymbols (AssemblyDefinition assembly) WriterParameters SaveSymbols (AssemblyDefinition assembly) { var parameters = new WriterParameters { - DeterministicMvid = Context.DeterministicOutput, - SymbolWriterProvider = new CustomSymbolWriterProvider (Context.PreserveSymbolPaths) + DeterministicMvid = Context.DeterministicOutput }; if (!Context.LinkSymbols) @@ -190,6 +189,7 @@ WriterParameters SaveSymbols (AssemblyDefinition assembly) return parameters; parameters.WriteSymbols = true; + parameters.SymbolWriterProvider = new CustomSymbolWriterProvider (Context.PreserveSymbolPaths); return parameters; } From 13b9eb7f0efde54351e37759c1f0d65ad5b8d092 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 11 Jun 2024 20:07:47 +0000 Subject: [PATCH 4/5] Add ILLink.Tasks tests --- .../src/linker/CompatibilitySuppressions.xml | 14 ++++++++++++- .../illink/src/linker/Linker/LinkContext.cs | 2 +- .../ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs | 21 +++++++++++++++++++ .../illink/test/ILLink.Tasks.Tests/Mock.cs | 1 + 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/tools/illink/src/linker/CompatibilitySuppressions.xml b/src/tools/illink/src/linker/CompatibilitySuppressions.xml index 08ab3bdda16593..af38ca258f95ba 100644 --- a/src/tools/illink/src/linker/CompatibilitySuppressions.xml +++ b/src/tools/illink/src/linker/CompatibilitySuppressions.xml @@ -1,4 +1,4 @@ - + @@ -1503,12 +1503,24 @@ ref/net9.0/illink.dll lib/net9.0/illink.dll + + CP0002 + M:Mono.Linker.LinkContext.get_PreserveSymbolPaths + ref/net9.0/illink.dll + lib/net9.0/illink.dll + CP0002 M:Mono.Linker.LinkContext.set_KeepComInterfaces(System.Boolean) ref/net9.0/illink.dll lib/net9.0/illink.dll + + CP0002 + M:Mono.Linker.LinkContext.set_PreserveSymbolPaths(System.Boolean) + ref/net9.0/illink.dll + lib/net9.0/illink.dll + CP0008 T:Mono.Linker.LinkContext diff --git a/src/tools/illink/src/linker/Linker/LinkContext.cs b/src/tools/illink/src/linker/Linker/LinkContext.cs index 41ada974422019..ce8dd2b0936616 100644 --- a/src/tools/illink/src/linker/Linker/LinkContext.cs +++ b/src/tools/illink/src/linker/Linker/LinkContext.cs @@ -108,7 +108,7 @@ public Pipeline Pipeline { public bool LinkSymbols { get; set; } - internal bool PreserveSymbolPaths { get; set; } + public bool PreserveSymbolPaths { get; set; } public bool KeepComInterfaces { get; set; } diff --git a/src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs b/src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs index ef05dde8632895..14db921f5ae85e 100644 --- a/src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs +++ b/src/tools/illink/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs @@ -588,6 +588,27 @@ public void TestRemoveSymbolsDefault () } } + [Theory] + [InlineData (true)] + [InlineData (false)] + public void TestPreserveSymbolPaths (bool preserveSymbolPaths) + { + var task = new MockTask () { + PreserveSymbolPaths = preserveSymbolPaths + }; + using (var driver = task.CreateDriver ()) { + Assert.Equal (preserveSymbolPaths, driver.Context.PreserveSymbolPaths); + } + } + + [Fact] + public void TestPreserveSymbolPathsDefault () + { + var task = new MockTask (); + using (var driver = task.CreateDriver ()) { + Assert.False (driver.Context.PreserveSymbolPaths); + } + } [Fact] public void TestKeepCustomMetadata () diff --git a/src/tools/illink/test/ILLink.Tasks.Tests/Mock.cs b/src/tools/illink/test/ILLink.Tasks.Tests/Mock.cs index 979d62b1448ac4..9a640c05b9f4ca 100644 --- a/src/tools/illink/test/ILLink.Tasks.Tests/Mock.cs +++ b/src/tools/illink/test/ILLink.Tasks.Tests/Mock.cs @@ -51,6 +51,7 @@ public void SetOptimization (string optimization, bool enabled) static readonly string[] nonOptimizationBooleanProperties = new string[] { "DumpDependencies", "RemoveSymbols", + "PreserveSymbolPaths", "TreatWarningsAsErrors", "SingleWarn" }; From a208510bd4d2535e724dc50176caf185176c1979 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Fri, 14 Jun 2024 20:39:15 +0000 Subject: [PATCH 5/5] Use const for codeview signature --- src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs b/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs index 5ff0f5d4c625c2..2380ea294a2807 100644 --- a/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs +++ b/src/tools/illink/src/linker/Linker/CustomSymbolWriter.cs @@ -25,6 +25,9 @@ public ISymbolWriter GetSymbolWriter (ModuleDefinition module, Stream symbolStre internal sealed class CustomSymbolWriter : ISymbolWriter { + // ASCII "RSDS": https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#codeview-debug-directory-entry-type-2 + const int CodeViewSignature = 0x53445352; + readonly ISymbolWriter _symbolWriter; readonly ModuleDefinition _module; readonly bool _preserveSymbolPaths; @@ -62,7 +65,7 @@ ImageDebugHeaderEntry ProcessEntry (ImageDebugHeaderEntry entry) var writer = new BinaryWriter (newDataStream); var sig = reader.ReadUInt32 (); - if (sig != 0x53445352) + if (sig != CodeViewSignature) return entry; writer.Write (sig); @@ -92,7 +95,7 @@ string GetOriginalPdbPath () var reader = new BinaryReader (new MemoryStream (entry.Data)); var sig = reader.ReadUInt32 (); - if (sig != 0x53445352) + if (sig != CodeViewSignature) return string.Empty; var stream = reader.BaseStream;