From a77ab81685acccb5795156b0637deed1b053efe3 Mon Sep 17 00:00:00 2001 From: fadimounir Date: Tue, 26 Mar 2019 14:32:50 -0700 Subject: [PATCH 1/2] Changing crossgen commands to use response files. Changing /Platform_Assemblies_Paths to a set of /r arguments Using resolved files list as assembly references to crossgen (temp hack until issue 3110 is fixed) --- .../PrepareForReadyToRunCompilation.cs | 14 -- .../RunReadyToRunCompiler.cs | 152 +++++++++++++++++- .../targets/Microsoft.NET.Publish.targets | 11 +- 3 files changed, 154 insertions(+), 23 deletions(-) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/PrepareForReadyToRunCompilation.cs b/src/Tasks/Microsoft.NET.Build.Tasks/PrepareForReadyToRunCompilation.cs index f267b090e22c..490434d4316d 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/PrepareForReadyToRunCompilation.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/PrepareForReadyToRunCompilation.cs @@ -25,8 +25,6 @@ public class PrepareForReadyToRunCompilation : TaskBase [Required] public string OutputPath { get; set; } [Required] - public string TargetFramework { get; set; } - [Required] public ITaskItem[] RuntimePacks { get; set; } [Required] public ITaskItem[] KnownFrameworkReferences { get; set; } @@ -69,9 +67,7 @@ public class PrepareForReadyToRunCompilation : TaskBase private string _packagePath; private string _crossgenPath; private string _clrjitPath; - private string _platformAssembliesPath; private string _diasymreaderPath; - private string _pathSeparatorCharacter = ";"; private Architecture _targetArchitecture; @@ -146,7 +142,6 @@ void ProcessInputFileList(ITaskItem[] inputFiles, List imageCompilati // an input to the ReadyToRunCompiler task TaskItem r2rCompilationEntry = new TaskItem(file); r2rCompilationEntry.SetMetadata("OutputR2RImage", outputR2RImage); - r2rCompilationEntry.SetMetadata("PlatformAssembliesPaths", $"{_platformAssembliesPath}{_pathSeparatorCharacter}{Path.GetDirectoryName(file.ItemSpec)}"); r2rCompilationEntry.RemoveMetadata(MetadataKeys.OriginalItemSpec); imageCompilationList.Add(r2rCompilationEntry); @@ -193,7 +188,6 @@ void ProcessInputFileList(ITaskItem[] inputFiles, List imageCompilati pdbCompilationEntry.ItemSpec = outputR2RImage; pdbCompilationEntry.SetMetadata("OutputPDBImage", outputPDBImage); pdbCompilationEntry.SetMetadata("CreatePDBCommand", createPDBCommand); - pdbCompilationEntry.SetMetadata("PlatformAssembliesPaths", $"{_platformAssembliesPath}{_pathSeparatorCharacter}{Path.GetDirectoryName(file.ItemSpec)}"); symbolsCompilationList.Add(pdbCompilationEntry); // This TaskItem corresponds to the output PDB image. It is equivalent to the input TaskItem, only the ItemSpec for it points to the new path @@ -386,8 +380,6 @@ bool GetCrossgenComponentsPaths() } else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) { - _pathSeparatorCharacter = ":"; - if (_targetArchitecture == Architecture.Arm || _targetArchitecture == Architecture.Arm64) { // We only have x64 hosted crossgen for both ARM target architectures @@ -406,8 +398,6 @@ bool GetCrossgenComponentsPaths() } else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - _pathSeparatorCharacter = ":"; - // Only x64 supported for OSX if (_targetArchitecture != Architecture.X64 || RuntimeInformation.OSArchitecture != Architecture.X64) return false; @@ -421,10 +411,6 @@ bool GetCrossgenComponentsPaths() return false; } - _platformAssembliesPath = - Path.Combine(_packagePath, "runtimes", _runtimeIdentifier, "native") + _pathSeparatorCharacter + - Path.Combine(_packagePath, "runtimes", _runtimeIdentifier, "lib", TargetFramework); - return true; } } diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs b/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs index 117f69f24b18..5459a1c5f38a 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs @@ -2,7 +2,11 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Generic; using System.IO; +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; +using System.Text; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; @@ -14,6 +18,8 @@ public class RunReadyToRunCompiler : ToolTask public ITaskItem CrossgenTool { get; set; } [Required] public ITaskItem CompilationEntry { get; set; } + [Required] + public ITaskItem[] ImplementationAssemblies { get; set; } private string _crossgenPath; private string _clrjitPath; @@ -22,7 +28,6 @@ public class RunReadyToRunCompiler : ToolTask private string _inputAssembly; private string _outputR2RImage; private string _outputPDBImage; - private string _platformAssembliesPaths; private string _createPDBCommand; private bool IsPdbCompilation => !String.IsNullOrEmpty(_createPDBCommand); @@ -48,7 +53,6 @@ protected override bool ValidateParameters() } _createPDBCommand = CompilationEntry.GetMetadata("CreatePDBCommand"); - _platformAssembliesPaths = CompilationEntry.GetMetadata("PlatformAssembliesPaths"); if (IsPdbCompilation) { @@ -80,18 +84,132 @@ protected override bool ValidateParameters() return true; } - protected override string GenerateCommandLineCommands() + #region TEMP LOGIC - SDK Should provide correct list - Delete this when fixing https://github.com/dotnet/sdk/issues/3110 + bool IsManagedAssemblyToUseAsCrossgenReference(ITaskItem file) + { + // Reference only managed assemblies that will be published to the root directory. + string relativeOutputPath = file.GetMetadata(MetadataKeys.RelativePath); + if (!String.IsNullOrEmpty(Path.GetDirectoryName(relativeOutputPath))) + { + return false; + } + + using (FileStream fs = new FileStream(file.ItemSpec, FileMode.Open, FileAccess.Read)) + { + try + { + using (var pereader = new PEReader(fs)) + { + if (!pereader.HasMetadata) + return false; + + MetadataReader mdReader = pereader.GetMetadataReader(); + if (!mdReader.IsAssembly) + return false; + + // Reference assemblies should never be given to crossgen + if (IsReferenceAssembly(mdReader)) + return false; + } + } + catch (BadImageFormatException) + { + // Not a valid assembly file + return false; + } + } + + return true; + } + + private bool IsReferenceAssembly(MetadataReader mdReader) { + foreach (var attributeHandle in mdReader.GetAssemblyDefinition().GetCustomAttributes()) + { + StringHandle attributeTypeName = default; + StringHandle attributeTypeNamespace = default; + EntityHandle attributeCtor = mdReader.GetCustomAttribute(attributeHandle).Constructor; + + if (attributeCtor.Kind == HandleKind.MemberReference) + { + EntityHandle attributeMemberParent = mdReader.GetMemberReference((MemberReferenceHandle)attributeCtor).Parent; + if (attributeMemberParent.Kind == HandleKind.TypeReference) + { + TypeReference attributeTypeRef = mdReader.GetTypeReference((TypeReferenceHandle)attributeMemberParent); + attributeTypeName = attributeTypeRef.Name; + attributeTypeNamespace = attributeTypeRef.Namespace; + } + } + else if (attributeCtor.Kind == HandleKind.MethodDefinition) + { + TypeDefinitionHandle attributeTypeDefHandle = mdReader.GetMethodDefinition((MethodDefinitionHandle)attributeCtor).GetDeclaringType(); + TypeDefinition attributeTypeDef = mdReader.GetTypeDefinition(attributeTypeDefHandle); + attributeTypeName = attributeTypeDef.Name; + attributeTypeNamespace = attributeTypeDef.Namespace; + } + + if (!attributeTypeName.IsNil && + !attributeTypeNamespace.IsNil && + mdReader.StringComparer.Equals(attributeTypeName, "ReferenceAssemblyAttribute") && + mdReader.StringComparer.Equals(attributeTypeNamespace, "System.Runtime.CompilerServices")) + { + return true; + } + } + + return false; + } + #endregion + + private string GetAssemblyReferencesCommands() + { + StringBuilder result = new StringBuilder(); + + // Add all runtime libraries to the list of references passed to crossgen + foreach (var runtimeAssembly in ImplementationAssemblies) + { + // When generating PDBs, we must not add a reference to the IL version of the R2R image for which we're trying to generate a PDB + if (IsPdbCompilation && String.Equals(Path.GetFileName(runtimeAssembly.ItemSpec), Path.GetFileName(_outputR2RImage), StringComparison.OrdinalIgnoreCase)) + continue; + + // TODO: Delete check when fixing https://github.com/dotnet/sdk/issues/3110 + if (!IsManagedAssemblyToUseAsCrossgenReference(runtimeAssembly)) + continue; + + result.AppendLine($"/r \"{runtimeAssembly}\""); + } + + return result.ToString(); + } + + protected override string GenerateResponseFileCommands() + { + StringBuilder result = new StringBuilder(); + + result.AppendLine("/nologo"); + if (IsPdbCompilation) { - return String.IsNullOrEmpty(_diasymreaderPath) ? - $"/nologo /Platform_Assemblies_Paths \"{_platformAssembliesPaths}\" {_createPDBCommand} \"{_outputR2RImage}\"" : - $"/nologo /Platform_Assemblies_Paths \"{_platformAssembliesPaths}\" /DiasymreaderPath \"{_diasymreaderPath}\" {_createPDBCommand} \"{_outputR2RImage}\""; + result.Append(GetAssemblyReferencesCommands()); + + if (!String.IsNullOrEmpty(_diasymreaderPath)) + { + result.AppendLine($"/DiasymreaderPath \"{_diasymreaderPath}\""); + } + + result.AppendLine(_createPDBCommand); + result.AppendLine($"\"{_outputR2RImage}\""); } else { - return $"/nologo /MissingDependenciesOK /JITPath \"{_clrjitPath}\" /Platform_Assemblies_Paths \"{_platformAssembliesPaths}\" /out \"{_outputR2RImage}\" \"{_inputAssembly}\""; + result.AppendLine("/MissingDependenciesOK"); + result.AppendLine($"/JITPath \"{_clrjitPath}\""); + result.Append(GetAssemblyReferencesCommands()); + result.AppendLine($"/out \"{_outputR2RImage}\""); + result.AppendLine($"\"{_inputAssembly}\""); } + + return result.ToString(); } protected override int ExecuteTool(string pathToTool, string responseFileCommands, string commandLineCommands) @@ -102,5 +220,25 @@ protected override int ExecuteTool(string pathToTool, string responseFileCommand return base.ExecuteTool(pathToTool, responseFileCommands, commandLineCommands); } + + #region TEMP - DELETE BEFORE MERGE (used for repros now) + protected override string GetResponseFileSwitch(string responseFilePath) + { + var rspExtension = Path.GetExtension(responseFilePath); + var responseFileCopy = Path.ChangeExtension(_outputR2RImage, rspExtension); + if (IsPdbCompilation) + { + responseFileCopy = Path.ChangeExtension(responseFileCopy, ".symbols" + rspExtension); + } + File.Copy(responseFilePath, responseFileCopy, true); + + var responseFileSwitch = base.GetResponseFileSwitch(responseFileCopy); + Log.LogMessage(MessageImportance.Normal, $"{ToolExe} {responseFileSwitch}"); + + return responseFileSwitch; + } + + protected override void LogToolCommand(string message) { /* Do not log the full command line because it's long */ } + #endregion } } diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets index 62122f2c181e..a555c8cd6f89 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets @@ -225,8 +225,7 @@ Copyright (c) .NET Foundation. All rights reserved. - + @@ -286,7 +287,9 @@ Copyright (c) .NET Foundation. All rights reserved. Inputs="@(_ReadyToRunSymbolsCompileListPreserveNewest)" Outputs="%(_ReadyToRunSymbolsCompileListPreserveNewest.OutputPDBImage)"> + @@ -309,7 +312,9 @@ Copyright (c) .NET Foundation. All rights reserved. Inputs="@(_ReadyToRunCompileListPublishAlways)" Outputs="%(_ReadyToRunCompileListPublishAlways.OutputR2RImage)"> + @@ -332,7 +337,9 @@ Copyright (c) .NET Foundation. All rights reserved. Inputs="@(_ReadyToRunSymbolsCompileListPublishAlways)" Outputs="%(_ReadyToRunSymbolsCompileListPublishAlways.OutputPDBImage)"> + From e60e05e673666a44b2450ace64f0965143d1c89a Mon Sep 17 00:00:00 2001 From: fadimounir Date: Tue, 16 Apr 2019 14:40:54 -0700 Subject: [PATCH 2/2] CR feedback, and removing temp debugging code --- .../RunReadyToRunCompiler.cs | 24 ++----------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs b/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs index 5459a1c5f38a..6d6430f22f2e 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/RunReadyToRunCompiler.cs @@ -84,7 +84,8 @@ protected override bool ValidateParameters() return true; } - #region TEMP LOGIC - SDK Should provide correct list - Delete this when fixing https://github.com/dotnet/sdk/issues/3110 + // TEMP LOGIC - SDK Should provide correct list - When fixing https://github.com/dotnet/sdk/issues/3110, delete + // both IsManagedAssemblyToUseAsCrossgenReference and IsReferenceAssembly bool IsManagedAssemblyToUseAsCrossgenReference(ITaskItem file) { // Reference only managed assemblies that will be published to the root directory. @@ -159,7 +160,6 @@ private bool IsReferenceAssembly(MetadataReader mdReader) return false; } - #endregion private string GetAssemblyReferencesCommands() { @@ -220,25 +220,5 @@ protected override int ExecuteTool(string pathToTool, string responseFileCommand return base.ExecuteTool(pathToTool, responseFileCommands, commandLineCommands); } - - #region TEMP - DELETE BEFORE MERGE (used for repros now) - protected override string GetResponseFileSwitch(string responseFilePath) - { - var rspExtension = Path.GetExtension(responseFilePath); - var responseFileCopy = Path.ChangeExtension(_outputR2RImage, rspExtension); - if (IsPdbCompilation) - { - responseFileCopy = Path.ChangeExtension(responseFileCopy, ".symbols" + rspExtension); - } - File.Copy(responseFilePath, responseFileCopy, true); - - var responseFileSwitch = base.GetResponseFileSwitch(responseFileCopy); - Log.LogMessage(MessageImportance.Normal, $"{ToolExe} {responseFileSwitch}"); - - return responseFileSwitch; - } - - protected override void LogToolCommand(string message) { /* Do not log the full command line because it's long */ } - #endregion } }