Skip to content

Commit 7706f54

Browse files
EgorBoegorbotCopilot
authored
[release/10.0] Fix bug in LowerCallMemmove (#125175)
Backport of #123907 to release/10.0 to fix #123748 /cc @EgorBo ## Customer Impact - [x] Customer reported - [ ] Found internally JIT crashes with an access violation (0xC0000005) when compiling calls to C++/CLI functions returning structs with aggregate initialization. The crash occurs in `LowerCallMemmove` during JIT compilation because `BlkOpKindUnroll` was used for `CORINFO_HELP_MEMCPY`, which bypasses the memmove-aware lowering paired with `genCodeForMemmove`. This leads to contained address nodes being passed where non-contained nodes are expected, causing the null dereference. ## Regression - [x] Yes - [ ] No ## Fix Use `BlkOpKindUnrollMemmove` uniformly for both `Memmove` and `MEMCPY` helpers. While slightly more expensive for LSRA (no addressing modes), `CORINFO_HELP_MEMCPY` is rarely used, so the impact is negligible. Also adds asserts to catch contained address nodes early. ## Testing No diffs in [SPMI replay](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1287687&view=ms.vss-build-web.run-extensions-tab). Verified on main via PR #123907. ## Risk Low. The change makes memcpy use the same safe code path as memmove, which is already well-tested. The only behavioral difference is slightly different register allocation for the rare `CORINFO_HELP_MEMCPY` case. Co-authored-by: egorbot <egorbot@egorbo.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 984dd1d commit 7706f54

File tree

1 file changed

+6
-5
lines changed

1 file changed

+6
-5
lines changed

src/coreclr/jit/lower.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2448,6 +2448,8 @@ bool Lowering::LowerCallMemmove(GenTreeCall* call, GenTree** next)
24482448

24492449
GenTree* dstAddr = call->gtArgs.GetUserArgByIndex(0)->GetNode();
24502450
GenTree* srcAddr = call->gtArgs.GetUserArgByIndex(1)->GetNode();
2451+
assert(!dstAddr->isContained());
2452+
assert(!srcAddr->isContained());
24512453

24522454
// TODO-CQ: Try to create an addressing mode
24532455
GenTreeIndir* srcBlk = comp->gtNewIndir(TYP_STRUCT, srcAddr);
@@ -2457,10 +2459,8 @@ bool Lowering::LowerCallMemmove(GenTreeCall* call, GenTree** next)
24572459
GenTreeBlk(GT_STORE_BLK, TYP_STRUCT, dstAddr, srcBlk, comp->typGetBlkLayout((unsigned)cnsSize));
24582460
storeBlk->gtFlags |= (GTF_IND_UNALIGNED | GTF_ASG | GTF_EXCEPT | GTF_GLOB_REF);
24592461

2460-
// TODO-CQ: Use GenTreeBlk::BlkOpKindUnroll here if srcAddr and dstAddr don't overlap, thus, we can
2461-
// unroll this memmove as memcpy - it doesn't require lots of temp registers
2462-
storeBlk->gtBlkOpKind = call->IsHelperCall(comp, CORINFO_HELP_MEMCPY) ? GenTreeBlk::BlkOpKindUnroll
2463-
: GenTreeBlk::BlkOpKindUnrollMemmove;
2462+
// For simplicity, we use BlkOpKindUnrollMemmove even for CORINFO_HELP_MEMCPY.
2463+
storeBlk->gtBlkOpKind = GenTreeBlk::BlkOpKindUnrollMemmove;
24642464

24652465
BlockRange().InsertBefore(call, srcBlk);
24662466
BlockRange().InsertBefore(call, storeBlk);
@@ -2478,7 +2478,8 @@ bool Lowering::LowerCallMemmove(GenTreeCall* call, GenTree** next)
24782478

24792479
JITDUMP("\nNew tree:\n")
24802480
DISPTREE(storeBlk);
2481-
// TODO: This skips lowering srcBlk and storeBlk.
2481+
// We've just lowered srcBlk and storeBlk here and it's now what genCodeForMemmove expects.
2482+
// So the next node to lower is whatever we have after the storeBlk.
24822483
*next = storeBlk->gtNext;
24832484
return true;
24842485
}

0 commit comments

Comments
 (0)