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 < %s | FileCheck %s
+;Exception handle may indirectly jump to catch pad, So we should add
+;ENDBR before catch pad instructions.
+
+;ModuleID = 't.cc'
+;int func () {
+;  try {
+;    throw 2;
+;  }
+;  catch (int x) {
+;    return x;
+;  }
+;  return 0;
+;}
+
+target triple = "x86_64-unknown-linux-gnu"
+
+@_ZTIi = external dso_local constant i8*
+
+; Function Attrs: noinline optnone uwtable
+define dso_local i32 @_Z4funcv() #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  %retval = alloca i32, align 4
+  %exn.slot = alloca i8*
+  %ehselector.slot = alloca i32
+  %x = alloca i32, align 4
+  %exception = call i8* @__cxa_allocate_exception(i64 4) #2
+  %0 = bitcast i8* %exception to i32*
+  store i32 2, i32* %0, align 16
+  invoke void @__cxa_throw(i8* %exception, i8* bitcast (i8** @_ZTIi to i8*), i8* null) #3
+          to label %unreachable unwind label %lpad
+
+lpad:                                             ; preds = %entry
+  %1 = landingpad { i8*, i32 }
+          catch i8* bitcast (i8** @_ZTIi to i8*)
+  %2 = extractvalue { i8*, i32 } %1, 0
+  store i8* %2, i8** %exn.slot, align 8
+  %3 = extractvalue { i8*, i32 } %1, 1
+  store i32 %3, i32* %ehselector.slot, align 4
+  br label %catch.dispatch
+
+catch.dispatch:                                   ; preds = %lpad
+  %sel = load i32, i32* %ehselector.slot, align 4
+  %4 = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIi to i8*)) #2
+  %matches = icmp eq i32 %sel, %4
+  br i1 %matches, label %catch, label %eh.resume
+
+catch:                                            ; preds = %catch.dispatch
+  %exn = load i8*, i8** %exn.slot, align 8
+  %5 = call i8* @__cxa_begin_catch(i8* %exn) #2
+  %6 = bitcast i8* %5 to i32*
+  %7 = load i32, i32* %6, align 4
+  store i32 %7, i32* %x, align 4
+  %8 = load i32, i32* %x, align 4
+  store i32 %8, i32* %retval, align 4
+  call void @__cxa_end_catch() #2
+  br label %return
+
+try.cont:                                         ; No predecessors!
+  store i32 0, i32* %retval, align 4
+  br label %return
+
+return:                                           ; preds = %try.cont, %catch
+  %9 = load i32, i32* %retval, align 4
+  ret i32 %9
+
+eh.resume:                                        ; preds = %catch.dispatch
+  %exn1 = load i8*, i8** %exn.slot, align 8
+  %sel2 = load i32, i32* %ehselector.slot, align 4
+  %lpad.val = insertvalue { i8*, i32 } undef, i8* %exn1, 0
+  %lpad.val3 = insertvalue { i8*, i32 } %lpad.val, i32 %sel2, 1
+  resume { i8*, i32 } %lpad.val3
+
+unreachable:                                      ; preds = %entry
+  unreachable
+}
+
+declare dso_local i8* @__cxa_allocate_exception(i64)
+
+declare dso_local void @__cxa_throw(i8*, i8*, i8*)
+
+declare dso_local i32 @__gxx_personality_v0(...)
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.eh.typeid.for(i8*) #1
+
+declare dso_local i8* @__cxa_begin_catch(i8*)
+
+declare dso_local void @__cxa_end_catch()
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"cf-protection-branch", i32 1}
+
+
+;CHECK-LABEL: Lfunc_begin0:
+;CHECK: # %bb.0: # %entry
+;CHECK-NEXT: endbr64
+
+;CHECK-LABEL: .LBB0_2: # %lpad
+;CHECK-LABEL: .Ltmp2:
+;CHECK-NEXT:  endbr64