Fix two issues detected by Valgrind

[janvorli]

When I have used Valgrind to investigate a memory corruption issue recently,
I've noticed that it has also reported two cases when a conditional jump
was using an uninitialized variable as one of the inputs to the condition.

This change fixes these.

[tmds]

@janvorli I tried running valgrind against dotnet, but it failed and gave an error (I believe about a machine instruction it didn't know).
Does it work well for you?

[jkotas]

Does this need to be under debug ifdef?

[janvorli]

Right, it does. I have not noticed that it is a debug build only thing.

[janvorli]

Does it work well for you?

It actually fails relatively early with .NET Core 5, but it worked fine with .NET Core 3.1.

[janvorli]

@tmds this is the Valgrind failure with .NET Core 5 I was getting:

vex amd64->IR: unhandled instruction bytes: 0x48 0xCF 0x48 0x8D 0x64 0x24 0x30 0x5D
vex amd64->IR:   REX=1 REX.W=1 REX.R=0 REX.X=0 REX.B=0
vex amd64->IR:   VEX=0 VEX.L=0 VEX.nVVVV=0x0 ESC=NONE
vex amd64->IR:   PFX.66=0 PFX.F2=0 PFX.F3=0
==24510== valgrind: Unrecognised instruction at address 0x6c62d36.

This looks like a bug in Valgrind, as there is a valid IRET instruction with REX.W prefix.

[am11]

With:

valgrind -v --tool=memcheck --leak-check=full --show-reachable=yes \
    --num-callers=40 --log-file=dotnet-valgrind.log \
    dotnet5 new console

valgrind-3.13.0 on Ubuntu bionic found 176 Conditional jump or move depends on uninitialised value(s) cases and the last line of donet-valgrind.log shows:

==112569== ERROR SUMMARY: 1819 errors from 170 contexts (suppressed: 0 from 0)

dotnet5 --version => 5.0.100-preview.3.20177.2.

[tmds]

On Fedora 31 with .NET 3.1 I get:

$ dotnet --version
3.1.201
$ valgrind --version
valgrind-3.15.0
$ valgrind dotnet new console -o console
...
vex amd64->IR: unhandled instruction bytes: 0x48 0xE9 0x70 0xB3 0x41 0xA9 0x49 0xBA 0x88 0xCD
vex amd64->IR:   REX=1 REX.W=1 REX.R=0 REX.X=0 REX.B=0
vex amd64->IR:   VEX=0 VEX.L=0 VEX.nVVVV=0x0 ESC=NONE
vex amd64->IR:   PFX.66=0 PFX.F2=0 PFX.F3=0
==178762== valgrind: Unrecognised instruction at address 0x5d09698a.

@janvorli what version of valgrind are you using? And does it still work with .NET Core 3.1?

[janvorli]

I was using the default valgrind package on Ubuntu 16.04, which is version 3.11.0. However, I have tested it just on an app reproducing the write behind allocated memory issue. So it is possible that more things would trouble it if it went through different code paths.
The instruction it has failed on in your case is just a pc relative jump with 32 bit offset (the first 6 bytes) with added REX.W prefix that has no effect. So I guess that's what has confused Valgrind.
I think I've actually seen this too in one of my test runs on .NET Core 5.0.

[jkotas]

Where are these superfluous REX.W prefixes coming from? Do we generate them, or is it something that clang generates?

[janvorli]

One comes from RtlRestoreContext here:

runtime/src/coreclr/src/pal/src/arch/amd64/context2.S

Line 183 in fcd862e

iretq

I don't know where the other stems from.

[jkotas]

Thanks. The prefix on iret is required - it is not superfluous, so we are good on that one.

[janvorli]

@jkotas the other one - the JMP with extra REX.W prefix seems to be coming from the JIT or a code that runtime generates.

[jkotas]

Is there a way to dump the disassembly before the offending JMP? Hopefully, we will be able to guess where it came from.

[janvorli]

@jkotas there is a way to run valgrind in a mode where one can attach a debugger externally, so I'll give it a try.

[janvorli]

@jkotas, I can see that the jump that it complains on is:
=> 0x000000003d5cb57a: 48 e9 00 30 41 c9 jmpq 0x69de580 <NDirectImportThunk>

So it comes from

runtime/src/coreclr/src/vm/i386/stublinkerx86.h

Line 587 in 42183b1

static const int Type = 0x48;

It is used here:

runtime/src/coreclr/src/vm/i386/stublinkerx86.cpp

Lines 5974 to 5978 in 42183b1

	void NDirectImportPrecode::Init(MethodDesc* pMD, LoaderAllocator *pLoaderAllocator)
	{
	WRAPPER_NO_CONTRACT;
	StubPrecode::Init(pMD, pLoaderAllocator, NDirectImportPrecode::Type, GetEEFuncEntryPoint(NDirectImportThunk));
	}