JIT: fix bad arm64 move peephole #68631
Conversation
`IF_DR_2G` `(mov sp/reg, reg/sp)` has an implicit reg2, so an unwary peephole opt might mistakenly think it is `mov reg, x0`. If the opt is unlucky and the next instruction is a real `mov reg, x0` the peephole opt might suppress the second instruction, leaving the wrong value in `reg`. Fix is to just disallow `IF_DR_2G` in the peephole. Fixes dotnet#68527.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue Details
Fix is to just disallow Fixes #68527.
|
|
@kunalspathak PTAL If we see any diffs here we might think about backporting this. But seems unlikely we'll see any. |
There was a problem hiding this comment.
That's a nasty one and I continue to have similar thoughts that I had in #57016 (review). "I can imagine the pain to get to the bottom of this!" LGTM.
It wasn't that bad. Once I got an updated windbg I used If you look at the diffs from #57016 the bug is there waiting for someone to spot it. Unfortunately, I assumed the previous assert for the various instr forms was sensible... |
ghost
locked as resolved and limited conversation to collaborators
IF_DR_2G(mov sp/reg, reg/sp)has an implicit reg2, so an unwary peephole optmight mistakenly think it is
mov reg, x0. If the opt is unlucky and the nextinstruction is a real
mov reg, x0the peephole opt might suppress the secondinstruction, leaving the wrong value in
reg.Fix is to just disallow
IF_DR_2Gin the peephole.Fixes #68527.