[X86] nocf_check: disable tail call

[MaskRay]

When a function pointer is annotated with
void (*fptr)(void) __attribute__((nocf_check));, calling it should use
the NOTRACK prefix, as the callee may not contain an ENDBR.

https://reviews.llvm.org/D41879 implemented NOTRACK variants for
X86ISD::CALL and ISD::BRIND but not for TCRETURN. Given that there are
so many tail call variants (e.g. conditional tailcall
https://reviews.llvm.org/D29856), let's just disable tailcall.
While nocf_check has some uses within the Linux kernel, it isn't a
popular attribute.

Fix #91228

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: Fangrui Song (MaskRay)

Changes

When a function pointer is annotated with
void (*fptr)(void) __attribute__((nocf_check));, calling it should use
the NOTRACK prefix, as the callee may not contain an ENDBR.

https://reviews.llvm.org/D41879 implemented NOTRACK variants for
X86ISD::CALL and ISD::BRIND but not for TCRETURN. Given that there are
so many tail call variants (e.g. conditional tailcall
https://reviews.llvm.org/D29856), let's just disable tailcall.
While nocf_check has some uses within the Linux kernel, it isn't a
popular attribute.

Fix #91228

---

Full diff: https://github.com/llvm/llvm-project/pull/131487.diff

2 Files Affected:

• (modified) llvm/lib/Target/X86/X86ISelLoweringCall.cpp (+9-3)
• (modified) llvm/test/CodeGen/X86/nocf_check.ll (+23)

diff --git a/llvm/lib/Target/X86/X86ISelLoweringCall.cpp b/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
index 80b4aeeda1e00..76451994bb73b 100644
--- a/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
+++ b/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
@@ -2034,11 +2034,17 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
   X86MachineFunctionInfo *X86Info = MF.getInfo<X86MachineFunctionInfo>();
   bool HasNCSR = (CB && isa<CallInst>(CB) &&
                   CB->hasFnAttr("no_caller_saved_registers"));
-  bool HasNoCfCheck = (CB && CB->doesNoCfCheck());
   bool IsIndirectCall = (CB && isa<CallInst>(CB) && CB->isIndirectCall());
   bool IsCFICall = IsIndirectCall && CLI.CFIType;
   const Module *M = MF.getFunction().getParent();
-  Metadata *IsCFProtectionSupported = M->getModuleFlag("cf-protection-branch");
+
+  // If the indirect call target has the nocf_check attribute, the call needs
+  // the NOTRACK prefix. For simplicity just disable tail calls as there there
+  // are too many variants
+  bool IsNoTrackIndirectCall = IsIndirectCall && CB->doesNoCfCheck() &&
+                               M->getModuleFlag("cf-protection-branch");
+  if (IsNoTrackIndirectCall)
+    isTailCall = false;
 
   MachineFunction::CallSiteInfo CSInfo;
   if (CallConv == CallingConv::X86_INTR)
@@ -2549,7 +2555,7 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
 
   // Returns a chain & a glue for retval copy to use.
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
-  if (HasNoCfCheck && IsCFProtectionSupported && IsIndirectCall) {
+  if (IsNoTrackIndirectCall) {
     Chain = DAG.getNode(X86ISD::NT_CALL, dl, NodeTys, Ops);
   } else if (CLI.CB && objcarc::hasAttachedCallOpBundle(CLI.CB)) {
     // Calls with a "clang.arc.attachedcall" bundle are special. They should be
diff --git a/llvm/test/CodeGen/X86/nocf_check.ll b/llvm/test/CodeGen/X86/nocf_check.ll
index d6c6edea55e14..7b184edf8c0b0 100644
--- a/llvm/test/CodeGen/X86/nocf_check.ll
+++ b/llvm/test/CodeGen/X86/nocf_check.ll
@@ -43,6 +43,29 @@ entry:
   ret void
 }
 
+;; NOTRACK tail call is not implemented, so nocf_check just disables tail call.
+define void @NoCfCheckTail(ptr %p) #1 {
+; CHECK-LABEL: NoCfCheckTail:
+; CHECK:       notrack callq *%rax
+  %f = load ptr, ptr %p, align 4
+  tail call void %f() #2
+  ret void
+}
+
+define void @NoCfCheckTailCond(ptr %f, i1 %x) #1 {
+; CHECK-LABEL: NoCfCheckTailCond:
+; CHECK:       notrack callq *%rdi
+; CHECK:       notrack callq *%rdi
+entry:
+  br i1 %x, label %bb1, label %bb2
+bb1:
+  tail call void %f() #2
+  ret void
+bb2:
+  tail call void %f() #2
+  ret void
+}
+
 attributes #0 = { nocf_check noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #1 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #2 = { nocf_check }

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 4286f4dccee9140eef3ef7d059619eaab73fc392 27631f17399363ec1c50a01a64991bc443262d58 --extensions cpp -- llvm/lib/Target/X86/X86ISelLoweringCall.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Target/X86/X86ISelLoweringCall.cpp b/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
index 739a29283b..995e1781c3 100644
--- a/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
+++ b/llvm/lib/Target/X86/X86ISelLoweringCall.cpp
@@ -2032,8 +2032,8 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
       CallConv == CallingConv::Tail || CallConv == CallingConv::SwiftTail;
   bool IsCalleePopSRet = !IsGuaranteeTCO && hasCalleePopSRet(Outs, Subtarget);
   X86MachineFunctionInfo *X86Info = MF.getInfo<X86MachineFunctionInfo>();
-  bool HasNCSR = (CB && isa<CallInst>(CB) &&
-                  CB->hasFnAttr("no_caller_saved_registers"));
+  bool HasNCSR =
+      (CB && isa<CallInst>(CB) && CB->hasFnAttr("no_caller_saved_registers"));
   bool IsIndirectCall = (CB && isa<CallInst>(CB) && CB->isIndirectCall());
   bool IsCFICall = IsIndirectCall && CLI.CFIType;
   const Module *M = MF.getFunction().getParent();

[phoebewang]

LGTM.