Fix an offset underflow bug in DwarfExpression when describing small values with subregisters

[aprantl]

Summary

DwarfExpression::addMachineReg() knows how to build a larger register that isn't expressible in DWARF by combining multiple subregisters. However, if the entire value fits into just one subregister, it would still emit the other subregisters, leading to all sorts of inconsistencies down the line.

This patch fixes that by moving an already existing(!) check whether the subregister's offset is before the end of the value to the right place.

rdar://problem/57294211

[vsk]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

168

Is it a requirement that all bits in MachineReg be described? What happens is there's an unset bit in Coverage?

[aprantl]

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

168

All "holes" are covered by empty "no DWARF register encoding" empty pieces (see line 164 and 176).

[vsk]

Lgtm!

[jyknight]

The test dwarf-size-field-overflow.test now takes over 60 seconds to run, which seems quite excessive.

Is there no way to test this in a less-expensive manner?

[aprantl]

In D70508#1754479, @jyknight wrote:

The test dwarf-size-field-overflow.test now takes over 60 seconds to run, which seems quite excessive.

Is there no way to test this in a less-expensive manner?

I almost halved the number of iterations in 1b9ef. On my iMac Pro in Release+Asserts it's running in 1.18s wall clock time now. Note however that even before my change it was only taking ~5s. Were you measuring a debug build?

[jyknight]

In D70508#1755232, @aprantl wrote:

In D70508#1754479, @jyknight wrote:

The test dwarf-size-field-overflow.test now takes over 60 seconds to run, which seems quite excessive.

Is there no way to test this in a less-expensive manner?

I almost halved the number of iterations in 1b9ef. On my iMac Pro in Release+Asserts it's running in 1.18s wall clock time now. Note however that even before my change it was only taking ~5s. Were you measuring a debug build?

Yes, in debug mode.

Thanks -- it now takes 22 sec, which is better. (Before the first change, it took 12 sec).

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

if (Reg < 0)
       continue;
 
     // Intersection between the bits we already emitted and the bits
     // covered by this subregister.
     SmallBitVector CurSubReg(RegSize, false);
     CurSubReg.set(Offset, Offset + Size);
 
     // If this sub-register has a DWARF number and we haven't covered
-    // its range, emit a DWARF piece for it.
-    if (CurSubReg.test(Coverage)) {
+    // its range, and its range covers the value, emit a DWARF piece for it.
+    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(
+            {-1, Offset - CurPos, "no DWARF register encoding"});
       DwarfRegs.push_back(
           {Reg, std::min<unsigned>(Size, MaxSize - Offset), "sub-register"});
-      if (Offset >= MaxSize)
-        break;
-
-      // Mark it as emitted.
-      Coverage.set(Offset, Offset + Size);
-      CurPos = Offset + Size;
-    }
-  }
+    }
   // Failed to find any DWARF encoding.
   if (CurPos == 0)
     return false;
   // Found a partial or complete DWARF encoding.
   if (CurPos < RegSize)
     DwarfRegs.push_back({-1, RegSize - CurPos, "no DWARF register encoding"});
   return true;
 }
while (ExprCursor) {
     switch (OpNum) {
     case dwarf::DW_OP_LLVM_fragment: {
       unsigned SizeInBits = Op->getArg(1);
       unsigned FragmentOffset = Op->getArg(0);
       // The fragment offset must have already been adjusted by emitting an
       // empty DW_OP_piece / DW_OP_bit_piece before we emitted the base
       // location.
       assert(OffsetInBits >= FragmentOffset && "fragment offset not added?");
+      assert(SizeInBits >= OffsetInBits - FragmentOffset && "size underflow");
 
       // If addMachineReg already emitted DW_OP_piece operations to represent
       // a super-register by splicing together sub-registers, subtract the size
       // of the pieces that was already emitted.
       SizeInBits -= OffsetInBits - FragmentOffset;
 
       // If addMachineReg requested a DW_OP_bit_piece to stencil out a
       // sub-register that is smaller than the current fragment's size, use it.

llvm/test/DebugInfo/MIR/ARM/larger-subregister.mir

+# RUN: llc -start-before=livedebugvalues -filetype=obj -o - %s | \
+# RUN:     llvm-dwarfdump -debug-info - | FileCheck %s
+#
+# In this test the first DBG_VALUE is 32 bits large and sits in 128-bit register Q8,
+# which has to be decomposed into two 64-bit register D16, D17.
+# This test ensures that the unused D17 is suppressed in the output.
+#
+# CHECK: .debug_info contents:
+# CHECK: DW_TAG_formal_parameter
+# CHECK-NEXT: DW_AT_location
+# CHECK-NEXT: DW_OP_regx D16, DW_OP_piece 0x4, DW_OP_regx D15, DW_OP_bit_piece 0x20 0x20
+# CHECK-NEXT: DW_AT_name {{.*}}"boundingRect"
+--- |
+  target triple = "thumbv7k-apple-watchos"
+  define swiftcc void @f() !dbg !5 {
+       ret void
+  }
+  !llvm.module.flags = !{!0}
+  !llvm.dbg.cu = !{!2}
+  !0 = !{i32 2, !"Debug Info Version", i32 3}
+  !2 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !3,
+  isOptimized: true, runtimeVersion: 5, emissionKind: FullDebug, enums: !4, globals: !4, imports: !4)
+  !3 = !DIFile(filename: "t.swift", directory: "/")
+  !4 = !{}
+  !5 = distinct !DISubprogram(name: "f", scope: !3, file: !3, line: 388, type: !9, scopeLine: 388, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !4)
+  !9 = !DISubroutineType(types: !4)
+  !10 = !DILocation(line: 0, scope: !5)
+  !12 = !DILocalVariable(name: "boundingRect", arg: 1, scope: !5, file: !3, line: 388, type: !13)
+  !13 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !14)
+  !14 = !DICompositeType(tag: DW_TAG_structure_type, name: "CGRect", scope: !5, file: !3, line: 46, size: 128, elements: !4, runtimeLang: DW_LANG_Swift, identifier: "$sSo6CGRectVD")
+name:            'f'
+body:             |
+  bb.0:
+    DBG_VALUE $s31, $noreg, !12, !DIExpression(DW_OP_LLVM_fragment, 32, 32), debug-location !10
+    DBG_VALUE $q8, $noreg, !12, !DIExpression(DW_OP_LLVM_fragment, 0, 32), debug-location !10
+    renamable $r0 = t2ADDri $r6, 144, 14, $noreg, $noreg, debug-location !10

llvm/test/MC/X86/dwarf-size-field-overflow.test

 # This test generates too many debug location entries to fit into 65KB required
 # by DWARF < 5. Check that the location is set to 0 instead of crashing.
 #
-# RUN: %python %s 4000 | llc -mtriple=x86_64-apple-darwin -filetype=obj -o %t
+# RUN: %python %s 10000 | llc -mtriple=x86_64-apple-darwin -filetype=obj -o %t
 # RUN: llvm-dwarfdump %t | FileCheck %s
 #
 # CHECK:       0x0000004d:       DW_TAG_formal_parameter
 # CHECK-NEXT:      DW_AT_location   (0x00000000
 # CHECK-NEXT:           [0x0000000000000000,  0x0000000000000008): )
 # CHECK-NEXT:      DW_AT_name   ("self")
 
 import sys

 !8 = distinct !DICompositeType(tag: DW_TAG_structure_type, scope: !6, file: !1, line: 36, size: 2384384, elements: !2, runtimeLang: DW_LANG_C)
 !22 = !{{i32 2, !"Debug Info Version", i32 3}}
 !24 = distinct !DISubprogram(name: "main", linkageName: "main", scope: !6, file: !1, line: 1, type: !25, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
 !25 = !DISubroutineType(types: !2)
 !30 = !DILocalVariable(name: "self", arg: 1, scope: !24, file: !1, line: 12, type: !8, flags: DIFlagArtificial)
 !34 = !DILocation(line: 12, column: 8, scope: !24)
 """
 
-CALL = "call void @llvm.dbg.value(metadata i64 {0}, metadata !30, metadata !DIExpression(DW_OP_LLVM_fragment, {0}, 64)), !dbg !34"
+CALL = "call void @llvm.dbg.value(metadata i64 {0}, metadata !30, metadata !DIExpression(DW_OP_LLVM_fragment, {1}, 8)), !dbg !34"
 
 if __name__ == '__main__':
     N = int(sys.argv[1])
     calls = []
     for i in range(0, N):
-        calls.append(CALL.format(i * 10**12))
+        calls.append(CALL.format(i * 10**12, i*8))
     print(SKELETON.format('\n'.join(calls)))