some jumps based on uninitialized values detected by valgrind

[tmds]

I ran valgrind on dotnet hello world, and it pointed out a few jumps based on uninitialized values.

1. moved to JIT: some jumps based on uninitialized values detected by valgrind #46961

2. moved to JIT: some jumps based on uninitialized values detected by valgrind #46961

3. BINDER_SPACE::AssemblyName::Init(IMDInternalImport*, _tagPEKIND, unsigned int, int) (src/coreclr/binder/assemblyname.cpp:144)

runtime/src/coreclr/binder/assemblyname.cpp

Line 144 in 36d132e

if (pvPublicKeyToken && dwPublicKeyToken)

valgrind is not happy about pvPublicKeyToken, but dwPublicKeyToken is 0 in those cases so pvPublicKeyToken is not used beyond the check. Maybe pvPublicKeyToken can be set to NULL in GetAssemblyRefProps, or we can swap the order in the if to make valgrind happy?

4. moved to JIT: some jumps based on uninitialized values detected by valgrind #46961

5. ==6406== at 0x563D079: PAL_VirtualUnwind (src/coreclr/pal/src/exception/seh-unwind.cpp:353) ==6406== by 0x545253E: LazyMachState::unwindLazyState(LazyMachState*, MachState*, unsigned int, int, HostCallPreference) (src/coreclr/vm/amd64/gmsamd64.cpp:63)

runtime/src/coreclr/pal/src/exception/seh-unwind.cpp

Line 353 in a1c4028

if ((context->ContextFlags & CONTEXT_EXCEPTION_ACTIVE) != 0)

ContextFlags is not initialized.
The context is a local at:

runtime/src/coreclr/vm/amd64/gmsamd64.cpp

Line 25 in a1c4028

CONTEXT ctx;

6. ==6406== by 0x5670B1D: FreeSignalAlternateStack (src/coreclr/pal/src/thread/thread.cpp:2603)

runtime/src/coreclr/pal/src/thread/thread.cpp

Lines 2597 to 2603 in a1c4028

	stack_t ss, oss;
	// The man page for sigaltstack says that when the ss.ss_flags is set to SS_DISABLE,
	// all other ss fields are ignored. However, MUSL implementation checks that the
	// ss_size is >= MINSIGSTKSZ even in this case.
	ss.ss_size = MINSIGSTKSZ;
	ss.ss_flags = SS_DISABLE;
	int st = sigaltstack(&ss, &oss);

valgrind doesn't likes we're passing an uninitialized bytes to sigaltstack.
It is a false positive since that field is ignored when using SS_DISABLE.
We can probably make this warning go away by setting ss_sp to NULL.

@janvorli @jkotas can you take a look, or cc someone that can?

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[janvorli]

cc: @dotnet/jit-contrib for the JIT code.

[janvorli]

@tmds thank you for reporting these!

[janvorli]

valgrind is not happy about pvPublicKeyToken
This is strange - pvPublicKeyToken is initialized to NULL here:

runtime/src/coreclr/binder/assemblyname.cpp

Line 51 in 36d132e

CONST VOID *pvPublicKeyToken = NULL;

[tmds]

This is strange - pvPublicKeyToken is initialized to NULL here:

From stepping through gdb it looks like GetAssemblyProps assigns a value to it.

(gdb) p dwPublicKeyToken
$81 = 0
(gdb) p pvPublicKeyToken
$82 = (const void *) 0x3
(gdb) p *this
$83 = {<BINDER_SPACE::AssemblyIdentity> = {m_simpleName = {<SBuffer> = {m_size = 16, m_allocation = 16, m_flags = 12, {m_buffer = 0x555555663980 "c", m_asStr = 0x555555663980 u"console"}}, static MINIMUM_GUESS = 20, 
      static s_EmptyBuffer = "\000", static s_ACP = 65001, static s_Empty = 0x7ffff7a44190 <s_EmptySpace>}, m_version = {static Unspecified = 4294967295, static UnspecifiedShort = 65535, m_dwMajor = 1, m_dwMinor = 0, m_dwBuild = 0, 
      m_dwRevision = 0}, m_cultureOrLanguage = {<SBuffer> = {m_size = 2, m_allocation = 2, m_flags = 8, {m_buffer = 0x555555663960 "", m_asStr = 0x555555663960 u""}}, static MINIMUM_GUESS = 20, 
      static s_EmptyBuffer = <same as static member of an already seen type>, static s_ACP = 65001, static s_Empty = 0x7ffff7a44190 <s_EmptySpace>}, m_publicKeyOrTokenBLOB = {m_size = 1, m_allocation = 1, m_flags = 16, {
        m_buffer = 0x7ffff78a67d8 <BINDER_SPACE::AssemblyIdentity::AssemblyIdentity()::byteArr> "", m_asStr = 0x7ffff78a67d8 <BINDER_SPACE::AssemblyIdentity::AssemblyIdentity()::byteArr> u"\x3100䌴牯䉥湩剤獥汵t"}}, 
    m_kProcessorArchitecture = peNone, m_kContentType = AssemblyContentType_Default, m_customBLOB = {m_size = 1, m_allocation = 1, m_flags = 16, {
        m_buffer = 0x7ffff78a67d8 <BINDER_SPACE::AssemblyIdentity::AssemblyIdentity()::byteArr> "", m_asStr = 0x7ffff78a67d8 <BINDER_SPACE::AssemblyIdentity::AssemblyIdentity()::byteArr> u"\x3100䌴牯䉥湩剤獥汵t"}}, 
    m_dwIdentityFlags = 275}, m_cRef = 1, m_dwNameFlags = 0}

[janvorli]

Maybe I didn't understand the issue then. I have thought valgrind was complaining that the pvPublicKeyToken is not initialized in some code paths. What was the exact complaint?

[tmds]

==5601== Conditional jump or move depends on uninitialised value(s)
==5601==    at 0x555E47F: BINDER_SPACE::AssemblyName::Init(IMDInternalImport*, _tagPEKIND, unsigned int, int) (src/coreclr/binder/assemblyname.cpp:144)

I think what GetAssemblyProps assigns to it may come from something uninitialized.

[tmds]

It can also be a false positive.
It seems strange to me that GetAssemblyProps returns 0 for length (dwPublicKeyToken) and non-zero for data pointer (pvPublicKeyToken). It may be a valid thing for this API, I am not familiar with it.

[BruceForstall]

I edited/split this issue and moved the JIT portion to #46961

[tmds]

I've added a 5th and 6th bullet.