[X86] Use the CFA as the DWARF frame base for better variable locatio…

…ns around calls. Prior to this patch, for the DWARF frame base LLVM uses the frame pointer register if available, otherwise the stack pointer register. If the stack pointer register is being used and a call or other code modifies the stack pointer during the body of the function this results in the locations being wrong and the debugger displaying the wrong values for variables. By using DW_OP_call_frame_cfa in these situations the emitted location for the variable will automatically handle changes in the stack pointer. The CFA needs to be adjusted for the offset between the frame pointer/stack pointer to allow the variable locations themselves to remain unchanged by this patch. Reviewed By: #debug-info, scott.linder, jryans Differential Revision: https://reviews.llvm.org/D143463
llvm · May 15, 2023 · d421f52 · d421f52
1 parent f516ad6
commit d421f52
Show file tree

Hide file tree

Showing 9 changed files with 50 additions and 14 deletions.
diff --git a/llvm/include/llvm/CodeGen/TargetFrameLowering.h b/llvm/include/llvm/CodeGen/TargetFrameLowering.h
@@ -54,15 +54,18 @@ class TargetFrameLowering {
   };
 
   struct DwarfFrameBase {
-    // The frame base may be either a register (the default), the CFA,
-    // or a WebAssembly-specific location description.
+    // The frame base may be either a register (the default), the CFA with an
+    // offset, or a WebAssembly-specific location description.
     enum FrameBaseKind { Register, CFA, WasmFrameBase } Kind;
     struct WasmFrameBase {
       unsigned Kind; // Wasm local, global, or value stack
       unsigned Index;
     };
     union {
+      // Used with FrameBaseKind::Register.
       unsigned Reg;
+      // Used with FrameBaseKind::CFA.
+      int Offset;
       struct WasmFrameBase WasmLoc;
     } Location;
   };

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
@@ -531,6 +531,11 @@ DIE &DwarfCompileUnit::updateSubprogramScopeDIEImpl(const DISubprogram *SP,
     case TargetFrameLowering::DwarfFrameBase::CFA: {
       DIELoc *Loc = new (DIEValueAllocator) DIELoc;
       addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_call_frame_cfa);
+      if (FrameBase.Location.Offset != 0) {
+        addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_consts);
+        addSInt(*Loc, dwarf::DW_FORM_sdata, FrameBase.Location.Offset);
+        addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_plus);
+      }
       addBlock(*SPDie, dwarf::DW_AT_frame_base, Loc);
       break;
     }

diff --git a/llvm/lib/Target/X86/X86FrameLowering.cpp b/llvm/lib/Target/X86/X86FrameLowering.cpp
@@ -3831,6 +3831,32 @@ X86FrameLowering::getInitialCFARegister(const MachineFunction &MF) const {
   return TRI->getDwarfRegNum(StackPtr, true);
 }
 
+TargetFrameLowering::DwarfFrameBase
+X86FrameLowering::getDwarfFrameBase(const MachineFunction &MF) const {
+  if (needsDwarfCFI(MF)) {
+    // TODO(khuey): Eventually we should emit the variable expressions in
+    // terms of the CFA, rather than adjusting the CFA to mimic the frame
+    // or stack pointers.
+    DwarfFrameBase FrameBase;
+    FrameBase.Kind = DwarfFrameBase::CFA;
+    FrameBase.Location.Offset = -getInitialCFAOffset(MF);
+    if (hasFP(MF)) {
+      // Adjust for one additional stack slot (for the saved frame pointer
+      // register), so that the frame base expression is equivalent to the
+      // value of the frame pointer.
+      FrameBase.Location.Offset -= TRI->getSlotSize();
+    } else {
+      // Adjust for the entire stack size, so that the frame base expression
+      // is equivalent to the value of the stack pointer once the stack
+      // frame is completely set up.
+      FrameBase.Location.Offset -= MF.getFrameInfo().getStackSize();
+    }
+    return FrameBase;
+  }
+
+  return TargetFrameLowering::getDwarfFrameBase(MF);
+}
+
 namespace {
 // Struct used by orderFrameObjects to help sort the stack objects.
 struct X86FrameSortingObject {

diff --git a/llvm/lib/Target/X86/X86FrameLowering.h b/llvm/lib/Target/X86/X86FrameLowering.h
@@ -193,6 +193,8 @@ class X86FrameLowering : public TargetFrameLowering {
 
   Register getInitialCFARegister(const MachineFunction &MF) const override;
 
+  DwarfFrameBase getDwarfFrameBase(const MachineFunction &MF) const override;
+
   /// Return true if the function has a redzone (accessible bytes past the
   /// frame of the top of stack function) as part of it's ABI.
   bool has128ByteRedZone(const MachineFunction& MF) const;

diff --git a/llvm/test/CodeGen/X86/dbg-baseptr.ll b/llvm/test/CodeGen/X86/dbg-baseptr.ll
@@ -1,14 +1,14 @@
 ; RUN: llc -o - %s | FileCheck %s
 ; RUN: llc -filetype=obj -o - %s | llvm-dwarfdump -v - | FileCheck %s --check-prefix=DWARF
 ; This test checks that parameters on the stack pointer are correctly
-; referenced by debug info.
+; referenced by debug info. X86 always uses the CFA when available now.
 target triple = "x86_64--"
 
 @glob = external global i64
 @ptr = external global ptr
 %struct.s = type { i32, i32, i32, i32, i32 }
 
-; Simple case: no FP, use offset from RSP.
+; Simple case: no FP, use CFA, 8 byte offset to RSP.
 
 ; CHECK-LABEL: f0:
 ; CHECK-NOT: pushq
@@ -22,14 +22,14 @@ define i32 @f0(ptr byval(%struct.s) align 8 %input) !dbg !8 {
 ; DWARF-LABEL: .debug_info contents:
 
 ; DWARF-LABEL: DW_TAG_subprogram
-; DWARF:   DW_AT_frame_base [DW_FORM_exprloc]      (DW_OP_reg7 RSP)
+; DWARF:   DW_AT_frame_base [DW_FORM_exprloc]      (DW_OP_call_frame_cfa, DW_OP_consts -8, DW_OP_plus)
 ; DWARF:   DW_AT_name [DW_FORM_strp]       ( {{.*}}"f0")
 ; DWARF:   DW_TAG_formal_parameter
 ; DWARF-NEXT:     DW_AT_location [DW_FORM_exprloc]      (DW_OP_fbreg +8)
 ; DWARF-NEXT:     DW_AT_name [DW_FORM_strp]     ( {{.*}}"input")
 
 
-; Dynamic alloca forces the use of RBP as the base pointer
+; Dynamic alloca forces a frame pointer, use CFA, 16 byte offset to RBP.
 
 ; CHECK-LABEL: f1:
 ; CHECK: pushq %rbp
@@ -46,7 +46,7 @@ define i32 @f1(ptr byval(%struct.s) align 8 %input) !dbg !19 {
 }
 
 ; DWARF-LABEL: DW_TAG_subprogram
-; DWARF:   DW_AT_frame_base [DW_FORM_exprloc]      (DW_OP_reg6 RBP)
+; DWARF:   DW_AT_frame_base [DW_FORM_exprloc]      (DW_OP_call_frame_cfa, DW_OP_consts -16, DW_OP_plus)
 ; DWARF:   DW_AT_name [DW_FORM_strp]       ( {{.*}}"f1")
 ; DWARF:   DW_TAG_formal_parameter
 ; DWARF-NEXT:     DW_AT_location [DW_FORM_exprloc]      (DW_OP_fbreg +16)
@@ -69,9 +69,9 @@ define i32 @f2(ptr byval(%struct.s) align 8 %input) !dbg !22 {
   ret i32 42, !dbg !24
 }
 
-; "input" should still be referred to through RBP.
+; "input" should still be referred to through CFA with a 16 byte offset to RBP.
 ; DWARF-LABEL: DW_TAG_subprogram
-; DWARF:   DW_AT_frame_base [DW_FORM_exprloc]      (DW_OP_reg6 RBP)
+; DWARF:   DW_AT_frame_base [DW_FORM_exprloc]      (DW_OP_call_frame_cfa, DW_OP_consts -16, DW_OP_plus)
 ; DWARF:   DW_AT_name [DW_FORM_strp]       ( {{.*}}"f2")
 ; DWARF:   DW_TAG_formal_parameter
 ; DWARF-NEXT:     DW_AT_location [DW_FORM_exprloc]      (DW_OP_fbreg +16)

diff --git a/llvm/test/DebugInfo/X86/dwarf-public-names.ll b/llvm/test/DebugInfo/X86/dwarf-public-names.ll
@@ -61,7 +61,7 @@
 
 ; Skip the output to the header of the pubnames section.
 ; LINUX: debug_pubnames
-; LINUX-NEXT: unit_size = 0x00000128
+; LINUX-NEXT: unit_size = 0x00000134
 
 ; Check for each name in the output.
 ; LINUX-DAG: "ns"

diff --git a/llvm/test/ExecutionEngine/OrcLazy/debug-objects-elf-minimal.ll b/llvm/test/ExecutionEngine/OrcLazy/debug-objects-elf-minimal.ll
@@ -10,7 +10,7 @@
 ;
 ; CHECK: -:	file format elf64-x86-64
 ; CHECK: .debug_info contents:
-; CHECK: 0x00000000: Compile Unit: length = 0x00000047, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x0000004b)
+; CHECK: 0x00000000: Compile Unit: length = 0x0000004a, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x0000004e)
 ; CHECK: DW_TAG_compile_unit
 ; CHECK:               DW_AT_producer	("compiler version")
 ; CHECK:               DW_AT_language	(DW_LANG_C99)
@@ -22,7 +22,7 @@
 ; CHECK:   DW_TAG_subprogram
 ; CHECK:                 DW_AT_low_pc	()
 ; CHECK:                 DW_AT_high_pc	()
-; CHECK:                 DW_AT_frame_base	(DW_OP_reg7 RSP)
+; CHECK:                 DW_AT_frame_base	(DW_OP_call_frame_cfa, DW_OP_consts -16, DW_OP_plus)
 ; CHECK:                 DW_AT_name	("main")
 ; CHECK:                 DW_AT_decl_file	("/workspace/source-file.c")
 ; CHECK:                 DW_AT_decl_line	(4)

diff --git a/llvm/test/MC/X86/dwarf-size-field-overflow.test b/llvm/test/MC/X86/dwarf-size-field-overflow.test
@@ -4,7 +4,7 @@
 # RUN: %python %s 5750 | llc -mtriple=x86_64-apple-darwin -filetype=obj -o - \
 # RUN:    | llvm-dwarfdump - | FileCheck %s
 #
-# CHECK:       0x0000004d:       DW_TAG_formal_parameter
+# CHECK:       0x00000050:       DW_TAG_formal_parameter
 # CHECK-NEXT:      DW_AT_location   (0x00000000
 # CHECK-NEXT:           [0x0000000000000000,  0x0000000000000008): <empty>)
 # CHECK-NEXT:      DW_AT_name   ("self")

diff --git a/llvm/test/tools/llvm-dwarfdump/X86/statistics.ll b/llvm/test/tools/llvm-dwarfdump/X86/statistics.ll
@@ -56,7 +56,7 @@
 ; CHECK:      "#bytes within inlined functions": [[INLINESIZE:[0-9]+]]
 ; CHECK:      "#bytes in __debug_loc": 35,
 ; CHECK-NEXT: "#bytes in __debug_abbrev": 384,
-; CHECK-NEXT: "#bytes in __debug_info": 459,
+; CHECK-NEXT: "#bytes in __debug_info": 471,
 ; CHECK-NEXT: "#bytes in __debug_str": 231,
 ; CHECK-NEXT: "#bytes in __apple_names": 348,
 ; CHECK-NEXT: "#bytes in __apple_objc": 36,