Skip to content

Commit

Permalink
Addressing Simon's PR feedback, msbuild project fixes for x86
Browse files Browse the repository at this point in the history
Based on Simon's suggestion I moved the PDB / PerfMap-specific code
to a new file SymbolFileBuilder as I concur that its squatting in
the MapFileBuilder was somewhat hacky. Both classes use a new helper
class ObjectInfoBuilder that collects the information common to
both - I was reluctant to make it a base class of the *FileBuilder's
as we'd have to collect all the elements twice.

Thanks

Tomas
  • Loading branch information
trylek committed Jan 15, 2021
1 parent 20f7ba0 commit ff15684
Show file tree
Hide file tree
Showing 10 changed files with 374 additions and 267 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<OutputType>Library</OutputType>
<AssemblyName>ILCompiler.Diagnostics</AssemblyName>
<OutputType>Library</OutputType>
<IsDotNetFrameworkProductAssembly>true</IsDotNetFrameworkProductAssembly>
<TargetFramework>$(NetCoreAppToolCurrent)</TargetFramework>
<DefineConstants>READYTORUN;$(DefineConstants)</DefineConstants>
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
<Platforms>x64;x86</Platforms>
<Platforms>x64;x86;arm;arm64</Platforms>
<PlatformTarget>AnyCPU</PlatformTarget>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,21 @@ internal class ReadyToRunObjectWriter
/// </summary>
private readonly IEnumerable<DependencyNode> _nodes;

/// <summary>
/// Set to non-null when the executable generator should output a map or symbol file.
/// </summary>
private readonly ObjectInfoBuilder _objectInfoBuilder;

/// <summary>
/// Set to non-null when the executable generator should output a map file.
/// </summary>
private readonly MapFileBuilder _mapFileBuilder;

/// <summary>
/// Set to non-null when generating symbol info (PDB / PerfMap).
/// </summary>
private readonly SymbolFileBuilder _symbolFileBuilder;

/// <summary>
/// True when the map file builder should emit a textual map file
/// </summary>
Expand Down Expand Up @@ -133,10 +143,23 @@ public ReadyToRunObjectWriter(
_pdbPath = pdbPath;
_generatePerfMapFile = generatePerfMapFile;
_perfMapPath = perfMapPath;

if (generateMapFile || generateMapCsvFile || generatePdbFile || generatePerfMapFile)

bool generateMap = (generateMapFile || generateMapCsvFile);
bool generateSymbols = (generatePdbFile || generatePerfMapFile);

if (generateMap || generateSymbols)
{
_mapFileBuilder = new MapFileBuilder();
_objectInfoBuilder = new ObjectInfoBuilder();

if (generateMap)
{
_mapFileBuilder = new MapFileBuilder(_objectInfoBuilder);
}

if (generateSymbols)
{
_symbolFileBuilder = new SymbolFileBuilder(_objectInfoBuilder);
}
}
}

Expand Down Expand Up @@ -244,12 +267,12 @@ public void EmitPortableExecutable()
}
}

EmitObjectData(r2rPeBuilder, nodeContents, nodeIndex, name, node.Section, _mapFileBuilder);
EmitObjectData(r2rPeBuilder, nodeContents, nodeIndex, name, node.Section);
lastWrittenObjectNode = node;

if ((_generatePdbFile || _generatePerfMapFile) && node is MethodWithGCInfo methodNode)
{
_mapFileBuilder.AddMethod(methodNode, nodeContents.DefinedSymbols[0]);
_objectInfoBuilder.AddMethod(methodNode, nodeContents.DefinedSymbols[0]);
}
}

Expand Down Expand Up @@ -289,9 +312,9 @@ public void EmitPortableExecutable()
}
}

if (_mapFileBuilder != null)
if (_objectInfoBuilder != null)
{
r2rPeBuilder.AddSections(_mapFileBuilder);
r2rPeBuilder.AddSections(_objectInfoBuilder);

if (_generateMapFile)
{
Expand All @@ -313,7 +336,7 @@ public void EmitPortableExecutable()
{
path = Path.GetDirectoryName(_objectFilePath);
}
_mapFileBuilder.SavePdb(path, _objectFilePath);
_symbolFileBuilder.SavePdb(path, _objectFilePath);
}

if (_generatePerfMapFile)
Expand All @@ -323,7 +346,7 @@ public void EmitPortableExecutable()
{
path = Path.GetDirectoryName(_objectFilePath);
}
_mapFileBuilder.SavePerfMap(path, _objectFilePath);
_symbolFileBuilder.SavePerfMap(path, _objectFilePath);
}
}

Expand Down Expand Up @@ -361,8 +384,7 @@ public void EmitPortableExecutable()
/// <param name="nodeIndex">Logical index of the emitted node for diagnostic purposes</param>
/// <param name="name">Textual representation of the ObjecData blob in the map file</param>
/// <param name="section">Section to emit the blob into</param>
/// <param name="mapFile">Map file output stream</param>
private void EmitObjectData(R2RPEBuilder r2rPeBuilder, ObjectData data, int nodeIndex, string name, ObjectNodeSection section, MapFileBuilder mapFileBuilder)
private void EmitObjectData(R2RPEBuilder r2rPeBuilder, ObjectData data, int nodeIndex, string name, ObjectNodeSection section)
{
#if DEBUG
for (int symbolIndex = 0; symbolIndex < data.DefinedSymbols.Length; symbolIndex++)
Expand All @@ -381,7 +403,7 @@ private void EmitObjectData(R2RPEBuilder r2rPeBuilder, ObjectData data, int node
}
#endif

r2rPeBuilder.AddObjectData(data, section, name, mapFileBuilder);
r2rPeBuilder.AddObjectData(data, section, name, _objectInfoBuilder);
}

public static void EmitObject(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<DefineConstants>READYTORUN;$(DefineConstants)</DefineConstants>
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
<Platforms>x64;x86</Platforms>
<Platforms>x64;x86;arm;arm64</Platforms>
<PlatformTarget>AnyCPU</PlatformTarget>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>

Expand Down Expand Up @@ -196,6 +196,8 @@
<Compile Include="IL\Stubs\PInvokeILEmitter.cs" />
<Compile Include="Interop\IL\Marshaller.ReadyToRun.cs" />
<Compile Include="Compiler\PerfEventSource.cs" />
<Compile Include="ObjectWriter\ObjectInfoBuilder.cs" />
<Compile Include="ObjectWriter\SymbolFileBuilder.cs" />
<Compile Include="Win32Resources\ResourceData.cs" />
<Compile Include="Win32Resources\ResourceData.Reader.cs" />
<Compile Include="Win32Resources\ResourceData.ResourcesDataModel.cs" />
Expand Down
Loading

0 comments on commit ff15684

Please sign in to comment.