[AArch64] Correct position of CFI Instruction for Pointer Authentication #137795

DanielKristofKiss · 2025-04-29T12:18:58Z

This reverts partially this commit 0b73b5a.
This is not a clear revert because other changes already landed.
CFI directives like .cfi_negate_ra_state must be emitted after the instruction.
If the execution is stopped before the paciasp instruction is executed the debugger/unwinder would try to authenticated the return address as the .cfi_negate_ra_state already indicates it got signed.

fixes: #137802

…hentication (llvm#121559)" This reverts commit 0b73b5a.

llvmbot · 2025-04-29T12:19:35Z

@llvm/pr-subscribers-backend-aarch64

Author: Daniel Kiss (DanielKristofKiss)

Changes

This reverts commit 0b73b5a.
This is not a clear revert because other changes already landed.
CFI directives like .cfi_negate_ra_state must be emitted after the instruction that causes the effect.
If the execution would stop before the paciasp instruction is executed the debugger/unwinder would try to authenticated the return address as the .cfi_negate_ra_state already indicates it got signed.
The problem of .cfi_negate_ra_state_with_pc to be solved later.

Patch is 51.78 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/137795.diff

14 Files Affected:

(modified) llvm/lib/Target/AArch64/AArch64PointerAuth.cpp (+4-4)
(modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-cfi.ll (+1-1)
(modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-diff-scope-same-key.ll (+6-6)
(modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-non-leaf.ll (+6-6)
(modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-regsave.mir (+1-1)
(modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-same-scope-diff-key.ll (+6-6)
(modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir (+17-17)
(modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-subtarget.ll (+3-3)
(modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-thunk.ll (+8-10)
(modified) llvm/test/CodeGen/AArch64/pacbti-llvm-generated-funcs-2.ll (+2-2)
(modified) llvm/test/CodeGen/AArch64/sign-return-address-cfi-negate-ra-state.ll (+12-12)
(modified) llvm/test/CodeGen/AArch64/sign-return-address-pauth-lr.ll (+35-35)
(modified) llvm/test/CodeGen/AArch64/sign-return-address.ll (+22-22)
(modified) llvm/test/CodeGen/MIR/AArch64/return-address-signing.mir (+3-3)

diff --git a/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp b/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
index 57e84ebfcf403..f0761583baff5 100644
--- a/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
+++ b/llvm/lib/Target/AArch64/AArch64PointerAuth.cpp
@@ -131,20 +131,20 @@ void AArch64PointerAuth::signLR(MachineFunction &MF,
   // No SEH opcode for this one; it doesn't materialize into an
   // instruction on Windows.
   if (MFnI.branchProtectionPAuthLR() && Subtarget->hasPAuthLR()) {
-    emitPACCFI(MBB, MBBI, MachineInstr::FrameSetup, EmitCFI);
     BuildMI(MBB, MBBI, DL,
             TII->get(MFnI.shouldSignWithBKey() ? AArch64::PACIBSPPC
                                                : AArch64::PACIASPPC))
         .setMIFlag(MachineInstr::FrameSetup)
         ->setPreInstrSymbol(MF, MFnI.getSigningInstrLabel());
+    emitPACCFI(MBB, MBBI, MachineInstr::FrameSetup, EmitCFI);
   } else {
     BuildPACM(*Subtarget, MBB, MBBI, DL, MachineInstr::FrameSetup);
-    emitPACCFI(MBB, MBBI, MachineInstr::FrameSetup, EmitCFI);
     BuildMI(MBB, MBBI, DL,
             TII->get(MFnI.shouldSignWithBKey() ? AArch64::PACIBSP
                                                : AArch64::PACIASP))
         .setMIFlag(MachineInstr::FrameSetup)
         ->setPreInstrSymbol(MF, MFnI.getSigningInstrLabel());
+    emitPACCFI(MBB, MBBI, MachineInstr::FrameSetup, EmitCFI);
   }
 
   if (!EmitCFI && NeedsWinCFI) {
@@ -199,17 +199,17 @@ void AArch64PointerAuth::authenticateLR(
     if (MFnI->branchProtectionPAuthLR() && Subtarget->hasPAuthLR()) {
       assert(PACSym && "No PAC instruction to refer to");
       emitPACSymOffsetIntoX16(*TII, MBB, MBBI, DL, PACSym);
-      emitPACCFI(MBB, MBBI, MachineInstr::FrameDestroy, EmitAsyncCFI);
       BuildMI(MBB, MBBI, DL,
               TII->get(UseBKey ? AArch64::AUTIBSPPCi : AArch64::AUTIASPPCi))
           .addSym(PACSym)
           .setMIFlag(MachineInstr::FrameDestroy);
+      emitPACCFI(MBB, MBBI, MachineInstr::FrameDestroy, EmitAsyncCFI);
     } else {
       BuildPACM(*Subtarget, MBB, MBBI, DL, MachineInstr::FrameDestroy, PACSym);
-      emitPACCFI(MBB, MBBI, MachineInstr::FrameDestroy, EmitAsyncCFI);
       BuildMI(MBB, MBBI, DL,
               TII->get(UseBKey ? AArch64::AUTIBSP : AArch64::AUTIASP))
           .setMIFlag(MachineInstr::FrameDestroy);
+      emitPACCFI(MBB, MBBI, MachineInstr::FrameDestroy, EmitAsyncCFI);
     }
 
     if (NeedsWinCFI) {
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-cfi.ll b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-cfi.ll
index e7de54036245a..4bbbe40176313 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-cfi.ll
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-cfi.ll
@@ -9,9 +9,9 @@ define void @a() "sign-return-address"="all" "sign-return-address-key"="b_key" {
 ; CHECK-LABEL:         a:                     // @a
 ; CHECK:               // %bb.0:
 ; CHECK-NEXT:          .cfi_b_key_frame
-; CHECK-NEXT:          .cfi_negate_ra_state
 ; V8A-NEXT:            hint #27
 ; V83A-NEXT:           pacibsp
+; CHECK-NEXT:          .cfi_negate_ra_state
   %1 = alloca i32, align 4
   %2 = alloca i32, align 4
   %3 = alloca i32, align 4
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-diff-scope-same-key.ll b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-diff-scope-same-key.ll
index a26dda1d5c1f1..6a11bef08c740 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-diff-scope-same-key.ll
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-diff-scope-same-key.ll
@@ -5,9 +5,9 @@
 
 define void @a() "sign-return-address"="all" {
 ; CHECK-LABEL:      a:                                     // @a
-; CHECK:      .cfi_negate_ra_state
-; V8A-NEXT:              hint #25
-; V83A-NEXT:             paciasp
+; V8A:              hint #25
+; V83A:             paciasp
+; CHECK-NEXT:      .cfi_negate_ra_state
   %1 = alloca i32, align 4
   %2 = alloca i32, align 4
   %3 = alloca i32, align 4
@@ -52,9 +52,9 @@ define void @b() "sign-return-address"="non-leaf" {
 
 define void @c() "sign-return-address"="all" {
 ; CHECK-LABEL:         c:              // @c
-; CHECK:      .cfi_negate_ra_state
-; V8A-NEXT:              hint #25
-; V83A-NEXT:             paciasp
+; V8A:                 hint #25
+; V83A:                paciasp
+; CHECK-NEXT          .cfi_negate_ra_state
   %1 = alloca i32, align 4
   %2 = alloca i32, align 4
   %3 = alloca i32, align 4
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-non-leaf.ll b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-non-leaf.ll
index 064b2b78c7bc7..1e7224683c6c8 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-non-leaf.ll
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-non-leaf.ll
@@ -8,8 +8,8 @@ define i64 @a(i64 %x) "sign-return-address"="non-leaf" "sign-return-address-key"
 ; V8A-LABEL: a:
 ; V8A:       // %bb.0:
 ; V8A-NEXT:    .cfi_b_key_frame
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #27
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    sub sp, sp, #32
 ; V8A-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; V8A-NEXT:    .cfi_def_cfa_offset 32
@@ -26,8 +26,8 @@ define i64 @a(i64 %x) "sign-return-address"="non-leaf" "sign-return-address-key"
 ; V83A-LABEL: a:
 ; V83A:       // %bb.0:
 ; V83A-NEXT:    .cfi_b_key_frame
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    pacibsp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    sub sp, sp, #32
 ; V83A-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; V83A-NEXT:    .cfi_def_cfa_offset 32
@@ -59,8 +59,8 @@ define i64 @b(i64 %x) "sign-return-address"="non-leaf" "sign-return-address-key"
 ; V8A-LABEL: b:
 ; V8A:       // %bb.0:
 ; V8A-NEXT:    .cfi_b_key_frame
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #27
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    sub sp, sp, #32
 ; V8A-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; V8A-NEXT:    .cfi_def_cfa_offset 32
@@ -77,8 +77,8 @@ define i64 @b(i64 %x) "sign-return-address"="non-leaf" "sign-return-address-key"
 ; V83A-LABEL: b:
 ; V83A:       // %bb.0:
 ; V83A-NEXT:    .cfi_b_key_frame
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    pacibsp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    sub sp, sp, #32
 ; V83A-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; V83A-NEXT:    .cfi_def_cfa_offset 32
@@ -110,8 +110,8 @@ define i64 @c(i64 %x) "sign-return-address"="non-leaf" "sign-return-address-key"
 ; V8A-LABEL: c:
 ; V8A:       // %bb.0:
 ; V8A-NEXT:    .cfi_b_key_frame
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #27
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    sub sp, sp, #32
 ; V8A-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; V8A-NEXT:    .cfi_def_cfa_offset 32
@@ -128,8 +128,8 @@ define i64 @c(i64 %x) "sign-return-address"="non-leaf" "sign-return-address-key"
 ; V83A-LABEL: c:
 ; V83A:       // %bb.0:
 ; V83A-NEXT:    .cfi_b_key_frame
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    pacibsp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    sub sp, sp, #32
 ; V83A-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; V83A-NEXT:    .cfi_def_cfa_offset 32
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-regsave.mir b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-regsave.mir
index 218ee6609c803..9a983cbd6714e 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-regsave.mir
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-regsave.mir
@@ -81,8 +81,8 @@ body:             |
 # CHECK:         name:            bar
 # CHECK:          bb.0:
 # CHECK:            frame-setup EMITBKEY
-# CHECK-NEXT:       frame-setup CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NEXT:       frame-setup PACIBSP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:       frame-setup CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NOT:        OUTLINED_FUNCTION_
 # CHECK:          bb.1:
 # CHECK-NOT:        OUTLINED_FUNCTION_
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-same-scope-diff-key.ll b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-same-scope-diff-key.ll
index 5c45373d8c1d6..87771f5de4f69 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-same-scope-diff-key.ll
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-same-scope-diff-key.ll
@@ -7,8 +7,8 @@
 define void @a() "sign-return-address"="all" {
 ; V8A-LABEL: a:
 ; V8A:       // %bb.0:
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #25
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    sub sp, sp, #32
 ; V8A-NEXT:    .cfi_def_cfa_offset 32
 ; V8A-NEXT:    mov w8, #1 // =0x1
@@ -26,8 +26,8 @@ define void @a() "sign-return-address"="all" {
 ;
 ; V83A-LABEL: a:
 ; V83A:       // %bb.0:
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    paciasp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    sub sp, sp, #32
 ; V83A-NEXT:    .cfi_def_cfa_offset 32
 ; V83A-NEXT:    mov w8, #1 // =0x1
@@ -60,8 +60,8 @@ define void @b() "sign-return-address"="all" "sign-return-address-key"="b_key" {
 ; V8A-LABEL: b:
 ; V8A:       // %bb.0:
 ; V8A-NEXT:    .cfi_b_key_frame
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #27
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    sub sp, sp, #32
 ; V8A-NEXT:    .cfi_def_cfa_offset 32
 ; V8A-NEXT:    mov w8, #1 // =0x1
@@ -80,8 +80,8 @@ define void @b() "sign-return-address"="all" "sign-return-address-key"="b_key" {
 ; V83A-LABEL: b:
 ; V83A:       // %bb.0:
 ; V83A-NEXT:    .cfi_b_key_frame
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    pacibsp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    sub sp, sp, #32
 ; V83A-NEXT:    .cfi_def_cfa_offset 32
 ; V83A-NEXT:    mov w8, #1 // =0x1
@@ -113,8 +113,8 @@ define void @b() "sign-return-address"="all" "sign-return-address-key"="b_key" {
 define void @c() "sign-return-address"="all" {
 ; V8A-LABEL: c:
 ; V8A:       // %bb.0:
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #25
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    sub sp, sp, #32
 ; V8A-NEXT:    .cfi_def_cfa_offset 32
 ; V8A-NEXT:    mov w8, #1 // =0x1
@@ -132,8 +132,8 @@ define void @c() "sign-return-address"="all" {
 ;
 ; V83A-LABEL: c:
 ; V83A:       // %bb.0:
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    paciasp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    sub sp, sp, #32
 ; V83A-NEXT:    .cfi_def_cfa_offset 32
 ; V83A-NEXT:    mov w8, #1 // =0x1
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir
index d4a4b886ec0e3..22e5edef2a939 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir
@@ -86,11 +86,11 @@ body:             |
 # CHECK:          body:             |
 # CHECK-NEXT:         bb.0 (%ir-block.0):
 # CHECK-NEXT:           liveins: $lr
-# CHECK:                frame-setup CFI_INSTRUCTION negate_ra_sign_state
-# CHECK-NEXT:           frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK:                frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:           frame-setup CFI_INSTRUCTION negate_ra_sign_state
 # CHECK:                BL @[[OUTLINED_FUNC:OUTLINED_FUNCTION_[0-9]+]]
-# CHECK:                frame-destroy CFI_INSTRUCTION negate_ra_sign_state
-# CHECK-NEXT:           frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK:                frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:           frame-destroy CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NEXT:           RET undef $lr
 
 ...
@@ -119,11 +119,11 @@ body:             |
 # CHECK:          body:             |
 # CHECK-NEXT:         bb.0 (%ir-block.0):
 # CHECK-NEXT:           liveins: $lr
-# CHECK:                frame-setup CFI_INSTRUCTION negate_ra_sign_state
-# CHECK-NEXT:           frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK:                frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:           frame-setup CFI_INSTRUCTION negate_ra_sign_state
 # CHECK:                BL @[[OUTLINED_FUNC]]
-# CHECK:                frame-destroy CFI_INSTRUCTION negate_ra_sign_state
-# CHECK-NEXT:           frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK:                frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:           frame-destroy CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NEXT:           RET undef $lr
 
 ...
@@ -174,22 +174,22 @@ body:             |
 # CHECK:          body:             |
 # CHECK-NEXT:         bb.0 (%ir-block.0):
 # CHECK-NEXT:           liveins: $lr
-# CHECK:                frame-setup CFI_INSTRUCTION negate_ra_sign_state
-# CHECK-NEXT:           frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK:                frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:           frame-setup CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NOT:            BL @OUTLINED_FUNCTION_{{.*}}
-# CHECK:                frame-destroy CFI_INSTRUCTION negate_ra_sign_state
-# CHECK-NEXT:           frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK:                frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:           frame-destroy CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NEXT:           RET undef $lr
 
 # CHECK-LABEL:    name:            illegal1
 # CHECK:          body:             |
 # CHECK-NEXT:         bb.0 (%ir-block.0):
 # CHECK-NEXT:           liveins: $lr
-# CHECK:                frame-setup CFI_INSTRUCTION negate_ra_sign_state
-# CHECK-NEXT:           frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK:                frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:           frame-setup CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NOT:            BL @OUTLINED_FUNCTION_{{.*}}
-# CHECK:                frame-destroy CFI_INSTRUCTION negate_ra_sign_state
-# CHECK-NEXT:           frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK:                frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:           frame-destroy CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NEXT:           RET undef $lr
 
 # Outlined function that contains only legal sp modifications
@@ -198,8 +198,8 @@ body:             |
 # CHECK-NEXT:       bb.0:
 # CHECK-NEXT: liveins: $lr
 # CHECK-NEXT: {{^  $}}
-# CHECK-NEXT:         frame-setup CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NEXT:         frame-setup PACIASP implicit-def $lr, implicit $lr, implicit $sp
+# CHECK-NEXT:         frame-setup CFI_INSTRUCTION negate_ra_sign_state
 # CHECK-NEXT:         $sp = frame-setup SUBXri $sp, 16, 0
 # CHECK:              $sp = frame-destroy ADDXri $sp, 16, 0
 # CHECK-NEXT:         frame-destroy AUTIASP implicit-def $lr, implicit $lr, implicit $sp
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-subtarget.ll b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-subtarget.ll
index cb43b3ba3e47e..a7ea32952f3b7 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-subtarget.ll
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-subtarget.ll
@@ -9,8 +9,8 @@ define void @a() #0 {
 ; CHECK-LABEL:      a:                                     // @a
 ; CHECK:            // %bb.0:
 ; CHECK-NEXT:               .cfi_b_key_frame
-; CHECK-NEXT:               .cfi_negate_ra_state
 ; CHECK-NEXT:               pacibsp
+; CHECK-NEXT:               .cfi_negate_ra_state
 ; CHECK-NOT:                OUTLINED_FUNCTION_
   %1 = alloca i32, align 4
   %2 = alloca i32, align 4
@@ -33,8 +33,8 @@ define void @b() #0 {
 ; CHECK-LABEL:      b:                                     // @b
 ; CHECK:            // %bb.0:
 ; CHECK-NEXT:               .cfi_b_key_frame
-; CHECK-NEXT:               .cfi_negate_ra_state
 ; CHECK-NEXT:               pacibsp
+; CHECK-NEXT:               .cfi_negate_ra_state
 ; CHECK-NOT:                OUTLINED_FUNCTION_
   %1 = alloca i32, align 4
   %2 = alloca i32, align 4
@@ -57,8 +57,8 @@ define void @c() #1 {
 ; CHECK-LABEL:      c:                                     // @c
 ; CHECK:            // %bb.0:
 ; CHECK-NEXT:               .cfi_b_key_frame
-; CHECK-NEXT:               .cfi_negate_ra_state
 ; CHECK-NEXT:               hint #27
+; CHECK-NEXT:               .cfi_negate_ra_state
 ; CHECK-NOT:                OUTLINED_FUNCTION_
   %1 = alloca i32, align 4
   %2 = alloca i32, align 4
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-thunk.ll b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-thunk.ll
index 0ba4455532925..da68ea5bf0dbc 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-thunk.ll
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-thunk.ll
@@ -10,8 +10,8 @@ declare i32 @thunk_called_fn(i32, i32, i32, i32)
 define i32 @a() #0 {
 ; V8A-LABEL: a:
 ; V8A:       // %bb.0: // %entry
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #25
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
 ; V8A-NEXT:    .cfi_def_cfa_offset 16
 ; V8A-NEXT:    .cfi_offset w30, -16
@@ -27,8 +27,8 @@ define i32 @a() #0 {
 ;
 ; V83A-LABEL: a:
 ; V83A:       // %bb.0: // %entry
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    paciasp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
 ; V83A-NEXT:    .cfi_def_cfa_offset 16
 ; V83A-NEXT:    .cfi_offset w30, -16
@@ -49,8 +49,8 @@ entry:
 define i32 @b() #0 {
 ; V8A-LABEL: b:
 ; V8A:       // %bb.0: // %entry
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #25
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
 ; V8A-NEXT:    .cfi_def_cfa_offset 16
 ; V8A-NEXT:    .cfi_offset w30, -16
@@ -66,8 +66,8 @@ define i32 @b() #0 {
 ;
 ; V83A-LABEL: b:
 ; V83A:       // %bb.0: // %entry
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    paciasp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
 ; V83A-NEXT:    .cfi_def_cfa_offset 16
 ; V83A-NEXT:    .cfi_offset w30, -16
@@ -88,8 +88,8 @@ entry:
 define hidden i32 @c(ptr %fptr) #0 {
 ; V8A-LABEL: c:
 ; V8A:       // %bb.0: // %entry
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #25
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
 ; V8A-NEXT:    .cfi_def_cfa_offset 16
 ; V8A-NEXT:    .cfi_offset w30, -16
@@ -106,8 +106,8 @@ define hidden i32 @c(ptr %fptr) #0 {
 ;
 ; V83A-LABEL: c:
 ; V83A:       // %bb.0: // %entry
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    paciasp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
 ; V83A-NEXT:    .cfi_def_cfa_offset 16
 ; V83A-NEXT:    .cfi_offset w30, -16
@@ -129,8 +129,8 @@ entry:
 define hidden i32 @d(ptr %fptr) #0 {
 ; V8A-LABEL: d:
 ; V8A:       // %bb.0: // %entry
-; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    hint #25
+; V8A-NEXT:    .cfi_negate_ra_state
 ; V8A-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
 ; V8A-NEXT:    .cfi_def_cfa_offset 16
 ; V8A-NEXT:    .cfi_offset w30, -16
@@ -147,8 +147,8 @@ define hidden i32 @d(ptr %fptr) #0 {
 ;
 ; V83A-LABEL: d:
 ; V83A:       // %bb.0: // %entry
-; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    paciasp
+; V83A-NEXT:    .cfi_negate_ra_state
 ; V83A-NEXT:    str x30, [sp, #-16]! // 8-byte Folded Spill
 ; V83A-NEXT:    .cfi_def_cfa_offset 16
 ; V83A-NEXT:    .cfi_offset w30, -16
@@ -176,5 +176,3 @@ attributes #0 = { "sign-return-address"="non-leaf" minsize }
 ; CHECK-NOT:         .cfi_negate_ra_state
 ; CHECK-NOT:         auti{{[a,b]}}sp
 ; CHECK-NOT:         hint #{{[29,31]}}
-;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
-; CHECK: {{.*}}
diff --git a/llvm/test/CodeGen/AArch64/pacbti-llvm-generated-funcs-2.ll b/llvm/test/CodeGen/AArch64/pacbti-llvm-generated-funcs-2.ll
index f823d2aa82ac0..373c4969a9405 100644
--- a/llvm/test/Cod...
[truncated]

ostannard · 2025-04-29T13:01:20Z

When PAuth_LR is used, libunwind relies on this behavior to capture the PC value which will be used to authenticate the return address. Reverting this will turn this from a wrong-debug-info bug into a wrong-code bug for C++ exceptions, so I don't think it should be committed as-is. The options I see are:

Revert this only for .cfi_negate_ra_state, giving us more time to find the right fix for .cfi_negate_ra_state_with_pc
Change the ABI to make it clear that the unwind opcode must be associated with the PC of the instruction after the PAC instruction. Then commit this revert along with a libunwind change to apply a 4-byte offset to the captured PC, so that exception unwinding continues to work. Alternatively, clarify that it must be at the same address as the PAC instruction, but that breaks the usual rules for debug info so I prefer the former option.
I think there's been some discussion about replacing these directives (at least .cfi_negate_ra_state_with_pc) with something which works better for functions with multiple returns, if that goes ahead maybe we could deprecate .cfi_negate_ra_state_with_pc in favour of it.

DanielKristofKiss · 2025-04-29T13:51:52Z

The depreciation of the cfi_negate_ra_state_with_pc is just posted here: ARM-software/abi-aa#327
I'd like to land .cfi_negate_ra_state part definitely as it is in active use in various deployments.

smithp35 · 2025-04-29T13:55:36Z

I've raised ARM-software/abi-aa#327 to cover the problem with .cfi_negate_ra_state_with_pc. The intention is that we'll propose a replacement for .cfi_negate_ra_state_with_pc and then deprecate it.

The consensus among the people discussing the ABI was that it was worth replacing .cfi_negate_ra_state_with_pc rather than attempting to fix it up. I did offer to change the ABI description as in your second bullet point [1] although there wasn't much enthusiasm for that given that it would be soon deprecated.

I don't have a strong preference on whether the revert is for both directives or just .cfi_negate_ra_state . My understanding is that due to lack of hardware there is unlikely to be existing code in production that uses .cfi_negate_ra_state_with_pc but there is for .cfi_negate_ra_state

[1]

Change the ABI to make it clear that the unwind opcode must be associated with the PC of the instruction after the PAC instruction. Then commit this revert along with a libunwind change to apply a 4-byte offset to the captured PC, so that exception unwinding continues to work

Stylie777 · 2025-05-01T09:01:34Z

I agree with @ostannard that if we are to revert this, to only change the behaviour of .cfi_negate_ra_state. cfi_negate_ra_state_with_pc needs a more thought out fix to address its issues.

github-actions · 2025-05-05T16:16:59Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Stylie777

LGTM

ostannard

LGTM

…ion (llvm#137795) This reverts partially this commit 0b73b5a. This is not a clear revert because other changes already landed. CFI directives like `.cfi_negate_ra_state` must be emitted after the instruction. If the execution is stopped before the `paciasp` instruction is executed the debugger/unwinder would try to authenticated the return address as the `.cfi_negate_ra_state` already indicates it got signed. fixes: llvm#137802

Revert "[AArch64] Correct position of CFI Instruction for Pointer Aut…

6ee1738

…hentication (llvm#121559)" This reverts commit 0b73b5a.

DanielKristofKiss requested a review from Stylie777 April 29, 2025 12:18

llvmbot added the backend:AArch64 label Apr 29, 2025

DanielKristofKiss requested review from davemgreen, smithp35 and ilinpv April 29, 2025 12:19

This was referenced Apr 29, 2025

[AArch64] Correct position of CFI Instruction for Pointer Authentication #137800

Open

[AArch64] cfi_negate_ra_state emitted incorrectly. #137802

Closed

Let's revert only the non-pauth-lr operations.

aba0b83

DanielKristofKiss changed the title ~~Revert "[AArch64] Correct position of CFI Instruction for Pointer Authentication (#121559)~~ [AArch64] Correct position of CFI Instruction for Pointer Authentication May 5, 2025

clang format

dfee50c

Stylie777 approved these changes May 6, 2025

View reviewed changes

ostannard approved these changes May 6, 2025

View reviewed changes

DanielKristofKiss merged commit 4cc152c into llvm:main May 6, 2025
11 checks passed

bgergely0 mentioned this pull request May 6, 2025

[BOLT][AArch64] Handle OpNegateRAState to enable optimizing binaries with pac-ret hardening #120064

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AArch64] Correct position of CFI Instruction for Pointer Authentication #137795

[AArch64] Correct position of CFI Instruction for Pointer Authentication #137795

DanielKristofKiss commented Apr 29, 2025 •

edited

Loading

llvmbot commented Apr 29, 2025

ostannard commented Apr 29, 2025

DanielKristofKiss commented Apr 29, 2025

smithp35 commented Apr 29, 2025

Stylie777 commented May 1, 2025

github-actions bot commented May 5, 2025 •

edited

Loading

Stylie777 left a comment

ostannard left a comment

[AArch64] Correct position of CFI Instruction for Pointer Authentication #137795

[AArch64] Correct position of CFI Instruction for Pointer Authentication #137795

Conversation

DanielKristofKiss commented Apr 29, 2025 • edited Loading

llvmbot commented Apr 29, 2025

ostannard commented Apr 29, 2025

DanielKristofKiss commented Apr 29, 2025

smithp35 commented Apr 29, 2025

Stylie777 commented May 1, 2025

github-actions bot commented May 5, 2025 • edited Loading

Stylie777 left a comment

Choose a reason for hiding this comment

ostannard left a comment

Choose a reason for hiding this comment

DanielKristofKiss commented Apr 29, 2025 •

edited

Loading

github-actions bot commented May 5, 2025 •

edited

Loading