[DwarfDebug] Restore code that make comments stay aligned in DwarfDebug::emitDebugLocEntry

[bjope]

Summary

Commit 2bddab25dba8d4b0 removed a piece of code from
DwarfDebug::emitDebugLocEntry that according to code comments
"Make sure comments stay aligned".

This patch restores that piece of code, together with the addition
of some extra checks in an existing lit test to work as a regression
test. Without this patch we incorrectly get

.byte   159                             # 0

instead of

.byte   159                             # DW_OP_stack_value

[dblaikie]

This seems like a fairly subtle/long distance dependence now, that "emitDIERef" will use the same ULEB128PadSize as this code uses? Which isn't necessarily true/could become divergent relatively easily.

[bjope]

In D117441#3249162, @dblaikie wrote:

This seems like a fairly subtle/long distance dependence now, that "emitDIERef" will use the same ULEB128PadSize as this code uses? Which isn't necessarily true/could become divergent relatively easily.

Yes, that could be true.

Honestly, I do not really know how it works with these comments. For example:

• How comes that we should skip (up-to) ULEB128PadSize comments for Encoding::BaseTypeRef? Why is it like that?
• Why are we just skipping comments and not printing them?

I was thinking that I could add a Twine argument to emitDIERef similarly to those found for emitInt8. But that felt weird since we just skip comments, not printing them. And I would need to pass the Iterator in order to step it multiple times.

Well, this patch simply restores the code needed to make our downstream test cases working again (we had some lit tests that used FileCheck lines to verify the debug section, including the comments, and those tests have been failing after commit 2bddab25dba8d4b0 due to the comments being totally messed up).

[dblaikie]

Perhaps a reasonable intermediate solution would be to return the pad size from emitDIERef? (or have an exposed constant in ByteStreamer that's used in emitDIERef and can be used from the DwarfDebug code) and use that value in the DwarfDebug code so the values can't diverge, at least...

[bjope]

Utilize a return value from APByteStreamer::emitDIERef to determine how many comments to skip.

[dblaikie]

Looks OK, thanks!

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 //===- llvm/CodeGen/DwarfDebug.cpp - Dwarf Debug Framework ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
assert(Op.getCode() != dwarf::DW_OP_const_type &&
            "3 operand ops not yet supported");
     Streamer.emitInt8(Op.getCode(), Comment != End ? *(Comment++) : "");
     Offset++;
     for (unsigned I = 0; I < 2; ++I) {
       if (Op.getDescription().Op[I] == Encoding::SizeNA)
         continue;
       if (Op.getDescription().Op[I] == Encoding::BaseTypeRef) {
         Streamer.emitDIERef(*CU->ExprRefedBaseTypes[Op.getRawOperand(I)].Die);
+        // Make sure comments stay aligned.
+        for (unsigned J = 0; J < ULEB128PadSize; ++J)
+          if (Comment != End)
+            Comment++;
       } else {
         for (uint64_t J = Offset; J < Op.getOperandEndOffset(I); ++J)
           Streamer.emitInt8(Data.getData()[J], Comment != End ? *(Comment++) : "");
       }
       Offset = Op.getOperandEndOffset(I);
     }
     assert(Offset == Op.getEndOffset());
   }

llvm/test/DebugInfo/X86/convert-loclist.ll

 ; identical and demonstrate that they have different DWO IDs.
 
 ; SPLIT: 0x00000000: Compile Unit: {{.*}} DWO_id = 0xa6edbf487b0a7acf
 
 ; Regression testing a fairly quirky bug where instead of hashing (see above),
 ; extra bytes would be emitted into the output assembly in no
 ; particular/intentional section - so let's check they don't show up at all:
 ; ASM-NOT: .asciz  "\200\200\200"
+; ASM:      .byte   10                              # Loc expr size
+; ASM-NEXT: .byte   17                              # DW_OP_consts
+; ASM-NEXT: .byte   7                               # 7
+; ASM-NEXT: .byte   48                              # DW_OP_lit0
+; ASM-NEXT: .byte   34                              # DW_OP_plus
+; ASM-NEXT: .byte   168                             # DW_OP_convert
+; ASM-NEXT: .asciz  "\232\200\200"                  #
+; ASM-NEXT: .byte   159                             # DW_OP_stack_value
+; ASM-NEXT: .byte   0                               # DW_LLE_end_of_list
+; ASM-NOT: .asciz  "\200\200\200"
 
 ; CHECK: 0x{{0*}}[[TYPE:.*]]: DW_TAG_base_type
 ; CHECK-NEXT:                   DW_AT_name ("DW_ATE_unsigned_32")
 
 ; CHECK: DW_LLE_offset_pair ({{.*}}): DW_OP_consts +7, DW_OP_lit0, DW_OP_plus, DW_OP_convert 0x[[TYPE]], DW_OP_stack_value
 
 ; Function Attrs: uwtable
 define dso_local void @_Z2f2v() local_unnamed_addr #0 !dbg !11 {