DisassemblyDiagnoser does not work in InProcessEmitToolchain

[timcassell]

Trying to use DisassemblyDiagnoser with InProcessEmitToolchain, I get an error:

// ! InProcessEmitExecutor, exception: System.InvalidOperationException: Sequence contains no matching element
   at System.Linq.ThrowHelper.ThrowNoMatchException()
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source, Func`2 predicate)
   at BenchmarkDotNet.Disassemblers.ClrMdV2Disassembler.AttachAndDisassemble(Settings settings)
   at BenchmarkDotNet.Disassemblers.SameArchitectureDisassembler.Disassemble(DiagnoserActionParameters parameters)
   at BenchmarkDotNet.Diagnosers.DisassemblyDiagnoser.Handle(HostSignal signal, DiagnoserActionParameters parameters)
   at BenchmarkDotNet.Diagnosers.CompositeDiagnoser.Handle(HostSignal signal, DiagnoserActionParameters parameters)
   at BenchmarkDotNet.Toolchains.InProcess.InProcessHost.SendSignal(HostSignal hostSignal)
   at BenchmarkDotNet.Engines.HostExtensions.AfterAll(IHost host)
   at BenchmarkDotNet.Toolchains.InProcess.Emit.Implementation.RunnableProgram.Run(BenchmarkId benchmarkId, Assembly partitionAssembly, BenchmarkCase benchmarkCase, IHost host)
   at BenchmarkDotNet.Toolchains.InProcess.Emit.InProcessEmitExecutor.ExecuteCore(IHost host, ExecuteParameters parameters)
Executable C:\Users\Tim\Documents\git\ProtoPromiseBenchmarks\AsynchronousBenchmarks\bin\Release\net6.0\001a3848-5fde-4a5d-a284-00d6519a2aaeEmitted.dll not found
ExitCode != 0 and no results reported
No Workload Results were obtained from the run.

The last version where it worked was 0.13.1. It seems ClrMdV2Disassembler broke this case in 0.13.2. @adamsitnik

[adamsitnik]

I am surprised that it ever worked (in the past we were attaching the debugger in a way it was performing full suspend of the debugged process, suspending yourself == deadlock).

We even have a check for that, but it checks only one of the InProcessToolchains:

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs

Lines 103 to 106 in d93490d

	if (benchmark.Job.Infrastructure.TryGetToolchain(out var toolchain) && toolchain is InProcessNoEmitToolchain)
	{
	yield return new ValidationError(true, "InProcessToolchain has no DisassemblyDiagnoser support", benchmark);
	}

I won't have the time to fix it anytime soon, but whoever wants to do it they need to check whether we are using the right name of the type:

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs

Line 40 in d93490d

ClrType typeWithBenchmark = state.Runtime.EnumerateModules().Select(module => module.GetTypeByName(settings.TypeName)).First(type => type != null);

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/SameArchitectureDisassembler.cs

Line 31 in d93490d

typeName: $"BenchmarkDotNet.Autogenerated.Runnable_{parameters.BenchmarkId.Value}",

It's the name of the type generated by the in process toolchain.

In case we can not get it working, we should just mark is as unsupported for all the in process toolchains.

[timcassell]

I did check the type, and it is correct. It just was unable to find it. I hope we can get it to work somehow, because I'm trying to debug #2334.

[adamsitnik]

It's possible that ClrMD does not include types emitted on the fly in reported types list.

Have you tried using the VS Disassemby Window: https://learn.microsoft.com/en-us/visualstudio/debugger/how-to-use-the-disassembly-window?view=vs-2022?

[timcassell]

I did look at that, yeah, but it's hard for me to make sense of because each method isn't nicely segregated like our disassembler does.

[leculver]

I have found and fixed this issue in ClrMD. It will be shipped in ClrMD 3 in the next week or two.

[adamsitnik]

@leculver thank you!

[timcassell]

@leculver Do you have a migration guide/ changelist for v3?

[leculver]

No but I'm happy to help if you get stuck. We didn't remove any features, and the breaking changes were mostly around things that were broken due to changes in the GC_REGIONS feature in the runtime. Most usage of ClrMD won't be affected by those changes at all. If you have migration problems, ping me.

[timcassell]

@leculver We're blocked by 2 things.

First, what's the replacement for SOSDacInterface? DacLibrary's only public members appear to be OwningLibrary and Dispose, which don't seem useful.

'DacLibrary' does not contain a definition for 'SOSDacInterface' and no accessible extension method 'SOSDacInterface' accepting a first argument of type 'DacLibrary' could be found (are you missing a using directive or an assembly reference?)

Second, v3 depends on System.Runtime.CompilerServices.Unsafe and System.Collections.Immutable >= 6.0.0. This is blocking because those versions do not support netcoreapp3.0 and older. Can you downgrade to 5.0.0 for netstandard2.0 target, or is 6.0.0 a hard requirement? #2333

[leculver]

First, what's the replacement for SOSDacInterface?

What is using the SOS/Dac interface directly? Whatever functionality is needed from there should be moving that over to the public surface area if it's needed. (I probably shouldn't have made SOSDacInterface public to begin with. Removing it from 3.0's surface area was intentional.)

The CLR debugging private APIs are messy and complicated to use, and often version dependent. If there's a hard dependency on something there in Benchmark.Net, I'm happy to go move the functionality to the public surface area so we don't use those implementation details.

Where in the code is it used? Is it just here?

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs

Line 256 in dd7a9b7

&& runtime.DacLibrary.SOSDacInterface.GetCodeHeaderData(method.NativeCode, out var codeHeaderData) == HResult.S_OK

If so I can go fix that bug...

DacLibrary's only public members appear to be OwningLibrary and Dispose, which don't seem useful.
DacLibrary is going away soon too. It'll be replaced with an abstract way to create ClrRuntimes, but it's really only expected to be used internally by the library.

because those versions do not support netcoreapp3.0 and older.
.Net 3.1 is already past its end-of-life. We won't keep supporting out of life versions of .Net Core.

However, ClrMD also targets netstandard2.0, which works for both desktop CLR and .Net Core back to 2.0. That version of the library should be the right version to use if you have to dip back into .Net 2.0, 3.0, or 3.1, right?

Unless I'm missing something there...

[timcassell]

Where in the code is it used? Is it just here?

And here

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs

Line 315 in dd7a9b7

var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address);

However, ClrMD also targets netstandard2.0, which works for both desktop CLR and .Net Core back to 2.0. That version of the library should be the right version to use if you have to dip back into .Net 2.0, 3.0, or 3.1, right?

Unless I'm missing something there...

Starting with 6.0.0, MS decided to start confusing people about netstandard2.0 support. Their 6.0.0 nuget packages specifically disallow netcoreapp3.0 and older runtimes from consuming them, even though the netstandard2.0 is supported on those runtimes (and future versions will continue to stop working on unsupported runtimes, despite targeting netstandard2.0). We had to downgrade some of our nuget dependencies to make netcoreapp3.0 and older work again (#2359).

You can try updating Microsoft.Diagnostics.Runtime to v3 in the core BenchmarkDotNet csproj, then try to compile BenchmarkDotNet.IntegrationTests.ManualRunning.MultipleFrameworks to see how it fails.

See open-telemetry/opentelemetry-dotnet#3448 for more info about it.

[leculver]

Where in the code is it used? Is it just here?

And here

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs

Line 315 in dd7a9b7

var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address);

That line should be var methodTableName = clrRuntime.GetTypeByMethodTable(mt)?.Name, which eliminates the need for SOSDacInterface, and uses the other caching we have in the library.

Their 6.0.0 nuget packages specifically disallow netcoreapp3.0 and older runtimes from consuming them

Ah, interesting. How long does Benchmark.Net plan to support 3.0 applications? 3.0 is out of lifecycle support and is no longer getting security updates.

This is a bit of a problem here, because you need ClrMD 3.0 to fully support the GC_REGIONS feature (.Net 8 default), but I'm not super excited to compile for netcore3.0 for years to come when it's out of lifecycle.

[timcassell]

How long does Benchmark.Net plan to support 3.0 applications?

#2333 (comment)

This is a bit of a problem here, because you need ClrMD 3.0 to fully support the GC_REGIONS feature (.Net 8 default), but I'm not super excited to compile for netcore3.0 for years to come when it's out of lifecycle.

If you could just use older nuget packages for netstandard2.0 target, it should be fine. You don't need to explicitly target netcoreapp3.0 (we don't either).

That line should be var methodTableName = clrRuntime.GetTypeByMethodTable(mt)?.Name, which eliminates the need for SOSDacInterface, and uses the other caching we have in the library.

Cool. You mentioned you will fix the other one? Will you send a PR? Or just mention what it should be here and they can both be fixed at the same time.

[leculver]

Cool. You mentioned you will fix the other one? Will you send a PR? Or just mention what it should be here and they can both be fixed at the same time.

I'll fix the issue in ClrMD and figure out dependencies in the next ClrMD release. I'm in the middle of some work, but I hopefully should have one out next week or the week after.

[leculver]

Fixed in ClrMD 3.1: https://www.nuget.org/packages/Microsoft.Diagnostics.Runtime/3.1.455904

Let me know if you find any blocking issues.

[timcassell]

@leculver It looks like that still has the same 6.0.0 dependencies.

[leculver]

The 5.0.0 version of Immutable is out of servicing support and no longer maintained. (The same is true for .Net Core 3.1.)

I won't roll back the version of these libraries because you are basically asking me: "Never roll forward any of these libraries, you have to keep ClrMD on a 5.0 Immutable/Unsafe forever". That's not a tenable position as a library maintainer, especially if a security issue is ever found.

My suggestion would be to move everything in Benchmark.Net forward and drop .Net 3.1 support. If customers need to continue benchmarking old, out of date runtimes then they should use an older version of Benchmark.Net. The burden will be on Benchmark.Net to maintain an older version of your library for older runtimes, but that's where the burden of support should lie. Not on the libraries it relies on.

(This is how we approached Desktop Framework CLR 3.5 support in PerfView: If you want to create a .gcdump for a .Net Framework 3.5 runtime, you have to use an old version of PerfView to do it.)

One other thing to keep in mind that as of .Net 7, the GC implemented GC_REGIONS as a feature. This feature required a lot of rewrites to ClrMD resulting in ClrMD 3.0. Eventually you will have to roll forward to ClrMD 3.0+ if you want to be able to accurately walk the heap in this tool. That's just the forward march of progress in the .Net space.

[timcassell]

Fair enough.

@adamsitnik What are your thoughts? We could simply drop support for older runtimes, or we could try to maintain a v3 disassembler for net6.0+ and a v2 disassembler for netstandard2.0 (or we could add a netcoreapp2.0 build target so v3 will also work in Framework).

[timcassell]

I think we should just drop netcoreapp2.0,2.1,3.0 support. I think trying to support v2 and v3 clrmd in the same BDN version would be too error prone. The current net6.0 nuget packages still support netcoreapp3.1 and newer. It's a fairly large gap between 3.0 and 9.0. @adamsitnik @AndreyAkinshin Any opinions?

[AndreyAkinshin]

@timcassell I agree. Here are the end support dates of these runtimes:
.NET Core 2.0: Oct 1, 2018
.NET Core 2.1: Aug 21, 2021
.NET Core 2.2: Dec 23, 2019
.NET Core 3.0: Mar 3, 2020

I feel like we have been supporting them long enough. Maybe it's finally time to say goodbye. I'm not a fan of dropping old TFMs just because they are expired while we can support them for free. However, if this support requires too much effort, it doesn't make sense to continue supporting them. If someone wants to measure the performance of these runtimes for archaeological purposes, they always can use the previous BenchmarkDotNet version.

[timcassell]

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs

Line 315 in dd7a9b7

var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address);

That line should be var methodTableName = clrRuntime.GetTypeByMethodTable(mt)?.Name, which eliminates the need for SOSDacInterface, and uses the other caching we have in the library.

@leculver How do I get the mt there? At that point runtime.GetMethodByInstructionPointer and runtime.GetMethodByHandle have both returned null, and I'm not seeing another way to get the method table from the address (v1 had runtime.GetMethodTableName which no longer exists).

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs

Line 256 in dd7a9b7

&& runtime.DacLibrary.SOSDacInterface.GetCodeHeaderData(method.NativeCode, out var codeHeaderData) == HResult.S_OK

If so I can go fix that bug...

Fixed in ClrMD 3.1

Ok, I see how that can remove WorkaroundGetMethodByInstructionPointerBug, but it's also used in GetCompleteNativeMap, which I'm not understanding how to fix up.
[Edit] Maybe it can just be reverted to how it was before the fixes in 4924f0e?

--

On another note, once we upgrade to clrmd v3, will we still need to keep the v1 disassembler around? Does v3 support dissassembling x86 process from x64 host and vice-versa? Or should those separate exes also be updated to v3?

[leculver]

@leculver How do I get the mt there? At that point runtime.GetMethodByInstructionPointer and runtime.GetMethodByHandle have both returned null, and I'm not seeing another way to get the method table from the address (v1 had runtime.GetMethodTableName which no longer exists).

In this context, mt meant MethodTable, which is what you are checking to see if address is. So this line:

var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address);

Should simply be:

var methodTableName = runtime.GetTypeByMethodTable(address)?.Name;

[Edit] Maybe it can just be reverted to how it was before the fixes in 4924f0e?

I believe so. The hot/cold regions should have the right data for older runtimes now.

Unfortunately, I don't think the IL map ranges will be correct with tiered jitting. ClrMD can't give that information accurately because the underlying APIs in the CLR debugging layer haven't been updated with tiered jit information. I'm not 100% sure about hot/cold info and native address ranges.

I can add it to my list to investigate, but if it's not working now with tiered jit, it will likely require a runtime feature to fix. That's doable, but it requires more work than simply updating ClrMD.

On another note, once we upgrade to clrmd v3, will we still need to keep the v1 disassembler around?

I don't know exactly what's meant by the "v1 disassembler". I can say that ClrMD v3/3.1 supports all Desktop CLR and .Net Core versions that are within lifecycle support. If you are debugging those, it will give you the best information that's available.

Does v3 support dissassembling x86 process from x64 host and vice-versa? Or should those separate exes also be updated to v3?

You will have to have separate processes for x86/x64.

All debugging of CLR requires you to match the bitness of your application to the bitness of the target process. This is because the underlying CLR debugging API (mscordaccore.dll/mscordacwks.dll) is a native DLL/so that's tied to processor architecture. ClrMD (and all .net debugging) relies on that dll, so all tools have to follow that rule too.

[timcassell]

I don't know exactly what's meant by the "v1 disassembler".

We ship separate exe disassemblers for x86 and x64 that still use clrmd v1 (BenchmarkDotNet.Disassembler.x64/x86 projects).

Another thought I had was maybe we can build the disassembler directly like we do the benchmark project, instead of having 2 separate pre-built exes and part of the core BDN library. We could strip the clrmd dependencies out of core BDN, include the project files as content files like we currently do with the exes, then just build it on the end user's machine when it's needed.

This would allow us to continue supporting netcoreapp3.0 and older (no breaking dependencies), as well as have the proper architecture and bitness (so we could support ARM 32-bit as well as ARM64). Any thoughts on that @AndreyAkinshin @adamsitnik?

[AndreyAkinshin]

This idea looks interesting. If we can deliver a reliable implementation that works in all environments, I don't mind.

By the way, what about cases when we can't build anything on the user machine? E.g., how do we use this approach with InProcessToolchain?

[timcassell]

By the way, what about cases when we can't build anything on the user machine? E.g., how do we use this approach with InProcessToolchain?

I could be wrong, but I don't think coreclr (the only runtime clrmd supports besides full framework?) runs on platforms that can't build. Those platforms mostly use Mono or NativeAot or some derivative (we have a mono disassembler and we don't support disassembling NativeAot atm).

Otherwise, disassembling in process with clrmd would need a separate package (if we want to keep supporting old runtimes).

[Edit] We can still build the disassembler even if the in process toolchain is used (as long as the dotnet sdk is installed).

[Edit2] It looks like we currently only support clrmd disassembler on Windows and Linux, so we should always be able to build it.

[AndreyAkinshin]

@adamsitnik what do you think?

[adamsitnik]

I believe we should simply drop the support for old runtimes that are not supported for years. Users can still benchmark them, if they need a dissasembly they simply need to use old BDN version. It's too expensive to maintain and most likely almost nobody uses it.

[timcassell]

Users can still benchmark them, if they need a dissasembly they simply need to use old BDN version.

Well, no. Benchmarks will not work even without disassembly. Users will have to use old BDN version for any benchmarks of those runtimes.