[DWARF] Set discriminator to 0 for DW_LNS_copy

[MaskRay]

Summary

Make DW_LNS_copy set the discriminator register to 0, to conform to
DWARF 4 & 5: "Then it sets the discriminator register to 0, and sets the
basic_block, prologue_end and epilogue_begin registers to false."

Because all of DW_LNE_end_sequence, DN_LNS_copy, and special opcodes reset
discriminator to 0, we can move discriminator=0 to appendRowToMatrix.

Also, make DW_LNS_copy print before appending the row, as it is similar
to a address+=0,line+=0 special opcode, which prints before appending
the row.

[probinson]

+Dehao who I believe did the discriminator work in the first place.

[MaskRay]

This behavior is consistent with another DWARF implementation https://sourceware.org/git/?p=elfutils.git;a=blob;f=libdw/dwarf_getsrclines.c;h=75ec9c7916dac65f7ef051423e77625e61908890;hb=HEAD#l859

[dblaikie]

Should there also be an LLVM test - given that LLVM does emit DW_LNS_copy by the looks of it, and if it's been being incorrectly interpreted/dumped, then that seems to imply there's some missing LLVM test coverage for that case, maybe?

[MaskRay]

In D60364#1460434, @dblaikie wrote:

Should there also be an LLVM test - given that LLVM does emit DW_LNS_copy by the looks of it, and if it's been being incorrectly interpreted/dumped, then that seems to imply there's some missing LLVM test coverage for that case, maybe?

Do you mean a test of lib/MC/MCDwarf.cpp? As I understand it, lib/DebugInfo/DWARF is mostly the reader part and MC the writer part. With my casual reading, MCDwarf.cpp seems to do the correct thing by doing Discriminator = 0;. I haven't looked into that part yet but I can investigate that and add a test if needed in a separate change.

[dblaikie]

In D60364#1460672, @MaskRay wrote:

In D60364#1460434, @dblaikie wrote:

Should there also be an LLVM test - given that LLVM does emit DW_LNS_copy by the looks of it, and if it's been being incorrectly interpreted/dumped, then that seems to imply there's some missing LLVM test coverage for that case, maybe?

Do you mean a test of lib/MC/MCDwarf.cpp? As I understand it, lib/DebugInfo/DWARF is mostly the reader part and MC the writer part. With my casual reading, MCDwarf.cpp seems to do the correct thing by doing Discriminator = 0;. I haven't looked into that part yet but I can investigate that and add a test if needed in a separate change.

Right (& yes, lib/DebugInfo is for reading, lib/MC is for writing) - it seems likely lib/MC is doing the right thing, but it might be missing some test coverage (given the bug in the dumper, it should've been visible in some test case).

But this patch looks generally good.

[MaskRay]

Rebase and fix DW_LNCT_directory_index in the test (2 -> 0)

[MaskRay]

.

llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

return createStringError(errc::invalid_argument,
                        " but it ended at 0x%8.8" PRIx64,
                        PrologueOffset, EndPrologueOffset, (uint64_t)*OffsetPtr);
   return Error::success();
 }
 
 DWARFDebugLine::Row::Row(bool DefaultIsStmt) { reset(DefaultIsStmt); }
 
 void DWARFDebugLine::Row::postAppend() {
+  Discriminator = 0;
   BasicBlock = false;
   PrologueEnd = false;
   EpilogueBegin = false;
 }
 
 void DWARFDebugLine::Row::reset(bool DefaultIsStmt) {
   Address.Address = 0;
   Address.SectionIndex = object::SectionedAddress::UndefSection;
if (Opcode == 0) {
                            ExtOffset, Len, *OffsetPtr - ExtOffset);
     } else if (Opcode < Prologue.OpcodeBase) {
       if (OS)
         *OS << LNStandardString(Opcode);
       switch (Opcode) {
       // Standard Opcodes
       case DW_LNS_copy:
         // Takes no arguments. Append a row to the matrix using the
-        // current values of the state-machine registers. Then set
-        // the basic_block register to false.
-        State.appendRowToMatrix();
+        // current values of the state-machine registers.
         if (OS) {
           *OS << "\n";
           OS->indent(12);
           State.Row.dump(*OS);
           *OS << "\n";
         }
+        State.appendRowToMatrix();
         break;
 
       case DW_LNS_advance_pc:
         // Takes a single unsigned LEB128 operand, multiplies it by the
         // min_inst_length field of the prologue, and adds the
         // result to the address register of the state machine.
         {
           uint64_t AddrOffset =
if (Opcode == 0) {
       if (OS) {
         *OS << "address += " << AddrOffset << ",  line += " << LineOffset
             << "\n";
         OS->indent(12);
         State.Row.dump(*OS);
       }
 
       State.appendRowToMatrix();
-      // Reset discriminator to 0.
-      State.Row.Discriminator = 0;
     }
     if(OS)
       *OS << "\n";
   }
 
   if (!State.Sequence.Empty)
     RecoverableErrorCallback(
         createStringError(errc::illegal_byte_sequence,

llvm/trunk/test/tools/llvm-dwarfdump/X86/debug-line-dw-lns-copy.s

+# RUN: llvm-mc -filetype obj -triple x86_64-pc-linux %s -o %t.o
+# RUN: llvm-dwarfdump -debug-line %t.o | FileCheck %s
+
+# CHECK:      Address            Line   Column File   ISA Discriminator Flags
+# CHECK-NEXT: ------------------ ------ ------ ------ --- ------------- -------------
+# CHECK-NEXT: 0x0000000000000000      1      0      1   0             1  is_stmt
+# CHECK-NEXT: 0x0000000000000001      2      0      1   0             0  is_stmt
+# CHECK-NEXT: 0x0000000000000001      2      0      1   0             0  is_stmt end_sequence
+
+.section .debug_line,"",@progbits
+.Line_table_start0:
+  .long   .Line_table_end0-.Line_table_start0-4   # Length of Unit
+  .short  5               # DWARF version number
+  .byte   8               # Address Size
+  .byte   0               # Segment Selector Size
+  .long   .Line_table_header_end0-.Line_table_params0     # Length of Prologue
+.Line_table_params0:
+  .byte   1               # Minimum Instruction Length
+  .byte   1               # Maximum Operations per Instruction
+  .byte   1               # Default is_stmt
+  .byte   -5              # Line Base
+  .byte   14              # Line Range
+  .byte   13              # Opcode Base
+  .byte   0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 # Standard Opcode Lengths
+  # Directory table format
+  .byte   1               # One element per directory entry
+  .byte   1               # DW_LNCT_path
+  .byte   0x08            # DW_FORM_string
+  # Directory table entries
+  .byte   1               # 1 directory
+  .asciz  "/tmp"
+  # File table format
+  .byte   2               # 2 elements per file entry
+  .byte   1               # DW_LNCT_path
+  .byte   0x08            # DW_FORM_string
+  .byte   2               # DW_LNCT_directory_index
+  .byte   0x0b            # DW_FORM_data1
+  # File table entries
+  .byte   1               # 1 file
+  .asciz  "a.c"
+  .byte   0
+.Line_table_header_end0:
+  .byte   0,2,4,1         # DW_LNE_set_discriminator 1
+  .byte   1               # DW_LNS_copy
+  .byte   33              # address += 1, line += 1
+  .byte   0,1,1           # DW_LNE_end_sequence
+.Line_table_end0: