[EnC] CoreCLR assertion in System.Reflection.Metadata.ApplyUpdateTest.TestGenericAddStaticField

[lambdageek]

The test triggers an assertion:

Assert failure(PID 2148 [0x00000864], Thread: 17217 [0x4341]): pos >= 0 && pos < pMT->GetNumStaticFields()
    File: /Users/runner/work/1/s/src/coreclr/vm/memberload.cpp Line: 369
    Image: /private/tmp/helix/working/B15A0A10/p/dotnet

Test was added in #87285

[ghost]

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

---

The test triggers an assertion:

Assert failure(PID 2148 [0x00000864], Thread: 17217 [0x4341]): pos >= 0 && pos < pMT->GetNumStaticFields()
    File: /Users/runner/work/1/s/src/coreclr/vm/memberload.cpp Line: 369
    Image: /private/tmp/helix/working/B15A0A10/p/dotnet

Test was added in #87285

Author:	lambdageek
Assignees:	-
Labels:	area-Diagnostics-coreclr
Milestone:	-

[lambdageek]

attn: @mikelle-rogers

From offline discussion with @AaronRobinsonMSFT

the comment points to GetFieldDescByIndex() which was updated. See

runtime/src/coreclr/vm/methodtable.cpp

Lines 2091 to 2116 in 2905fe7

	// Check if the field index is for an EnC field lookup.
	// See GetIndexForFieldDesc() for when this is applied and why.
	if ((fieldIndex & EnCFieldIndex) == EnCFieldIndex)
	{
	DWORD rid = fieldIndex & ~EnCFieldIndex;
	LOG((LF_ENC, LL_INFO100, "MT:GFDBI: rid:0x%08x\n", rid));
	mdFieldDef tokenToFind = TokenFromRid(rid, mdtFieldDef);
	EncApproxFieldDescIterator fdIterator(
	this,
	ApproxFieldDescIterator::ALL_FIELDS,
	(EncApproxFieldDescIterator::FixUpEncFields | EncApproxFieldDescIterator::OnlyEncFields));
	PTR_FieldDesc pField;
	while ((pField = fdIterator.Next()) != NULL)
	{
	mdFieldDef token = pField->GetMemberDef();
	if (tokenToFind == token)
	{
	LOG((LF_ENC, LL_INFO100, "MT:GFDBI: Found pField:%p\n", pField));
	return pField;
	}
	}
	LOG((LF_ENC, LL_INFO100, "MT:GFDBI: Failed to find rid:0x%08x\n", rid));
	return NULL;
	}

runtime/src/coreclr/vm/memberload.cpp

Lines 361 to 375 in 2905fe7

	if (pFD->IsStatic() && pMT->HasGenericsStaticsInfo())
	{
	//
	// <NICE> this is duplicated logic GetFieldDescByIndex </NICE>
	//
	INDEBUG(mdFieldDef token = pFD->GetMemberDef();)
	DWORD pos = static_cast<DWORD>(pFD - (pMT->GetApproxFieldDescListRaw() + pMT->GetNumIntroducedInstanceFields()));
	_ASSERTE(pos >= 0 && pos < pMT->GetNumStaticFields());
	pFD = pMT->GetGenericsStaticFieldDescs() + pos;
	_ASSERTE(pFD->GetMemberDef() == token);
	_ASSERTE(!pFD->IsSharedByGenericInstantiations());
	_ASSERTE(pFD->GetEnclosingMethodTable() == pMT);
	}

We need to add a test and reconcile these two paths. Hopefully removing the code duplication to avoid this in the future.

[AaronRobinsonMSFT]

@mikelle-rogers Please let me know if you'd like me to look into this.

[mikelle-rogers]

After taking out the assert _ASSERTE(pos >= 0 && pos < pMT->GetNumStaticFields()); this fails in memberload.cpp, in GetDescFromMemberRef, with the field with name <>9__1_0. It fails with an AV in clr when this field calls GetMemberDef().

@AaronRobinsonMSFT

[mikelle-rogers]

Close via pull request #89145

[jkotas]

We still have tests disabled against this issue

runtime/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs

Line 886 in 3f28b1a

[ActiveIssue("https://github.com/dotnet/runtime/issues/87574", TestRuntimes.CoreCLR)]

[tommcdon]

Moving to future as this issue does not seem to be affecting any customer related scenarios