[AArch64] Move SLS later in pass pipeline

[olista01]

Summary

Currently, the SLS hardening pass is run before the machine outliner,
which means that the outliner creates new functions and calls which do
not have the SLS hardening applied.

The fix for this is to move the SLS passes to after the outliner, as has
recently been done for the return address signing pass.

This also avoids a bug where the SLS outliner emits code with
instructions after a return, which the outliner doesn't correctly
handle.

This results in the heuristics used by the outliner being wrong, since
it doesn't know about the extra instructions the SLS pass will add to
every call and return. That's not a correctness problem, so I'll update
them in a separate patch.

Previous summary:

The SLS hardening pass inserts barrier instructions after return
instructions, so it was causing the machine outliner to fail to notice
return blocks, so it could outline at places where LR is live. The fix
for this is to check all terminator instructions when deciding if a
block ends in a return, not just the least one.

I think there's a deeper issue that we don't correctly track the
liveness of LR around function returns. This appears to be deliberate,
according to the comment on RET_ReallyLR in AArch64InstrInfo.td. I tried
changing this to make LR an implicit operand of all returns, but haven't
found a way to do that which doesn't cause a large number of MIR
verifier failures.

I think there's also an issue here that the outliner can create code
which does not have the SLS barrier instructions after a tail-call.
@Kristof, do you know if the SLS pass could be moved to after the
outliner to fix this, or does it need to happen as early as it does?

[kristof.beyls]

I don't remember any reason why the SLS pass needs to run before the outliner.
It seems to me that indeed it may be easier to move to SLS pass to run after the outliner - i.e. run after any pass that may need to interpret the semantics of return instructions.

[olista01]

• Move SLS pass later in the pass pipeline

[kristof.beyls]

llvm/lib/Target/AArch64/AArch64SLSHardening.cpp

225–227

I guess this change is a side-effect of moving the AArch64IndirectThunks pass later in the pipeline. But I cannot easily guess what exactly triggers this change in behavior. I wonder if you happen to know?

[olista01]

llvm/lib/Target/AArch64/AArch64SLSHardening.cpp

225–227

IndirectThunks.h:84 has a comment explaining that it does not create the entry block, but I've not worked out where it was being created before.

[kristof.beyls]

LGTM, thanks!

llvm/lib/Target/AArch64/AArch64SLSHardening.cpp

225–227

Thanks. I guess it doesn't really matter much.

llvm/lib/Target/AArch64/AArch64SLSHardening.cpp

void SLSBLRThunkInserter::populateThunk(MachineFunction &MF) {
   assert(MF.getName().startswith(getThunkPrefix()));
   auto ThunkIt = llvm::find_if(
       SLSBLRThunks, [&MF](auto T) { return T.Name == MF.getName(); });
   assert(ThunkIt != std::end(SLSBLRThunks));
   Register ThunkReg = ThunkIt->Reg;
 
   const TargetInstrInfo *TII =
       MF.getSubtarget<AArch64Subtarget>().getInstrInfo();
-  assert (MF.size() == 1);
+  assert (MF.size() == 0);
+  MF.push_back(MF.CreateMachineBasicBlock());
   MachineBasicBlock *Entry = &MF.front();
   Entry->clear();
 
   //  These thunks need to consist of the following instructions:
   //  __llvm_slsblr_thunk_xN:
   //      BR xN
   //      barrierInsts
   Entry->addLiveIn(ThunkReg);
   // MOV X16, ThunkReg == ORR X16, XZR, ThunkReg, LSL #0

llvm/lib/Target/AArch64/AArch64TargetMachine.cpp

void AArch64PassConfig::addPreSched2() {
 
   // The AArch64SpeculationHardeningPass destroys dominator tree and natural
   // loop info, which is needed for the FalkorHWPFFixPass and also later on.
   // Therefore, run the AArch64SpeculationHardeningPass before the
   // FalkorHWPFFixPass to avoid recomputing dominator tree and natural loop
   // info.
   addPass(createAArch64SpeculationHardeningPass());
 
-  addPass(createAArch64IndirectThunks());
-  addPass(createAArch64SLSHardeningPass());
-
   if (TM->getOptLevel() != CodeGenOptLevel::None) {
     if (EnableFalkorHWPFFix)
       addPass(createFalkorHWPFFixPass());
   }
 }
 
 void AArch64PassConfig::addPreEmitPass() {
   // Machine Block Placement might have created new opportunities when run
void AArch64PassConfig::addPreEmitPass() {
   }
 
   if (TM->getOptLevel() != CodeGenOptLevel::None && EnableCollectLOH &&
       TM->getTargetTriple().isOSBinFormatMachO())
     addPass(createAArch64CollectLOHPass());
 }
 
 void AArch64PassConfig::addPostBBSections() {
+  addPass(createAArch64IndirectThunks());
+  addPass(createAArch64SLSHardeningPass());
   addPass(createAArch64PointerAuthPass());
   if (EnableBranchTargets)
     addPass(createAArch64BranchTargetsPass());
   // Relax conditional branch instructions if they're otherwise out of
   // range of their destination.
   if (BranchRelaxation)
     addPass(&BranchRelaxationPassID);
 

llvm/test/CodeGen/AArch64/O0-pipeline.ll

 ; CHECK-NEXT:       Fixup Statepoint Caller Saved
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       Prologue/Epilogue Insertion & Frame Finalization
 ; CHECK-NEXT:       Post-RA pseudo instruction expansion pass
 ; CHECK-NEXT:       AArch64 pseudo instruction expansion pass
 ; CHECK-NEXT:       Insert KCFI indirect call checks
 ; CHECK-NEXT:       AArch64 speculation hardening pass
-; CHECK-NEXT:       AArch64 Indirect Thunks
-; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       Analyze Machine Code For Garbage Collection
 ; CHECK-NEXT:       Insert fentry calls
 ; CHECK-NEXT:       Insert XRay ops
 ; CHECK-NEXT:       Implement the 'patchable-function' attribute
 ; CHECK-NEXT:       Workaround A53 erratum 835769 pass
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       StackMap Liveness Analysis
 ; CHECK-NEXT:       Live DEBUG_VALUE analysis
 ; CHECK-NEXT:       Machine Sanitizer Binary Metadata
+; CHECK-NEXT:       AArch64 Indirect Thunks
+; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       AArch64 Pointer Authentication
 ; CHECK-NEXT:       AArch64 Branch Targets
 ; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       Insert CFI remember/restore state instructions
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       Stack Frame Layout Analysis
 ; CHECK-NEXT:       Unpack machine instruction bundles
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       AArch64 Assembly Printer
 ; CHECK-NEXT:       Free MachineFunction
 
 define void @f() {
   ret void
 }

llvm/test/CodeGen/AArch64/O3-pipeline.ll

 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Tail Duplication
 ; CHECK-NEXT:       Machine Copy Propagation Pass
 ; CHECK-NEXT:       Post-RA pseudo instruction expansion pass
 ; CHECK-NEXT:       AArch64 pseudo instruction expansion pass
 ; CHECK-NEXT:       AArch64 load / store optimization pass
 ; CHECK-NEXT:       Insert KCFI indirect call checks
 ; CHECK-NEXT:       AArch64 speculation hardening pass
-; CHECK-NEXT:       AArch64 Indirect Thunks
-; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       MachineDominator Tree Construction
 ; CHECK-NEXT:       Machine Natural Loop Construction
 ; CHECK-NEXT:       Falkor HW Prefetch Fix Late Phase
 ; CHECK-NEXT:       PostRA Machine Instruction Scheduler
 ; CHECK-NEXT:       Analyze Machine Code For Garbage Collection
 ; CHECK-NEXT:       Machine Block Frequency Analysis
 ; CHECK-NEXT:       MachinePostDominator Tree Construction
 ; CHECK-NEXT:       Branch Probability Basic Block Placement
 ; CHECK-NEXT:       Insert fentry calls
 ; CHECK-NEXT:       Insert XRay ops
 ; CHECK-NEXT:       Implement the 'patchable-function' attribute
 ; CHECK-NEXT:       AArch64 load / store optimization pass
 ; CHECK-NEXT:       Machine Copy Propagation Pass
 ; CHECK-NEXT:       Workaround A53 erratum 835769 pass
 ; CHECK-NEXT:       Contiguously Lay Out Funclets
 ; CHECK-NEXT:       StackMap Liveness Analysis
 ; CHECK-NEXT:       Live DEBUG_VALUE analysis
 ; CHECK-NEXT:       Machine Sanitizer Binary Metadata
 ; CHECK-NEXT:     Machine Outliner
 ; CHECK-NEXT:     FunctionPass Manager
+; CHECK-NEXT:       AArch64 Indirect Thunks
+; CHECK-NEXT:       AArch64 sls hardening pass
 ; CHECK-NEXT:       AArch64 Pointer Authentication
 ; CHECK-NEXT:       AArch64 Branch Targets
 ; CHECK-NEXT:       Branch relaxation pass
 ; CHECK-NEXT:       AArch64 Compress Jump Tables
 ; CHECK-NEXT:       Insert CFI remember/restore state instructions
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       Stack Frame Layout Analysis

llvm/test/CodeGen/AArch64/arm64-opt-remarks-lazy-bfi.ll

 
 
 ; Verify that we only populate MachineBFI on behalf of ORE when hotness is
 ; requested.  (This hard-codes the previous pass to the Assembly Printer,
 ; please adjust accordingly.)
 
 ; HOTNESS:      Freeing Pass 'Machine Outliner'
 ; HOTNESS-NEXT:  Executing Pass 'Function Pass Manager'
-; HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
-; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
+; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT: Executing Pass 'AArch64 Indirect Thunks' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'AArch64 Indirect Thunks' on Function 'empty_func'...
+; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT: Executing Pass 'AArch64 sls hardening pass' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'AArch64 sls hardening pass' on Function 'empty_func'...
+; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; HOTNESS-NEXT: Executing Pass 'AArch64 Pointer Authentication' on Function 'empty_func'...
 ; HOTNESS-NEXT: Freeing Pass 'AArch64 Pointer Authentication' on Function 'empty_func'...
 ; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; HOTNESS-NEXT: Executing Pass 'AArch64 Branch Targets' on Function 'empty_func'...
 ; HOTNESS-NEXT: Freeing Pass 'AArch64 Branch Targets' on Function 'empty_func'...
 ; HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; HOTNESS-NEXT: Freeing Pass 'Verify generated machine code' on Function 'empty_func'...

 ; HOTNESS-NOT: Executing Pass
 ; HOTNESS: Executing Pass 'AArch64 Assembly Printer'
 
 ; HOTNESS: arm64-summary-remarks.ll:5:0: 1 instructions in function (hotness: 33)
 
 
 ; NO_HOTNESS:      Freeing Pass 'Machine Outliner'
 ; NO_HOTNESS-NEXT:  Executing Pass 'Function Pass Manager'
-; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code'
-; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code'
+; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT: Executing Pass 'AArch64 Indirect Thunks' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'AArch64 Indirect Thunks' on Function 'empty_func'...
+; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT: Executing Pass 'AArch64 sls hardening pass' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'AArch64 sls hardening pass' on Function 'empty_func'...
+; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
+; NO_HOTNESS-NEXT:  Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Executing Pass 'AArch64 Pointer Authentication' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Freeing Pass 'AArch64 Pointer Authentication' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Executing Pass 'AArch64 Branch Targets' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Freeing Pass 'AArch64 Branch Targets' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Executing Pass 'Verify generated machine code' on Function 'empty_func'...
 ; NO_HOTNESS-NEXT: Freeing Pass 'Verify generated machine code' on Function 'empty_func'...

llvm/test/CodeGen/AArch64/sls-stackprotector-outliner.ll

 ; CHECK-NEXT:    sub sp, sp, #32
 ; CHECK-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    .cfi_offset w30, -16
 ; CHECK-NEXT:    bl OUTLINED_FUNCTION_0
 ; CHECK-NEXT:    b.ne .LBB0_2
 ; CHECK-NEXT:  // %bb.1: // %entry
 ; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
-; CHECK-NEXT:    bl OUTLINED_FUNCTION_1
+; CHECK-NEXT:    add x0, x0, x8
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    b _ZN2C6D1Ev
 ; CHECK-NEXT:    dsb sy
 ; CHECK-NEXT:    isb
 ; CHECK-NEXT:  .LBB0_2: // %entry
 ; CHECK-NEXT:    bl __stack_chk_fail
 entry:
   %0 = load ptr, ptr %this, align 8
   %1 = getelementptr inbounds i8, ptr %0, i64 -24

 ; CHECK-NEXT:    sub sp, sp, #32
 ; CHECK-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    .cfi_offset w30, -16
 ; CHECK-NEXT:    bl OUTLINED_FUNCTION_0
 ; CHECK-NEXT:    b.ne .LBB1_2
 ; CHECK-NEXT:  // %bb.1: // %entry
 ; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
-; CHECK-NEXT:    bl OUTLINED_FUNCTION_1
+; CHECK-NEXT:    add x0, x0, x8
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    b _ZN2C6D0Ev
 ; CHECK-NEXT:    dsb sy
 ; CHECK-NEXT:    isb
 ; CHECK-NEXT:  .LBB1_2: // %entry
 ; CHECK-NEXT:    bl __stack_chk_fail
 entry:
   %0 = load ptr, ptr %this, align 8
   %1 = getelementptr inbounds i8, ptr %0, i64 -24

 ; CHECK-NEXT:    sub sp, sp, #32
 ; CHECK-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    .cfi_offset w30, -16
 ; CHECK-NEXT:    bl OUTLINED_FUNCTION_0
 ; CHECK-NEXT:    b.ne .LBB2_2
 ; CHECK-NEXT:  // %bb.1: // %entry
 ; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
-; CHECK-NEXT:    bl OUTLINED_FUNCTION_1
+; CHECK-NEXT:    add x0, x0, x8
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    b _ZN3C10D1Ev
 ; CHECK-NEXT:    dsb sy
 ; CHECK-NEXT:    isb
 ; CHECK-NEXT:  .LBB2_2: // %entry
 ; CHECK-NEXT:    bl __stack_chk_fail
 entry:
   %0 = load ptr, ptr %this, align 8
   %1 = getelementptr inbounds i8, ptr %0, i64 -24

 ; CHECK-NEXT:    sub sp, sp, #32
 ; CHECK-NEXT:    str x30, [sp, #16] // 8-byte Folded Spill
 ; CHECK-NEXT:    .cfi_def_cfa_offset 32
 ; CHECK-NEXT:    .cfi_offset w30, -16
 ; CHECK-NEXT:    bl OUTLINED_FUNCTION_0
 ; CHECK-NEXT:    b.ne .LBB3_2
 ; CHECK-NEXT:  // %bb.1: // %entry
 ; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
-; CHECK-NEXT:    bl OUTLINED_FUNCTION_1
+; CHECK-NEXT:    add x0, x0, x8
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    b _ZN3C10D0Ev
 ; CHECK-NEXT:    dsb sy
 ; CHECK-NEXT:    isb
 ; CHECK-NEXT:  .LBB3_2: // %entry
 ; CHECK-NEXT:    bl __stack_chk_fail
 entry:
   %0 = load ptr, ptr %this, align 8
   %1 = getelementptr inbounds i8, ptr %0, i64 -24