From becfc23bfe74751bdf4b08bc7871339b1c8ce39b Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 30 Nov 2022 10:59:21 +0100 Subject: [PATCH] JIT: Fix initblk/cpblk and STORE_DYN_BLK size mismatch (#78930) STORE_DYN_BLK turns into a call to JIT_MemSet/JIT_MemCpy that go quite directly to memset/memcpy, so the size is actually a native uint. This can cause problems since the JIT does not make any normalization guarantees above 32 bits. Fix #78912 --- src/coreclr/jit/codegencommon.cpp | 2 +- src/coreclr/jit/codegenlinear.cpp | 3 ++- src/coreclr/jit/gtlist.h | 2 +- src/coreclr/jit/importer.cpp | 11 +++++++++++ src/coreclr/jit/morphblock.cpp | 3 +-- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 32d5ec4c934bc..dcd69bd5a974f 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -9393,7 +9393,7 @@ void CodeGen::genPoisonFrame(regMaskTP regLiveIn) #else GetEmitter()->emitIns_R_S(INS_lea, EA_PTRSIZE, REG_ARG_0, (int)varNum, 0); instGen_Set_Reg_To_Imm(EA_4BYTE, REG_ARG_1, static_cast(poisonVal)); - instGen_Set_Reg_To_Imm(EA_4BYTE, REG_ARG_2, size); + instGen_Set_Reg_To_Imm(EA_PTRSIZE, REG_ARG_2, size); genEmitHelperCall(CORINFO_HELP_MEMSET, 0, EA_UNKNOWN); // May kill REG_SCRATCH, so we need to reload it. hasPoisonImm = false; diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index b2c29c24c7939..7fdbe01480603 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -1922,7 +1922,8 @@ void CodeGen::genSetBlockSize(GenTreeBlk* blkNode, regNumber sizeReg) if (!blkNode->OperIs(GT_STORE_DYN_BLK)) { assert((blkNode->gtRsvdRegs & genRegMask(sizeReg)) != 0); - instGen_Set_Reg_To_Imm(EA_4BYTE, sizeReg, blockSize); + // This can go via helper which takes the size as a native uint. + instGen_Set_Reg_To_Imm(EA_PTRSIZE, sizeReg, blockSize); } else { diff --git a/src/coreclr/jit/gtlist.h b/src/coreclr/jit/gtlist.h index cde991961ab81..2593bae80f2ff 100644 --- a/src/coreclr/jit/gtlist.h +++ b/src/coreclr/jit/gtlist.h @@ -87,7 +87,7 @@ GTNODE(OBJ , GenTreeObj ,0,GTK_UNOP|GTK_EXOP) GTNODE(STORE_OBJ , GenTreeObj ,0,GTK_BINOP|GTK_EXOP|GTK_NOVALUE) // Object that MAY have gc pointers, and thus includes the relevant gc layout info. GTNODE(BLK , GenTreeBlk ,0,GTK_UNOP|GTK_EXOP) // Block/object with no gc pointers, and with a known size (e.g. a struct with no gc fields) GTNODE(STORE_BLK , GenTreeBlk ,0,GTK_BINOP|GTK_EXOP|GTK_NOVALUE) // Block/object with no gc pointers, and with a known size (e.g. a struct with no gc fields) -GTNODE(STORE_DYN_BLK , GenTreeStoreDynBlk ,0,GTK_SPECIAL|GTK_NOVALUE) // Dynamically sized block store +GTNODE(STORE_DYN_BLK , GenTreeStoreDynBlk ,0,GTK_SPECIAL|GTK_NOVALUE) // Dynamically sized block store, with native uint size GTNODE(NULLCHECK , GenTreeIndir ,0,GTK_UNOP|GTK_NOVALUE) // Null checks the source GTNODE(ARR_LENGTH , GenTreeArrLen ,0,GTK_UNOP|GTK_EXOP) // single-dimension (SZ) array length diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 3aa4c138d267d..295d72e6f76f8 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -10883,6 +10883,11 @@ void Compiler::impImportBlockCode(BasicBlock* block) op2 = gtNewOperNode(GT_INIT_VAL, TYP_INT, op2); } +#ifdef TARGET_64BIT + // STORE_DYN_BLK takes a native uint size as it turns into call to memset. + op3 = gtNewCastNode(TYP_I_IMPL, op3, /* fromUnsigned */ true, TYP_U_IMPL); +#endif + op1 = new (this, GT_STORE_DYN_BLK) GenTreeStoreDynBlk(op1, op2, op3); size = 0; @@ -10910,6 +10915,12 @@ void Compiler::impImportBlockCode(BasicBlock* block) else { op2 = gtNewOperNode(GT_IND, TYP_STRUCT, op2); + +#ifdef TARGET_64BIT + // STORE_DYN_BLK takes a native uint size as it turns into call to memcpy. + op3 = gtNewCastNode(TYP_I_IMPL, op3, /* fromUnsigned */ true, TYP_U_IMPL); +#endif + op1 = new (this, GT_STORE_DYN_BLK) GenTreeStoreDynBlk(op1, op2, op3); if ((prefixFlags & PREFIX_VOLATILE) != 0) diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp index 1ff93cf9e4628..5bffde1a74477 100644 --- a/src/coreclr/jit/morphblock.cpp +++ b/src/coreclr/jit/morphblock.cpp @@ -1571,9 +1571,8 @@ GenTree* Compiler::fgMorphStoreDynBlock(GenTreeStoreDynBlk* tree) if (tree->gtDynamicSize->IsIntegralConst()) { int64_t size = tree->gtDynamicSize->AsIntConCommon()->IntegralValue(); - assert(FitsIn(size)); - if (size != 0) + if ((size != 0) && FitsIn(size)) { GenTree* lhs = gtNewBlockVal(tree->Addr(), static_cast(size)); GenTree* asg = gtNewAssignNode(lhs, tree->Data());