[PATCH] D111108: [X86] Fix fentry handling in X86IndirectBranchTracking.cpp

[Joao Moreira via Phabricator via llvm-commits]

joaomoreira created this revision.
joaomoreira added reviewers: craig.topper, xiangzhangllvm, oren_ben_simhon, amilburn.
joaomoreira added a project: LLVM.
Herald added subscribers: pengfei, hiraditya.
joaomoreira requested review of this revision.
Herald added a subscriber: llvm-commits.

When compiling with indirect branch tracking and fentry (-fcf-protection=branch -mfentry -pg) the X86IndirectBranchTrackingPass will attempt to place endbr in basic blocks, checking for Calls/IsCallReturnTwice. For calling the function IsCallReturnTwice(), the pass attempts to retrieve the first operand of the respective machine instruction. Since FENTRY_CALL is considered a call, and it does not have any argument, the condition inside the pass will attempt to call IsCallReturnTwice on the machine instruction, but since it does not have operands, it will lead into a crash.

      

Kudos to Alyssa Milburn for helping in the issue triage. The diff brings a test, but to reproduce the problem, follow the steps below.

      

  echo "int main() {};" > repro.c
  clang repro.c -fcf-protection=branch -mfentry -pg


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111108

Files:
  llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
  llvm/test/CodeGen/X86/fentry-ibt.ll


Index: llvm/test/CodeGen/X86/fentry-ibt.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/fentry-ibt.ll
@@ -0,0 +1,17 @@
+; RUN: llc %s -o - -verify-machineinstrs | FileCheck %s
+
+define void @test1() #0 {
+entry:
+  ret void
+
+; CHECK-LABEL: @test1
+; CHECK: endbr64
+; CHECK: callq __fentry__
+; CHECK-NOT: mcount
+; CHECK: retq
+}
+
+!llvm.module.flags = !{!0}
+
+attributes #0 = { "fentry-call"="true" }
+!0 = !{i32 4, !"cf-protection-branch", i32 1}
Index: llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
===================================================================
--- llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
+++ llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
@@ -137,8 +137,10 @@
       Changed |= addENDBR(MBB, MBB.begin());
 
     for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ++I) {
-      if (I->isCall() && IsCallReturnTwice(I->getOperand(0)))
+      if (I->isCall() && I->getNumOperands() > 0 &&
+          IsCallReturnTwice(I->getOperand(0))) {
         Changed |= addENDBR(MBB, std::next(I));
+      }
     }
 
     // Exception handle may indirectly jump to catch pad, So we should add


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111108.377067.patch
Type: text/x-patch
Size: 1207 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211005/d4d179a1/attachment.bin>