[X86] Check if call is indirect before emitting NT_CALL

[joaomoreira]

Summary

The notrack prefix is a relaxation of CET policies which makes it possible to indirectly call targets which do not have an ENDBR instruction in the landing address. To emit a call with this prefix, the special attribute "nocf_check" is used. When used as a function attribute, a CallInst targeting the respective function will return true for the method "doesNoCfCheck()", no matter if it is a direct call (and such should remain like this, as the information that the to-be-called function won't perform control-flow checks is useful in other contexts). Yet, when emitting an X86ISD::NT_CALL, the respective CallInst should be verified for its indirection, allowing that the prefixed calls are only emitted in the right situations.

Update the respective testing unit to also verify for direct calls to functions with ''nocf_check'' attributes.

The bug can also be reproduced through compiling the following C code using the -fcf-protection=full flag.

int __attribute__((nocf_check)) foo(int a) {};

int main() {
	foo(42);
}

[xiangzhangllvm]

Should we add a test here ?

[RKSimon]

This needs test coverage.

Also, please regenerate the diff with context (e.g. git diff -U9999 HEAD)

[joaomoreira]

The test was updated in the last revision, diff was also updated for context. Is there anything else needed for this?

[craig.topper]

LGTM

llvm/lib/Target/X86/X86ISelLowering.cpp

X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
   X86MachineFunctionInfo *X86Info = MF.getInfo<X86MachineFunctionInfo>();
   const auto *CI = dyn_cast_or_null<CallInst>(CLI.CB);
   const Function *Fn = CI ? CI->getCalledFunction() : nullptr;
   bool HasNCSR = (CI && CI->hasFnAttr("no_caller_saved_registers")) ||
                  (Fn && Fn->hasFnAttribute("no_caller_saved_registers"));
   const auto *II = dyn_cast_or_null<InvokeInst>(CLI.CB);
   bool HasNoCfCheck =
       (CI && CI->doesNoCfCheck()) || (II && II->doesNoCfCheck());
+  bool IsIndirectCall = (CI && CI->isIndirectCall());
   const Module *M = MF.getMMI().getModule();
   Metadata *IsCFProtectionSupported = M->getModuleFlag("cf-protection-branch");
 
   MachineFunction::CallSiteInfo CSInfo;
   if (CallConv == CallingConv::X86_INTR)
     report_fatal_error("X86 interrupts may not be called directly");
 
   if (Subtarget.isPICStyleGOT() && !IsGuaranteeTCO) {
if (isTailCall) {
     // should be computed from returns not tail calls.  Consider a void
     // function making a tail call to a function returning int.
     MF.getFrameInfo().setHasTailCall();
     SDValue Ret = DAG.getNode(X86ISD::TC_RETURN, dl, NodeTys, Ops);
     DAG.addCallSiteInfo(Ret.getNode(), std::move(CSInfo));
     return Ret;
   }
 
-  if (HasNoCfCheck && IsCFProtectionSupported) {
+  if (HasNoCfCheck && IsCFProtectionSupported && IsIndirectCall) {
     Chain = DAG.getNode(X86ISD::NT_CALL, dl, NodeTys, Ops);
   } else {
     Chain = DAG.getNode(X86ISD::CALL, dl, NodeTys, Ops);
   }
   InFlag = Chain.getValue(1);
   DAG.addNoMergeSiteInfo(Chain.getNode(), CLI.NoMerge);
   DAG.addCallSiteInfo(Chain.getNode(), std::move(CSInfo));
 

llvm/test/CodeGen/X86/nocf_check.ll

 ; RUN: llc -mtriple=x86_64-unknown-unknown -x86-indirect-branch-tracking < %s | FileCheck %s
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; This test verify the handling of ''nocf_check'' attribute by the backend. ;;
 ;; The file was generated using the following C code:                        ;;
 ;;                                                                           ;;
 ;; void __attribute__((nocf_check)) NoCfCheckFunc(void) {}                   ;;
 ;;                                                                           ;;
 ;; typedef void(*FuncPointer)(void);                                         ;;
 ;; void NoCfCheckCall(FuncPointer f) {                                       ;;
 ;;   __attribute__((nocf_check)) FuncPointer p = f;                          ;;
 ;;   (*p)();                                                                 ;;
+;;   NoCfCheckFunc();                                                        ;;
 ;; }                                                                         ;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 ; Make sure that a function with ''nocf_check'' attribute is not instrumented
 ; with endbr instruction at the beginning.
 define void @NoCfCheckFunc() #0 {
 ; CHECK-LABEL: NoCfCheckFunc
 ; CHECK-NOT:   endbr64
 ; CHECK:       retq
 entry:
   ret void
 }
 
-; Make sure that notrack prefix is added before a call with ''nocf_check'' attribute.
-define void @NoCfCheckCall(void ()* %f) {
+; Ensure the notrack prefix is added before an indirect call using a pointer
+; with ''nocf_check'' attribute. Also ensure a direct call to a function with
+; the ''nocf_check'' attribute is correctly generated without notrack prefix.
+define void @NoCfCheckCall(void ()* %f) #1 {
 ; CHECK-LABEL: NoCfCheckCall
 ; CHECK:       notrack call
+; CHECK:       callq NoCfCheckFunc
 entry:
   %f.addr = alloca void ()*, align 4
   %p = alloca void ()*, align 4
   store void ()* %f, void ()** %f.addr, align 4
   %0 = load void ()*, void ()** %f.addr, align 4
   store void ()* %0, void ()** %p, align 4
   %1 = load void ()*, void ()** %p, align 4
-  call void %1() #1
+  call void %1() #2
+  call void @NoCfCheckFunc() #2
   ret void
 }
 
-attributes #0 = { noinline nocf_check nounwind optnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "frame-pointer"="none" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
-attributes #1 = { nocf_check }
+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 }
 
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 4, !"cf-protection-branch", i32 1}