Fix linux crash from disassembler

[timcassell]

dotnet/runtime#90794

Fixes #2404

[adamsitnik]

@timcassell thank you for providing a quick fix!

After reading dotnet/runtime#90794 (comment) I wonder whether we should simply unconditionally not try to map 0 and -1. Thoughts?

[timcassell]

@adamsitnik It looks like the IntelDisassembler already checks for it if (address > ushort.MaxValue) but the Arm64Disassembler does not.

BenchmarkDotNet/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs

Lines 85 to 91 in c35dcb2

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

But I guess that doesn't really catch it, because the address we're checking is ulong, so -1 is translated to ulong.MaxValue and passes the check.

[timcassell]

I changed it to always check for 0 or -1, though I'm not sure if it needs to only check that in non-windows.
Also, I'm not sure if addresses less than ushort.MaxValue are only invalid in x86, or also in ARM and if we should add that check in the Arm64Disassembler.

[timcassell]

@janvorli Is -1 address invalid in every environment? Also are addresses less than ushort.MaxValue invalid in every environment?

@leculver It might make sense to explicitly check for -1 address in ClrMd also.

[janvorli]

Is -1 address invalid in every environment?

Yes, -1 is treated in a special way in DAC. See e.g.:
https://github.com/dotnet/runtime/blob/4a5695b6ea0e461e6527de057420872369345282/src/coreclr/debug/daccess/dacfn.cpp#L384-L388

Also are addresses less than ushort.MaxValue invalid in every environment?

This is true for Windows (actually, less or equal to that value are invalid). For Unix, this can differ based on the memory page size. On Linux, it is 4096 in most cases and 65536 for the case when the Linux kernel is configured for 64kB page size (some arm64 systems use that). But it can be different. For example, in WSL2 on Windows, it is 65536. The /proc/sys/vm/mmap_min_addr can be used to get the actual value on Linux.
On macOS, it is also different. On x64 it is be 4096 and on arm64 it is 4GB (0x100000000).

[timcassell]

@janvorli How can I use that to get the value on Linux? I tried /proc/sys/vm/mmap_min_addr on a GitHub runner to test, and it gave back Permission denied.

[leculver]

@janvorli How can I use that to get the value on Linux? I tried /proc/sys/vm/mmap_min_addr on a GitHub runner to test, and it gave back Permission denied.

You are trying to run it, if that's the command you gave.

cat /proc/sys/vm/mmap_min_addr
or
sudo cat /proc/sys/vm/mmap_min_addr
if it's protected

[timcassell]

@adamsitnik This should be good to go, if you could take a look.

[adamsitnik]

The changes look OK, but we have lost a possibility to map certain addresses.

Every time we introduce changes to the disassembler I run a sample benchmark with --disasmFilter * to get the disassembly of everything that we can on Win x64, Lin x64 and arm64.

dotnet run -c Release -f net7.0 --filter BenchmarkDotNet.Samples.IntroDisassembly.SumLocal --disasmFilter '*'

I've run this command on master branch, created a copy of the exported asm file and then run it against your branch. When eyeballing the diff I've found:

The smallest repro is:

dotnet run -c Release -f net7.0 --filter BenchmarkDotNet.Samples.IntroDisassembly.SumLocal --disasmFilter "BenchmarkDotNet.Characteristics.CharacteristicSetPresenter..ctor()"

@timcassell PTAL

[timcassell]

@adamsitnik I spent a few hours trying to figure out why that was happening. I finally found that if I delete my BenchmarkDotNet.Samples\bin directory before running, it works as expected. If I run it twice in a row without deleting it, I get the missing ctor method. I see the same behavior on master, so it's unrelated to my changes.

[adamsitnik]

LGTM, thank you @timcassell

I spent a few hours trying to figure out why that was happening. I finally found that if I delete my BenchmarkDotNet.Samples\bin directory before running, it works as expected. If I run it twice in a row without deleting it, I get the missing ctor method. I see the same behavior on master, so it's unrelated to my changes.

@janvorli it would be great if we could debug it one day

[janvorli]

I have experimented with this a bit - just changing the runtime version in the test command line to 8.0. I cannot reproduce the behavior that @timcassell has seen, it fails to discover the method name for the constructor call all the time, even if I deleted the bin/obj folders before.
However, I've re-read some of my notes from the time I have implemented decoding the target from the stubs it calls and I've found that a case that doesn't work is when the target is called via a ready to run delayload helper. So, I've tried to disable ready to run by setting DOTNET_ReadyToRun=0 and the System.Object..ctor got decoded correctly. And then I've set it to 1 - and it was not decoded. So, I guess that is the culprit.
My notes say that real target extraction for calls via the ready to run delayload helpers would not be reasonably feasible from the disassembler code.