From 8680d6f28f572051b0d06d6264e9a59a0c701927 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Sim=C3=B5es?= Date: Fri, 24 Jul 2026 11:53:05 +0100 Subject: [PATCH 1/3] Fix nested generic constructor - Now properly resolving nested types in CEE_NEWOBJ. - Update CLAUDE with reasoning and new finding. Assisted by Claude Opus 4.8. --- src/CLR/Core/CLAUDE.md | 59 +++++++--------- src/CLR/Core/Interpreter.cpp | 133 ++++++++++++++++++++++++++++++++++- 2 files changed, 156 insertions(+), 36 deletions(-) diff --git a/src/CLR/Core/CLAUDE.md b/src/CLR/Core/CLAUDE.md index fd8bafa779..47805c94eb 100644 --- a/src/CLR/Core/CLAUDE.md +++ b/src/CLR/Core/CLAUDE.md @@ -349,40 +349,31 @@ slots: Passing no instance here would leave every VAR field wrongly typed as the OBJECT block that `ExtractHeapBlocksForObjects` produced. -### KNOWN LIMITATION — nested generic construction (deferred) - -A generic type whose `.cctor` constructs a **different** generic type -parameterized by the holder's own type parameter is not yet supported. -Canonical shape: - -```csharp -static class Holder { - static readonly Box Field = new Box(); // Box — open target -} -``` - -`Holder`'s `.cctor` emits `newobj Box::.ctor()` where `!0` is -`Holder`'s `T`. `CEE_NEWOBJ` (`Interpreter.cpp`, §7) only knows how to -prefer the caller's closed TypeSpec when caller and callee share a -**typedef** (the `List`-builds-`List` case). Here the typedefs -differ (`Holder` vs `Box`), so the pushed `Box::.ctor` frame inherits the -**open** `Box` as its `genericType`. Any `new T[]` / VAR use inside that -ctor then fails to resolve `T`, the `.cctor` aborts before its `stsfld`, -and the per-instantiation field stays null. Symptom: `NullReferenceException` -on first use of the static field, even though the field slot was allocated. - -The demand gate (§10) and the bare-VAR storage fix above are **not** the -problem — the `.cctor` is scheduled correctly; it dies inside its own body. - -A proper fix must resolve the callee's open type argument (`Box`) to the -concrete closed form (`Box`) via the caller's closed `genericType`, and -flow that closed instantiation into **both** the `NewObject` object tag and -the pushed ctor frame's `genericType` (so single-VAR cases can also lean on -the `arrayElementType` chain in §5/§8). You cannot rely on a matching closed -TypeSpec row already existing in metadata — it only exists when some other -non-generic site happens to use the same closed instantiation. Repro: -`Generic_StaticField_GenericHolder` in `NFUnitTestClasses/UnitTestGenericStaticTests.cs` -(currently commented out with a pointer here). +### Nested generic construction + +When a generic `.cctor` constructs a **different** generic type parameterized +by the holder's own type parameter (`Holder` building `Box`), the +`newobj Box::.ctor()` leaves the callee's owner TypeSpec **open** because +caller and callee have different typedefs — the §7 caller-preference does not +fire. The pushed ctor frame inherits the open `Box`, so `new T[]` inside +the ctor can't resolve `T`; the `.cctor` aborts before `stsfld` and the +static field stays null. + +Resolution is in `CEE_NEWOBJ` (after `GetDeclaringType`): parse the open +owner TypeSpec, resolve each argument against the caller's closed TypeSpec +via `GetGenericParam`, then either find a matching closed TypeSpec row in +metadata or fall back to `arrayElementType` for single-VAR cases. + +**Invariant: never set both `genericType` and `arrayElementType`** on the +same ctor frame — doing so corrupts `newarr`'s element-type resolution +(native `AccessViolation`). The `arrayElementType` fallback is used **only** +when no closed TypeSpec row exists. + +**Why not widen the `ResolveToken` gate:** `ResolveToken` has a closed-row +search for MVAR / `arrayElementType` cases, gated so it skips pure-VAR +callers. Widening that gate made the search fire for every generic +`newobj`/`call`, crashing during startup `.cctor` crawling. The fix is +scoped inside `CEE_NEWOBJ` behind a strict nested-case guard. --- diff --git a/src/CLR/Core/Interpreter.cpp b/src/CLR/Core/Interpreter.cpp index add2ba6c75..ef982c6197 100644 --- a/src/CLR/Core/Interpreter.cpp +++ b/src/CLR/Core/Interpreter.cpp @@ -3067,6 +3067,130 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg) NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE); } + // Nested generic construction (Holder builds Box). See CLAUDE.md §9. + const CLR_RT_TypeSpec_Index *nestedGenericTag = nullptr; + + if (cls.target->genericParamCount > 0 && stack->m_call.genericType != nullptr && + NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType) && calleeInst.genericType != nullptr && + NANOCLR_INDEX_IS_VALID(*calleeInst.genericType)) + { + CLR_RT_TypeSpec_Instance callerTs{}; + CLR_RT_TypeSpec_Instance calleeOwnerTs{}; + + if (callerTs.InitializeFromIndex(*stack->m_call.genericType) && + calleeOwnerTs.InitializeFromIndex(*calleeInst.genericType) && + NANOCLR_INDEX_IS_VALID(calleeOwnerTs.genericTypeDef) && + calleeOwnerTs.genericTypeDef.data == cls.data && callerTs.genericTypeDef.data != cls.data && + callerTs.IsClosedGenericType() && !calleeOwnerTs.IsClosedGenericType()) + { + CLR_RT_SignatureParser ownerParser{}; + ownerParser.Initialize_TypeSpec(calleeOwnerTs); + + CLR_RT_SignatureParser::Element ownerElem{}; + if (SUCCEEDED(ownerParser.Advance(ownerElem)) && ownerElem.DataType == DATATYPE_GENERICINST && + SUCCEEDED(ownerParser.Advance(ownerElem))) + { + int argCount = ownerElem.GenParamCount; + CLR_RT_TypeDef_Index resolvedArgs[8]; + bool allResolved = (argCount > 0 && argCount <= 8); + + for (int a = 0; a < argCount && allResolved; a++) + { + CLR_RT_SignatureParser::Element argElem{}; + if (FAILED(ownerParser.Advance(argElem))) + { + allResolved = false; + break; + } + + if (argElem.DataType == DATATYPE_VAR) + { + CLR_RT_SignatureParser::Element paramElem{}; + if (callerTs.GetGenericParam(argElem.GenericParamPosition, paramElem) && + NANOCLR_INDEX_IS_VALID(paramElem.Class)) + { + resolvedArgs[a] = paramElem.Class; + } + else + { + allResolved = false; + } + } + else if (NANOCLR_INDEX_IS_VALID(argElem.Class)) + { + resolvedArgs[a] = argElem.Class; + } + else + { + allResolved = false; + } + } + + if (allResolved) + { + const CLR_RT_TypeSpec_Index *closedMatch = nullptr; + + for (size_t ai = 0; ai < g_CLR_RT_TypeSystem.m_assembliesMax && closedMatch == nullptr; + ai++) + { + CLR_RT_Assembly *pASSM = g_CLR_RT_TypeSystem.m_assemblies[ai]; + if (pASSM == nullptr) + { + continue; + } + + for (int tsIdx = 0; tsIdx < pASSM->tablesSize[TBL_TypeSpec]; tsIdx++) + { + const CLR_RT_TypeSpec_Index *candidateIdx = + &pASSM->crossReferenceTypeSpec[tsIdx].genericType; + + if (!NANOCLR_INDEX_IS_VALID(*candidateIdx)) + { + continue; + } + + CLR_RT_TypeSpec_Instance candidateInst{}; + if (!candidateInst.InitializeFromIndex(*candidateIdx) || + candidateInst.genericTypeDef.data != cls.data || + !candidateInst.IsClosedGenericType()) + { + continue; + } + + bool argsMatch = true; + for (int a = 0; a < argCount && argsMatch; a++) + { + CLR_RT_SignatureParser::Element candidateArg{}; + if (!candidateInst.GetGenericParam(a, candidateArg) || + candidateArg.Class.data != resolvedArgs[a].data) + { + argsMatch = false; + } + } + + if (argsMatch) + { + closedMatch = candidateIdx; + break; + } + } + } + + if (closedMatch != nullptr) + { + calleeInst.genericType = closedMatch; + nestedGenericTag = closedMatch; + } + else if (argCount == 1) + { + // Never set alongside genericType above — see CLAUDE.md §9. + calleeInst.arrayElementType = resolvedArgs[0]; + } + } + } + } + } + { const CLR_RT_TypeSpec_Index *tsForCctor = nullptr; @@ -3172,11 +3296,16 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg) top->SetObjectReference(nullptr); // NEWOBJ: prefer the caller's closed TypeSpec over the callee's (open) - // MethodRef TypeSpec. See CLAUDE.md "NEWOBJ on generic types". + // MethodRef TypeSpec. See CLAUDE.md §7, §9. const CLR_RT_TypeSpec_Index *genericTypeForContext = nullptr; + if (nestedGenericTag != nullptr) + { + genericTypeForContext = nestedGenericTag; + } // Prefer the caller's generic type if available and valid - if (stack->m_call.genericType != nullptr && NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType)) + else if (stack->m_call.genericType != nullptr && + NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType)) { genericTypeForContext = stack->m_call.genericType; } From ca318d7e22a34aef1afdeeb2524f866a4a69d801 Mon Sep 17 00:00:00 2001 From: nfbot Date: Fri, 24 Jul 2026 11:34:24 +0000 Subject: [PATCH 2/3] Code style fixes Automated fixes for code style. --- src/CLR/Core/Interpreter.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/CLR/Core/Interpreter.cpp b/src/CLR/Core/Interpreter.cpp index ef982c6197..33dc2c00db 100644 --- a/src/CLR/Core/Interpreter.cpp +++ b/src/CLR/Core/Interpreter.cpp @@ -3087,8 +3087,8 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg) ownerParser.Initialize_TypeSpec(calleeOwnerTs); CLR_RT_SignatureParser::Element ownerElem{}; - if (SUCCEEDED(ownerParser.Advance(ownerElem)) && ownerElem.DataType == DATATYPE_GENERICINST && - SUCCEEDED(ownerParser.Advance(ownerElem))) + if (SUCCEEDED(ownerParser.Advance(ownerElem)) && + ownerElem.DataType == DATATYPE_GENERICINST && SUCCEEDED(ownerParser.Advance(ownerElem))) { int argCount = ownerElem.GenParamCount; CLR_RT_TypeDef_Index resolvedArgs[8]; @@ -3130,7 +3130,8 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg) { const CLR_RT_TypeSpec_Index *closedMatch = nullptr; - for (size_t ai = 0; ai < g_CLR_RT_TypeSystem.m_assembliesMax && closedMatch == nullptr; + for (size_t ai = 0; + ai < g_CLR_RT_TypeSystem.m_assembliesMax && closedMatch == nullptr; ai++) { CLR_RT_Assembly *pASSM = g_CLR_RT_TypeSystem.m_assemblies[ai]; @@ -3304,8 +3305,8 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg) genericTypeForContext = nestedGenericTag; } // Prefer the caller's generic type if available and valid - else if (stack->m_call.genericType != nullptr && - NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType)) + else if ( + stack->m_call.genericType != nullptr && NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType)) { genericTypeForContext = stack->m_call.genericType; } From 7bec3403c63e91329eb8248655364e48ff19996c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Sim=C3=B5es?= Date: Fri, 24 Jul 2026 13:00:11 +0100 Subject: [PATCH 3/3] Fix from code review --- src/CLR/Core/Interpreter.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/CLR/Core/Interpreter.cpp b/src/CLR/Core/Interpreter.cpp index 33dc2c00db..b1e6cce32b 100644 --- a/src/CLR/Core/Interpreter.cpp +++ b/src/CLR/Core/Interpreter.cpp @@ -3096,6 +3096,8 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg) for (int a = 0; a < argCount && allResolved; a++) { + int targetAvail = ownerParser.Available() - 1; + CLR_RT_SignatureParser::Element argElem{}; if (FAILED(ownerParser.Advance(argElem))) { @@ -3103,7 +3105,11 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg) break; } - if (argElem.DataType == DATATYPE_VAR) + if (argElem.Levels > 0 || argElem.DataType == DATATYPE_GENERICINST) + { + allResolved = false; + } + else if (argElem.DataType == DATATYPE_VAR) { CLR_RT_SignatureParser::Element paramElem{}; if (callerTs.GetGenericParam(argElem.GenericParamPosition, paramElem) && @@ -3124,6 +3130,16 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg) { allResolved = false; } + + while (ownerParser.Available() > targetAvail) + { + CLR_RT_SignatureParser::Element drained{}; + if (FAILED(ownerParser.Advance(drained))) + { + allResolved = false; + break; + } + } } if (allResolved)