CET for Exception Handle

[xiangzhangllvm]

Summary

Bug fix for https://bugs.llvm.org/show_bug.cgi?id=45182
Exception handle may indirectly jump to catch pad, So we should add ENDBR instruction before catch pad instructions.

[LuoYuanke]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

132

Line 131 ~ 134 can be merged to "bool EHPadIBTNeeded = MBB.isEHPad();"

140

Does it looks better to insert endbr first for EHPAD block?

MachineBasicBlock::iterator I = MBB.begin();
if (MBB.isEHPad()) {
  for (; I != MBB.end(); ++I) {
    if (I->isDebugInstr()) continue;
  }
  Changed |= addENDBR(MBB, std::next(I));
  break;
}
for (; I != MBB.end(); ++I) {
  if (!I->isCall()) continue;
  ...
}

[xiangzhangllvm]

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

132

Yes, you are right, thank you!

140

I have thought that, but it may cost 2 times loop to do it.

[LuoYuanke]

LGTM

[MaskRay]

@pengfei You can strip unneeded Phabricator tags with:

% which arcfilter
arcfilter () {
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,
"");print}' | git commit --amend -F -
}

Subscribers: and Reviewers: are just annoying. It is also not appropriate to use Patch By: Xiang Zhang (xiangzhangllvm). See https://llvm.org/docs/DeveloperPolicy.html#commit-messages " git commit --amend --author="John Doe <[e-mail]> to correct the author property"

[pengfei]

In D76190#1948762, @MaskRay wrote:

@pengfei You can strip unneeded Phabricator tags with:

% which arcfilter
arcfilter () {
        git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: /,
"");print}' | git commit --amend -F -
}

Subscribers: and Reviewers: are just annoying. It is also not appropriate to use Patch By: Xiang Zhang (xiangzhangllvm). See https://llvm.org/docs/DeveloperPolicy.html#commit-messages " git commit --amend --author="John Doe <[e-mail]> to correct the author property"

Thanks for your information. It's very helpful.

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();
+
     for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ++I) {
-      if (!I->isCall())
-        continue;
-      if (IsCallReturnTwice(I->getOperand(0)))
+      if (I->isCall() && IsCallReturnTwice(I->getOperand(0)))
+        Changed |= addENDBR(MBB, std::next(I));
+
+      if (EHPadIBTNeeded && I->isEHLabel()) {
           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
+
+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*) {
+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}