Skip to content

Commit

Permalink
JIT: Fix initblk/cpblk and STORE_DYN_BLK size mismatch
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 committed Nov 28, 2022
1 parent f35444a commit 49bd3b8
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
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 @@ -10893,6 +10893,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 @@ -10920,6 +10925,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 49bd3b8

Please sign in to comment.