[PATCH] D69723: [RISCV][RFC] Fix wrong CFI directives

[Luís Marques via Phabricator via llvm-commits]

luismarques created this revision.
luismarques added reviewers: asb, lenary, shiva0217.
Herald added subscribers: llvm-commits, sameer.abuasal, pzheng, s.egerton, Jim, benna, psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01, atanasyan, edward-jones, zzheng, MaskRay, jrtc27, kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar, hiraditya, arichardson, sdardis.
Herald added a project: LLVM.
luismarques added a parent revision: D69385: [RISCV] Fix CFA when doing split sp adjustment with fp.

Currently there are CFI directives being emitted whose effects incorrectly propagate beyond the basic block they are in. For instance:

  define void @branch_and_tail_call(i1 %a) {
  ; RV32-LABEL: branch_and_tail_call:
  ; RV32:       # %bb.0:
  ; RV32-NEXT:    addi sp, sp, -16
  ; RV32-NEXT:    .cfi_def_cfa_offset 16
  ; RV32-NEXT:    sw ra, 12(sp)
  ; RV32-NEXT:    .cfi_offset ra, -4
  ; RV32-NEXT:    andi a0, a0, 1
  ; RV32-NEXT:    beqz a0, .LBB2_2
  ; RV32-NEXT:  # %bb.1: # %blue_pill
  ; RV32-NEXT:    lw ra, 12(sp)
  ; RV32-NEXT:    .cfi_restore ra
  ; RV32-NEXT:    addi sp, sp, 16
  ; RV32-NEXT:    .cfi_def_cfa_offset 0
  ; RV32-NEXT:    tail foo
  ; RV32-NEXT:  .LBB2_2: # %red_pill
  ; RV32-NEXT:    call bar
  ; RV32-NEXT:    lw ra, 12(sp)
  ; RV32-NEXT:    .cfi_restore ra
  ; RV32-NEXT:    addi sp, sp, 16
  ; RV32-NEXT:    .cfi_def_cfa_offset 0
  ; RV32-NEXT:    ret

The last `.cfi_restore ra` will try to restore `ra` based on the incorrect offset of the ` .cfi_def_cfa_offset 0`, when it should still be based on a `.cfi_def_cfa_offset 16`.

The emission of incorrect offsets affects the stack unwinding. For instance, without this patch the following program will not correctly unwind, and will get stuck (compile with clang, link with a gnu toolchain, test with qemu):

  void three() {
      throw 7;
  }
  
  void two(void) {
      try {
          three();
      } catch(int &c) {
          throw 42;
      }
  }
  
  int main() {
      try {
          two();
      }
      catch(...) {
      }
      return 0;
  }

This patch tries to solve this problem by not emitting the epilogue CFI directives, as is done for other LLVM targets (e.g. MIPS). The epilogue CFI directives were probably added to follow the footsteps of GCC. But GCC is able to solve this problem by, for instance, making use of `.cfi_remember_state` and `.cfi_restore_state` when necessary to make sure that the effects of CFI directives does not propagate beyond the intended BBs. Since adding support for that approach would probably not be trivial, for now we just disable emitting the epilogue directives. Feedback on whether there are issues with this simpler approach (and why it doesn't affect other LLVM targets) would be appreciated.

This patch deletes code fixed by D69385 <https://reviews.llvm.org/D69385>. Since we might want to reintroduce the deleted code (e.g. if we start making use of `.cfi_remember_state`) it's IMO a good idea to first apply D69385 <https://reviews.llvm.org/D69385>, so that if the code is reintroduced no bugs are reintroduced as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69723

Files:
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/test/CodeGen/RISCV/exception-pointer-register.ll
  llvm/test/CodeGen/RISCV/frame-info.ll
  llvm/test/CodeGen/RISCV/large-stack.ll
  llvm/test/CodeGen/RISCV/split-offsets.ll
  llvm/test/CodeGen/RISCV/srem-lkk.ll
  llvm/test/CodeGen/RISCV/srem-vector-lkk.ll
  llvm/test/CodeGen/RISCV/urem-lkk.ll
  llvm/test/CodeGen/RISCV/urem-vector-lkk.ll
  llvm/test/CodeGen/RISCV/vararg.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69723.227486.patch
Type: text/x-patch
Size: 49986 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191101/37bf7cec/attachment-0001.bin>