[CodeView] Add support for local S_CONSTANT records

[thieta]

Summary

CodeView doesn't have the ability to represent variables
in other ways than as in registers or memory values, but
LLVM very often transforms simple values into constants,
consider this program:

int f () { int i = 123; return i; }

LLVM will transform i into a constant value and just
leave behind a llvm.dbg.value, this can't be represented
as a S_LOCAL record in CodeView. But we can represent it
as a S_CONSTANT record.

This patch checks if the location of a debug value is null,
then we will insert a S_CONSTANT record instead of a S_LOCAL
value with the flag "OptimizedAway".

In lld we then output the S_CONSTANT in the right scope, before
they where always inserted in the global stream, now we check
the scope before inserting it.

This has shown to improve debugging for our developers
internally.

Fixes to llvm/llvm-project#55958

[aganea]

Thanks, that's a nice improvement! Out of curiosity does this optimization happen at -O0 too? (when using clang-cl)
Does MSVC generate S_CONSTANT as well for your test-case?

lld/test/COFF/Inputs/pdb-local-constants.s

192

Since we have -gno-codeview-command-line can you leave out this part? It adds unnecessary noise which is not required for this test.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

1347

s/becasue/because/

1349

'since' repeated twice, not sure if this could be rephrased a bit?

1355

nit: could also be Val.ConstantValue = APSInt(APInt(64, Op.getImm()), false);

2798

unwanted indent?

2800–2801

I would add the extra braces at the end of the for line, to enclose the scope below.

2805

Could this comment fit on the line above? You can also write the entire comment on one line, and then git clang-format reformat it.

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

107

I think the way to go forward is to use std::optional.

[thieta]

In D138995#3961742, @aganea wrote:

Thanks, that's a nice improvement! Out of curiosity does this optimization happen at -O0 too? (when using clang-cl)
Does MSVC generate S_CONSTANT as well for your test-case?

In general -O0 doesn't have this problem. I have seen it happen, but unsure what triggers it.

No MSVC doesn't generate S_CONSTANTS for any of my cases. They only use it when you actually do a constant, in most of the cases MSVC doesn't output anything or outputs a S_LOCAL with the optimized away flag set. I tried with /Zo as well and couldn't get it do anything differently.

I even tried to debug several programs with msvc and their default flags, and with /O2 they always had same debugging info as clang-cl.
They do behave better with /O2, mostly because clang really don't want to emit all declarations if they are global and can const propagate some variables.

[thieta]

Handled some review feedback

[aganea]

LGTM, but wait until next week before landing, in case others would like to comment. + @akhuang

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

1346

Bracket on wrong line, please apply git clang-format.

[akhuang]

Looks good to me!

lld/COFF/PDB.cpp

static void scopeStackClose(SmallVectorImpl<uint32_t> &stack,
   scopeRec->ptrParent = offParent;
   scopeRec->ptrEnd = offEnd;
 }
 
 static bool symbolGoesInModuleStream(const CVSymbol &sym,
                                      unsigned symbolScopeDepth) {
   switch (sym.kind()) {
   case SymbolKind::S_GDATA32:
-  case SymbolKind::S_CONSTANT:
   case SymbolKind::S_GTHREAD32:
   // We really should not be seeing S_PROCREF and S_LPROCREF in the first place
   // since they are synthesized by the linker in response to S_GPROC32 and
   // S_LPROC32, but if we do see them, don't put them in the module stream I
   // guess.
   case SymbolKind::S_PROCREF:
   case SymbolKind::S_LPROCREF:
     return false;
-  // S_UDT records go in the module stream if it is not a global S_UDT.
+  // S_UDT and S_CONSTANT records go in the module stream if it is not a global record.
   case SymbolKind::S_UDT:
+  case SymbolKind::S_CONSTANT:
     return symbolScopeDepth > 0;
   // S_GDATA32 does not go in the module stream, but S_LDATA32 does.
   case SymbolKind::S_LDATA32:
   case SymbolKind::S_LTHREAD32:
   default:
     return true;
   }
 }
 
 static bool symbolGoesInGlobalsStream(const CVSymbol &sym,
                                       unsigned symbolScopeDepth) {
   switch (sym.kind()) {
-  case SymbolKind::S_CONSTANT:
   case SymbolKind::S_GDATA32:
   case SymbolKind::S_GTHREAD32:
   case SymbolKind::S_GPROC32:
   case SymbolKind::S_LPROC32:
   case SymbolKind::S_GPROC32_ID:
   case SymbolKind::S_LPROC32_ID:
   // We really should not be seeing S_PROCREF and S_LPROCREF in the first place
   // since they are synthesized by the linker in response to S_GPROC32 and
   // S_LPROC32, but if we do see them, copy them straight through.
   case SymbolKind::S_PROCREF:
   case SymbolKind::S_LPROCREF:
     return true;
   // Records that go in the globals stream, unless they are function-local.
   case SymbolKind::S_UDT:
   case SymbolKind::S_LDATA32:
   case SymbolKind::S_LTHREAD32:
+  case SymbolKind::S_CONSTANT:
     return symbolScopeDepth == 0;
   default:
     return false;
   }
 }
 
 static void addGlobalSymbol(pdb::GSIStreamBuilder &builder, uint16_t modIndex,
                             unsigned symOffset,

lld/test/COFF/Inputs/pdb-local-constants.s

+  .text
+  .def  @feat.00;
+  .scl  3;
+  .type 0;
+  .endef
+  .globl  @feat.00
+.set @feat.00, 0
+  .file "local_constant.cpp"
+  .def  main;
+  .scl  2;
+  .type 32;
+  .endef
+  .globl  main                            # -- Begin function main
+  .p2align  4, 0x90
+main:                                   # @main
+.Lfunc_begin0:
+  .cv_func_id 0
+# %bb.0:                                # %entry
+  #DEBUG_VALUE: main:i <- 123
+  .cv_file  1 "/home/tobias/code/llvm-project/build/local_constant.cpp" "C33315002D9B48E67EB3E617E430BC02" 1
+  .cv_loc 0 1 7 0                         # local_constant.cpp:7:0
+  movl  $444, %eax                      # imm = 0x1BC
+  retq
+.Ltmp0:
+.Lfunc_end0:
+                                        # -- End function
+  .section  .debug$S,"dr"
+  .p2align  2, 0x0
+  .long 4                               # Debug section magic
+  .long 241
+  .long .Ltmp2-.Ltmp1                   # Subsection size
+.Ltmp1:
+  .short  .Ltmp4-.Ltmp3                   # Record length
+.Ltmp3:
+  .short  4353                            # Record kind: S_OBJNAME
+  .long 0                               # Signature
+  .byte 0                               # Object name
+  .p2align  2, 0x0
+.Ltmp4:
+  .short  .Ltmp6-.Ltmp5                   # Record length
+.Ltmp5:
+  .short  4412                            # Record kind: S_COMPILE3
+  .long 1                               # Flags and language
+  .short  208                             # CPUType
+  .short  16                              # Frontend version
+  .short  0
+  .short  0
+  .short  0
+  .short  16000                           # Backend version
+  .short  0
+  .short  0
+  .short  0
+  .asciz  "clang version 16.0.0 ([e-mail]:llvm/llvm-project.git eef89bd2b3f4a13efcad176bb4c4dda1b1e202ce)" # Null-terminated compiler version string
+  .p2align  2, 0x0
+.Ltmp6:
+.Ltmp2:
+  .p2align  2, 0x0
+  .long 241                             # Symbol subsection for main
+  .long .Ltmp8-.Ltmp7                   # Subsection size
+.Ltmp7:
+  .short  .Ltmp10-.Ltmp9                  # Record length
+.Ltmp9:
+  .short  4423                            # Record kind: S_GPROC32_ID
+  .long 0                               # PtrParent
+  .long 0                               # PtrEnd
+  .long 0                               # PtrNext
+  .long .Lfunc_end0-main                # Code size
+  .long 0                               # Offset after prologue
+  .long 0                               # Offset before epilogue
+  .long 4098                            # Function type index
+  .secrel32 main                    # Function section relative address
+  .secidx main                            # Function section index
+  .byte 0                               # Flags
+  .asciz  "main"                          # Function name
+  .p2align  2, 0x0
+.Ltmp10:
+  .short  .Ltmp12-.Ltmp11                 # Record length
+.Ltmp11:
+  .short  4114                            # Record kind: S_FRAMEPROC
+  .long 0                               # FrameSize
+  .long 0                               # Padding
+  .long 0                               # Offset of padding
+  .long 0                               # Bytes of callee saved registers
+  .long 0                               # Exception handler offset
+  .short  0                               # Exception handler section
+  .long 1056768                         # Flags (defines frame register)
+  .p2align  2, 0x0
+.Ltmp12:
+  .short  .Ltmp14-.Ltmp13                 # Record length
+.Ltmp13:
+  .short  4359                            # Record kind: S_CONSTANT
+  .long 116                             # Type
+  .byte 0x7b, 0x00                      # Value
+  .asciz  "i"                             # Name
+  .p2align  2, 0x0
+.Ltmp14:
+  .short  2                               # Record length
+  .short  4431                            # Record kind: S_PROC_ID_END
+.Ltmp8:
+  .p2align  2, 0x0
+  .cv_linetable 0, main, .Lfunc_end0
+  .long 241                             # Symbol subsection for globals
+  .long .Ltmp16-.Ltmp15                 # Subsection size
+.Ltmp15:
+  .short  .Ltmp18-.Ltmp17                 # Record length
+.Ltmp17:
+  .short  4359                            # Record kind: S_CONSTANT
+  .long 4099                            # Type
+  .byte 0x41, 0x01                      # Value
+  .asciz  "g_const"                       # Name
+  .p2align  2, 0x0
+.Ltmp18:
+.Ltmp16:
+  .p2align  2, 0x0
+  .cv_filechecksums                       # File index to string table offset subsection
+  .cv_stringtable                         # String table
+  .long 241
+  .long .Ltmp20-.Ltmp19                 # Subsection size
+.Ltmp19:
+  .short  .Ltmp22-.Ltmp21                 # Record length
+.Ltmp21:
+  .short  4428                            # Record kind: S_BUILDINFO
+  .long 4103                            # LF_BUILDINFO index
+  .p2align  2, 0x0
+.Ltmp22:
+.Ltmp20:
+  .p2align  2, 0x0
+  .section  .debug$T,"dr"
+  .p2align  2, 0x0
+  .long 4                               # Debug section magic
+  # ArgList (0x1000)
+  .short  0x6                             # Record length
+  .short  0x1201                          # Record kind: LF_ARGLIST
+  .long 0x0                             # NumArgs
+  # Procedure (0x1001)
+  .short  0xe                             # Record length
+  .short  0x1008                          # Record kind: LF_PROCEDURE
+  .long 0x74                            # ReturnType: int
+  .byte 0x0                             # CallingConvention: NearC
+  .byte 0x0                             # FunctionOptions
+  .short  0x0                             # NumParameters
+  .long 0x1000                          # ArgListType: ()
+  # FuncId (0x1002)
+  .short  0x12                            # Record length
+  .short  0x1601                          # Record kind: LF_FUNC_ID
+  .long 0x0                             # ParentScope
+  .long 0x1001                          # FunctionType: int ()
+  .asciz  "main"                          # Name
+  .byte 243
+  .byte 242
+  .byte 241
+  # Modifier (0x1003)
+  .short  0xa                             # Record length
+  .short  0x1001                          # Record kind: LF_MODIFIER
+  .long 0x74                            # ModifiedType: int
+  .short  0x1                             # Modifiers ( Const (0x1) )
+  .byte 242
+  .byte 241
+  # StringId (0x1004)
+  .short  0x2e                            # Record length
+  .short  0x1605                          # Record kind: LF_STRING_ID
+  .long 0x0                             # Id
+  .asciz  "/home/tobias/code/llvm-project/build" # StringData
+  .byte 243
+  .byte 242
+  .byte 241
+  # StringId (0x1005)
+  .short  0x1a                            # Record length
+  .short  0x1605                          # Record kind: LF_STRING_ID
+  .long 0x0                             # Id
+  .asciz  "local_constant.cpp"            # StringData
+  .byte 241
+  # StringId (0x1006)
+  .short  0xa                             # Record length
+  .short  0x1605                          # Record kind: LF_STRING_ID
+  .long 0x0                             # Id
+  .byte 0                               # StringData
+  .byte 243
+  .byte 242
+  .byte 241
+  # BuildInfo (0x1007)
+  .short  0x1a                            # Record length
+  .short  0x1603                          # Record kind: LF_BUILDINFO
+  .short  0x5                             # NumArgs
+  .long 0x1004                          # Argument: /home/tobias/code/llvm-project/build
+  .long 0x0                             # Argument
+  .long 0x1005                          # Argument: local_constant.cpp
+  .long 0x1006                          # Argument
+  .long 0x0                             # Argument
+  .byte 242
+  .byte 241
+  .addrsig

lld/test/COFF/pdb-local-constants.test

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj %S/Inputs/pdb-local-constants.s -o %t.obj -triple x86_64-windows-msvc
+# RUN: lld-link -entry:main -nodefaultlib %t.obj -out:%t.exe -pdb:%t.pdb -debug
+# RUN: llvm-pdbutil dump -globals -symbols %t.pdb | FileCheck %s
+#
+# Check that LLD puts the S_CONSTANT records in the right scope
+#
+# Compiled from this C code, using
+# clang t.cpp -g -gcodeview -S
+#
+# const int g_const = 321;
+# int main() { int i = 123; return i + g_const; }
+#
+
+CHECK:                       Global Symbols
+CHECK:           40 | S_CONSTANT [size = 20] `g_const`
+CHECK-NEXT:           type = 0x1002 (const int), value = 321
+
+CHECK:                          Symbols
+CHECK:           220 | S_CONSTANT [size = 12] `i`
+CHECK-NEXT            type = 0x0074 (int), value = 123
 No newline at end of file

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

private:
 
   /// Similar to DbgVariable in DwarfDebug, but not dwarf-specific.
   struct LocalVariable {
     const DILocalVariable *DIVar = nullptr;
     MapVector<LocalVarDef,
               SmallVector<std::pair<const MCSymbol *, const MCSymbol *>, 1>>
         DefRanges;
     bool UseReferenceType = false;
+    std::optional<APSInt> ConstantValue;
   };
 
   struct CVGlobalVariable {
     const DIGlobalVariable *DIGV;
     PointerUnion<const GlobalVariable *, const DIExpression *> GVInfo;
   };
 
   struct InlineSite {

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

if (!Entry.isDbgValue())
       continue;
     const MachineInstr *DVInst = Entry.getInstr();
     assert(DVInst->isDebugValue() && "Invalid History entry");
     // FIXME: Find a way to represent constant variables, since they are
     // relatively common.
     Optional<DbgVariableLocation> Location =
         DbgVariableLocation::extractFromMachineInstruction(*DVInst);
     if (!Location)
+    {
+      // When we don't have a location this is usually because LLVM has
+      // transformed it into a constant and we only have an llvm.dbg.value. We
+      // can't represent these well in CodeView since S_LOCAL only works on
+      // registers and memory locations. Instead, we will pretend this to be a
+      // constant value to at least have it show up in the debugger.
+      auto Op = DVInst->getDebugOperand(0);
+      if (Op.isImm())
+        Var.ConstantValue = APSInt(APInt(64, Op.getImm()), false);
       continue;
+    }
 
     // CodeView can only express variables in register and variables in memory
     // at a constant offset from a register. However, for variables passed
     // indirectly by pointer, it is common for that pointer to be spilled to a
     // stack location. For the special case of one offseted load followed by a
     // zero offset load (a pointer spilled to the stack), we change the type of
     // the local variable from a value type to a reference type. This tricks the
     // debugger into doing the load for us.
void CodeViewDebug::emitLocalVariableList(const FunctionInfo &FI,
   SmallVector<const LocalVariable *, 6> Params;
   for (const LocalVariable &L : Locals)
     if (L.DIVar->isParameter())
       Params.push_back(&L);
   llvm::sort(Params, [](const LocalVariable *L, const LocalVariable *R) {
     return L->DIVar->getArg() < R->DIVar->getArg();
   });
   for (const LocalVariable *L : Params)
     emitLocalVariable(FI, *L);
 
   // Next emit all non-parameters in the order that we found them.
-  for (const LocalVariable &L : Locals)
-    if (!L.DIVar->isParameter())
+  for (const LocalVariable &L : Locals) {
+    if (!L.DIVar->isParameter()) {
+      if (L.ConstantValue) {
+        // If ConstantValue is set we will emit it as a S_CONSTANT instead of a
+        // S_LOCAL in order to be able to represent it at all.
+        const DIType *Ty = L.DIVar->getType();
+        APSInt Val(L.ConstantValue.value());
+        emitConstantSymbolRecord(Ty, Val, std::string(L.DIVar->getName()));
+      } else {
         emitLocalVariable(FI, L);
       }
+    }
+  }
+}
 
 void CodeViewDebug::emitLocalVariable(const FunctionInfo &FI,
                                       const LocalVariable &Var) {
   // LocalSym record, see SymbolRecord.h for more info.
   MCSymbol *LocalEnd = beginSymbolRecord(SymbolKind::S_LOCAL);
 
   LocalSymFlags Flags = LocalSymFlags::None;
   if (Var.DIVar->isParameter())

llvm/test/DebugInfo/COFF/local-constant.ll

 ; RUN: llc -mtriple=x86_64-windows-msvc < %s -filetype=obj | llvm-readobj --codeview - | FileCheck %s --check-prefix=OBJ
 
 ; This LL file was generated by running 'clang -g -gcodeview' on the
 ; following code:
 ; void useint(int);
 ; void constant_var() {
 ;   int x = 42;
 ;   useint(x);
 ;   useint(x);
 ; }
 
-; FIXME: Find a way to describe variables optimized to constants.
-
 ; OBJ:        {{.*}}Proc{{.*}}Sym {
 ; OBJ:           DisplayName: constant_var
 ; OBJ:         }
-; OBJ:         LocalSym {
-; OBJ-NEXT:      Kind:
+; OBJ:         ConstantSym {
+; OBJ-NEXT:      Kind: S_CONSTANT
 ; OBJ-NEXT:      Type: int (0x74)
-; OBJ-NEXT:      Flags [ (0x100)
-; OBJ-NEXT:        IsOptimizedOut (0x100)
-; OBJ-NEXT:      ]
-; OBJ-NEXT:      VarName: x
+; OBJ-NEXT:      Value: 42
+; OBJ-NEXT:      Name: x
 ; OBJ-NEXT:    }
 ; OBJ-NOT:     DefRange
 ; OBJ:         ProcEnd
 
 ; ModuleID = 't.cpp'
 target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc18.0.0"
 

llvm/test/DebugInfo/COFF/pieces.ll

 ; ASM:         movl    [[offset_o_x]](%rsp), %eax          # 4-byte Reload
 ; ASM:         retq
 ; ASM: [[spill_o_x_end_func:\.Ltmp[0-9]+]]:
 ; ASM-NEXT: .Lfunc_end4:
 
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "loop_csr"              # Function name
-; ASM:        .short  4414                    # Record kind: S_LOCAL
+; ASM:        .short  4359                    # Record kind: S_CONSTANT
+; ASM:        .long 4099                      # Type
+; ASM:        .byte 0x00, 0x00                # Value
 ; ASM:        .asciz  "o"
-; ASM:        .cv_def_range    [[oy_ox_start]] [[loopskip_start]], subfield_reg, 24, 0
-; ASM:        .cv_def_range    [[oy_ox_start]] [[loopskip_start]], subfield_reg, 23, 4
 
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
 ; OBJ:         DisplayName: loop_csr
 ; OBJ:       }
-; OBJ:       LocalSym {
-; OBJ:         VarName: o
-; OBJ:       }
-; OBJ:       DefRangeSubfieldRegisterSym {
-; OBJ:         Register: EDI (0x18)
-; OBJ:         MayHaveNoName: 0
-; OBJ:         OffsetInParent: 0
-; OBJ:         LocalVariableAddrRange {
-; OBJ:         }
-; OBJ:       }
-; OBJ:       DefRangeSubfieldRegisterSym {
-; OBJ:         Register: ESI (0x17)
-; OBJ:         MayHaveNoName: 0
-; OBJ:         OffsetInParent: 4
-; OBJ:         LocalVariableAddrRange {
-; OBJ:         }
+; OBJ:       ConstantSym {
+; OBJ:         Kind: S_CONSTANT (0x1107)
+; OBJ:         Type: IntPair (0x1003)
+; OBJ:         Value: 0
+; OBJ:         Name: o
 ; OBJ:       }
 ; OBJ:       ProcEnd {
 ; OBJ:       }
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "pad_right"             # Function name
 ; ASM:        .short  4414                    # Record kind: S_LOCAL
 ; ASM:        .asciz  "o"

 ; OBJ:         }
 ; OBJ:       }
 ; OBJ:       ProcEnd {
 ; OBJ:       }
 
 
 ; ASM-LABEL:  .short  4423                    # Record kind: S_GPROC32_ID
 ; ASM:        .asciz  "bitpiece_spill"        # Function name
-; ASM:        .short  4414                    # Record kind: S_LOCAL
-; ASM:        .asciz  "o"
-; ASM:        .cv_def_range    [[spill_o_x_start]] .Lfunc_end4, reg_rel, 335, 65, 36
+; ASM:        .short  4359                    # Record kind: S_CONSTANT
+; ASM:        .long 4099                      # Type
+; ASM:        .byte 0x00, 0x00                # Value
+; ASM:        .asciz  "o"                     # Name
 
 ; OBJ-LABEL: GlobalProcIdSym {
 ; OBJ:         Kind: S_GPROC32_ID (0x1147)
 ; OBJ:         DisplayName: bitpiece_spill
 ; OBJ:       }
-; OBJ:       LocalSym {
-; OBJ:         VarName: o
-; OBJ:       }
-; OBJ:       DefRangeRegisterRelSym {
-; OBJ:         BaseRegister: RSP (0x14F)
-; OBJ:         HasSpilledUDTMember: Yes
-; OBJ:         OffsetInParent: 4
-; OBJ:         BasePointerOffset: 36
-; OBJ:         LocalVariableAddrRange {
-; OBJ:         }
+; OBJ:       ConstantSym {
+; OBJ:         Kind: S_CONSTANT (0x1107)
+; OBJ:         Type: IntPair (0x1003)
+; OBJ:         Value: 0
+; OBJ:         Name: o
 ; OBJ:       }
 ; OBJ:       ProcEnd {
 ; OBJ:       }
 
 
 
 ; ModuleID = 't.c'
 source_filename = "t.c"