Handle CET for -exception-model sjlj

[xiangzhangllvm]

Summary

In SjLj exception mode, the old landingpad BB will create a new landingpad BB and use indirect branch jump to the old landingpad BB in lowering.
So we should add 2 endbr for this exception model.

[LuoYuanke]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

153

Can you comments with an example to elaborate which pointer endbr is needed? Do we need to insert endbr on each EHPad? Two EHPad must be in different MBB, right?

[LuoYuanke]

llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll

6

The test just show an endbr is inserted, but it doesn't demo where the endbr is inserted.

[LuoYuanke]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

157

After build a local test case, I can see in line 39 it has indirect jump to lpad. Can you explain why checking "MF.hasCallSiteLandingPad(Sym)" is needed? Is there any test case for it?

37 .LBB0_7:                                #   in Loop: Header=BB0_6 Depth=1
38         leaq    .LJTI0_0(%rip), %rcx
39         jmpq    *(%rcx,%rax,8)
40 .LBB0_1:                                # %lpad
41                                         #   in Loop: Header=BB0_6 Depth=1
42 .Ltmp2:
43         movl    -132(%rbp), %edi
44         movl    -128(%rbp), %eax

[xiangzhangllvm]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

153

Do we need to insert endbr on each EHPad?

Yes
Two EHPad must be in different MBB, right?
Right

I'll update the test case, let it easy to see. Thank you!

157

Because before converting to SjLj exception mode, this information "MF.hasCallSiteLandingPad(Sym) is true" has been established. And it do not changed after converting to SjLj exception mode.

in fact, line 39 jump to line 42(not line 40), .Ltmp2 is a hasCallSiteLandingPad symbol.

[hjl.tools]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

142

Why do you have to iterate the same loop twice?

[xiangzhangllvm]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

142

I move out the condition out of the loop, because in most time the "if" and "else if" is false.
It will rarely come in to the loops (line 149-173).
I think it is more performance than putting these conditions into loop (line 141).
And the code is more readable than before.

llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll

6

Yes, here just shows there one more endbr in sjlj model than other model. I'll refine it.

[LuoYuanke]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

143

The isEHPad() include CatchSwitch, CatchPad, CleanupPad and LandingPad. Do we want to insert endbr to all of them or just for LandingPad?

[xiangzhangllvm]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

143

The LandingPad is the exception where will be indirectly jump when the exception happen. The others uses direct jump, them belong to the "internal exception handle block" following the LandingPad.

[LuoYuanke]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

143

Yes. Could you extend your test case to cover CatchPad or some other Pad to make sure we don't insert endbr on these blocks.

[xiangzhangllvm]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

143

OK, I'll use CHECK-COUNT =n to limit the count of endbr in the tests.

[xiangzhangllvm]

Update test about endbr checking for sjlj model.
And from the test you can see the endbr is not add before catchpad, total 3 endbr, one at the entry of function, 2nd one before "old landpad" (line62) and last one at the begin of BB (new landpad without name) generated before "old landpad"(line 51)

[LuoYuanke]

llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll

27

Also add a cache pad. Some code as this.

catch:                                            ; preds = %lpad
  %5 = tail call i8* @__cxa_begin_catch(i8* %2) #6
  %6 = bitcast i8* %5 to i32*

[xiangzhangllvm]

Add a test to enhance the patch.

llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll

27

OK, but this will make test a little complex, in fact the patch is focus on dealing with the change of landpad (old +new). So current test is written very simple to show this.
Catch branch is handled in catch.dispatch which usually follow the landpad, they are direct branch.

[xiangzhangllvm]

Add a test to enhance the patch.

[LuoYuanke]

LGTM.

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

bool X86IndirectBranchTrackingPass::runOnMachineFunction(MachineFunction &MF) {
   }
 
   for (auto &MBB : MF) {
     // Find all basic blocks that their address was taken (for example
     // in the case of indirect jump) and add ENDBR instruction.
     if (MBB.hasAddressTaken())
       Changed |= addENDBR(MBB, MBB.begin());
 
-    // Exception handle may indirectly jump to catch pad, So we should add
-    // ENDBR before catch pad instructions.
-    bool EHPadIBTNeeded = MBB.isEHPad();
+    bool EHPadIBTNeeded = true;
+    bool isEHPadBB = MBB.isEHPad();
 
     for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ++I) {
       if (I->isCall() && IsCallReturnTwice(I->getOperand(0)))
         Changed |= addENDBR(MBB, std::next(I));
 
-      if (EHPadIBTNeeded && I->isEHLabel()) {
+      // Exception handle may indirectly jump to catch pad, So we should add
+      // ENDBR before catch pad instructions. For some exception mode, the old
+      // landingpad BB may create a new landingpad BB and use indirect branch
+      // jump to the old landingpad BB in lowering.
+      if (isEHPadBB && !I->isDebugInstr() && EHPadIBTNeeded) {
+        if (I->isEHLabel())
+          Changed |= addENDBR(MBB, std::next(I));
+        else
+          Changed |= addENDBR(MBB, I);
+        EHPadIBTNeeded = false;
+      }
+      if (!isEHPadBB && I->isEHLabel() && EHPadIBTNeeded) {
+        MCSymbol *Sym = I->getOperand(0).getMCSymbol();
+        if (MF.hasCallSiteLandingPad(Sym))
           Changed |= addENDBR(MBB, std::next(I));
           EHPadIBTNeeded = false;
       }
     }
   }
   return Changed;
 }

llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll

-; RUN: llc -mtriple=x86_64-unknown-unknown < %s | FileCheck %s
-; RUN: llc -mtriple=i386-unknown-unknown < %s | FileCheck %s
-
-;There should be 2 endbr* instruction at entry and catch pad.
-;CHECK-COUNT-2: endbr
+; RUN: llc -mtriple=x86_64-unknown-unknown < %s | FileCheck %s --check-prefix=X86_64
+; RUN: llc -mtriple=i386-unknown-unknown < %s | FileCheck %s --check-prefix=X86
+; RUN: llc -mtriple i386-windows-gnu -exception-model sjlj -verify-machineinstrs=0 < %s | FileCheck %s --check-prefix=SJLJ
+
+; ALL-LABEL:   test_eh
+; X86_64:      endbr64
+; X86:         endbr32
+; SJLJ:        endbr32
+; ALL:         call{{.}}   __Z20function_that_throwsv
+; SJLJ:        LBB0_{{.}}:
+; SJLJ:        endbr32
+; ALL:         LBB0_{{.}}:   # %lpad
+; X86_64:      endbr64
+; X86:         endbr32
+; SJLJ:        endbr32
 
 declare void @_Z20function_that_throwsv()
 declare i32 @__gxx_personality_sj0(...)
 declare i8* @__cxa_begin_catch(i8*)
 declare void @__cxa_end_catch()
 
-define void @test8() personality i8* bitcast (i32 (...)* @__gxx_personality_sj0 to i8*) {
+define void @test_eh() personality i8* bitcast (i32 (...)* @__gxx_personality_sj0 to i8*) {
 entry:
   invoke void @_Z20function_that_throwsv()
           to label %try.cont unwind label %lpad
 
 lpad:
   %0 = landingpad { i8*, i32 }
           catch i8* null
   %1 = extractvalue { i8*, i32 } %0, 0
   %2 = tail call i8* @__cxa_begin_catch(i8* %1)
   tail call void @__cxa_end_catch()
   br label %try.cont
 
 try.cont:
   ret void
 }
 
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 4, !"cf-protection-branch", i32 1}