Skip to content

Commit

Permalink
Disassembler realiability fixes (#2234)
Browse files Browse the repository at this point in the history
* re-enable the failing tests

* add a fix

* translate only jump and call instruction addresses, allows for avoiding AV
  • Loading branch information
adamsitnik committed Dec 23, 2022
1 parent 82f03f4 commit 9684485
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 5 deletions.
2 changes: 2 additions & 0 deletions src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat
// is at an address one memory page higher than the code.
byte[] buffer = new byte[12];

FlushCachedDataIfNeeded(state.Runtime.DataTarget.DataReader, address, buffer);

if (state.Runtime.DataTarget.DataReader.Read(address, buffer) == buffer.Length)
{
if (buffer.SequenceEqual(callCountingStubTemplate))
Expand Down
26 changes: 24 additions & 2 deletions src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,18 @@
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using BenchmarkDotNet.Portability;

namespace BenchmarkDotNet.Disassemblers
{
// This Disassembler uses ClrMd v2x. Please keep it in sync with ClrMdV1Disassembler (if possible).
internal abstract class ClrMdV2Disassembler
{
// Translating an address to a method can cause AV and a process crash (https://github.com/dotnet/BenchmarkDotNet/issues/2070).
// It was fixed in https://github.com/dotnet/runtime/pull/79846,
// and most likely will be backported to 7.0.2 very soon (https://github.com/dotnet/runtime/pull/79862).
protected static readonly bool IsVulnerableToAvInDac = !RuntimeInformation.IsWindows() && Environment.Version < new Version(7, 0, 2);

internal DisassemblyResult AttachAndDisassemble(Settings settings)
{
using (var dataTarget = DataTarget.AttachToProcess(
Expand Down Expand Up @@ -270,7 +276,7 @@ protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD,

if (method is null)
{
if (isAddressPrecodeMD || this is not Arm64Disassembler)
if (isAddressPrecodeMD || !IsVulnerableToAvInDac)
{
var methodDescriptor = runtime.GetMethodByHandle(address);
if (!(methodDescriptor is null))
Expand All @@ -287,7 +293,7 @@ protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD,
}
}

if (this is not Arm64Disassembler)
if (!IsVulnerableToAvInDac)
{
var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address);
if (!string.IsNullOrEmpty(methodTableName))
Expand All @@ -311,6 +317,22 @@ protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD,
state.AddressToNameMapping.Add(address, methodName);
}

protected void FlushCachedDataIfNeeded(IDataReader dataTargetDataReader, ulong address, byte[] buffer)
{
if (!RuntimeInformation.IsWindows())
{
if (dataTargetDataReader.Read(address, buffer) <= 0)
{
// We don't suspend the benchmark process for the time of disassembling,
// as it would require sudo privileges.
// Because of that, the Tiered JIT thread might still re-compile some methods
// in the meantime when the host process it trying to disassemble the code.
// In such case, Tiered JIT thread might create stubs which requires flushing of the cached data.
dataTargetDataReader.FlushCachedData();
}
}
}

// 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)
Expand Down
18 changes: 17 additions & 1 deletion src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat
// is at an address one memory page higher than the code.
byte[] buffer = new byte[10];

FlushCachedDataIfNeeded(state.Runtime.DataTarget.DataReader, address, buffer);

if (state.Runtime.DataTarget.DataReader.Read(address, buffer) == buffer.Length && buffer.SequenceEqual(callCountingStubTemplate))
{
const ulong TargetMethodAddressSlotOffset = 8;
Expand Down Expand Up @@ -82,7 +84,10 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat

if (address > ushort.MaxValue)
{
TryTranslateAddressToName(address, isPrestubMD, state, isIndirect, depth, currentMethod);
if (!IsVulnerableToAvInDac || IsCallOrJump(instruction))
{
TryTranslateAddressToName(address, isPrestubMD, state, isIndirect, depth, currentMethod);
}
}
}

Expand All @@ -97,6 +102,17 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat
}
}

private static bool IsCallOrJump(Instruction instruction)
=> instruction.FlowControl switch
{
FlowControl.Call => true,
FlowControl.IndirectCall => true,
FlowControl.ConditionalBranch => true,
FlowControl.IndirectBranch => true,
FlowControl.UnconditionalBranch => true,
_ => false
};

private static bool TryGetReferencedAddress(Instruction instruction, uint pointerSize, out ulong referencedAddress)
{
for (int i = 0; i < instruction.OpCount; i++)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ public void Recursive()
public void CanDisassembleAllMethodCalls(Jit jit, Platform platform, Runtime runtime)
{
if (RuntimeInformation.IsMacOS()) return; // currently not supported
if (RuntimeInformation.IsLinux()) return; // https://github.com/dotnet/BenchmarkDotNet/issues/2223

var disassemblyDiagnoser = new DisassemblyDiagnoser(
new DisassemblyDiagnoserConfig(printSource: true, maxDepth: 3));
Expand Down Expand Up @@ -150,7 +149,6 @@ [Benchmark] public void JustReturn() { }
public void CanDisassembleInlinableBenchmarks(Jit jit, Platform platform, Runtime runtime)
{
if (RuntimeInformation.IsMacOS()) return; // currently not supported
if (RuntimeInformation.IsLinux()) return; // https://github.com/dotnet/BenchmarkDotNet/issues/2223

var disassemblyDiagnoser = new DisassemblyDiagnoser(
new DisassemblyDiagnoserConfig(printSource: true, maxDepth: 3));
Expand Down

0 comments on commit 9684485

Please sign in to comment.