Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update clrmd to 3.1 #2488

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+31 −74
Open

Update clrmd to 3.1 #2488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/BenchmarkDotNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<PackageReference Include="CommandLineParser" Version="2.9.1" />
<PackageReference Include="Gee.External.Capstone" Version="2.3.0" />
<PackageReference Include="Iced" Version="1.17.0" />
<PackageReference Include="Microsoft.Diagnostics.Runtime" Version="2.2.332302" />
<PackageReference Include="Microsoft.Diagnostics.Runtime" Version="3.1.512801" />
<PackageReference Include="Perfolizer" Version="[0.3.17]" />
<PackageReference Include="Microsoft.Diagnostics.Tracing.TraceEvent" Version="3.1.8" PrivateAssets="contentfiles;analyzers" />
<PackageReference Include="Microsoft.DotNet.PlatformAbstractions" Version="3.1.6" />
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void Feed(Arm64Instruction instruction)
public Arm64RegisterId RegisterId { get { return _registerId; } }
}

internal class Arm64Disassembler : ClrMdV2Disassembler
internal class Arm64Disassembler : ClrMdV3Disassembler
{
internal sealed class RuntimeSpecificData
{
Expand Down
93 changes: 25 additions & 68 deletions ...tNet/Disassemblers/ClrMdV2Disassembler.cs → ...tNet/Disassemblers/ClrMdV3Disassembler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@

namespace BenchmarkDotNet.Disassemblers
{
// This Disassembler uses ClrMd v2x. Please keep it in sync with ClrMdV1Disassembler (if possible).
internal abstract class ClrMdV2Disassembler
// This Disassembler uses ClrMd v3x. Please keep it in sync with ClrMdV1Disassembler (if possible).
internal abstract class ClrMdV3Disassembler

{
private static readonly ulong MinValidAddress = GetMinValidAddress();

Expand Down Expand Up @@ -64,7 +65,7 @@ internal DisassemblyResult AttachAndDisassemble(Settings settings)
state.Todo.Enqueue(
new MethodInfo(
// the Disassembler Entry Method is always parameterless, so check by name is enough
typeWithBenchmark.Methods.Single(method => method.IsPublic && method.Name == settings.MethodName),
typeWithBenchmark.Methods.Single(method => method.Attributes.HasFlag(System.Reflection.MethodAttributes.Public) && method.Name == settings.MethodName),
0));
}

Expand Down Expand Up @@ -149,9 +150,10 @@ private DisassembledMethod DisassembleMethod(MethodInfo methodInfo, State state,

if (!CanBeDisassembled(method))
{
if (method.IsPInvoke)
if (method.Attributes.HasFlag(System.Reflection.MethodAttributes.PinvokeImpl))
return CreateEmpty(method, "PInvoke method");
if (method.IL is null || method.IL.Length == 0)
var ilInfo = method.GetILInfo();
if (ilInfo is null || ilInfo.Length == 0)
return CreateEmpty(method, "Extern method");
if (method.CompilationType == MethodCompilationType.None)
return CreateEmpty(method, "Method was not JITted yet.");
Expand Down Expand Up @@ -214,60 +216,30 @@ private IEnumerable<Asm> Decode(ILToNativeMap map, State state, int depth, ClrMe

private static ILToNativeMap[] GetCompleteNativeMap(ClrMethod method, ClrRuntime runtime)
{
if (!TryReadNativeCodeAddresses(runtime, method, out ulong startAddress, out ulong endAddress))
// it's better to use one single map rather than few small ones
// it's simply easier to get next instruction when decoding ;)

var hotColdInfo = method.HotColdInfo;
if (hotColdInfo.HotSize > 0 && hotColdInfo.HotStart > 0)
{
startAddress = method.NativeCode;
endAddress = ulong.MaxValue;
return hotColdInfo.ColdSize <= 0
? new[] { new ILToNativeMap() { StartAddress = hotColdInfo.HotStart, EndAddress = hotColdInfo.HotStart + hotColdInfo.HotSize, ILOffset = -1 } }
: new[]
{
new ILToNativeMap() { StartAddress = hotColdInfo.HotStart, EndAddress = hotColdInfo.HotStart + hotColdInfo.HotSize, ILOffset = -1 },
new ILToNativeMap() { StartAddress = hotColdInfo.ColdStart, EndAddress = hotColdInfo.ColdStart + hotColdInfo.ColdSize, ILOffset = -1 }
};
}

ILToNativeMap[] sortedMaps = method.ILOffsetMap // CanBeDisassembled ensures that there is at least one map in ILOffsetMap
.Where(map => map.StartAddress >= startAddress && map.StartAddress < endAddress) // can be false for Tier 0 maps, EndAddress is not checked on purpose here
.Where(map => map.StartAddress < map.EndAddress) // some maps have 0 length (they don't have corresponding assembly code?)
return method.ILOffsetMap
.Where(map => map.StartAddress < map.EndAddress) // some maps have 0 length?
.OrderBy(map => map.StartAddress) // we need to print in the machine code order, not IL! #536
.Select(map => new ILToNativeMap()
{
StartAddress = map.StartAddress,
// some maps have EndAddress > codeHeaderData.MethodStart + codeHeaderData.MethodSize and contain garbage (#2074). They need to be fixed!
EndAddress = Math.Min(map.EndAddress, endAddress),
ILOffset = map.ILOffset
})
.ToArray();

if (sortedMaps.Length == 0)
{
// In such situation ILOffsetMap most likely describes Tier 0, while CodeHeaderData Tier 1.
// Since we care about Tier 1 (if it's present), we "fake" a Tier 1 map.
return new[] { new ILToNativeMap() { StartAddress = startAddress, EndAddress = endAddress } };
}
else if (sortedMaps[0].StartAddress != startAddress || (sortedMaps[sortedMaps.Length - 1].EndAddress != endAddress && endAddress != ulong.MaxValue))
{
// In such situation ILOffsetMap most likely is missing few bytes. We just "extend" it to avoid producing "bad" instructions.
return new[] { new ILToNativeMap() { StartAddress = startAddress, EndAddress = endAddress } };
}

return sortedMaps;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a lot of time to workaround all ClrMd bugs. If we really want to remove these workarounds we need to do a very extensive testing:

  1. Run a simple benchmark and ask BDN to disassemble ALL methods for main. It should produce a markdown file with all CLR methods disassembled.
  2. Same as above, but for this PR.
  3. Diff two produced .md files and ensure that all instructions are correctly resolved and there are no missing things.

}

private static DisassembledMethod CreateEmpty(ClrMethod method, string reason)
=> DisassembledMethod.Empty(method.Signature, method.NativeCode, reason);

protected static bool TryReadNativeCodeAddresses(ClrRuntime runtime, ClrMethod method, out ulong startAddress, out ulong endAddress)
{
if (method is not null
&& runtime.DacLibrary.SOSDacInterface.GetCodeHeaderData(method.NativeCode, out var codeHeaderData) == HResult.S_OK
&& codeHeaderData.MethodSize > 0) // false for extern methods!
{
// HotSize can be missing or be invalid (https://github.com/microsoft/clrmd/issues/1036).
// So we fetch the method size on our own.
startAddress = codeHeaderData.MethodStart;
endAddress = codeHeaderData.MethodStart + codeHeaderData.MethodSize;
return true;
}

startAddress = endAddress = 0;
return false;
}

protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD, State state, int depth, ClrMethod currentMethod)
{
if (!IsValidAddress(address) || state.AddressToNameMapping.ContainsKey(address))
Expand All @@ -283,18 +255,10 @@ protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD,
}

var method = runtime.GetMethodByInstructionPointer(address);
if (method is null && (address & ((uint) runtime.DataTarget.DataReader.PointerSize - 1)) == 0)
{
if (runtime.DataTarget.DataReader.ReadPointer(address, out ulong newAddress) && IsValidAddress(newAddress))
{
method = runtime.GetMethodByInstructionPointer(newAddress);

method = WorkaroundGetMethodByInstructionPointerBug(runtime, method, newAddress);
}
}
else
if (method is null && (address & ((uint) runtime.DataTarget.DataReader.PointerSize - 1)) == 0
&& runtime.DataTarget.DataReader.ReadPointer(address, out ulong newAddress) && IsValidAddress(newAddress))
{
method = WorkaroundGetMethodByInstructionPointerBug(runtime, method, address);
method = runtime.GetMethodByInstructionPointer(newAddress);
}

if (method is null)
Expand All @@ -313,7 +277,7 @@ protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD,
return;
}

var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address);
var methodTableName = runtime.GetTypeByMethodTable(address)?.Name;
if (!string.IsNullOrEmpty(methodTableName))
{
state.AddressToNameMapping.Add(address, $"MT_{methodTableName}");
Expand Down Expand Up @@ -349,13 +313,6 @@ protected void FlushCachedDataIfNeeded(IDataReader dataTargetDataReader, ulong a
}
}

// GetMethodByInstructionPointer sometimes returns wrong methods.
// In case given address does not belong to the methods range, null is returned.
private static ClrMethod WorkaroundGetMethodByInstructionPointerBug(ClrRuntime runtime, ClrMethod method, ulong newAddress)
=> TryReadNativeCodeAddresses(runtime, method, out ulong startAddress, out ulong endAddress) && !(startAddress >= newAddress && newAddress <= endAddress)
? null
: method;

private class SharpComparer : IEqualityComparer<Sharp>
{
public bool Equals(Sharp x, Sharp y)
Expand Down
2 changes: 1 addition & 1 deletion src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace BenchmarkDotNet.Disassemblers
{
internal class IntelDisassembler : ClrMdV2Disassembler
internal class IntelDisassembler : ClrMdV3Disassembler
{
internal sealed class RuntimeSpecificData
{
Expand Down
6 changes: 3 additions & 3 deletions src/BenchmarkDotNet/Disassemblers/SameArchitectureDisassembler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ namespace BenchmarkDotNet.Disassemblers
internal class SameArchitectureDisassembler
{
private readonly DisassemblyDiagnoserConfig config;
private ClrMdV2Disassembler? clrMdV2Disassembler;
private ClrMdV3Disassembler? clrMdV3Disassembler;

internal SameArchitectureDisassembler(DisassemblyDiagnoserConfig config) => this.config = config;

internal DisassemblyResult Disassemble(DiagnoserActionParameters parameters)
// delay the creation to avoid exceptions at DisassemblyDiagnoser ctor
=> (clrMdV2Disassembler ??= CreateDisassemblerForCurrentArchitecture())
=> (clrMdV3Disassembler ??= CreateDisassemblerForCurrentArchitecture())
.AttachAndDisassemble(BuildDisassemblerSettings(parameters));

private static ClrMdV2Disassembler CreateDisassemblerForCurrentArchitecture()
private static ClrMdV3Disassembler CreateDisassemblerForCurrentArchitecture()
=> RuntimeInformation.GetCurrentPlatform() switch
{
Platform.X86 or Platform.X64 => new IntelDisassembler(),
Expand Down