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: retThe 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.
Should we remove this cfi directive, too?