Some functions with outlined prologs fail to catch exceptions

[ellishg]

We have been investigating a bug in an iOS app where a function is failing to catch an exception, leading to a crash. The related functions had their prologs outlined using -homogeneous-prolog-epilog, and disabling that optimization fixed the crash. We also found that reverting 6e54fcc (https://reviews.llvm.org/D153098) also fixed crash when the optimization is enabled.

Since I don't know a lot about CFI instructions, I don't have a small reproducer, but I am happy to provide IR for a few functions if that helps.

@igorkudrin @MaskRay @smeenai Do any of you have some idea of what's going on? Can this diff be safely reverted? Or can we fix somehow?

[efriedma-quic]

If you can provide the asm for a function with/without homogeneous-prolog-epilog that's causing an issue, it might be something obvious, like we're accidentally dropping a CFI directive somewhere.

[ellishg]

I'll try to bisect the functions to identify which one is broken. If I find it, I'll share the assembly.

[ellishg]

We've isolated the problematic function (which was in the middle of the call stack between the function that threw and the function that should have caught the exception) and dumped the MIR before and after the revert. The only difference is the location of these CFI instructions.

--- a/before-revert.txt
+++ b/after-revert.txt
bb.0 (%ir-block.2):
   successors: %bb.2(0x30000000), %bb.1(0x50000000); %bb.2(37.50%), %bb.1(62.50%)
   liveins: $x0, $x8, $lr, $x19, $x20
   frame-setup HOM_Prolog $lr, $fp, $x19, $x20, 16
-  $sp = frame-setup SUBXri $sp, 16, 0
   frame-setup CFI_INSTRUCTION def_cfa $w29, 16
   frame-setup CFI_INSTRUCTION offset $w30, -8
   frame-setup CFI_INSTRUCTION offset $w29, -16
   frame-setup CFI_INSTRUCTION offset $w19, -24
   frame-setup CFI_INSTRUCTION offset $w20, -32
+  $sp = frame-setup SUBXri $sp, 16, 0
   renamable $x19 = COPY $x8
   DBG_VALUE 0, $noreg, !"manifest_block", !DIExpression()
   DBG_VALUE $x19, $noreg, !"manifest_store", !DIExpression(DW_OP_deref)

[ellishg]

I was able to find a reduced test that shows that .cfi_def_cfa_offset is missing.

; RUN: llc -mtriple=aarch64-linux-gnu < %s | FileCheck %s

target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
target triple = "arm64-apple-ios13.4.0"

%"class.std::__1::vector" = type { ptr, ptr, %"class.std::__1::__compressed_pair.1" }
%"class.std::__1::__compressed_pair.1" = type { %"struct.std::__1::__compressed_pair_elem.2" }
%"struct.std::__1::__compressed_pair_elem.2" = type { ptr }

define void @foo(ptr sret(%"class.std::__1::vector") %0) #0 {
; CHECK-LABEL: foo:
; CHECK:       // %bb.0:
; CHECK-NEXT:     sub	sp, sp, #48
; The following CFI instruction is missing after 6e54fcc
; CHECK-NEXT:     .cfi_def_cfa_offset 48
; CHECK-NEXT:     stp	x29, x30, [sp, #16]             // 16-byte Folded Spill
; CHECK-NEXT:     str	x19, [sp, #32]                  // 8-byte Folded Spill
; CHECK-NEXT:     add	x29, sp, #16
; CHECK-NEXT:     .cfi_def_cfa w29, 32
; CHECK-NEXT:     .cfi_offset w19, -16
; CHECK-NEXT:     .cfi_offset w30, -24
; CHECK-NEXT:     .cfi_offset w29, -32
; CHECK-NEXT:     mov	x19, x8

  %2 = alloca ptr, i32 0, align 8
  %3 = alloca i64, i32 0, align 8
  call void @bar()
  call void @llvm.memset.p0.i64(ptr %0, i8 0, i64 1, i1 false)
  ret void
}

declare void @bar()

; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: write)
declare void @llvm.memset.p0.i64(ptr nocapture writeonly, i8, i64, i1 immarg) #1

attributes #0 = { "frame-pointer"="non-leaf" }
attributes #1 = { nocallback nofree nounwind willreturn memory(argmem: write) }

I found these comments to be suspicious: https://reviews.llvm.org/D153098#4462424

In D153098#4462255, @MaskRay wrote:
Have you checked all the lost .cfi_def_cfa_offset and verified that they are all non-important?

The patch preserves the last .cfi_def_cfa or .cfi_def_cfa_offset instruction in the prolog, which is enough for synchronous unwind tables.

But for this test, there is no .cfi_def_cfa_offset instruction in the prolog at all. You can also see in the diff that these instructions are completely removed from foo in llvm/test/CodeGen/AArch64/arm64-shrink-wrapping.ll.

llvm-project/llvm/test/CodeGen/AArch64/arm64-shrink-wrapping.ll

Lines 9 to 20 in 7840c00

	define i32 @foo(i32 %a, i32 %b) {
	; ENABLE-LABEL: foo:
	; ENABLE: ; %bb.0:
	; ENABLE-NEXT: cmp w0, w1
	; ENABLE-NEXT: b.ge LBB0_2
	; ENABLE-NEXT: ; %bb.1: ; %true
	; ENABLE-NEXT: sub sp, sp, #32
	; ENABLE-NEXT: stp x29, x30, [sp, #16] ; 16-byte Folded Spill
	; ENABLE-NEXT: add x29, sp, #16
	; ENABLE-NEXT: .cfi_def_cfa w29, 16
	; ENABLE-NEXT: .cfi_offset w30, -8
	; ENABLE-NEXT: .cfi_offset w29, -16

[efriedma-quic]

The "CFA" is a fixed position on the stack, the value of the stack pointer on entry to the function. cfi_def_cfa_offset/cfi_def_cfa provide different ways to define it: relative to the current sp, or relative to some other register. So the directive is redundant, and your "reduced" testcase is doesn't show a bug, I think.

Can you show llvm-dwarfdump output for the function that's causing the issue before/after the patch?

[ellishg]

Here are the dwarfdump outputs:
before-revert.txt
after-revert.txt

[efriedma-quic]

That doesn't have what I'm looking for... I guess llvm-dwarfdump can't see the unwind. Maybe try llvm-objdump --unwind-info

[ellishg]

I'm not seeing any unwind flag in llvm-dwarfdump :(

[efriedma-quic]

I said llvm-objdump?

[ellishg]

Oh sorry, here is the output of llvm-objdump -u <object>:
after-revert.txt
before-revert.txt

Add here are the addresses of the problematic function:

$ nm after-revert | grep _foo
000000000015b4e4 t __foo
$ nm before-revert | grep _foo
00000000001590cc t __foo

[efriedma-quic]

So before-revert, the compact unwind encoding is 0x0000000. That seems suspicious. Compact-unwind is MachO specific, though; I'm not really familiar with how it's supposed to work.

[ellishg]

I did notice that, but I'm not familiar with the compact unwind format either. @thevinster @int3 do you have an idea of what's going on?

[drodriguez]

The original change https://reviews.llvm.org/D153098 recreates a set of variables in emitDefineCFAWithFP:

llvm-project/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

Lines 1650 to 1653 in 43b9888

	const AArch64Subtarget &STI = MF.getSubtarget<AArch64Subtarget>();
	const AArch64RegisterInfo *TRI = STI.getRegisterInfo();
	const TargetInstrInfo *TII = STI.getInstrInfo();
	AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();

However, the variable AFI in emitPrologue was further customized with calls setRedZone in

llvm-project/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

Line 1744 in 43b9888

AFI->setHasRedZone(false);

, setTaggedPointerBase in

llvm-project/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

Lines 1825 to 1827 in 43b9888

	AFI->setTaggedBasePointerOffset(-MFI.getObjectOffset(*TBPI));
	else
	AFI->setTaggedBasePointerOffset(MFI.getStackSize());

, setLocalStackSize in

llvm-project/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

Line 1844 in 43b9888

AFI->setLocalStackSize(NumBytes);

, and maybe others.

I don't know if any of those calls are important for the code of emitDefineCFAWithFP, but if they are, and since both calls of emitDefineCFAWithFP are part of emitPrologue, why those variables were not passed as arguments, so their setup is exactly the same as it was in emitPrologue? If one of the calls (the original change seems to add the second one) needs a cleaner AFI, it can still generate one and pass it to the function.