From 46b3c0171709c48f58966fdf2665b5f292ff6467 Mon Sep 17 00:00:00 2001 From: Tim Cassell <35501420+timcassell@users.noreply.github.com> Date: Fri, 22 Sep 2023 17:34:18 -0400 Subject: [PATCH] Fix linux crash from disassembler (#2413) * Update dac vulnerable version. * Skip 0 and -1 addresses. * Check for minimum address. * Read file instead of bash. * Use SystemPageSize * Add OS validation. * Some cleanup. --- .../Disassemblers/Arm64Disassembler.cs | 2 +- .../Disassemblers/ClrMdV2Disassembler.cs | 73 +++++++++++-------- .../Disassemblers/DisassemblyDiagnoser.cs | 35 +++++---- .../Disassemblers/IntelDisassembler.cs | 20 +---- 4 files changed, 67 insertions(+), 63 deletions(-) diff --git a/src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs b/src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs index 5b15a1e6ef..16ebbde219 100644 --- a/src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs +++ b/src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs @@ -247,7 +247,7 @@ protected override IEnumerable Decode(byte[] code, ulong startAddress, Stat } } } - TryTranslateAddressToName(address, isPrestubMD, state, isIndirect, depth, currentMethod); + TryTranslateAddressToName(address, isPrestubMD, state, depth, currentMethod); } accumulator.Feed(instruction); diff --git a/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs b/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs index 0087b2b9a5..5a0ddccafb 100644 --- a/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs +++ b/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs @@ -14,10 +14,32 @@ 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); + private static readonly ulong MinValidAddress = GetMinValidAddress(); + + private static ulong GetMinValidAddress() + { + // https://github.com/dotnet/BenchmarkDotNet/pull/2413#issuecomment-1688100117 + if (RuntimeInformation.IsWindows()) + return ushort.MaxValue + 1; + if (RuntimeInformation.IsLinux()) + return (ulong) Environment.SystemPageSize; + if (RuntimeInformation.IsMacOS()) + return RuntimeInformation.GetCurrentPlatform() switch + { + Environments.Platform.X86 or Environments.Platform.X64 => 4096, + Environments.Platform.Arm64 => 0x100000000, + _ => throw new NotSupportedException($"{RuntimeInformation.GetCurrentPlatform()} is not supported") + }; + throw new NotSupportedException($"{System.Runtime.InteropServices.RuntimeInformation.OSDescription} is not supported"); + } + + private static bool IsValidAddress(ulong address) + // -1 (ulong.MaxValue) address is invalid, and will crash the runtime in older runtimes. https://github.com/dotnet/runtime/pull/90794 + // 0 is NULL and therefore never valid. + // Addresses less than the minimum virtual address are also invalid. + => address != ulong.MaxValue + && address != 0 + && address >= MinValidAddress; internal DisassemblyResult AttachAndDisassemble(Settings settings) { @@ -245,13 +267,13 @@ protected static bool TryReadNativeCodeAddresses(ClrRuntime runtime, ClrMethod m return false; } - protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD, State state, bool isIndirectCallOrJump, int depth, ClrMethod currentMethod) + protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD, State state, int depth, ClrMethod currentMethod) { - var runtime = state.Runtime; - - if (state.AddressToNameMapping.ContainsKey(address)) + if (!IsValidAddress(address) || state.AddressToNameMapping.ContainsKey(address)) return; + var runtime = state.Runtime; + var jitHelperFunctionName = runtime.GetJitHelperFunctionName(address); if (!string.IsNullOrEmpty(jitHelperFunctionName)) { @@ -260,9 +282,9 @@ 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 (method is null && (address & ((uint) runtime.DataTarget.DataReader.PointerSize - 1)) == 0) { - if (runtime.DataTarget.DataReader.ReadPointer(address, out ulong newAddress) && newAddress > ushort.MaxValue) + if (runtime.DataTarget.DataReader.ReadPointer(address, out ulong newAddress) && IsValidAddress(newAddress)) { method = runtime.GetMethodByInstructionPointer(newAddress); @@ -276,31 +298,24 @@ protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD, if (method is null) { - if (isAddressPrecodeMD || !IsVulnerableToAvInDac) + var methodDescriptor = runtime.GetMethodByHandle(address); + if (methodDescriptor is not null) { - var methodDescriptor = runtime.GetMethodByHandle(address); - if (!(methodDescriptor is null)) + if (isAddressPrecodeMD) { - if (isAddressPrecodeMD) - { - state.AddressToNameMapping.Add(address, $"Precode of {methodDescriptor.Signature}"); - } - else - { - state.AddressToNameMapping.Add(address, $"MD_{methodDescriptor.Signature}"); - } - return; + state.AddressToNameMapping.Add(address, $"Precode of {methodDescriptor.Signature}"); } + else + { + state.AddressToNameMapping.Add(address, $"MD_{methodDescriptor.Signature}"); + } + return; } - if (!IsVulnerableToAvInDac) + var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address); + if (!string.IsNullOrEmpty(methodTableName)) { - var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address); - if (!string.IsNullOrEmpty(methodTableName)) - { - state.AddressToNameMapping.Add(address, $"MT_{methodTableName}"); - return; - } + state.AddressToNameMapping.Add(address, $"MT_{methodTableName}"); } return; } diff --git a/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs b/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs index 2c6eb62f79..31e7570e86 100644 --- a/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs +++ b/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs @@ -109,24 +109,31 @@ public IEnumerable Validate(ValidationParameters validationPara yield return new ValidationError(true, "Currently NativeAOT has no DisassemblyDiagnoser support", benchmark); } - if (RuntimeInformation.IsLinux() && ShouldUseClrMdDisassembler(benchmark)) + if (ShouldUseClrMdDisassembler(benchmark)) { - var runtime = benchmark.Job.ResolveValue(EnvironmentMode.RuntimeCharacteristic, EnvironmentResolver.Instance); - - if (runtime.RuntimeMoniker < RuntimeMoniker.NetCoreApp30) - { - yield return new ValidationError(true, $"{nameof(DisassemblyDiagnoser)} supports only .NET Core 3.0+", benchmark); - } - - if (ptrace_scope.Value == "2") + if (RuntimeInformation.IsLinux()) { - yield return new ValidationError(false, $"ptrace_scope is set to 2, {nameof(DisassemblyDiagnoser)} is going to work only if you run as sudo"); - } - else if (ptrace_scope.Value == "3") - { - yield return new ValidationError(true, $"ptrace_scope is set to 3, {nameof(DisassemblyDiagnoser)} is not going to work"); + var runtime = benchmark.Job.ResolveValue(EnvironmentMode.RuntimeCharacteristic, EnvironmentResolver.Instance); + + if (runtime.RuntimeMoniker < RuntimeMoniker.NetCoreApp30) + { + yield return new ValidationError(true, $"{nameof(DisassemblyDiagnoser)} supports only .NET Core 3.0+", benchmark); + } + + if (ptrace_scope.Value == "2") + { + yield return new ValidationError(false, $"ptrace_scope is set to 2, {nameof(DisassemblyDiagnoser)} is going to work only if you run as sudo"); + } + else if (ptrace_scope.Value == "3") + { + yield return new ValidationError(true, $"ptrace_scope is set to 3, {nameof(DisassemblyDiagnoser)} is not going to work"); + } } } + else if (!ShouldUseMonoDisassembler(benchmark)) + { + yield return new ValidationError(true, $"Only Windows and Linux are supported in DisassemblyDiagnoser without Mono. Current OS is {System.Runtime.InteropServices.RuntimeInformation.OSDescription}"); + } } } diff --git a/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs b/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs index 2c024f587d..f29faae037 100644 --- a/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs +++ b/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs @@ -109,14 +109,7 @@ protected override IEnumerable Decode(byte[] code, ulong startAddress, Stat } } } - - if (address > ushort.MaxValue) - { - if (!IsVulnerableToAvInDac || IsCallOrJump(instruction)) - { - TryTranslateAddressToName(address, isPrestubMD, state, isIndirect, depth, currentMethod); - } - } + TryTranslateAddressToName(address, isPrestubMD, state, depth, currentMethod); } yield return new IntelAsm @@ -130,17 +123,6 @@ protected override IEnumerable 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++)