[PATCH] D126861: [RISCV] Fix missing stack pointer recover

[Kito Cheng via Phabricator via llvm-commits]

kito-cheng added a comment.

In D126861#3553679 <https://reviews.llvm.org/D126861#3553679>, @reames wrote:

> I'm missing something here.
>
> If we have a frame pointer, shouldn't restoring SP simply be a register move?  Why bother with all of the offset adjustments in this case at all?  Shouldn't we be able to structure the code as:
> if (hasFP) {
>
>   copy SP to FP
>
> } else {
>
>   do it the hard way
>
> }

I think that might less intuitive on the `sp` recover code here, ideally we could restore `sp` by moving `s0` to `sp` and `addi sp, sp, 32` could be removed like this,:

  mv sp, s0, -32
  ld ra, -8(sp) # 8-byte Folded Reload
  ld s0, -16(sp) # 8-byte Folded Reload
  ld s1, -24(sp) # 8-byte Folded Reload

but we can't do that in practice since that is not signal-safe/interrupt-safe, when the `sp` changing means we already released the space of stack, and then anything could happen on those area.

Give a practical example here, we got a signal after execute `mv sp, s0, -32`, and then kernel start to prepare the arguments and put into stack, ok - you could imagine what happened: the stack is corrupt, ra, s0 and s1 in stack might be corrupt, but technically it's not corrupt since you already move the stack pointer that means we release that, and that's same for the interrupt situation.

  mv sp, s0, -32
  ; Get signal or interrupt here!
  ld ra, -8(sp) # 8-byte Folded Reload   ; Signal handler or interrupt handler might overwrite this area,
  ld s0, -16(sp) # 8-byte Folded Reload ; because we already released the stack space!
  ld s1, -24(sp) # 8-byte Folded Reload


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126861/new/

https://reviews.llvm.org/D126861