Skip to content

Commit c22c179

Browse files
EgorBoCopilot
andcommitted
Fix latent stale gtCompileTimeHandle poisoning EmbeddedHandleMap
`GenTree::BashToConst<T>` repurposes an arbitrary node into a constant but never resets `AsIntCon()->gtCompileTimeHandle`, so the field retains stale data from the node's prior life. `optConstantAssertionProp` then re-applies `GTF_ICON_CLASS_HDL` on top of the bashed node based on the VN's handle flags, leaving a CNS_INT with `iconValue = realClassHandle`, the class-handle flag set, but a stale `gtCompileTimeHandle`. When `fgValueNumberTreeConst` later sees this node it overwrites the existing entry in the EmbeddedHandleMap with that stale handle (via `AddOrUpdate`), poisoning lookups. Consumers such as `IsVNTypeHandle` and `VNEvalForCast` then read the garbage back, and `compareTypesForCast` crashes when dereferencing the bogus type handle. Three small fixes: 1. `GenTree::BashToConst<T>`: zero `gtCompileTimeHandle` alongside the existing `gtFieldSeq = nullptr`, matching the "node is now a fresh constant" semantics. 2. `fgValueNumberTreeConst`: only register an entry in the EmbeddedHandleMap when `gtCompileTimeHandle != 0`, so a re-flagged bashed node cannot overwrite a valid mapping placed by a real `gtNewIconEmbHndNode` that shares the same iconValue. 3. `fgValueNumberSpecialIntrinsic`: fix an inverted bail-out that proceeds to `getRuntimeTypePointer` with a 0 handle when the map lookup fails (`&&` + `clsHandle != 0` made the bail unreachable because `clsHandle` is initialized to 0). With fix #2, "no entry" becomes more common, and this site must bail explicitly. Reproduced on System.Memory.Tests / System.Linq.Tests with DOTNET_TieredCompilation=1 against the PR's `IsVNTypeHandle` consumer. Tests pass after the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9e37596 commit c22c179

2 files changed

Lines changed: 12 additions & 5 deletions

File tree

src/coreclr/jit/compiler.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2101,7 +2101,8 @@ void GenTree::BashToConst(T value, var_types type /* = TYP_UNDEF */)
21012101
}
21022102

21032103
AsIntCon()->SetIconValue(static_cast<ssize_t>(value));
2104-
AsIntCon()->gtFieldSeq = nullptr;
2104+
AsIntCon()->gtFieldSeq = nullptr;
2105+
AsIntCon()->gtCompileTimeHandle = 0;
21052106
break;
21062107

21072108
#if !defined(TARGET_64BIT)

src/coreclr/jit/valuenum.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12287,8 +12287,12 @@ void Compiler::fgValueNumberTreeConst(GenTree* tree)
1228712287
const GenTreeIntCon* cns = tree->AsIntCon();
1228812288
const GenTreeFlags handleFlags = tree->GetIconHandleFlag();
1228912289
tree->gtVNPair.SetBoth(vnStore->VNForHandle(cns->IconValue(), handleFlags));
12290-
if (handleFlags == GTF_ICON_CLASS_HDL)
12290+
if ((handleFlags == GTF_ICON_CLASS_HDL) && (cns->gtCompileTimeHandle != 0))
1229112291
{
12292+
// Skip registration when gtCompileTimeHandle is unknown (e.g., the node was created by
12293+
// BashToConst+gtFlags|=GTF_ICON_CLASS_HDL in optConstantAssertionProp). Overwriting an
12294+
// existing valid mapping with 0 would poison the map for any constant assertion-based
12295+
// re-flagging that happens to share the same iconValue as a real embedded handle node.
1229212296
vnStore->AddToEmbeddedHandleMap(cns->IconValue(), cns->gtCompileTimeHandle);
1229312297
}
1229412298
}
@@ -14430,9 +14434,11 @@ bool Compiler::fgValueNumberSpecialIntrinsic(GenTreeCall* call)
1443014434
ValueNum clsVN = typeHandleFuncApp.m_args[0];
1443114435
ssize_t clsHandle = 0;
1443214436

14433-
// NOTE: EmbeddedHandleMapLookup may return 0 for non-0 embedded handle
14434-
if (!vnStore->EmbeddedHandleMapLookup(vnStore->ConstantValue<ssize_t>(clsVN), &clsHandle) &&
14435-
(clsHandle != 0))
14437+
// EmbeddedHandleMapLookup may return false (no entry) or true with a 0 handle when the
14438+
// backing iconNode was produced by BashToConst (i.e., it has no compile-time handle).
14439+
// Either way, we cannot resolve the runtime type, so bail.
14440+
if (!vnStore->EmbeddedHandleMapLookup(vnStore->ConstantValue<ssize_t>(clsVN), &clsHandle) ||
14441+
(clsHandle == 0))
1443614442
{
1443714443
break;
1443814444
}

0 commit comments

Comments
 (0)