Miscompilation of calls to setjmp after "Preserve regmask when expanding the BLR_BTI pseudo instruction"

[mstorsjo]

The fix to issue #73787, in commit 99d4859, from pull request #73927, seem to cause miscompilations in other similar-looking cases.

The issues can be triggered with ffmpeg, with the very latest versions of ffmpeg, that include commit FFmpeg/FFmpeg@6573969. To reproduce it, build it like this:

$ git clone https://github.com/ffmpeg/ffmpeg
$ cd ffmpeg
$ git checkout 65739691b90012c9d93b2e5e0e89a55d8de9eb7b
$ cd ..
$ mkdir ffmpeg-build
$ cd ffmpeg-build
$ ../ffmpeg/configure --cc=clang --extra-cflags='-mbranch-protection=standard' --extra-ldflags='-mbranch-protection=standard'
$ make -j$(nproc) fate-checkasm

The symptom is that when calling sigsetjmp, the first parameter, in register x0, has been lost and is passed as a null pointer.

The issue can be observed with this attached LLVM IR file as well, which is the code from tests/checkasm/idctdsp.c in such a build. idctdsp-ll.zip

It can be injected in such a build to test whether it is correctly compiled or not:

$ clang -mbranch-protection=standard -O3 idctdsp.ll -c -o tests/checkasm/idctdsp.o && make -j$(nproc) fate-checkasm-idctdsp

The difference in generated code can be observed by looking at the output of clang -mbranch-protection=standard -O3 idctdsp.ll -c -o tests/checkasm/idctdsp.o -mllvm -print-after-all before/after this change. That diff looks like this:

--- log-good    2024-01-12 14:24:33.730645287 +0200
+++ log-bad     2024-01-12 14:24:26.046612441 +0200
@@ -67330,8 +67330,8 @@
   renamable $x27 = LDRXroX killed renamable $x8, killed renamable $x9, 0, 0 :: (load (s64) from %ir.13, !tbaa !13)
   $x0 = ORRXrs $xzr, $fp, 0
   renamable $w1 = MOVZWi 1, 0
-  BUNDLE implicit-def $lr, implicit-def $w30, implicit $sp {
-    BL @__sigsetjmp, implicit-def $lr, implicit $sp
+  BUNDLE implicit-def $lr, implicit-def $w30, implicit-def $sp, implicit-def $wsp, implicit-def $w0, implicit $sp {
+    BL @__sigsetjmp, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def $lr, implicit $sp, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $w0
     HINT 36
   }
   BL @checkasm_handle_signal, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def dead $lr, implicit $sp, implicit $w0, implicit-def $sp, implicit-def dead $w0
@@ -73086,14 +73086,13 @@
   liveins: $fp, $x8, $x20, $x21, $x22, $x23, $x24, $x25, $x26, $x28
   $x9 = ADDXri $sp, 248, 0
   STRXui renamable $x8, $sp, 21 :: (store (s64) into %stack.16)
-  $x0 = ORRXrs $xzr, $fp, 0
   renamable $x19 = ADDXrs killed renamable $x9, killed renamable $x8, 4
   $x9 = ADDXri $sp, 296, 0
   renamable $w1 = MOVZWi 1, 0
   renamable $x8 = LDRXui renamable $x19, 1 :: (load (s64) from %ir.offset, !tbaa !7)
   renamable $x27 = LDRXroX killed renamable $x8, killed renamable $x9, 0, 0 :: (load (s64) from %ir.13, !tbaa !13)
-  BUNDLE implicit-def $lr, implicit-def $w30, implicit $sp {
-    BL @__sigsetjmp, implicit-def $lr, implicit $sp
+  BUNDLE implicit-def $lr, implicit-def $w30, implicit-def $sp, implicit-def $wsp, implicit-def $w0, implicit $sp {
+    BL @__sigsetjmp, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def $lr, implicit $sp, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $w0
     HINT 36
   }
   BL @checkasm_handle_signal, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit-def $sp, implicit-def dead $w0

(I left out a number of identical hunks of the diff.)

The first change is the one that looks like the right thing, the fix for #73787. However further on, this causes the $x0 = ORRXrs $xzr, $fp, 0, which is used as input to the __sigsetjmp call, gets lost.

CC @efriedma-quic @DavidSpickett

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Martin Storsjö (mstorsjo)

The fix to issue #73787, in commit 99d4859, from pull request #73927, seem to cause miscompilations in other similar-looking cases.

The issues can be triggered with ffmpeg, with the very latest versions of ffmpeg, that include commit FFmpeg/FFmpeg@6573969. To reproduce it, build it like this:

$ git clone https://github.com/ffmpeg/ffmpeg
$ cd ffmpeg
$ git checkout 65739691b90012c9d93b2e5e0e89a55d8de9eb7b
$ cd ..
$ mkdir ffmpeg-build
$ cd ffmpeg-build
$ ../ffmpeg/configure --cc=clang --extra-cflags='-mbranch-protection=standard' --extra-ldflags='-mbranch-protection=standard'
$ make -j$(nproc) fate-checkasm

The symptom is that when calling sigsetjmp, the first parameter, in register x0, has been lost and is passed as a null pointer.

The issue can be observed with this attached LLVM IR file as well, which is the code from tests/checkasm/idctdsp.c in such a build. idctdsp-ll.zip

It can be injected in such a build to test whether it is correctly compiled or not:

$ clang -mbranch-protection=standard -O3 idctdsp.ll -c -o tests/checkasm/idctdsp.o && make -j$(nproc) fate-checkasm-idctdsp

The difference in generated code can be observed by looking at the output of clang -mbranch-protection=standard -O3 idctdsp.ll -c -o tests/checkasm/idctdsp.o -mllvm -print-after-all before/after this change. That diff looks like this:

--- log-good    2024-01-12 14:24:33.730645287 +0200
+++ log-bad     2024-01-12 14:24:26.046612441 +0200
@@ -67330,8 +67330,8 @@
   renamable $x27 = LDRXroX killed renamable $x8, killed renamable $x9, 0, 0 :: (load (s64) from %ir.13, !tbaa !13)
   $x0 = ORRXrs $xzr, $fp, 0
   renamable $w1 = MOVZWi 1, 0
-  BUNDLE implicit-def $lr, implicit-def $w30, implicit $sp {
-    BL @<!-- -->__sigsetjmp, implicit-def $lr, implicit $sp
+  BUNDLE implicit-def $lr, implicit-def $w30, implicit-def $sp, implicit-def $wsp, implicit-def $w0, implicit $sp {
+    BL @<!-- -->__sigsetjmp, &lt;regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...&gt;, implicit-def $lr, implicit $sp, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $w0
     HINT 36
   }
   BL @<!-- -->checkasm_handle_signal, &lt;regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...&gt;, implicit-def dead $lr, implicit $sp, implicit $w0, implicit-def $sp, implicit-def dead $w0
@@ -73086,14 +73086,13 @@
   liveins: $fp, $x8, $x20, $x21, $x22, $x23, $x24, $x25, $x26, $x28
   $x9 = ADDXri $sp, 248, 0
   STRXui renamable $x8, $sp, 21 :: (store (s64) into %stack.16)
-  $x0 = ORRXrs $xzr, $fp, 0
   renamable $x19 = ADDXrs killed renamable $x9, killed renamable $x8, 4
   $x9 = ADDXri $sp, 296, 0
   renamable $w1 = MOVZWi 1, 0
   renamable $x8 = LDRXui renamable $x19, 1 :: (load (s64) from %ir.offset, !tbaa !7)
   renamable $x27 = LDRXroX killed renamable $x8, killed renamable $x9, 0, 0 :: (load (s64) from %ir.13, !tbaa !13)
-  BUNDLE implicit-def $lr, implicit-def $w30, implicit $sp {
-    BL @<!-- -->__sigsetjmp, implicit-def $lr, implicit $sp
+  BUNDLE implicit-def $lr, implicit-def $w30, implicit-def $sp, implicit-def $wsp, implicit-def $w0, implicit $sp {
+    BL @<!-- -->__sigsetjmp, &lt;regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...&gt;, implicit-def $lr, implicit $sp, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $w0
     HINT 36
   }
   BL @<!-- -->checkasm_handle_signal, &lt;regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...&gt;, implicit-def dead $lr, implicit $sp, implicit killed $w0, implicit-def $sp, implicit-def dead $w0

(I left out a number of identical hunks of the diff.)

The first change is the one that looks like the right thing, the fix for #73787. However further on, this causes the $x0 = ORRXrs $xzr, $fp, 0, which is used as input to the __sigsetjmp call, gets lost.

CC @efriedma-quic @DavidSpickett

[mstorsjo]

I see that the bundling/expansion from BLR_BTI to BUNDLE { BL, HINT } loses the input registers. When digging in the output of -print-after-all, before expanding/bundling, we have BLR_BTI @__sigsetjmp, $x0, $w1, <regmask ..., after we have

 BUNDLE implicit-def $lr, implicit-def $w30, implicit-def $sp, implicit-def $wsp, implicit-def $w0, implicit $sp {
    BL @__sigsetjmp, <regmask ...

So due to that, we lose the information that the value of x0 and w1 actually are needed here, so they're optimized out later.

[DavidSpickett]

I am not experienced in this area. So I'm a bit lost telling one type of operand/argument from another.

before expanding/bundling, we have BLR_BTI @__sigsetjmp, $x0, $w1, <regmask ...

Are you seeing this literally in the output or are you showing what is implicitly there inside of llvm but not shown in the text? In other words, if the call knew it took in those arguments, would that be shown in the IR or just in the internal state of llvm?

For some reason the IR file you uploaded wouldn't compile with main clang but I generated one from the C file and that does not produce output like that. It's always BLR_BTI @_sigsetjmp <regmask ....

I also don't see arguments like that when BTI protection is not enabled. It's always BL @__sigsetjmp, <regmask.

Toward the end the BLR_BTI is:

BLR_BTI @__sigsetjmp, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def dead $lr, implicit $sp, implicit killed $x0, implicit killed $w1, implicit-def $w0

And I wonder if the implicit killed is what you mean?

But I think not because after the bundle is expanded:

BL @__sigsetjmp, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def $lr, implicit $sp, implicit-def dead $lr, implicit $sp, implicit killed $x0, implicit killed $w1, implicit-def $w0

The implicit killed are still there, and I see the test failing, so something must be missing.

TLDR: I'm not sure what correct looks like here, so I don't know what to look for.

[mstorsjo]

I am not experienced in this area. So I'm a bit lost telling one type of operand/argument from another.

before expanding/bundling, we have BLR_BTI @__sigsetjmp, $x0, $w1, <regmask ...

Are you seeing this literally in the output or are you showing what is implicitly there inside of llvm but not shown in the text? In other words, if the call knew it took in those arguments, would that be shown in the IR or just in the internal state of llvm?

I see that in the output from -print-after-all, from the pass before expanding BLR_BTI into BUNDLE { BL ...}.

For some reason the IR file you uploaded wouldn't compile with main clang but I generated one from the C file and that does not produce output like that. It's always BLR_BTI @_sigsetjmp <regmask ....

Hmm, that's odd. FWIW, I produced these files after bisecting, with clang built at 99d4859 - but I also retested it now, I built clang at 08e4386, and executed clang -target aarch64-linux-gnu -c idctdsp.ll -mbranch-protection=standard -O3 -mllvm -print-after-all and that runs just fine.

I also don't see arguments like that when BTI protection is not enabled. It's always BL @__sigsetjmp, <regmask.

That's weird. For me, as long as I have branch protection enabled, I still with today's clang do get BLR_BTI @__sigsetjmp, $x0, $w1, <regmask ...>, implicit-def $w0. After expansion, it is BUNDLE ... implicit-def $w0, ... { BL @__sigsetjmp, <regmask ...>, implicit-def $w0.

Without branch protection, when compiling from C, I get BL @__sigsetjmp, <regmask ...>, implicit $x0, implicit $w1, implicit-def $w0.

I'm not entirely sure what exact role these have here, but I presume that without implicit $x0, the implicit-def $w0 indicates that the earlier write to x0 is dead and can be removed.

Toward the end the BLR_BTI is:

BLR_BTI @__sigsetjmp, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def dead $lr, implicit $sp, implicit killed $x0, implicit killed $w1, implicit-def $w0

And I wonder if the implicit killed is what you mean?

But I think not because after the bundle is expanded:

BL @__sigsetjmp, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def $lr, implicit $sp, implicit-def dead $lr, implicit $sp, implicit killed $x0, implicit killed $w1, implicit-def $w0

The implicit killed are still there, and I see the test failing, so something must be missing.

TLDR: I'm not sure what correct looks like here, so I don't know what to look for.

[DavidSpickett]

I think 1. I managed to corrupt the IR file on the way to a remote machine and 2. some detail of how I made my own IR changed the output.

I do see the operands at the start now that I'm using the file you uploaded:

  BLR_BTI @__sigsetjmp, $x0, $w1, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $w0

Then they are gone once the bundle is built:

  BL @__sigsetjmp, <regmask $fp $lr $wzr $xzr $b8 $b9 $b10 $b11 $b12 $b13 $b14 $b15 $d8 $d9 $d10 $d11 $d12 $d13 $d14 $d15 $h8 $h9 $h10 $h11 $h12 $h13 $h14 $h15 $s8 $s9 $s10 $s11 $s12 and 55 more...>, implicit-def $lr, implicit $sp, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $w0

There is another bundle like this that's handled by expandCALL_RVMARKER which has some extra steps that I did not include in expandCALL_BTI because I couldn't figure out what their purpose was at the time. I will start there.