Re-submit r289925 (Update .debug_line section version to match DWARF version)

[kromanova]

Summary

This is one more attempt to re-land patch r289925 that was reverted by Nico a while ago (because it caused problem for Chromium's symbolizer).

Nico filed a bug PR# 31407, https://bugs.llvm.org/show_bug.cgi?id=31407, but unfortunately, the reproducer was not created for several months.
A couple of days ago Caroline confirmed that the problem was with the symbolizer, not with the patch itself. See all the details in PR# 31407.

So now I could re-submit the patch again.

I rebased my old patch (https://reviews.llvm.org/D16697) and made a few small changes. Namely, I updated debug_line table length/prologue length for some of newly added tests
and temporarily lowered the line table version to 2, when generation of DWARF V5 is requested (simply because line table V5 is not supported yet and a lot of newly added DWARF 5 tests started to fail).

[dblaikie]

lib/MC/MCDwarf.cpp

282–286

That seems weird - why not LineTableVersion 4? (why would going from DWARF 4 to DWARF 5 make the line table version go from 4 to 2?)

[kromanova]

Thank you for the review, David! I have changed debug line version to V4, when DWARF V5 is requested (this is just a temporary measure until debug line V5 is supported).

[probinson]

LGTM. Sorry, I hadn't realized nobody formally approved this yet! I'm starting to work on emitting the v5 line-table header so it would be nice to get this in.

[davide]

I'm afraid this commit and the subsequent by @probinson broke large part of the debugger test suite.
I'll try updating all the tests to see if it's just as easy as that, but in case it won't I'll consider asking you to revert this commit.

[davide]

More generally, DWARF changes should not impact the lldb test suite (apparently one of the bots, green dragon, failed, and now has been red for days).

[davide]

This is the current status.

===================
Test Result Summary
===================
Test Methods:       2679
Reruns:                3
Success:            1626
Expected Failure:    106
Failure:             510
Error:                67
Exceptional Exit:      0
Unexpected Success:   44
Skip:                323
Timeout:               3
Expected Timeout:      0

I'm going to revert the two commits to bring the bot green again, then we can probably evaluate a strategy on how to tackle this.
@probinson/ @kromanova are you OK with this?

[probinson]

In D38002#947253, @davide wrote:

More generally, DWARF changes should not impact the lldb test suite (apparently one of the bots, green dragon, failed, and now has been red for days).

The issue is not the lldb test suite per se, but the lldb test suite as run on certain Apple bots and depending on certain system utilities.
The lldb tests that I ran on Linux were fine (ninja check-lldb check-lldb-lit, using the just-built clang as the test compiler).

The lldb bots did not send me a blame email, either. I followed up with the greendragon bot that was having the exact same problem with the debuginfo-tests. I was not aware of the lldb bot problem until today when somebody mentioned it on lldb-dev.

[davide]

In D38002#947272, @probinson wrote:

In D38002#947253, @davide wrote:

More generally, DWARF changes should not impact the lldb test suite (apparently one of the bots, green dragon, failed, and now has been red for days).

The issue is not the lldb test suite per se, but the lldb test suite as run on certain Apple bots and depending on certain system utilities.
The lldb tests that I ran on Linux were fine (ninja check-lldb check-lldb-lit, using the just-built clang as the test compiler).

This, FWIW, breaks also on my desktop (and probably and stock macOS configuration).
I understand it's not your fault, and I'll try to monitor fix the bots to send mails, but, in the meanwhile, I'd rather unblock this situation. Are you OK with temporarily reverting?

[probinson]

Let me see if I can find a way to force version 2 for Darwin only. I really don't want to revert again, it took months to get Chrome to fix their tools and reverting would block further work.

[davide]

Thank you Paul. I'm OK with this. I don't want to bother you more than needed, probably I can try to take a look at how to implement myself.

[davide]

(as I also have the means to check on the bots etc..)

[probinson]

Ah, thanks for the help Davide! Looks like you can do this:
if (context.getObjectFileInfo()->getTargetTriple().isOSDarwin()) LineTableVersion = 2;
and that should take care of it.

[davide]

No worries, happy to get this solved. FWIW, there was a configuration problem and this why you didn't receive the mail.
This was fixed so you should be able to receive blame mails in future (please let me know if you don't).

lib/MC/MCDwarf.cpp

MCDwarfLineTableHeader::Emit(MCStreamer *MCOS, MCDwarfLineTableParams Params,
   // Create a symbol for the end of the section (to be set when we get there).
   MCSymbol *LineEndSym = context.createTempSymbol();
 
   // The first 4 bytes is the total length of the information for this
   // compilation unit (not including these 4 bytes for the length).
   emitAbsValue(*MCOS,
                MakeStartMinusEndExpr(*MCOS, *LineStartSym, *LineEndSym, 4), 4);
 
-  // Next 2 bytes is the Version, which is Dwarf 2.
-  MCOS->EmitIntValue(2, 2);
+  // Next 2 bytes is the Version.
+  unsigned LineTableVersion = context.getDwarfVersion();
+  // FIXME: Right now the compiler doesn't support line table V5. Until it's
+  // supported keep generating line table V4, when Dwarf Info version V5 is used.  
+  if (LineTableVersion >= 5) 
+    LineTableVersion = 4; 
+  MCOS->EmitIntValue(LineTableVersion, 2);
 
   // Create a symbol for the end of the prologue (to be set when we get there).
   MCSymbol *ProEndSym = context.createTempSymbol(); // Lprologue_end
 
   // Length of the prologue, is the next 4 bytes.  Which is the start of the
   // section to the end of the prologue.  Not including the 4 bytes for the
   // total length, the 2 bytes for the version, and these 4 bytes for the
   // length of the prologue.
   emitAbsValue(
       *MCOS,
       MakeStartMinusEndExpr(*MCOS, *LineStartSym, *ProEndSym, (4 + 2 + 4)), 4);
 
   // Parameters of the state machine, are next.
   MCOS->EmitIntValue(context.getAsmInfo()->getMinInstAlignment(), 1);
+  // maximum_operations_per_instruction 
+  // For non-VLIW architectures this field is always 1.
+  // FIXME: VLIW architectures need to update this field accordingly.
+  if (context.getDwarfVersion() >= 4)
+    MCOS->EmitIntValue(1, 1);
   MCOS->EmitIntValue(DWARF2_LINE_DEFAULT_IS_STMT, 1);
   MCOS->EmitIntValue(Params.DWARF2LineBase, 1);
   MCOS->EmitIntValue(Params.DWARF2LineRange, 1);
   MCOS->EmitIntValue(StandardOpcodeLengths.size() + 1, 1);
 
   // Standard opcode lengths
   for (char Length : StandardOpcodeLengths)
     MCOS->EmitIntValue(Length, 1);

test/CodeGen/X86/dbg-line-0-no-discriminator.ll

 !8 = !{null}
 !9 = !DILocation(line: 4, column: 3, scope: !10)
 !10 = !DILexicalBlockFile(scope: !6, file: !1, discriminator: 1)
 !11 = !DILocation(line: 6, column: 1, scope: !6)
 
 ; Look at the lengths. We can't verify the line-number-program size
 ; directly, but the difference in the two lengths should not change
 ; unexpectedly.
-; CHECK:    total_length: 0x00000043
-; CHECK: prologue_length: 0x0000001e
+; CHECK:    total_length: 0x00000044
+; CHECK: prologue_length: 0x0000001f
 ;
 ; Verify that we see a line entry with a discriminator, and the next entry
 ; has line 0 and no discriminator.
 ;             line column file ISA discriminator
 ; CHECK:      4    3      1    0   1
 ; CHECK-NEXT: 0    3      1    0   0

test/DebugInfo/AArch64/line-header.ll

 ; RUN: llc -mtriple=aarch64-none-linux -O0 -filetype=obj - < %S/../Inputs/line.ll | llvm-dwarfdump -v - | FileCheck %s
 ; RUN: llc -mtriple=aarch64_be-none-linux -O0 -filetype=obj - < %S/../Inputs/line.ll | llvm-dwarfdump -v - | FileCheck %s
 
 ; check line table length is correctly calculated for both big and little endian
 CHECK-LABEL: .debug_line contents:
-CHECK: total_length: 0x0000003e
+CHECK: total_length: 0x0000003f

test/DebugInfo/Generic/empty.ll

 ; REQUIRES: object-emission
 
 ; RUN: %llc_dwarf < %s -filetype=obj | llvm-dwarfdump -v - | FileCheck %s
 ; RUN: %llc_dwarf -split-dwarf-file=foo.dwo < %s -filetype=obj | llvm-dwarfdump -v - | FileCheck --check-prefix=FISSION %s
 
 ; darwin has a workaround for a linker bug so it always emits one line table entry
 ; XFAIL: darwin
 
 ; Expect no line table entry since there are no functions and file references in this compile unit
 ; CHECK: .debug_line contents:
 ; CHECK: Line table prologue:
-; CHECK: total_length: 0x00000019
+; CHECK: total_length: 0x0000001a
 ; CHECK-NOT: file_names[
 
 ; CHECK-NOT: .debug_pubnames contents:
 ; CHECK: contents:
 
 ; Don't emit DW_AT_addr_base when there are no addresses.
 ; FISSION-NOT: DW_AT_GNU_addr_base [DW_FORM_sec_offset]
 

test/DebugInfo/Generic/nodebug.ll

 ; Check that there's no DW_TAG_subprogram, not even for the 'f2' function.
 ; CHECK: .debug_info contents:
 ; CHECK: DW_TAG_compile_unit
 ; CHECK-NOT: DW_TAG_subprogram
 
 ; Expect no line table entry since there are no functions and file references in this compile unit
 ; CHECK: .debug_line contents:
 ; CHECK: Line table prologue:
-; CHECK: total_length: 0x00000019
+; CHECK: total_length: 0x0000001a
 ; CHECK-NOT: file_names[
 
 @i = external global i32
 
 ; Function Attrs: uwtable
 define void @_Z2f2v() #0 {
 entry:
   store i32 3, i32* @i, align 4, !dbg !11

test/DebugInfo/X86/empty.ll

 ; RUN: llc -mtriple i686-pc-cygwin < %s -filetype=obj | llvm-dwarfdump -v - | FileCheck %s
 ; RUN: llc -mtriple i686-pc-cygwin -split-dwarf-file=foo.dwo < %s -filetype=obj | llvm-dwarfdump -v - | FileCheck --check-prefix=FISSION %s
 
 ; Expect no line table entry since there are no functions and file references in this compile unit
 ; CHECK: .debug_line contents:
 ; CHECK: Line table prologue:
-; CHECK: total_length: 0x00000019
+; CHECK: total_length: 0x0000001a
 ; CHECK-NOT: file_names[
 
 ; CHECK: .debug_pubnames contents:
 ; CHECK-NEXT: length = 0x0000000e
 ; CHECK-NEXT: Offset
 ; CHECK-NEXT: {{^$}}
 
 ; Don't emit DW_AT_addr_base when there are no addresses.

test/DebugInfo/X86/stmt-list-multiple-compile-units.ll

 ; CHECK: DW_AT_stmt_list [DW_FORM_sec_offset]   (0x00000000)
 ; CHECK: DW_AT_low_pc [DW_FORM_addr]            (0x0000000000000000)
 ; CHECK: DW_AT_high_pc [DW_FORM_data4]          (0x00000010)
 ; CHECK: DW_TAG_subprogram
 ; CHECK: DW_AT_low_pc [DW_FORM_addr]            (0x0000000000000000)
 ; CHECK: DW_AT_high_pc [DW_FORM_data4]          (0x00000010)
 
 ; CHECK: DW_TAG_compile_unit
-; CHECK: DW_AT_stmt_list [DW_FORM_sec_offset]   (0x0000003c)
+; CHECK: DW_AT_stmt_list [DW_FORM_sec_offset]   (0x0000003d)
 ; CHECK: DW_AT_low_pc [DW_FORM_addr]            (0x0000000000000010)
 ; CHECK: DW_AT_high_pc [DW_FORM_data4]          (0x00000009)
 ; CHECK: DW_TAG_subprogram
 ; CHECK: DW_AT_low_pc [DW_FORM_addr]            (0x0000000000000010)
 ; CHECK: DW_AT_high_pc [DW_FORM_data4]          (0x00000009)
 
 
 ; CHECK: .debug_line contents:
 ; CHECK-NEXT: Line table prologue:
-; CHECK-NEXT: total_length: 0x00000038
+; CHECK-NEXT: total_length: 0x00000039
 ; CHECK: file_names[  1]    0 0x00000000 0x00000000 simple.c
 ; CHECK: Line table prologue:
-; CHECK-NEXT: total_length: 0x00000039
+; CHECK-NEXT: total_length: 0x0000003a
 ; CHECK: file_names[  1]    0 0x00000000 0x00000000 simple2.c
 ; CHECK-NOT: file_names
 
 ; DWARF3: .debug_info contents:
 ; DWARF3: DW_TAG_compile_unit
 ; DWARF3: DW_AT_stmt_list [DW_FORM_data4]    (0x00000000)
 
 ; DWARF3: DW_TAG_compile_unit

test/MC/ELF/debug-line.s

 
 // CHECK:        Section {
 // CHECK:          Name: .debug_line
 // CHECK-NEXT:     Type: SHT_PROGBITS
 // CHECK-NEXT:     Flags [
 // CHECK-NEXT:     ]
 // CHECK-NEXT:     Address: 0x0
 // CHECK-NEXT:     Offset:
-// CHECK-NEXT:     Size: 57
+// CHECK-NEXT:     Size: 58
 // CHECK-NEXT:     Link: 0
 // CHECK-NEXT:     Info: 0
 // CHECK-NEXT:     AddressAlignment: 1
 // CHECK-NEXT:     EntrySize: 0
 // CHECK-NEXT:     SectionData (
 
-// CHECK-NEXT:       0000: 35000000 02001C00 00000101 FB0E0D00
-// CHECK-NEXT:       0010: 01010101 00000001 00000100 666F6F2E
-// CHECK-NEXT:       0020: 63000000 00000009 02000000 00000000
-// CHECK-NEXT:       0030: 00154B21 02080001 01
+// CHECK-NEXT:       0000: 36000000 04001D00 00000101 01FB0E0D
+// CHECK-NEXT:       0010: 00010101 01000000 01000001 00666F6F  
+// CHECK-NEXT:       0020: 2E630000 00000000 09020000 00000000  
+// CHECK-NEXT:       0030: 0000154B 21020800 0101              
 // CHECK-NEXT:     )
 // CHECK-NEXT:   }
 
   .section  .debug_line,"",@progbits
   .text
 
   .file 1 "foo.c"
   .loc 1 4 0

test/MC/ELF/debug-line2.s

 // RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -s -sd | FileCheck %s
 
 // Test that two subsequent .loc directives generate two
 // distinct line table entries.
 
 // CHECK:        Section {
 // CHECK:          Name: .debug_line
 // CHECK-NEXT:     Type: SHT_PROGBITS
 // CHECK-NEXT:     Flags [
 // CHECK-NEXT:     ]
 // CHECK-NEXT:     Address: 0x0
 // CHECK-NEXT:     Offset:
-// CHECK-NEXT:     Size: 56
+// CHECK-NEXT:     Size: 57
 // CHECK-NEXT:     Link: 0
 // CHECK-NEXT:     Info: 0
 // CHECK-NEXT:     AddressAlignment: 1
 // CHECK-NEXT:     EntrySize: 0
 // CHECK-NEXT:     SectionData (
-// CHECK-NEXT:       0000: 34000000 02001C00 00000101 FB0E0D00
-// CHECK-NEXT:       0010: 01010101 00000001 00000100 666F6F2E
-// CHECK-NEXT:       0020: 63000000 00000009 02000000 00000000
-// CHECK-NEXT:       0030: 00011302 01000101
+// CHECK-NEXT:       0000: 35000000 04001D00 00000101 01FB0E0D  |5...............|
+// CHECK-NEXT:       0010: 00010101 01000000 01000001 00666F6F  |.............foo|
+// CHECK-NEXT:       0020: 2E630000 00000000 09020000 00000000  |.c..............|
+// CHECK-NEXT:       0030: 00000113 02010001 01                 |.........|
 // CHECK-NEXT:     )
 // CHECK-NEXT:   }
 
   .section  .debug_line,"",@progbits
   .text
 
   .file 1 "foo.c"
   .loc 1 1 0
   .loc 1 2 0
   nop

test/MC/ELF/debug-loc.s

 // CHECK:        Section {
 // CHECK:          Index:
 // CHECK:          Name: .debug_line
 // CHECK-NEXT:     Type: SHT_PROGBITS
 // CHECK-NEXT:     Flags [
 // CHECK-NEXT:     ]
 // CHECK-NEXT:     Address: 0x0
 // CHECK-NEXT:     Offset:
-// CHECK-NEXT:     Size: 61
+// CHECK-NEXT:     Size: 62
 // CHECK-NEXT:     Link: 0
 // CHECK-NEXT:     Info: 0
 // CHECK-NEXT:     AddressAlignment: 1
 // CHECK-NEXT:     EntrySize: 0
 // CHECK-NEXT:   }
 
   .section  .debug_line,"",@progbits
   .text
 foo:
   .file 1 "Driver.ii"
   .loc 1 2 0
         nop
   .loc 1 4 0
         nop
   .loc 1 3 0
         nop

test/MC/ELF/discriminator.s

.L.debug_abbrev_begin:
         .byte   0                       # EOM(1)
         .byte   0                       # EOM(2)
         .byte   0                       # EOM(3)
 .L.debug_abbrev_end:
 
 
 # CHECK:      Relocations [
 # CHECK:        Section ({{[^ ]+}}) .rel.debug_line {
-# CHECK-NEXT:     0x2D R_386_32 .text 0x0
+# CHECK-NEXT:     0x2E R_386_32 .text 0x0
 # CHECK-NEXT:   }
 
 # DWARF-DUMP: Address            Line   Column File   ISA Discriminator Flags
 # DWARF-DUMP: ------------------ ------ ------ ------ --- ------------- -------------
 # DWARF-DUMP: 0x0001021300000000     1      0      1   0             1  is_stmt

test/MC/ELF/empty-dwarf-lines.s

 // CHECK:        Section {
 // CHECK:          Index:
 // CHECK:          Name: .debug_line
 // CHECK-NEXT:     Type: SHT_PROGBITS
 // CHECK-NEXT:     Flags [
 // CHECK-NEXT:     ]
 // CHECK-NEXT:     Address: 0x0
 // CHECK-NEXT:     Offset: 0x44
-// CHECK-NEXT:     Size: 39
+// CHECK-NEXT:     Size: 40
 // CHECK-NEXT:     Link: 0
 // CHECK-NEXT:     Info: 0
 // CHECK-NEXT:     AddressAlignment: 1
 // CHECK-NEXT:     EntrySize: 0
 // CHECK-NEXT:   }

test/MC/MachO/file.s

 // RUN: llvm-mc -triple i386-apple-darwin9 %s -filetype=obj -o - | llvm-readobj -s -section-data | FileCheck %s
 
         .file 1 "dir/foo"
         nop
 
 // CHECK:       Section {
 // CHECK:         Index: 1
 // CHECK-NEXT:    Name: __debug_line
 // CHECK-NEXT:    Segment: __DWARF
 // CHECK-NEXT:    Address: 0x1
-// CHECK-NEXT:    Size: 0x28
+// CHECK-NEXT:    Size: 0x29
 // CHECK-NEXT:    Offset: 237
 // CHECK-NEXT:    Alignment: 0
 // CHECK-NEXT:    RelocationOffset: 0x0
 // CHECK-NEXT:    RelocationCount: 0
 // CHECK-NEXT:    Type: 0x0
 // CHECK-NEXT:    Attributes [ (0x20000)
 // CHECK-NEXT:      Debug (0x20000)
 // CHECK-NEXT:    ]
 // CHECK-NEXT:    Reserved1: 0x0
 // CHECK-NEXT:    Reserved2: 0x0
 // CHECK-NEXT:    SectionData (
-// CHECK-NEXT:      0000: 24000000 02001E00 00000101 FB0E0D00
-// CHECK-NEXT:      0010: 01010101 00000001 00000164 69720000
-// CHECK-NEXT:      0020: 666F6F00 01000000
+// CHECK-NEXT:      0000: 25000000 04001F00 00000101 01FB0E0D  |%...............|
+// CHECK-NEXT:      0010: 00010101 01000000 01000001 64697200  |............dir.|
+// CHECK-NEXT:      0020: 00666F6F 00010000 00                 |.foo.....|
 // CHECK-NEXT:    )
 // CHECK-NEXT:  }

test/MC/MachO/gen-dwarf.s

 // CHECK:    NULL
 
 // CHECK: .debug_aranges contents:
 // CHECK: Address Range Header: length = 0x0000001c, version = 0x0002, cu_offset = 0x00000000, addr_size = 0x04, seg_size = 0x00
 
 // CHECK: .debug_line contents:
 // CHECK: Line table prologue:
 // We don't check the total_length as it includes lengths of temp paths
-// CHECK:         version: 2
+// CHECK:         version: 4
 // We don't check the prologue_length as it too includes lengths of temp paths
 // CHECK: min_inst_length: 1
 // CHECK: default_is_stmt: 1
 // CHECK:       line_base: -5
 // CHECK:      line_range: 14
 // CHECK:     opcode_base: 13
 // CHECK: standard_opcode_lengths[DW_LNS_copy] = 0
 // CHECK: standard_opcode_lengths[DW_LNS_advance_pc] = 1

test/MC/MachO/loc.s

 // RUN: llvm-mc -triple i386-apple-darwin9 %s -filetype=obj -o - | llvm-readobj -r -s -sd | FileCheck %s
 
         .file 1 "foo"
   .loc  1 64 0
         nop
 
 // CHECK: Section {
 // CHECK:     Index: 1
 // CHECK:     Name: __debug_line (5F 5F 64 65 62 75 67 5F 6C 69 6E 65 00 00 00 00)
 // CHECK:     Segment: __DWARF (5F 5F 44 57 41 52 46 00 00 00 00 00 00 00 00 00)
 // CHECK:     Address: 0x1
-// CHECK:     Size: 0x33
+// CHECK:     Size: 0x34
 // CHECK:     Offset: 237
 // CHECK:     Alignment: 0
-// CHECK:     RelocationOffset: 0x120
+// CHECK:     RelocationOffset: 0x124
 // CHECK:     RelocationCount: 1
 // CHECK:     Type: 0x0
 // CHECK:     Attributes [ (0x20000)
 // CHECK:       Debug (0x20000)
 // CHECK:     ]
 // CHECK:     Reserved1: 0x0
 // CHECK:     Reserved2: 0x0
 // CHECK:     SectionData (
-// CHECK:       0000: 2F000000 02001A00 00000101 FB0E0D00  |/...............|
-// CHECK:       0010: 01010101 00000001 00000100 666F6F00  |............foo.|
-// CHECK:       0020: 00000000 00050200 00000003 3F010201  |............?...|
-// CHECK:       0030: 000101                               |...|
+// CHECK:       0000: 30000000 04001B00 00000101 01FB0E0D  |0...............|
+// CHECK:       0010: 00010101 01000000 01000001 00666F6F  |.............foo|
+// CHECK:       0020: 00000000 00000502 00000000 033F0102  |.............?..|
+// CHECK:       0030: 01000101   
 // CHECK:     )
 // CHECK:   }
 // CHECK: ]
 // CHECK: Relocations [
 // CHECK:   Section __debug_line {
-// CHECK:     0x27 0 2 0 GENERIC_RELOC_VANILLA 0 __text
+// CHECK:     0x28 0 2 0 GENERIC_RELOC_VANILLA 0 __text
 // CHECK:   }
 // CHECK: ]