[DebugInfo] Avoid crash from dropped fragments in LiveDebugValues

[jmorse]

Summary

This patch is spun out of D65368, and purely aims to avoid a crash caused by DW_OP_LLVM_fragments being dropped from DIExpressions by LiveDebugValues spill-restore code. There are two hunks:

• On spill restore, use fragment information from the spilt-location-expression, avoiding a later assertion failure
• When spilling, don't spill any DBG_VALUE with a complex expression, as it can't be safely restored and will definitely lead to an incorrect variable location

The additional function in live-debug-values-restore.mir triggers an assertion failure on a unpatched x86_64 Debug build of LLVM, on Ubuntu 18. This crash will be in LLVM-9 too, so the intention is to cherry pick it there.

[aprantl]

This is patch 1. LGTM.

lib/CodeGen/LiveDebugValues.cpp

LLVM_DEBUG(dbgs() << "Creating DBG_VALUE inst for spill: ";
                                     /*AddNewLine*/true, TII));
     return;
   }
   case TransferKind::TransferRestore: {
     assert(NewReg &&
            "No register supplied when handling a restore of a debug value");
     MachineFunction *MF = MI.getMF();
     DIBuilder DIB(*const_cast<Function &>(MF->getFunction()).getParent());
+
+    const DIExpression *NewExpr;
+    if (auto Fragment = DebugInstr->getDebugExpression()->getFragmentInfo())
+      NewExpr = *DIExpression::createFragmentExpression(DIB.createExpression(),
+        Fragment->OffsetInBits, Fragment->SizeInBits);
+    else
+      NewExpr = DIB.createExpression();
+
     NewDebugInstr =
         BuildMI(*MF, DebugInstr->getDebugLoc(), DebugInstr->getDesc(), false,
-                NewReg, DebugInstr->getDebugVariable(), DIB.createExpression());
+                NewReg, DebugInstr->getDebugVariable(), NewExpr);
     VarLoc VL(*NewDebugInstr, LS);
     ProcessVarLoc(VL, NewDebugInstr);
     LLVM_DEBUG(dbgs() << "Creating DBG_VALUE inst for register restore: ";
                NewDebugInstr->print(dbgs(), /*IsStandalone*/false,
                                     /*SkipOpers*/false, /*SkipDebugLoc*/false,
                                     /*AddNewLine*/true, TII));
     return;
   }
if (isSpillInstruction(MI, MF, Reg)) {
     if (!(Loc = isRestoreInstruction(MI, MF, Reg)))
       return;
     TKind = TransferKind::TransferRestore;
     LLVM_DEBUG(dbgs() << "Recognized as restore: "; MI.dump(););
     LLVM_DEBUG(dbgs() << "Register: " << Reg << " " << printReg(Reg, TRI)
                       << "\n");
   }
   // Check if the register or spill location is the location of a debug value.
+  // FIXME: Don't create a spill transfer if there is a complex expression,
+  // because we currently cannot recover the original expression on restore.
   for (unsigned ID : OpenRanges.getVarLocs()) {
+    const MachineInstr *DebugInstr = &VarLocIDs[ID].MI;
+
     if (TKind == TransferKind::TransferSpill &&
-        VarLocIDs[ID].isDescribedByReg() == Reg) {
+        VarLocIDs[ID].isDescribedByReg() == Reg &&
+        !DebugInstr->getDebugExpression()->isComplex()) {
       LLVM_DEBUG(dbgs() << "Spilling Register " << printReg(Reg, TRI) << '('
                         << VarLocIDs[ID].Var.getVar()->getName() << ")\n");
     } else if (TKind == TransferKind::TransferRestore &&
                VarLocIDs[ID].Loc.SpillLocation == *Loc) {
       LLVM_DEBUG(dbgs() << "Restoring Register " << printReg(Reg, TRI) << '('
                         << VarLocIDs[ID].Var.getVar()->getName() << ")\n");
     } else
       continue;

test/DebugInfo/MIR/X86/live-debug-values-restore.mir

 # RUN: llc -run-pass livedebugvalues -march=x86-64 -o - %s | FileCheck %s
 
 # Generated from the following source with:
 # clang -g -mllvm -stop-before=livedebugvalues -S -O2 test.c -o test.mir
 
 # #define FORCE_SPILL() \
 #   __asm volatile("" : : : \
 #                    "rax", "rbx", "rcx", "rdx", "rsi", "rdi", "rbp", "r8", \
 #                    "r9", "r10", "r11", "r12", "r13", "r14", "r15")
 # int f(int *p) {
 #   if (p) {
 #     FORCE_SPILL();
 #   }
 #   return *(p + 1);
 # }
 
+# Pick out DILocalVariable numbers for "p" and "q"
+# CHECK: ![[PVAR:[0-9]+]] = !DILocalVariable(name: "p",
+# CHECK: ![[QVAR:[0-9]+]] = !DILocalVariable(name: "q",
+
 # Ascertain that the spill has been recognized and manifested in a DBG_VALUE.
 # CHECK: MOV64mr $rsp,{{.*-8.*}}killed{{.*}}$rdi :: (store 8 into %stack.0)
-# CHECK-NEXT: DBG_VALUE $rsp,{{.*}}![[MDIX:[0-9]+]],{{.*}}!DIExpression(DW_OP_constu, 8, DW_OP_minus)
+# CHECK-NEXT: DBG_VALUE $rsp,{{.*}}![[PVAR]],{{.*}}!DIExpression(DW_OP_constu, 8, DW_OP_minus)
 
 # Check for the restore.
 # CHECK: $rdi = MOV64rm $rsp,{{.*-8.*}}:: (load 8 from %stack.0)
-# CHECK-NEXT: DBG_VALUE $rdi,{{.*}}![[MDIX]], !DIExpression()
+# CHECK-NEXT: DBG_VALUE $rdi,{{.*}}![[PVAR]], !DIExpression()
 
 --- |
   define dso_local i32 @f(i32* readonly %p) local_unnamed_addr !dbg !7 {
   entry:
     call void @llvm.dbg.value(metadata i32* %p, metadata !13, metadata !DIExpression()), !dbg !14
     %tobool = icmp eq i32* %p, null, !dbg !15
     br i1 %tobool, label %if.end, label %if.then, !dbg !17
   
   if.then:                                          ; preds = %entry
     tail call void asm sideeffect "", "~{rax},~{rbx},~{rcx},~{rdx},~{rsi},~{rdi},~{rbp},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15},~{dirflag},~{fpsr},~{flags}"(), !dbg !18, !srcloc !20
     br label %if.end, !dbg !21
   
   if.end:                                           ; preds = %entry, %if.then
     %add.ptr = getelementptr inbounds i32, i32* %p, i64 1, !dbg !22
     %0 = load i32, i32* %add.ptr, align 4, !dbg !23, !tbaa !24
     ret i32 %0, !dbg !28
   }
   
+  define dso_local i32 @g(i32* readonly %p) local_unnamed_addr !dbg !107 {
+  entry:
+    call void @llvm.dbg.value(metadata i32* %p, metadata !113, metadata !DIExpression()), !dbg !114
+    %tobool = icmp eq i32* %p, null, !dbg !115
+    br i1 %tobool, label %if.end, label %if.then, !dbg !117
+  
+  if.then:                                          ; preds = %entry
+    tail call void asm sideeffect "", "~{rax},~{rbx},~{rcx},~{rdx},~{rsi},~{rdi},~{rbp},~{r8},~{r9},~{r10},~{r11},~{r12},~{r13},~{r14},~{r15},~{dirflag},~{fpsr},~{flags}"(), !dbg !118, !srcloc !120
+    br label %if.end, !dbg !121
+  
+  if.end:                                           ; preds = %entry, %if.then
+    %add.ptr = getelementptr inbounds i32, i32* %p, i64 1, !dbg !122
+    %0 = load i32, i32* %add.ptr, align 4, !dbg !123, !tbaa !24
+    ret i32 %0, !dbg !128
+  }
+ 
   declare void @llvm.dbg.value(metadata, metadata, metadata)
   
   !llvm.dbg.cu = !{!0}
   !llvm.module.flags = !{!3, !4, !5}
   !llvm.ident = !{!6}
   
   !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (https://git.llvm.org/git/clang.git/ 57a6ce7ac318de98e3e777e09cb9ed8282b5cc03) (https://git.llvm.org/git/llvm.git/ ff54a19e4912d7f15cb02798a7f2048441bff751)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
   !1 = !DIFile(filename: "test2.c", directory: "/home/test")
--- |
   !21 = !DILocation(line: 8, column: 3, scope: !19)
   !22 = !DILocation(line: 9, column: 14, scope: !7)
   !23 = !DILocation(line: 9, column: 10, scope: !7)
   !24 = !{!25, !25, i64 0}
   !25 = !{!"int", !26, i64 0}
   !26 = !{!"omnipotent char", !27, i64 0}
   !27 = !{!"Simple C/C++ TBAA"}
   !28 = !DILocation(line: 9, column: 3, scope: !7)
+  !101 = !DIBasicType(name: "looong int", size: 64, encoding: DW_ATE_signed)
+  !107 = distinct !DISubprogram(name: "g", scope: !1, file: !1, line: 105, type: !8, scopeLine: 105, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !112)
+  !112 = !{!113}
+  !113 = !DILocalVariable(name: "q", arg: 1, scope: !107, file: !1, line: 105, type: !101)
+  !114 = !DILocation(line: 105, column: 12, scope: !107)
+  !115 = !DILocation(line: 106, column: 7, scope: !116)
+  !116 = distinct !DILexicalBlock(scope: !107, file: !1, line: 106, column: 7)
+  !117 = !DILocation(line: 106, column: 7, scope: !107)
+  !118 = !DILocation(line: 107, column: 5, scope: !119)
+  !119 = distinct !DILexicalBlock(scope: !116, file: !1, line: 106, column: 10)
+  !120 = !{i32 -2147471544}
+  !121 = !DILocation(line: 108, column: 3, scope: !119)
+  !122 = !DILocation(line: 109, column: 14, scope: !107)
+  !123 = !DILocation(line: 109, column: 10, scope: !107)
+  !128 = !DILocation(line: 109, column: 3, scope: !107)
+
 
 ...
 ---
 name:            f
 alignment:       4
 exposesReturnsTwice: false
 legalized:       false
 regBankSelected: false
body:             |
   
   bb.2.if.end:
     liveins: $rdi, $rbx, $r12, $r13, $r14, $r15, $rbp
   
     renamable $eax = MOV32rm killed renamable $rdi, 1, $noreg, 4, $noreg, debug-location !23 :: (load 4 from %ir.add.ptr, !tbaa !24)
     RETQ $eax, debug-location !28
 
 ...
+---
+# This second function has been appended as a regression test against a
+# crash, caused by expressions being created from spill restores that did
+# not preserve fragment information. Test that no empty expressions are
+# created at all, and the last block describes both variable fragments.
+
+# CHECK-LABEL: name: g
+# CHECK-NOT: !DIExpression()
+# CHECK-LABEL: bb.2.if.end:
+# CHECK:       DBG_VALUE $rdi, $noreg, ![[QVAR]], !DIExpression(DW_OP_LLVM_fragment, 0, 32)
+# CHECK-NEXT:  DBG_VALUE $rbx, $noreg, ![[QVAR]], !DIExpression(DW_OP_LLVM_fragment, 32, 32)
+
+name:            g
+alignment:       4
+tracksRegLiveness: true
+liveins:         
+  - { reg: '$rdi', virtual-reg: '' }
+frameInfo:       
+  stackSize:       48
+  offsetAdjustment: -48
+  maxAlignment:    8
+  cvBytesOfCalleeSavedRegisters: 48
+  localFrameSize:  0
+fixedStack:      
+  - { id: 0, type: spill-slot, offset: -56, size: 8, alignment: 8, stack-id: default,
+      callee-saved-register: '$rbx', callee-saved-restored: true, debug-info-variable: '', 
+      debug-info-expression: '', debug-info-location: '' }
+  - { id: 1, type: spill-slot, offset: -48, size: 8, alignment: 16, stack-id: default,
+      callee-saved-register: '$r12', callee-saved-restored: true, debug-info-variable: '', 
+      debug-info-expression: '', debug-info-location: '' }
+  - { id: 2, type: spill-slot, offset: -40, size: 8, alignment: 8, stack-id: default,
+      callee-saved-register: '$r13', callee-saved-restored: true, debug-info-variable: '', 
+      debug-info-expression: '', debug-info-location: '' }
+  - { id: 3, type: spill-slot, offset: -32, size: 8, alignment: 16, stack-id: default,
+      callee-saved-register: '$r14', callee-saved-restored: true, debug-info-variable: '', 
+      debug-info-expression: '', debug-info-location: '' }
+  - { id: 4, type: spill-slot, offset: -24, size: 8, alignment: 8, stack-id: default,
+      callee-saved-register: '$r15', callee-saved-restored: true, debug-info-variable: '', 
+      debug-info-expression: '', debug-info-location: '' }
+  - { id: 5, type: spill-slot, offset: -16, size: 8, alignment: 16, stack-id: default,
+      callee-saved-register: '$rbp', callee-saved-restored: true, debug-info-variable: '', 
+      debug-info-expression: '', debug-info-location: '' }
+stack:           
+  - { id: 0, name: '', type: spill-slot, offset: -64, size: 8, alignment: 8, 
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+constants:       []
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x50000000)
+    liveins: $rdi, $rbx, $r12, $r13, $r14, $r15, $rbp
+  
+    DBG_VALUE $rdi, $noreg, !113, !DIExpression(DW_OP_LLVM_fragment, 0, 32), debug-location !114
+    TEST64rr renamable $rdi, renamable $rdi, implicit-def $eflags, debug-location !115
+    JMP_1 %bb.1, implicit $eflags, debug-location !117
+  
+  bb.1.if.then:
+    successors: %bb.2(0x80000000)
+    liveins: $rdi, $rbp, $r15, $r14, $r13, $r12, $rbx
+  
+    MOV64mr $rsp, 1, $noreg, -8, $noreg, killed renamable $rdi :: (store 8 into %stack.0)
+    renamable $rdi = MOV64rm $rsp, 1, $noreg, -8, $noreg :: (load 8 from %stack.0)
+  
+  bb.2.if.end:
+    liveins: $rdi, $rbx, $r12, $r13, $r14, $r15, $rbp
+  
+    DBG_VALUE $rbx, $noreg, !113, !DIExpression(DW_OP_LLVM_fragment, 32, 32), debug-location !114
+    MOV64mr $rsp, 1, $noreg, -8, $noreg, killed renamable $rbx :: (store 8 into %stack.0)
+    renamable $rsi = MOV64rm $rsp, 1, $noreg, -8, $noreg :: (load 8 from %stack.0)
+
+    renamable $eax = MOV32rm killed renamable $rsi, 1, $noreg, 4, $noreg, debug-location !123 :: (load 4 from %ir.add.ptr, !tbaa !24)
+    $rdi = MOV64ri 0
+    RETQ $eax, debug-location !128
+
+...