Fix an assertion failure in DwarfExpression's subregister composition

adrian-prantl · adrian-prantl · commit a095d149c2c8 · 2020-01-27T12:44:37.000-08:00
This patch fixes an assertion failure in DwarfExpression that is triggered when a complex fragment has exactly the size of a subregister of the register the DBG_VALUE points to *and* there is no DWARF encoding for the super-register. I took the opportunity to replace/document some magic values with static constructor functions to make this code less confusing to read. rdar://problem/58489125 Differential Revision: https://reviews.llvm.org/D72938
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
@@ -100,7 +100,7 @@ bool DwarfExpression::addMachineReg(const TargetRegisterInfo &TRI,
                                     unsigned MachineReg, unsigned MaxSize) {
   if (!llvm::Register::isPhysicalRegister(MachineReg)) {
     if (isFrameRegister(TRI, MachineReg)) {
-      DwarfRegs.push_back({-1, 0, nullptr});
+      DwarfRegs.push_back(Register::createRegister(-1, nullptr));
       return true;
     }
     return false;
@@ -110,7 +110,7 @@ bool DwarfExpression::addMachineReg(const TargetRegisterInfo &TRI,
 
   // If this is a valid register number, emit it.
   if (Reg >= 0) {
-    DwarfRegs.push_back({Reg, 0, nullptr});
+    DwarfRegs.push_back(Register::createRegister(Reg, nullptr));
     return true;
   }
 
@@ -122,7 +122,7 @@ bool DwarfExpression::addMachineReg(const TargetRegisterInfo &TRI,
       unsigned Idx = TRI.getSubRegIndex(*SR, MachineReg);
       unsigned Size = TRI.getSubRegIdxSize(Idx);
       unsigned RegOffset = TRI.getSubRegIdxOffset(Idx);
-      DwarfRegs.push_back({Reg, 0, "super-register"});
+      DwarfRegs.push_back(Register::createRegister(Reg, "super-register"));
       // Use a DW_OP_bit_piece to describe the sub-register.
       setSubRegisterPiece(Size, RegOffset);
       return true;
@@ -149,8 +149,8 @@ bool DwarfExpression::addMachineReg(const TargetRegisterInfo &TRI,
     if (Reg < 0)
       continue;
 
-    // Intersection between the bits we already emitted and the bits
-    // covered by this subregister.
+    // Used to build the intersection between the bits we already
+    // emitted and the bits covered by this subregister.
     SmallBitVector CurSubReg(RegSize, false);
     CurSubReg.set(Offset, Offset + Size);
 
@@ -159,10 +159,13 @@ bool DwarfExpression::addMachineReg(const TargetRegisterInfo &TRI,
     if (Offset < MaxSize && CurSubReg.test(Coverage)) {
       // Emit a piece for any gap in the coverage.
       if (Offset > CurPos)
-        DwarfRegs.push_back(
-            {-1, Offset - CurPos, "no DWARF register encoding"});
-      DwarfRegs.push_back(
-          {Reg, std::min<unsigned>(Size, MaxSize - Offset), "sub-register"});
+        DwarfRegs.push_back(Register::createSubRegister(
+            -1, Offset - CurPos, "no DWARF register encoding"));
+      if (Offset == 0 && Size >= MaxSize)
+        DwarfRegs.push_back(Register::createRegister(Reg, "sub-register"));
+      else
+        DwarfRegs.push_back(Register::createSubRegister(
+            Reg, std::min<unsigned>(Size, MaxSize - Offset), "sub-register"));
     }
     // Mark it as emitted.
     Coverage.set(Offset, Offset + Size);
@@ -173,7 +176,8 @@ bool DwarfExpression::addMachineReg(const TargetRegisterInfo &TRI,
     return false;
   // Found a partial or complete DWARF encoding.
   if (CurPos < RegSize)
-    DwarfRegs.push_back({-1, RegSize - CurPos, "no DWARF register encoding"});
+    DwarfRegs.push_back(Register::createSubRegister(
+        -1, RegSize - CurPos, "no DWARF register encoding"));
   return true;
 }
 
@@ -249,7 +253,7 @@ bool DwarfExpression::addMachineRegExpression(const TargetRegisterInfo &TRI,
     for (auto &Reg : DwarfRegs) {
       if (Reg.DwarfRegNo >= 0)
         addReg(Reg.DwarfRegNo, Reg.Comment);
-      addOpPiece(Reg.Size);
+      addOpPiece(Reg.SubRegSize);
     }
 
     if (isEntryValue())
@@ -276,7 +280,7 @@ bool DwarfExpression::addMachineRegExpression(const TargetRegisterInfo &TRI,
   auto Reg = DwarfRegs[0];
   bool FBReg = isFrameRegister(TRI, MachineReg);
   int SignedOffset = 0;
-  assert(Reg.Size == 0 && "subregister has same size as superregister");
+  assert(!Reg.isSubRegister() && "full register expected");
 
   // Pattern-match combinations for which more efficient representations exist.
   // [Reg, DW_OP_plus_uconst, Offset] --> [DW_OP_breg, Offset].
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
@@ -107,8 +107,21 @@ class DwarfExpression {
   /// Holds information about all subregisters comprising a register location.
   struct Register {
     int DwarfRegNo;
-    unsigned Size;
+    unsigned SubRegSize;
     const char *Comment;
+
+    /// Create a full register, no extra DW_OP_piece operators necessary.
+    static Register createRegister(int RegNo, const char *Comment) {
+      return {RegNo, 0, Comment};
+    }
+
+    /// Create a subregister that needs a DW_OP_piece operator with SizeInBits.
+    static Register createSubRegister(int RegNo, unsigned SizeInBits,
+                                      const char *Comment) {
+      return {RegNo, SizeInBits, Comment};
+    }
+
+    bool isSubRegister() const { return SubRegSize; }
   };
 
   /// Whether we are currently emitting an entry value operation.
diff --git a/llvm/test/DebugInfo/MIR/ARM/subregister-full-piece.mir b/llvm/test/DebugInfo/MIR/ARM/subregister-full-piece.mir
@@ -0,0 +1,47 @@
+# RUN: llc -start-after=livedebugvalues -filetype=obj -o - %s | \
+# RUN:    llvm-dwarfdump - | FileCheck %s
+#
+# This tests the edge-case where a complex fragment has exactly
+# the size of a subregister of the register the DBG_VALUE points to.
+#
+# CHECK: .debug_info contents:
+# CHECK: DW_TAG_variable
+# CHECK-NOT: DW_TAG
+# CHECK:   DW_AT_location
+#                                    Q8 = {D16, D17}
+# CHECK-NEXT:                   DW_OP_regx D16, DW_OP_piece 0x8)
+# CHECK-NOT: DW_TAG
+# CHECK:   DW_AT_name	("q8")
+# CHECK: DW_TAG_variable
+# CHECK-NOT: DW_TAG
+# CHECK:   DW_AT_location
+#                                     Q9 = {D18, D19}
+# CHECK-NEXT:                    DW_OP_regx D18, DW_OP_piece 0x7)
+# CHECK-NOT: DW_TAG
+# CHECK:   DW_AT_name	("q9")
+
+--- |
+  target triple = "thumbv7s-apple-ios"
+  define hidden void @f() !dbg !5 {
+  for.body:
+  ret void, !dbg !20
+  }
+  !llvm.module.flags = !{!1, !2}
+  !llvm.dbg.cu = !{!3}
+  !1 = !{i32 7, !"Dwarf Version", i32 4}
+  !2 = !{i32 2, !"Debug Info Version", i32 3}
+  !3 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !4, isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+  !4 = !DIFile(filename: "t.cpp", directory: "/")
+  !5 = distinct !DISubprogram(name: "f",scope: !4, file: !4, line: 1, type: !6, scopeLine: 1, unit: !3)
+  !6 = !DISubroutineType(types: !{})
+  !7 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "uint8x8x2_t", file: !4, line: 113, size: 128, flags: DIFlagTypePassByValue, elements: !{})
+  !8 = !DILocalVariable(name: "q8", scope: !5, file: !4, line: 1, type: !7)
+  !9 = !DILocalVariable(name: "q9", scope: !5, file: !4, line: 1, type: !7)
+  !20 = !DILocation(line: 0, scope: !5)
+name:            f
+body:             |
+  bb.2.for.body:
+    t2Bcc %bb.2.for.body, 0, killed $cpsr, debug-location !20
+    DBG_VALUE $q8, $noreg, !8, !DIExpression(DW_OP_LLVM_fragment, 0, 64), debug-location !20
+    DBG_VALUE $q9, $noreg, !9, !DIExpression(DW_OP_LLVM_fragment, 0, 56), debug-location !20
+    tB %bb.2.for.body, 14, $noreg
