[AArch64] Miscompilation with -mbranch-protection=standard since LLVM 15.0

[mstorsjo]

Since LLVM 15.0, or more precisely, 7c13ae6 (reviewed at https://reviews.llvm.org/D125335), there's a regression when compiling code for aarch64 with -mbranch-protection=standard, causing a miscompilation, leading to crashes at runtime.

The issue can be observed with a very reduced testcase like this:

void _setjmp();
void a(int b) {
  _setjmp();
  for (; b;)
    ;
}

If compiled with Clang after the regressing commit,

$ clang -target aarch64-linux-gnu -S -O3 test.c -mbranch-protection=standard -o - -fno-asynchronous-unwind-tables
        .globl  a                               // -- Begin function a
a:                                      // @a
        hint    #25
        stp     x29, x30, [sp, #-32]!           // 16-byte Folded Spill
        str     x19, [sp, #16]                  // 8-byte Folded Spill
        mov     x29, sp
        mov     w19, w0
        bl      _setjmp
        hint    #36
        cbz     w0, .LBB0_2
.LBB0_1:                                // %for.cond
                                        // =>This Inner Loop Header: Depth=1
        b       .LBB0_1
.LBB0_2:                                // %for.end
        ldr     x19, [sp, #16]                  // 8-byte Folded Reload
        ldp     x29, x30, [sp], #32             // 16-byte Folded Reload
        hint    #29
        ret

(-fno-asynchronous-unwind-tables is only included to improve readability of the output code, it's irrelevant for the actual issue at hand.)

The breakage exists in this part of the compiler output:

        mov     w19, w0
        bl      _setjmp
        hint    #36
        cbz     w0, .LBB0_2

Before the regression, the output was:

        mov     w19, w0
        bl      _setjmp
        hint    #36
        cbz     w19, .LBB0_2

The value in w0 is backed up in w19, before calling the function _setjmp. After calling another function, the registers x0- x15 are to be considered clobbered - but the cbz instruction inspects the register w0, while it was supposed to be read from w19 instead.

CC @davemgreen who reviewed it. (The patch author doesn't seem to be reachable on github any longer.) I'm unsure if this is an issue with the patch itself at hand, or if this was a preexisting issue that was uncovered by the new pass.

If inspecting the output with -mllvm -print-after-all, the first difference in the output (after the new pass) is this:

 bb.0.entry:
   successors: %bb.2(0x30000000), %bb.1(0x50000000); %bb.2(37.50%), %bb.1(62.50%)
   liveins: $w0, $lr, $x19
   frame-setup PACIASP implicit-def $lr, implicit killed $lr, implicit $sp
   early-clobber $sp = frame-setup STPXpre $fp, killed $lr, $sp(tied-def 0), -4 :: (store (s64) into %stack.2), (store (s64) into %stack.1)
   frame-setup STRXui killed $x19, $sp, 2 :: (store (s64) into %stack.0)
   $fp = frame-setup ADDXri $sp, 0, 0
-  $w19 = ORRWrs $wzr, killed $w0, 0
+  $w19 = ORRWrs $wzr, $w0, 0
   BUNDLE implicit-def $lr, implicit-def $w30, implicit $sp {
     BL @_setjmp, implicit-def $lr, implicit $sp
     HINT 36
   }
-  CBZW killed renamable $w19, %bb.2
+  CBZW $w0, %bb.2

For function calls other than _setjmp, there wouldn't be a bundled HINT instruction following it, which might be causing the issue.

Also attached is the full original, unreduced test file where the issue was triggered: setjmp-bti-breakage.zip

When compiling that file, with the flags above, there's the following diff in the output:

--- good        2023-11-29 14:06:55.907069198 +0200
+++ bad 2023-11-29 14:06:57.343074970 +0200
@@ -369,21 +369,21 @@
        mov     w21, w5
        str     x1, [sp, #352]                  // 8-byte Folded Spill
        mov     w20, w3
        ldr     x0, [x0, :got_lo12:checkasm_context_buf]
        mov     w19, w2
        bl      _setjmp
        hint    #36
        mov     w8, w19
        lsl     w19, w19, #2
-       str     w21, [sp, #364]                 // 4-byte Folded Spill
+       str     w5, [sp, #364]                  // 4-byte Folded Spill
        str     w8, [sp, #348]                  // 4-byte Folded Spill
-       cbz     w21, .LBB1_2
+       cbz     w5, .LBB1_2

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Martin Storsjö (mstorsjo)

Since LLVM 15.0, or more precisely, 7c13ae6 (reviewed at https://reviews.llvm.org/D125335), there's a regression when compiling code for aarch64 with `-mbranch-protection=standard`, causing a miscompilation, leading to crashes at runtime.

The issue can be observed with a very reduced testcase like this:

void _setjmp();
void a(int b) {
  _setjmp();
  for (; b;)
    ;
}

If compiled with Clang after the regressing commit,

$ clang -target aarch64-linux-gnu -S -O3 test.c -mbranch-protection=standard -o - -fno-asynchronous-unwind-tables
        .globl  a                               // -- Begin function a
a:                                      // @<!-- -->a
        hint    #<!-- -->25
        stp     x29, x30, [sp, #-32]!           // 16-byte Folded Spill
        str     x19, [sp, #<!-- -->16]                  // 8-byte Folded Spill
        mov     x29, sp
        mov     w19, w0
        bl      _setjmp
        hint    #<!-- -->36
        cbz     w0, .LBB0_2
.LBB0_1:                                // %for.cond
                                        // =&gt;This Inner Loop Header: Depth=1
        b       .LBB0_1
.LBB0_2:                                // %for.end
        ldr     x19, [sp, #<!-- -->16]                  // 8-byte Folded Reload
        ldp     x29, x30, [sp], #<!-- -->32             // 16-byte Folded Reload
        hint    #<!-- -->29
        ret

(-fno-asynchronous-unwind-tables is only included to improve readability of the output code, it's irrelevant for the actual issue at hand.)

The breakage exists in this part of the compiler output:

        mov     w19, w0
        bl      _setjmp
        hint    #<!-- -->36
        cbz     w0, .LBB0_2

Before the regression, the output was:

        mov     w19, w0
        bl      _setjmp
        hint    #<!-- -->36
        cbz     w19, .LBB0_2

The value in w0 is backed up in w19, before calling the function _setjmp. After calling another function, the registers x0- x15 are to be considered clobbered - but the cbz instruction inspects the register w0, while it was supposed to be read from w19 instead.

CC @davemgreen who reviewed it. (The patch author doesn't seem to be reachable on github any longer.) I'm unsure if this is an issue with the patch itself at hand, or if this was a preexisting issue that was uncovered by the new pass.

If inspecting the output with -mllvm -print-after-all, the first difference in the output (after the new pass) is this:

 bb.0.entry:
   successors: %bb.2(0x30000000), %bb.1(0x50000000); %bb.2(37.50%), %bb.1(62.50%)
   liveins: $w0, $lr, $x19
   frame-setup PACIASP implicit-def $lr, implicit killed $lr, implicit $sp
   early-clobber $sp = frame-setup STPXpre $fp, killed $lr, $sp(tied-def 0), -4 :: (store (s64) into %stack.2), (store (s64) into %stack.1)
   frame-setup STRXui killed $x19, $sp, 2 :: (store (s64) into %stack.0)
   $fp = frame-setup ADDXri $sp, 0, 0
-  $w19 = ORRWrs $wzr, killed $w0, 0
+  $w19 = ORRWrs $wzr, $w0, 0
   BUNDLE implicit-def $lr, implicit-def $w30, implicit $sp {
     BL @<!-- -->_setjmp, implicit-def $lr, implicit $sp
     HINT 36
   }
-  CBZW killed renamable $w19, %bb.2
+  CBZW $w0, %bb.2

For function calls other than _setjmp, there wouldn't be a bundled HINT instruction following it, which might be causing the issue.

Also attached is the full original, unreduced test file where the issue was triggered: setjmp-bti-breakage.zip

When compiling that file, with the flags above, there's the following diff in the output:

--- good        2023-11-29 14:06:55.907069198 +0200
+++ bad 2023-11-29 14:06:57.343074970 +0200
@@ -369,21 +369,21 @@
        mov     w21, w5
        str     x1, [sp, #<!-- -->352]                  // 8-byte Folded Spill
        mov     w20, w3
        ldr     x0, [x0, :got_lo12:checkasm_context_buf]
        mov     w19, w2
        bl      _setjmp
        hint    #<!-- -->36
        mov     w8, w19
        lsl     w19, w19, #<!-- -->2
-       str     w21, [sp, #<!-- -->364]                 // 4-byte Folded Spill
+       str     w5, [sp, #<!-- -->364]                  // 4-byte Folded Spill
        str     w8, [sp, #<!-- -->348]                  // 4-byte Folded Spill
-       cbz     w21, .LBB1_2
+       cbz     w5, .LBB1_2

[efriedma-quic]

AArch64ExpandPseudo::expandCALL_BTI is throwing away the regmask on the call, so later optimizations can't tell the setjmp() call clobbers w0. Pretty clearly an "uncovered" issue.

[mstorsjo]

AArch64ExpandPseudo::expandCALL_BTI is throwing away the regmask on the call, so later optimizations can't tell the setjmp() call clobbers w0. Pretty clearly an "uncovered" issue.

Thanks, that explains it!

Do you happen to know someone who might have time to look at fixing this?

[efriedma-quic]

CC @DavidSpickett @DanielKristofKiss from https://reviews.llvm.org/D121707