[X86] Use the CFA as the DWARF frame base for better variable locations around calls.
authorKyle Huey <khuey@pernos.co>
Mon, 15 May 2023 14:08:18 +0000 (15:08 +0100)
committerJ. Ryan Stinnett <jryans@gmail.com>
Mon, 15 May 2023 14:10:02 +0000 (15:10 +0100)
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/include/llvm/CodeGen/TargetFrameLowering.h
llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
llvm/lib/Target/X86/X86FrameLowering.cpp
llvm/lib/Target/X86/X86FrameLowering.h
llvm/test/CodeGen/X86/dbg-baseptr.ll
llvm/test/DebugInfo/X86/dwarf-public-names.ll
llvm/test/ExecutionEngine/OrcLazy/debug-objects-elf-minimal.ll
llvm/test/MC/X86/dwarf-size-field-overflow.test
llvm/test/tools/llvm-dwarfdump/X86/statistics.ll

index 3d81ed6..94de304 100644 (file)
@@ -54,15 +54,18 @@ public:
   };
 
   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;
   };
index dd5dbf4..0252fbb 100644 (file)
@@ -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;
     }
index f512de1..ae43ccc 100644 (file)
@@ -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 {
index 3b76f29..2dc9ecc 100644 (file)
@@ -193,6 +193,8 @@ public:
 
   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;
index 9194ce9..83a6c5c 100644 (file)
@@ -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)
index c227451..2ce38ca 100644 (file)
@@ -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"
index 0d5aba3..9d1b565 100644 (file)
@@ -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)
index 1a0fb0b..2a63a4a 100644 (file)
@@ -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")
index a454bf1..c7976f5 100644 (file)
@@ -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,