[musttail] Don't forward incoming registers over call site registers

[rnk]

Summary

AL is only used for varargs on SysV platforms. Don't forward it on
Windows. This allows control flow guard to set up an extra hidden
parameter in RAX, as described in PR44049.

This also has the effect of freeing up RAX for use in virtual member
pointer thunks, which may also be a nice little code size improvement on
Win64.

Fixes PR44049

[efriedma]

llvm/lib/Target/X86/X86ISelLowering.cpp

3912

This doesn't seem right... either we need to forward the value, or we don't.

We shouldn't be forwarding AL for Windows callling conventions; that convention for specifying the number of xmm arguments only applies to the "Linux" varargs ABI.

[rnk]

llvm/lib/Target/X86/X86ISelLowering.cpp

3912

So, I actually feel like musttail thunks really shouldn't assume a calling convention at all, they should simply preserve all non-CSR registers, including things like RAX, R11, R10, which are often scratch, and used for nest parameters, this CFG side parameter, and retpoline thunks. This makes it a much more general device for doing perfect forwarding. This change here is a step in that direction.

[efriedma]

llvm/lib/Target/X86/X86ISelLowering.cpp

3912

I guess my concern here, really, is that the "CCInfo.isAllocated" check seems sort of fragile? But I guess it's a fundamental part of the way the forwarding works; we have argument registers that may or may not be used by a known argument.

It still seems confusing if getForwardedMustTailRegParms() contains a register we know doesn't actually need to forward. And having a comment here describing that unnecessary functionality doesn't help.

[ajpaverd]

Thanks for finding and fixing this @rnk! I've double-checked that Windows doesn't use AL for varargs (https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=vs-2019#varargs), but having a general mechanism for forwarding that doesn't assume a particular calling convention sounds like a good approach. This looks good from the control flow guard perspective.

[rnk]

• Don't forward AL on Windows instead.

[efriedma]

LGTM

[rnk]

Thanks for the quick review, sorry for creating the extra back & forth.

llvm/lib/Target/X86/X86ISelLowering.cpp

if (isVarArg && MFI.hasMustTailInVarArgFunc()) {
     if (VecVT != MVT::Other)
       RegParmTypes.push_back(VecVT);
 
     // Compute the set of forwarded registers. The rest are scratch.
     SmallVectorImpl<ForwardedRegister> &Forwards =
         FuncInfo->getForwardedMustTailRegParms();
     CCInfo.analyzeMustTailForwardedRegisters(Forwards, RegParmTypes, CC_X86);
 
-    // Conservatively forward AL on x86_64, since it might be used for varargs.
-    if (Is64Bit && !CCInfo.isAllocated(X86::AL)) {
+    // Forward AL for SysV x86_64 targets, since it is used for varargs.
+    if (Is64Bit && !IsWin64 && !CCInfo.isAllocated(X86::AL)) {
       unsigned ALVReg = MF.addLiveIn(X86::AL, &X86::GR8RegClass);
       Forwards.push_back(ForwardedRegister(ALVReg, X86::AL, MVT::i8));
     }
 
     // Copy all forwards from physical to virtual registers.
     for (ForwardedRegister &FR : Forwards) {
       // FIXME: Can we use a less constrained schedule?
       SDValue RegVal = DAG.getCopyFromReg(Chain, dl, FR.VReg, FR.VT);
RegsToPass.push_back(std::make_pair(unsigned(X86::AL),
                                                         MVT::i8)));
   }
 
   if (isVarArg && IsMustTail) {
     const auto &Forwards = X86Info->getForwardedMustTailRegParms();
     for (const auto &F : Forwards) {
       SDValue Val = DAG.getCopyFromReg(Chain, dl, F.VReg, F.VT);
       RegsToPass.push_back(std::make_pair(unsigned(F.PReg), Val));
     }
   }
 
   // For tail calls lower the arguments to the 'real' stack slots.  Sibcalls
   // don't need this because the eligibility check rejects calls that require
   // shuffling arguments passed in memory.
   if (!IsSibcall && isTailCall) {
     // Force all the incoming stack arguments to be loaded from the stack
     // before any new outgoing arguments are stored to the stack, because the

llvm/test/CodeGen/X86/cfguard-checks.ll

entry:
 
   ; X64-LABEL: func_cf_tail
   ; X64:       leaq target_func(%rip), %rax
   ; X64:       movq __guard_dispatch_icall_fptr(%rip), %rcx
   ; X64:       rex64 jmpq *%rcx            # TAILCALL
   ; X64-NOT:   callq
 }
 
+%struct.Foo = type { i32 (%struct.Foo*)** }
+
+; Test that Control Flow Guard checks are correctly added for variadic musttail
+; calls. These are used for MS C++ ABI virtual member pointer thunks.
+; PR44049
+define i32 @vmptr_thunk(%struct.Foo* inreg %p) {
+entry:
+  %vptr.addr = getelementptr inbounds %struct.Foo, %struct.Foo* %p, i32 0, i32 0
+  %vptr = load i32 (%struct.Foo*)**, i32 (%struct.Foo*)*** %vptr.addr
+  %slot = getelementptr inbounds i32 (%struct.Foo*)*, i32 (%struct.Foo*)** %vptr, i32 1
+  %vmethod = load i32 (%struct.Foo*)*, i32 (%struct.Foo*)** %slot
+  %rv = musttail call i32 %vmethod(%struct.Foo* inreg %p)
+  ret i32 %rv
+
+  ; On i686, the call to __guard_check_icall_fptr should come immediately before the call to the target function.
+  ; X32-LABEL: _vmptr_thunk:
+  ; X32:       movl %eax, %esi
+  ; X32:       movl (%eax), %eax
+  ; X32:       movl 4(%eax), %ecx
+  ; X32:       calll *___guard_check_icall_fptr
+  ; X32:       movl %esi, %eax
+  ; X32:       jmpl       *%ecx                  # TAILCALL
+  ; X32-NOT:   calll
+
+  ; Use NEXT here because we previously had an extra instruction in this sequence.
+  ; X64-LABEL: vmptr_thunk:
+  ; X64:            movq (%rcx), %rax
+  ; X64-NEXT:       movq 8(%rax), %rax
+  ; X64-NEXT:       movq __guard_dispatch_icall_fptr(%rip), %rdx
+  ; X64-NEXT:       addq $40, %rsp
+  ; X64-NEXT:       rex64 jmpq *%rdx            # TAILCALL
+  ; X64-NOT:   callq
+}
 
 ; Test that longjmp targets have public labels and are included in the .gljmp section.
 %struct._SETJMP_FLOAT128 = type { [2 x i64] }
 @buf1 = internal global [16 x %struct._SETJMP_FLOAT128] zeroinitializer, align 16
 
 define i32 @func_cf_setjmp() {
   %1 = alloca i32, align 4
   %2 = alloca i32, align 4

llvm/test/CodeGen/X86/musttail-varargs.ll

 ; WINDOWS-LABEL: f_thunk:
 ; WINDOWS:       # %bb.0:
 ; WINDOWS-NEXT:    pushq %r14
 ; WINDOWS-NEXT:    .seh_pushreg %r14
 ; WINDOWS-NEXT:    pushq %rsi
 ; WINDOWS-NEXT:    .seh_pushreg %rsi
 ; WINDOWS-NEXT:    pushq %rdi
 ; WINDOWS-NEXT:    .seh_pushreg %rdi
-; WINDOWS-NEXT:    pushq %rbp
-; WINDOWS-NEXT:    .seh_pushreg %rbp
 ; WINDOWS-NEXT:    pushq %rbx
 ; WINDOWS-NEXT:    .seh_pushreg %rbx
-; WINDOWS-NEXT:    subq $64, %rsp
-; WINDOWS-NEXT:    .seh_stackalloc 64
+; WINDOWS-NEXT:    subq $72, %rsp
+; WINDOWS-NEXT:    .seh_stackalloc 72
 ; WINDOWS-NEXT:    .seh_endprologue
-; WINDOWS-NEXT:    movl %eax, %r14d
-; WINDOWS-NEXT:    movq %r9, %rsi
+; WINDOWS-NEXT:    movq %r9, %r14
 ; WINDOWS-NEXT:    movq %r8, %rdi
 ; WINDOWS-NEXT:    movq %rdx, %rbx
-; WINDOWS-NEXT:    movq %rcx, %rbp
+; WINDOWS-NEXT:    movq %rcx, %rsi
 ; WINDOWS-NEXT:    movq %rdx, {{[0-9]+}}(%rsp)
 ; WINDOWS-NEXT:    movq %r8, {{[0-9]+}}(%rsp)
 ; WINDOWS-NEXT:    movq %r9, {{[0-9]+}}(%rsp)
 ; WINDOWS-NEXT:    leaq {{[0-9]+}}(%rsp), %rax
 ; WINDOWS-NEXT:    movq %rax, {{[0-9]+}}(%rsp)
 ; WINDOWS-NEXT:    callq get_f
-; WINDOWS-NEXT:    movq %rax, %r10
-; WINDOWS-NEXT:    movq %rbp, %rcx
+; WINDOWS-NEXT:    movq %rsi, %rcx
 ; WINDOWS-NEXT:    movq %rbx, %rdx
 ; WINDOWS-NEXT:    movq %rdi, %r8
-; WINDOWS-NEXT:    movq %rsi, %r9
-; WINDOWS-NEXT:    movl %r14d, %eax
-; WINDOWS-NEXT:    addq $64, %rsp
+; WINDOWS-NEXT:    movq %r14, %r9
+; WINDOWS-NEXT:    addq $72, %rsp
 ; WINDOWS-NEXT:    popq %rbx
-; WINDOWS-NEXT:    popq %rbp
 ; WINDOWS-NEXT:    popq %rdi
 ; WINDOWS-NEXT:    popq %rsi
 ; WINDOWS-NEXT:    popq %r14
-; WINDOWS-NEXT:    rex64 jmpq *%r10 # TAILCALL
+; WINDOWS-NEXT:    rex64 jmpq *%rax # TAILCALL
 ; WINDOWS-NEXT:    .seh_handlerdata
 ; WINDOWS-NEXT:    .text
 ; WINDOWS-NEXT:    .seh_endproc
 ;
 ; X86-NOSSE-LABEL: f_thunk:
 ; X86-NOSSE:       # %bb.0:
 ; X86-NOSSE-NEXT:    pushl %ebp
 ; X86-NOSSE-NEXT:    movl %esp, %ebp

 ; WINDOWS-LABEL: h_thunk:
 ; WINDOWS:       # %bb.0:
 ; WINDOWS-NEXT:    subq $40, %rsp
 ; WINDOWS-NEXT:    .seh_stackalloc 40
 ; WINDOWS-NEXT:    .seh_endprologue
 ; WINDOWS-NEXT:    cmpb $1, (%rcx)
 ; WINDOWS-NEXT:    jne .LBB2_2
 ; WINDOWS-NEXT:  # %bb.1: # %then
-; WINDOWS-NEXT:    movq 8(%rcx), %r10
+; WINDOWS-NEXT:    movq 8(%rcx), %rax
 ; WINDOWS-NEXT:    addq $40, %rsp
-; WINDOWS-NEXT:    rex64 jmpq *%r10 # TAILCALL
+; WINDOWS-NEXT:    rex64 jmpq *%rax # TAILCALL
 ; WINDOWS-NEXT:  .LBB2_2: # %else
-; WINDOWS-NEXT:    movq 16(%rcx), %r10
+; WINDOWS-NEXT:    movq 16(%rcx), %rax
 ; WINDOWS-NEXT:    movl $42, {{.*}}(%rip)
 ; WINDOWS-NEXT:    addq $40, %rsp
-; WINDOWS-NEXT:    rex64 jmpq *%r10 # TAILCALL
+; WINDOWS-NEXT:    rex64 jmpq *%rax # TAILCALL
 ; WINDOWS-NEXT:    .seh_handlerdata
 ; WINDOWS-NEXT:    .text
 ; WINDOWS-NEXT:    .seh_endproc
 ;
 ; X86-LABEL: h_thunk:
 ; X86:       # %bb.0:
 ; X86-NEXT:    pushl %eax
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax