[AArch64] Fix BTI instruction emission.

[danielkiss]

Summary

SCTLR_EL1.BT[01] controls the PACI[AB]SP compatibility with PBYTE 11
(see [1])
This bit will be set to zero so PACI[AB]SP are equal to BTI C
instruction only.

[1] https://developer.arm.com/docs/ddi0595/b/aarch64-system-registers/sctlr_el1

[ostannard]

LGTM

[danielkiss]

Patch rebased, now should apply clearly.

[tamas.petz]

LGTM

[chill]

llvm/lib/Target/AArch64/AArch64BranchTargets.cpp

188–192

What if they are set to a different value?

[chill]

llvm/lib/Target/AArch64/AArch64BranchTargets.cpp

188–192

Sorry for commenting at this late stage, I didn't know that behavior was configurable.
Shouldn't there be a test isTargetAndroid() and/or isTargetLinux() ?

[nickdesaulniers]

llvm/lib/Target/AArch64/AArch64BranchTargets.cpp

188–192

At least for fixing the reported bug in Linux kernels, we shouldn't guard on isTargetAndroid; Linux kernels used in Android target aarch64-linux-gnu triples. (Apologies if I misunderstood your question, or if you knew that already).

Re: https://groups.google.com/g/clang-built-linux/c/t2cgw_2fA2w

[danielkiss]

llvm/lib/Target/AArch64/AArch64BranchTargets.cpp

188–192

If the SCTLR_EL1.BT[01] is set it won't break the generated code, it will allow to jump to PACIASP/PACIBSP too with BR with not X16 or X17. This is not an issue here because we anyway adding a landing pad. It is issue only where PACIASP present but we shouldn't jump there, therefore this setting is not expected to be used widely.
Kernel and Userspace could have different setting.
If there will be a platfrom that enforces the SCTLR_EL1.BT to be set then we need a flag here to save a few additional landing pads. I'm not aware of such an ABI.
This bug found in kernel and user space too.

llvm/lib/Target/AArch64/AArch64BranchTargets.cpp

void AArch64BranchTargets::addBTI(MachineBasicBlock &MBB, bool CouldCall,
 
   auto MBBI = MBB.begin();
 
   // Skip the meta instructions, those will be removed anyway.
   for (; MBBI != MBB.end() && (MBBI->isMetaInstruction() || MBBI->isPseudo());
        ++MBBI)
     ;
 
-  // PACI[AB]SP are implicitly BTI JC, so no BTI instruction needed there.
-  if (MBBI != MBB.end() && (MBBI->getOpcode() == AArch64::PACIASP ||
-                            MBBI->getOpcode() == AArch64::PACIBSP))
+  // SCTLR_EL1.BT[01] is set to 0 by default which means
+  // PACI[AB]SP are implicitly BTI C so no BTI C instruction is needed there.
+  if (MBBI != MBB.end() && HintNum == 34 &&
+      (MBBI->getOpcode() == AArch64::PACIASP ||
     return;
 
   BuildMI(MBB, MBB.begin(), MBB.findDebugLoc(MBB.begin()),
           TII->get(AArch64::HINT))
       .addImm(HintNum);
 }

llvm/test/CodeGen/AArch64/branch-target-enforcement.mir

bb.2.lab2 (address-taken):
     renamable $w0 = ORRWri $wzr, 1984
     renamable $x9 = ADDXri killed $x9, target-flags(aarch64-pageoff, aarch64-nc) blockaddress(@label_address, %ir-block.return), 0
     STRXui killed renamable $x9, killed renamable $x8, target-flags(aarch64-pageoff, aarch64-nc) @label_address.addr :: (store 8 into @label_address.addr)
     RET undef $lr, implicit killed $w0
 
 ---
 # Function takes address of the entry block, so the entry block needs a BTI JC.
 name:            label_address_entry
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -16, size: 8, alignment: 16,
+      stack-id: default, callee-saved-register: '$lr', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
 body:             |
   bb.0.entry (address-taken):
     ; CHECK-LABEL: label_address_entry
     ; CHECK: bb.0.entry (address-taken):
     ; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
     ; CHECK-NEXT: {{  }}
     ; CHECK-NEXT: HINT 38
     ; CHECK: BR killed renamable $x9
     successors: %bb.1(0x40000000), %bb.2(0x40000000)
 
     renamable $x8 = ADRP target-flags(aarch64-page) @label_address.addr
     renamable $x9 = LDRXui renamable $x8, target-flags(aarch64-pageoff, aarch64-nc) @label_address.addr :: (dereferenceable load 8 from @label_address.addr)
     BR killed renamable $x9
 
   bb.1.return (address-taken):
     ; CHECK: bb.1.return (address-taken):
     ; CHECK-NEXT: HINT 36
     liveins: $x8
-
+    frame-setup PACIASP implicit-def $lr, implicit killed $lr, implicit $sp
+    frame-setup CFI_INSTRUCTION negate_ra_sign_state
+    early-clobber $sp = frame-setup STRXpre killed $lr, $sp, -16 :: (store 8 into %stack.0)
+    INLINEASM &"", 1, 12, implicit-def dead early-clobber $lr
     $x9 = ADRP target-flags(aarch64-page) blockaddress(@label_address, %ir-block.entry)
     renamable $w0 = ORRWri $wzr, 0
     renamable $x9 = ADDXri killed $x9, target-flags(aarch64-pageoff, aarch64-nc) blockaddress(@label_address, %ir-block.entry), 0
     STRXui killed renamable $x9, killed renamable $x8, target-flags(aarch64-pageoff, aarch64-nc) @label_address.addr :: (store 8 into @label_address.addr)
-    RET undef $lr, implicit killed $w0
+    early-clobber $sp, $lr = frame-destroy LDRXpost $sp, 16 :: (load 8 from %stack.0)
+    RETAA implicit $sp, implicit $lr, implicit killed $w0
 
   bb.2.lab2:
     ; CHECK: bb.2.lab2:
     ; CHECK-NOT: HINT
     liveins: $x8
 
     $x9 = ADRP target-flags(aarch64-page) blockaddress(@label_address, %ir-block.return)
     renamable $w0 = ORRWri $wzr, 1984