Skip to content

Commit

Permalink
JIT: Fix initblk/cpblk and STORE_DYN_BLK size mismatch (#78930)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jakobbotsch authored Nov 30, 2022
1 parent 8f543a1 commit becfc23
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<char>(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;
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/codegenlinear.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/gtlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/morphblock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1571,9 +1571,8 @@ GenTree* Compiler::fgMorphStoreDynBlock(GenTreeStoreDynBlk* tree)
if (tree->gtDynamicSize->IsIntegralConst())
{
int64_t size = tree->gtDynamicSize->AsIntConCommon()->IntegralValue();
assert(FitsIn<int32_t>(size));

if (size != 0)
if ((size != 0) && FitsIn<int32_t>(size))
{
GenTree* lhs = gtNewBlockVal(tree->Addr(), static_cast<unsigned>(size));
GenTree* asg = gtNewAssignNode(lhs, tree->Data());
Expand Down

0 comments on commit becfc23

Please sign in to comment.