[codeview] Fix 32-bit x86 variable locations in realigned stack frames

Add the .cv_fpo_stackalign directive so that we can define $T0, or the
VFRAME virtual register, with it. This was overlooked in the initial
implementation because unlike MSVC, we push CSRs before allocating stack
space, so this value is only needed to describe local variable
locations. Variables that the compiler now addresses via ESP are instead
described as being stored at offsets from VFRAME, which for us is ESP
after alignment in the prologue.

This adds tests that show that we use the VFRAME register properly in
our S_DEFRANGE records, and that we emit the correct FPO data to define
it.

Fixes PR38857

llvm-svn: 343603

8 files changed, 90 insertions, 8 deletions

diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index aaa1dc95757..707ee410d76 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -1255,6 +1255,7 @@ void CodeViewDebug::beginFunctionImpl(const MachineFunction *MF) {
   // instruction (AArch64), this will be zero.
   CurFn->CSRSize = MFI.getCVBytesOfCalleeSavedRegisters();
   CurFn->FrameSize = MFI.getStackSize();
+  CurFn->HasStackRealignment = TRI->needsStackRealignment(*MF);
 
   // For this function S_FRAMEPROC record, figure out which codeview register
   // will be the frame pointer.
@@ -1267,7 +1268,7 @@ void CodeViewDebug::beginFunctionImpl(const MachineFunction *MF) {
     } else {
       // If there is an FP, parameters are always relative to it.
       CurFn->EncodedParamFramePtrReg = EncodedFramePtrReg::FramePtr;
-      if (TRI->needsStackRealignment(*MF)) {
+      if (CurFn->HasStackRealignment) {
         // If the stack needs realignment, locals are relative to SP or VFRAME.
         CurFn->EncodedLocalFramePtrReg = EncodedFramePtrReg::StackPtr;
       } else {
@@ -2502,13 +2503,18 @@ void CodeViewDebug::emitLocalVariable(const FunctionInfo &FI,
       int Offset = DefRange.DataOffset;
       unsigned Reg = DefRange.CVRegister;
 
-      // x86 call sequences often use PUSH instructions, which disrupt
+      // 32-bit x86 call sequences often use PUSH instructions, which disrupt
       // ESP-relative offsets. Use the virtual frame pointer, VFRAME or $T0,
-      // instead. In simple cases, $T0 will be the CFA. If the frame required
-      // re-alignment, it will be the CFA aligned downwards.
+      // instead. In simple cases, $T0 will be the CFA.
       if (RegisterId(Reg) == RegisterId::ESP) {
         Reg = unsigned(RegisterId::VFRAME);
         Offset -= FI.FrameSize;
+
+        // If the frame requires realignment, VFRAME will be ESP after it is
+        // aligned. We have to remove the ESP adjustments made to push CSRs and
+        // EBP. EBP is not included in CSRSize.
+        if (FI.HasStackRealignment)
+          Offset += FI.CSRSize + 4;
       }
 
       // If we can use the chosen frame pointer for the frame and this isn't a
diff --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
index 3836862f0a8..b2864306e50 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
@@ -165,6 +165,8 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
 
     codeview::FrameProcedureOptions FrameProcOpts;
 
+    bool HasStackRealignment = false;
+
     bool HaveLineInfo = false;
   };
   FunctionInfo *CurFn = nullptr;
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 8b7b250e1a0..e67daa5d857 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -852,6 +852,7 @@ private:
   bool parseDirectiveFPOSetFrame(SMLoc L);
   bool parseDirectiveFPOPushReg(SMLoc L);
   bool parseDirectiveFPOStackAlloc(SMLoc L);
+  bool parseDirectiveFPOStackAlign(SMLoc L);
   bool parseDirectiveFPOEndPrologue(SMLoc L);
   bool parseDirectiveFPOEndProc(SMLoc L);
   bool parseDirectiveFPOData(SMLoc L);
@@ -3315,6 +3316,8 @@ bool X86AsmParser::ParseDirective(AsmToken DirectiveID) {
     return parseDirectiveFPOPushReg(DirectiveID.getLoc());
   else if (IDVal == ".cv_fpo_stackalloc")
     return parseDirectiveFPOStackAlloc(DirectiveID.getLoc());
+  else if (IDVal == ".cv_fpo_stackalign")
+    return parseDirectiveFPOStackAlign(DirectiveID.getLoc());
   else if (IDVal == ".cv_fpo_endprologue")
     return parseDirectiveFPOEndPrologue(DirectiveID.getLoc());
   else if (IDVal == ".cv_fpo_endproc")
@@ -3429,6 +3432,16 @@ bool X86AsmParser::parseDirectiveFPOStackAlloc(SMLoc L) {
   return getTargetStreamer().emitFPOStackAlloc(Offset, L);
 }
 
+// .cv_fpo_stackalign 8
+bool X86AsmParser::parseDirectiveFPOStackAlign(SMLoc L) {
+  MCAsmParser &Parser = getParser();
+  int64_t Offset;
+  if (Parser.parseIntToken(Offset, "expected offset") ||
+      Parser.parseEOL("unexpected tokens"))
+    return addErrorSuffix(" in '.cv_fpo_stackalign' directive");
+  return getTargetStreamer().emitFPOStackAlign(Offset, L);
+}
+
 // .cv_fpo_endprologue
 bool X86AsmParser::parseDirectiveFPOEndPrologue(SMLoc L) {
   MCAsmParser &Parser = getParser();
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86TargetStreamer.h b/llvm/lib/Target/X86/MCTargetDesc/X86TargetStreamer.h
index 8d38cd32b82..10a282dd296 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86TargetStreamer.h
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86TargetStreamer.h
@@ -26,6 +26,7 @@ public:
   virtual bool emitFPOData(const MCSymbol *ProcSym, SMLoc L = {}) = 0;
   virtual bool emitFPOPushReg(unsigned Reg, SMLoc L = {}) = 0;
   virtual bool emitFPOStackAlloc(unsigned StackAlloc, SMLoc L = {}) = 0;
+  virtual bool emitFPOStackAlign(unsigned Align, SMLoc L = {}) = 0;
   virtual bool emitFPOSetFrame(unsigned Reg, SMLoc L = {}) = 0;
 };
 
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp
index 093dab4f2f9..bee9b704633 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp
@@ -38,6 +38,7 @@ public:
   bool emitFPOData(const MCSymbol *ProcSym, SMLoc L) override;
   bool emitFPOPushReg(unsigned Reg, SMLoc L) override;
   bool emitFPOStackAlloc(unsigned StackAlloc, SMLoc L) override;
+  bool emitFPOStackAlign(unsigned Align, SMLoc L) override;
   bool emitFPOSetFrame(unsigned Reg, SMLoc L) override;
 };
 
@@ -47,6 +48,7 @@ struct FPOInstruction {
   enum Operation {
     PushReg,
     StackAlloc,
+    StackAlign,
     SetFrame,
   } Op;
   unsigned RegOrOffset;
@@ -90,6 +92,7 @@ public:
   bool emitFPOData(const MCSymbol *ProcSym, SMLoc L) override;
   bool emitFPOPushReg(unsigned Reg, SMLoc L) override;
   bool emitFPOStackAlloc(unsigned StackAlloc, SMLoc L) override;
+  bool emitFPOStackAlign(unsigned Align, SMLoc L) override;
   bool emitFPOSetFrame(unsigned Reg, SMLoc L) override;
 };
 } // end namespace
@@ -133,6 +136,11 @@ bool X86WinCOFFAsmTargetStreamer::emitFPOStackAlloc(unsigned StackAlloc,
   return false;
 }
 
+bool X86WinCOFFAsmTargetStreamer::emitFPOStackAlign(unsigned Align, SMLoc L) {
+  OS << "\t.cv_fpo_stackalign\t" << Align << '\n';
+  return false;
+}
+
 bool X86WinCOFFAsmTargetStreamer::emitFPOSetFrame(unsigned Reg, SMLoc L) {
   OS << "\t.cv_fpo_setframe\t";
   InstPrinter.printRegName(OS, Reg);
@@ -226,6 +234,24 @@ bool X86WinCOFFTargetStreamer::emitFPOStackAlloc(unsigned StackAlloc, SMLoc L) {
   return false;
 }
 
+bool X86WinCOFFTargetStreamer::emitFPOStackAlign(unsigned Align, SMLoc L) {
+  if (checkInFPOPrologue(L))
+    return true;
+  if (!llvm::any_of(CurFPOData->Instructions, [](const FPOInstruction &Inst) {
+        return Inst.Op == FPOInstruction::SetFrame;
+      })) {
+    getContext().reportError(
+        L, "a frame register must be established before aligning the stack");
+    return true;
+  }
+  FPOInstruction Inst;
+  Inst.Label = emitFPOLabel();
+  Inst.Op = FPOInstruction::StackAlign;
+  Inst.RegOrOffset = Align;
+  CurFPOData->Instructions.push_back(Inst);
+  return false;
+}
+
 bool X86WinCOFFTargetStreamer::emitFPOEndPrologue(SMLoc L) {
   if (checkInFPOPrologue(L))
     return true;
@@ -250,6 +276,8 @@ struct FPOStateMachine {
   unsigned CurOffset = 0;
   unsigned LocalSize = 0;
   unsigned SavedRegSize = 0;
+  unsigned StackOffsetBeforeAlign = 0;
+  unsigned StackAlign = 0;
   unsigned Flags = 0; // FIXME: Set HasSEH / HasEH.
 
   SmallString<128> FrameFunc;
@@ -291,24 +319,39 @@ void FPOStateMachine::emitFrameDataRecord(MCStreamer &OS, MCSymbol *Label) {
   FrameFunc.clear();
   raw_svector_ostream FuncOS(FrameFunc);
   const MCRegisterInfo *MRI = OS.getContext().getRegisterInfo();
+  assert((StackAlign == 0 || FrameReg != 0) &&
+         "cannot align stack without frame reg");
+  StringRef CFAVar = StackAlign == 0 ? "$T0" : "$T1";
+
   if (FrameReg) {
     // CFA is FrameReg + FrameRegOff.
-    FuncOS << "$T0 " << printFPOReg(MRI, FrameReg) << " " << FrameRegOff
+    FuncOS << CFAVar << ' ' << printFPOReg(MRI, FrameReg) << ' ' << FrameRegOff
            << " + = ";
+
+    // Assign $T0, the VFRAME register, the value of ESP after it is aligned.
+    // Starting from the CFA, we subtract the size of all pushed registers, and
+    // align the result. While we don't store any CSRs in this area, $T0 is used
+    // by S_DEFRANGE_FRAMEPOINTER_REL records to find local variables.
+    if (StackAlign) {
+      FuncOS << "$T0 " << CFAVar << ' ' << StackOffsetBeforeAlign << " - "
+             << StackAlign << " @ = ";
+    }
   } else {
     // The address of return address is ESP + CurOffset, but we use .raSearch to
     // match MSVC. This seems to ask the debugger to subtract some combination
     // of LocalSize and SavedRegSize from ESP and grovel around in that memory
     // to find the address of a plausible return address.
-    FuncOS << "$T0 .raSearch = ";
+    FuncOS << CFAVar << " .raSearch = ";
   }
 
   // Caller's $eip should be dereferenced CFA, and $esp should be CFA plus 4.
-  FuncOS << "$eip $T0 ^ = $esp $T0 4 + = ";
+  FuncOS << "$eip " << CFAVar << " ^ = ";
+  FuncOS << "$esp " << CFAVar << " 4 + = ";
 
   // Each saved register is stored at an unchanging negative CFA offset.
   for (RegSaveOffset RO : RegSaveOffsets)
-    FuncOS << printFPOReg(MRI, RO.Reg) << " $T0 " << RO.Offset << " - ^ = ";
+    FuncOS << printFPOReg(MRI, RO.Reg) << ' ' << CFAVar << ' ' << RO.Offset
+           << " - ^ = ";
 
   // Add it to the CV string table.
   CodeViewContext &CVCtx = OS.getContext().getCVContext();
@@ -380,6 +423,10 @@ bool X86WinCOFFTargetStreamer::emitFPOData(const MCSymbol *ProcSym, SMLoc L) {
       FSM.FrameReg = Inst.RegOrOffset;
       FSM.FrameRegOff = FSM.CurOffset;
       break;
+    case FPOInstruction::StackAlign:
+      FSM.StackOffsetBeforeAlign = FSM.CurOffset;
+      FSM.StackAlign = Inst.RegOrOffset;
+      break;
     case FPOInstruction::StackAlloc:
       FSM.CurOffset += Inst.RegOrOffset;
       FSM.LocalSize += Inst.RegOrOffset;
diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
index 2d660ac8826..1eb9fa0bc1e 100644
--- a/llvm/lib/Target/X86/X86FrameLowering.cpp
+++ b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -1208,6 +1208,13 @@ void X86FrameLowering::emitPrologue(MachineFunction &MF,
   if (!IsWin64Prologue && !IsFunclet && TRI->needsStackRealignment(MF)) {
     assert(HasFP && "There should be a frame pointer if stack is realigned.");
     BuildStackAlignAND(MBB, MBBI, DL, StackPtr, MaxAlign);
+
+    if (NeedsWinCFI) {
+      HasWinCFI = true;
+      BuildMI(MBB, MBBI, DL, TII.get(X86::SEH_StackAlign))
+          .addImm(MaxAlign)
+          .setMIFlag(MachineInstr::FrameSetup);
+    }
   }
 
   // If there is an SUB32ri of ESP immediately before this instruction, merge
diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index 111dc28f2eb..ca354fae833 100644
--- a/llvm/lib/Target/X86/X86InstrCompiler.td
+++ b/llvm/lib/Target/X86/X86InstrCompiler.td
@@ -240,6 +240,8 @@ let isPseudo = 1, SchedRW = [WriteSystem] in {
                             "#SEH_SaveXMM $reg, $dst", []>;
   def SEH_StackAlloc : I<0, Pseudo, (outs), (ins i32imm:$size),
                             "#SEH_StackAlloc $size", []>;
+  def SEH_StackAlign : I<0, Pseudo, (outs), (ins i32imm:$align),
+                            "#SEH_StackAlign $align", []>;
   def SEH_SetFrame : I<0, Pseudo, (outs), (ins i32imm:$reg, i32imm:$offset),
                             "#SEH_SetFrame $reg, $offset", []>;
   def SEH_PushFrame : I<0, Pseudo, (outs), (ins i1imm:$mode),
diff --git a/llvm/lib/Target/X86/X86MCInstLower.cpp b/llvm/lib/Target/X86/X86MCInstLower.cpp
index a6251ffb2e1..acb2bc20858 100644
--- a/llvm/lib/Target/X86/X86MCInstLower.cpp
+++ b/llvm/lib/Target/X86/X86MCInstLower.cpp
@@ -1532,6 +1532,9 @@ void X86AsmPrinter::EmitSEHInstruction(const MachineInstr *MI) {
     case X86::SEH_StackAlloc:
       XTS->emitFPOStackAlloc(MI->getOperand(0).getImm());
       break;
+    case X86::SEH_StackAlign:
+      XTS->emitFPOStackAlign(MI->getOperand(0).getImm());
+      break;
     case X86::SEH_SetFrame:
       assert(MI->getOperand(1).getImm() == 0 &&
              ".cv_fpo_setframe takes no offset");
@@ -1809,6 +1812,7 @@ void X86AsmPrinter::EmitInstruction(const MachineInstr *MI) {
   case X86::SEH_SaveReg:
   case X86::SEH_SaveXMM:
   case X86::SEH_StackAlloc:
+  case X86::SEH_StackAlign:
   case X86::SEH_SetFrame:
   case X86::SEH_PushFrame:
   case X86::SEH_EndPrologue: