Don't emit CFI instructions at the end of a function

[aprantl]

Summary

When functions are terminated by unreachable instructions, the last
instruction might trigger a CFI instruction to be generated. However,
emitting it would be be illegal since the function (and thus the FDE the CFI is in)
has already ended with the previous instruction.

Darwin's dwarfdump --verify --eh-frame complains about this and the
specification supports this.
Relevant bits from the DWARF 5 standard (6.4 Call Frame Information):

"[The] address_range [field in an FDE]: The number of bytes of
 program instructions described by this entry."
    
"Row creation instructions: [...]
 The new location value is always greater than the current one."

The first quotation implies that a CFI cannot describe a target
address outside of the enclosing FDE's range.

rdar://problem/26244988

[MatzeB]

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

941–947 ↗

(On Diff #95821)

Should this look ignore MachineInstr::isTransient() instructions in general as they all shouldn't generate any actual machine code.

[aprantl]

Address review feedback. Thanks!

[MatzeB]

LGTM. Though maybe wait a little longer in case there is feedback from someone who actually knows the AsmPrinter.

[echristo]

(I do know the asm printer and this looks ok to me :)

llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::emitCFIInstruction(const MachineInstr &MI) {
   ExceptionHandling ExceptionHandlingType = MAI->getExceptionHandlingType();
   if (ExceptionHandlingType != ExceptionHandling::DwarfCFI &&
       ExceptionHandlingType != ExceptionHandling::ARM)
     return;
 
   if (needsCFIMoves() == CFI_M_None)
     return;
 
+  // If there is no "real" instruction following this CFI instruction, skip
+  // emitting it; it would be beyond the end of the function's FDE range.
+  auto *MBB = MI.getParent();
+  auto I = std::next(MI.getIterator());
+  while (I != MBB->end() && I->isTransient())
+    ++I;
+  if (I == MBB->instr_end() &&
+      MBB->getReverseIterator() == MBB->getParent()->rbegin())
+    return;
+
   const std::vector<MCCFIInstruction> &Instrs = MF->getFrameInstructions();
   unsigned CFIIndex = MI.getOperand(0).getCFIIndex();
   const MCCFIInstruction &CFI = Instrs[CFIIndex];
   emitCFIInstruction(CFI);
 }
 
 void AsmPrinter::emitFrameAlloc(const MachineInstr &MI) {
   // The operands are the MCSymbol and the frame offset of the allocation.

llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll

 ; LINUX-NO-FP: func:
 ; LINUX-NO-FP-NEXT: {{^}}.L[[BEGIN:.*]]:{{$}}
 ; LINUX-NO-FP-NEXT: .cfi_startproc
 ; LINUX-NO-FP-NEXT: {{^}}#
 ; LINUX-NO-FP-NEXT: {{^}}.L[[END:.*]]:{{$}}
 ; LINUX-NO-FP-NEXT: .size   func, .L[[END]]-.L[[BEGIN]]
 ; LINUX-NO-FP-NEXT: .cfi_endproc
 
-; A cfi directive can point to the end of a function. It (and in fact the
-; entire body) could be optimized out because of the unreachable, but we
-; don't do it right now.
+; A cfi directive cannot point to the end of a function.
 ; LINUX-FP: func:
 ; LINUX-FP-NEXT: {{^}}.L[[BEGIN:.*]]:{{$}}
 ; LINUX-FP-NEXT: .cfi_startproc
 ; LINUX-FP-NEXT: {{^}}#
 ; LINUX-FP-NEXT: stwu 1, -16(1)
 ; LINUX-FP-NEXT: stw 31, 12(1)
 ; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT:  .cfi_def_cfa_offset 16
 ; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT: .cfi_offset r31, -4
 ; LINUX-FP-NEXT: mr 31, 1
-; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}
-; LINUX-FP-NEXT: .cfi_def_cfa_register r31
 ; LINUX-FP-NEXT: {{^}}.L[[END:.*]]:{{$}}
 ; LINUX-FP-NEXT: .size   func, .L[[END]]-.L[[BEGIN]]
 ; LINUX-FP-NEXT: .cfi_endproc

llvm/trunk/test/CodeGen/SPARC/empty-functions.ll

 ; RUN: llc < %s -mtriple=sparc-linux-gnu | FileCheck -check-prefix=LINUX-NO-FP %s
 ; RUN: llc < %s -mtriple=sparc-linux-gnu -disable-fp-elim | FileCheck -check-prefix=LINUX-FP %s
 
 define void @func() {
 entry:
   unreachable
 }
 
 ; An empty function is perfectly fine on ELF.
 ; LINUX-NO-FP: func:
 ; LINUX-NO-FP-NEXT: .cfi_startproc
 ; LINUX-NO-FP-NEXT: {{^}}!
 ; LINUX-NO-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-NO-FP-NEXT: .size   func, .L{{.*}}-func
 ; LINUX-NO-FP-NEXT: .cfi_endproc
 
-; A cfi directive can point to the end of a function. It (and in fact the
-; entire body) could be optimized out because of the unreachable, but we
-; don't do it right now.
+; A cfi directive cannot point to the end of a function.
 ; LINUX-FP: func:
 ; LINUX-FP-NEXT: .cfi_startproc
 ; LINUX-FP-NEXT: {{^}}!
 ; LINUX-FP-NEXT: save %sp, -96, %sp
 ; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
-; LINUX-FP-NEXT: .cfi_def_cfa_register %fp
-; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
-; LINUX-FP-NEXT: .cfi_window_save
-; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
-; LINUX-FP-NEXT: .cfi_register 15, 31
-; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT: .size   func, .Lfunc_end0-func
 ; LINUX-FP-NEXT: .cfi_endproc

llvm/trunk/test/CodeGen/X86/eh-frame-unreachable.ll

+; RUN: llc < %s -mtriple=x86_64-apple-darwin | FileCheck %s
+; Test that we don't emit a row that extends beyond the FDE's range_size.
+;
+; CHECK: movq %rsp, %rbp
+; CHECK-NEXT: .cfi_endproc
+; CHECK-NOT: .cfi
+
+define void @f() #0 {
+  unreachable
+}
+attributes #0 = { "no-frame-pointer-elim"="true" }

llvm/trunk/test/CodeGen/X86/empty-functions.ll

 ; CHECK-FP-NEXT: .cfi_startproc
 ; CHECK-FP-NEXT: :
 ; CHECK-FP-NEXT: pushq %rbp
 ; CHECK-FP-NEXT: :
 ; CHECK-FP-NEXT: .cfi_def_cfa_offset 16
 ; CHECK-FP-NEXT: :
 ; CHECK-FP-NEXT: .cfi_offset %rbp, -16
 ; CHECK-FP-NEXT: movq %rsp, %rbp
-; CHECK-FP-NEXT: :
-; CHECK-FP-NEXT: .cfi_def_cfa_register %rbp
 ; CHECK-FP-NEXT: .cfi_endproc
 
 ; An empty function is perfectly fine on ELF.
 ; LINUX-NO-FP: func:
 ; LINUX-NO-FP-NEXT: .cfi_startproc
 ; LINUX-NO-FP-NEXT: {{^}}#
 ; LINUX-NO-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-NO-FP-NEXT: .size   func, .L{{.*}}-func
 ; LINUX-NO-FP-NEXT: .cfi_endproc
 
-; A cfi directive can point to the end of a function. It (and in fact the
-; entire body) could be optimized out because of the unreachable, but we
-; don't do it right now.
+; A cfi directive cannot point to the end of a function.
 ; LINUX-FP: func:
 ; LINUX-FP-NEXT: .cfi_startproc
 ; LINUX-FP-NEXT: {{^}}#
 ; LINUX-FP-NEXT: pushq %rbp
 ; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT:  .cfi_def_cfa_offset 16
 ; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT: .cfi_offset %rbp, -16
 ; LINUX-FP-NEXT: movq        %rsp, %rbp
 ; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}
-; LINUX-FP-NEXT: .cfi_def_cfa_register %rbp
-; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT: .size   func, .Lfunc_end0-func
 ; LINUX-FP-NEXT: .cfi_endproc