JIT: Fix initblk/cpblk and STORE_DYN_BLK size mismatch #78930
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
ghost
assigned 
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsSTORE_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
|
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 dotnet#78912
jakobbotsch
force-pushed
the
fix-78912
branch
from
32e1c7e to
49bd3b8
Compare
|
cc @dotnet/jit-contrib Small number of diffs. Alternatively we could move the normalization into |
I assume you meant to add new 32bit overloads instead of changing existing ones, right? |
I don't think introducing overloads would be necessary, there are very few uses of these helpers. |
Ah I thought that |
|
|
||
| if (size != 0) | ||
| if ((size != 0) && FitsIn<int32_t>(size)) |
There was a problem hiding this comment.
If i understand correctly, with your casting changes in importer, we would hit this assert and hence moving this to if-check is necessary?
There was a problem hiding this comment.
We should not see this in practice since we only create these nodes in very limited circumstances, when we see initblk/cpblk. But it wouldn't be considered illegal to have a CNS_INT node in this position with a native int > 2^31 anymore, so we shouldn't have asserts about that.
|
I am wondering what exposed this bug now? |
Don't know, unfortunately that's not easy to figure out. The repro requires a small property to not be inlined which is occurring in stress, so perhaps inlining changes or stress decision changes caused it to be exposed. (edit: furthermore, it requires stress to trigger a multi-reg return to be spilled to a spill temp in that small method) |
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