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

172

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

67

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

176

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

172

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!

176

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

151

Why do you have to iterate the same loop twice?

[xiangzhangllvm]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

151

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

67

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

152

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

152

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

152

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

152

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

78

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

78

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

#endif
   }
 
   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();
-
     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 SjLj exception model, it will
+    // create a new BB(new landingpad) indirectly jump to the old landingpad.
+    if (TM->Options.ExceptionModel == ExceptionHandling::SjLj) {
+      for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ++I) {
+        // New Landingpad BB without EHLabel.
+        if (MBB.isEHPad()) {
+          if (I->isDebugInstr())
+            continue;
+          Changed |= addENDBR(MBB, I);
+          break;
+        } else if (I->isEHLabel()) {
+          // Old Landingpad BB (is not Landingpad now) with
+          // the the old "callee" EHLabel.
+          MCSymbol *Sym = I->getOperand(0).getMCSymbol();
+          if (!MF.hasCallSiteLandingPad(Sym))
+            continue;
+          Changed |= addENDBR(MBB, std::next(I));
+          break;
+        }
+      }
+    } else if (MBB.isEHPad()){
+      for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ++I) {
+        if (!I->isEHLabel())
+          continue;
         Changed |= addENDBR(MBB, std::next(I));
-        EHPadIBTNeeded = false;
+        break;
       }
     }
   }
   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
+; 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
+
+; X86_64:       test_eh:                                # @test_eh
+; X86_64-NEXT:  .Lfunc_begin0:
+; X86_64:       # %bb.0:                                # %entry
+; X86_64-NEXT:          endbr64
+; X86_64-NEXT:          pushq   %rax
+; X86_64:       .Ltmp0:
+; X86_64-NEXT:          callq   _Z20function_that_throwsv
+; X86_64-NEXT:  .Ltmp1:
+; X86_64-NEXT:  # %bb.1:                                # %try.cont
+; X86_64:               retq
+; X86_64-NEXT:  .LBB0_2:                                # %lpad
+; X86_64-NEXT:          .cfi_def_cfa_offset 16
+; X86_64-NEXT:  .Ltmp2:
+; X86_64-NEXT:          endbr64
+; X86_64:               callq   __cxa_begin_catch
+
+
+; X86:       test_eh:                                # @test_eh
+; X86-NEXT:  .Lfunc_begin0:
+; X86:       # %bb.0:                                # %entry
+; X86-NEXT:          endbr32
+; X86-NEXT:  .Ltmp0:
+; X86:               calll   _Z20function_that_throwsv
+; X86-NEXT:  .Ltmp1:
+; X86-NEXT:  # %bb.1:                                # %try.cont
+; X86-NEXT:          retl
+; X86-NEXT:  .LBB0_2:                                # %lpad
+; X86-NEXT:  .Ltmp2:
+; X86-NEXT:          endbr32
+; X86:               calll   __cxa_begin_catch
+
+
+; SJLJ:       test_eh:
+; SJLJ-NEXT:  Lfunc_begin0:
+; SJLJ-NEXT:  # %bb.0:                                # %entry
+; SJLJ-NEXT:          endbr32
+; SJLJ:               calll   __Unwind_SjLj_Register
+; SJLJ:       Ltmp0:
+; SJLJ:               calll   __Z20function_that_throwsv
+; SJLJ:       LBB0_2:                                 # %try.cont
+; SJLJ:               calll   __Unwind_SjLj_Unregister
+; SJLJ:               retl
+
+; SJLJ:       LBB0_3:
+; SJLJ-NEXT:          endbr32
+; SJLJ-NEXT:          leal
+; SJLJ-NEXT:          movl
+; SJLJ-NEXT:          cmpl
+; SJLJ-NEXT:          jb      LBB0_4
+
+; SJLJ:       LBB0_4:
+; SJLJ-NEXT:          jmpl    *LJTI0_0(,%eax,4)
+
+; SJLJ:       LBB0_6:                                 # %lpad
+; SJLJ-NEXT:  Ltmp2:
+; SJLJ-NEXT:          endbr32
+; SJLJ:               calll   ___cxa_begin_catch
+; SJLJ:               jmp     LBB0_2
+; SJLJ:       LJTI0_0:
+; SJLJ-NEXT:          .long   LBB0_6
+
 
-;There should be 2 endbr* instruction at entry and catch pad.
-;CHECK-COUNT-2: endbr
 
 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}