Skip to content

Commit c99ba30

Browse files
authored
More disassembler improvements (#2078)
* handle "missing" bytes * Don't require the users to provide `--disasm true` when they already provide custom disassembler settings like --disasmDepth or --disasmFilter
1 parent 4924f0e commit c99ba30

File tree

3 files changed

+27
-2
lines changed

3 files changed

+27
-2
lines changed

src/BenchmarkDotNet/ConsoleArguments/CommandLineOptions.cs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ namespace BenchmarkDotNet.ConsoleArguments
1919
[SuppressMessage("ReSharper", "UnusedAutoPropertyAccessor.Global")]
2020
public class CommandLineOptions
2121
{
22+
private const int DefaultDisassemblerRecursiveDepth = 1;
23+
private bool useDisassemblyDiagnoser;
24+
2225
[Option('j', "job", Required = false, Default = "Default", HelpText = "Dry/Short/Medium/Long or Default")]
2326
public string BaseJob { get; set; }
2427

@@ -35,7 +38,11 @@ public class CommandLineOptions
3538
public bool UseThreadingDiagnoser { get; set; }
3639

3740
[Option('d', "disasm", Required = false, Default = false, HelpText = "Gets disassembly of benchmarked code")]
38-
public bool UseDisassemblyDiagnoser { get; set; }
41+
public bool UseDisassemblyDiagnoser
42+
{
43+
get => useDisassemblyDiagnoser || DisassemblerRecursiveDepth != DefaultDisassemblerRecursiveDepth || DisassemblerFilters.Any();
44+
set => useDisassemblyDiagnoser = value;
45+
}
3946

4047
[Option('p', "profiler", Required = false, HelpText = "Profiles benchmarked code using selected profiler. Available options: EP/ETW/CV/NativeMemory")]
4148
public string Profiler { get; set; }
@@ -145,7 +152,7 @@ public class CommandLineOptions
145152
[Option("list", Required = false, Default = ListBenchmarkCaseMode.Disabled, HelpText = "Prints all of the available benchmark names. Flat/Tree")]
146153
public ListBenchmarkCaseMode ListBenchmarkCaseMode { get; set; }
147154

148-
[Option("disasmDepth", Required = false, Default = 1, HelpText = "Sets the recursive depth for the disassembler.")]
155+
[Option("disasmDepth", Required = false, Default = DefaultDisassemblerRecursiveDepth, HelpText = "Sets the recursive depth for the disassembler.")]
149156
public int DisassemblerRecursiveDepth { get; set; }
150157

151158
[Option("disasmFilter", Required = false, HelpText = "Glob patterns applied to full method signatures by the the disassembler.")]

src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,11 @@ private static ILToNativeMap[] GetCompleteNativeMap(ClrMethod method, ClrRuntime
323323
// Since we care about Tier 1 (if it's present), we "fake" a Tier 1 map.
324324
return new[] { new ILToNativeMap() { StartAddress = startAddress, EndAddress = endAddress }};
325325
}
326+
else if (sortedMaps[0].StartAddress != startAddress || (sortedMaps[sortedMaps.Length - 1].EndAddress != endAddress && endAddress != ulong.MaxValue))
327+
{
328+
// In such situation ILOffsetMap most likely is missing few bytes. We just "extend" it to avoid producing "bad" instructions.
329+
return new[] { new ILToNativeMap() { StartAddress = startAddress, EndAddress = endAddress } };
330+
}
326331

327332
return sortedMaps;
328333
}

tests/BenchmarkDotNet.Tests/ConfigParserTests.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,19 @@ public void CanParseDisassemblerWithCustomRecursiveDepth()
474474
Assert.Equal(depth, diagnoser.Config.MaxDepth);
475475
}
476476

477+
[Fact]
478+
public void WhenCustomDisassemblerSettingsAreProvidedItsEnabledByDefault()
479+
{
480+
Verify(new[] { "--disasmDepth", "2" });
481+
Verify(new[] { "--disasmFilter", "*" });
482+
483+
void Verify(string[] args)
484+
{
485+
var config = ConfigParser.Parse(args, new OutputLogger(Output)).config;
486+
Assert.Single(config.GetDiagnosers().OfType<DisassemblyDiagnoser>());
487+
}
488+
}
489+
477490
[Fact]
478491
public void CanParseInfo()
479492
{

0 commit comments

Comments
 (0)