[DWARF] Fix handling of extended line-number opcodes

[probinson]

Summary

Tripped over this while writing a different test (and doing the line-number program incorrectly).
There are UI aspects here that I thought people might want to express an opinion about.

This one doesn't depend on others of my in-flight patches, although it's obviously in the same area.

[JDevlieghere]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

528

Would it be useful to print the expected and actual length? Can you add a test for this?

[probinson]

Print the expected and actual lengths for bad opcodes.
Add a second line-table with a test for this case.

[JDevlieghere]

LGTM!

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

while (*OffsetPtr < EndOffset) {
     uint8_t Opcode = DebugLineData.getU8(OffsetPtr);
 
     if (OS)
       *OS << format("%02.02" PRIx8 " ", Opcode);
 
     if (Opcode == 0) {
       // Extended Opcodes always start with a zero opcode followed by
       // a uleb128 length so you can skip ones you don't know about
-      uint32_t ExtOffset = *OffsetPtr;
       uint64_t Len = DebugLineData.getULEB128(OffsetPtr);
-      uint32_t ArgSize = Len - (*OffsetPtr - ExtOffset);
+      uint32_t ExtOffset = *OffsetPtr;
+
+      // Tolerate zero-length; assume length is correct and soldier on.
+      if (Len == 0) {
+        if (OS)
+          *OS << "Badly formed extended line op\n";
+        continue;
+      }
 
       uint8_t SubOpcode = DebugLineData.getU8(OffsetPtr);
       if (OS)
         *OS << LNExtendedString(SubOpcode);
       switch (SubOpcode) {
       case DW_LNE_end_sequence:
         // Set the end_sequence register of the state machine to true and
         // append a row to the matrix using the current values of the
if (Opcode == 0) {
 
       case DW_LNE_set_discriminator:
         State.Row.Discriminator = DebugLineData.getULEB128(OffsetPtr);
         if (OS)
           *OS << " (" << State.Row.Discriminator << ")";
         break;
 
       default:
-        // Length doesn't include the zero opcode byte or the length itself, but
-        // it does include the sub_opcode, so we have to adjust for that below
-        (*OffsetPtr) += ArgSize;
+        if (OS)
+          *OS << format("Unrecognized extended op 0x%02.02" PRIx8, SubOpcode)
+              << format(" length %" PRIx64, Len);
+        // Len doesn't include the zero opcode byte or the length itself, but
+        // it does include the sub_opcode, so we have to adjust for that.
+        (*OffsetPtr) += Len - 1;
         break;
       }
+      // Make sure the stated and parsed lengths are the same.
+      // Otherwise we have an unparseable line-number program.
+      if (*OffsetPtr - ExtOffset != Len) {
+        fprintf(stderr, "Unexpected length of extended opcode at offset "
+                        "0x%8.8" PRIx32 "\n", ExtOffset);
+        *OffsetPtr = EndOffset;
+        return false;
+      }
     } 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

llvm/test/DebugInfo/X86/dwarfdump-bogus-LNE.s

+# Test object to verify dwarfdump handles a syntactically correct line-number
+# program containing unrecognized extended opcodes.
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o - | \
+# RUN: llvm-dwarfdump -v - | FileCheck %s
+
+        .section .text
+        # Dummy function
+foo:    ret
+
+# FIXME: When we can dump a line-table without a unit, we could remove
+# the .debug_abbrev and .debug_info sections from this test.
+        .section .debug_abbrev,"",@progbits
+        .byte 0x01  # Abbrev code
+        .byte 0x11  # DW_TAG_compile_unit
+        .byte 0x00  # DW_CHILDREN_no
+        .byte 0x10  # DW_AT_stmt_list
+        .byte 0x17  # DW_FORM_sec_offset
+        .byte 0x00  # EOM(1)
+        .byte 0x00  # EOM(2)
+
+        .section .debug_info,"",@progbits
+        .long  CU_end-CU_version  # Length of Unit
+CU_version:
+        .short 4               # DWARF version number
+        .long .debug_abbrev    # Offset Into Abbrev. Section
+        .byte 8                # Address Size (in bytes)
+# The compile-unit DIE, with DW_AT_stmt_list.
+        .byte 1
+        .long LT_start
+        .byte 0 # NULL
+CU_end:
+
+        .section .debug_line,"",@progbits
+# CHECK-LABEL: .debug_line contents:
+
+# DWARF v4 line-table header.
+LT_start:
+        .long   LT_end-LT_version   # Length of Unit (DWARF-32 format)
+LT_version:
+        .short  4               # DWARF version number
+        .long   LT_header_end-LT_params     # Length of Prologue
+LT_params:
+        .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               # Standard Opcode Lengths
+        .byte   1
+        .byte   1
+        .byte   1
+        .byte   1
+        .byte   0
+        .byte   0
+        .byte   0
+        .byte   1
+        .byte   0
+        .byte   0
+        .byte   1
+        # No directories.
+        .byte   0
+        # No files.
+        .byte   0
+LT_header_end:
+        # Bogus extended opcode with zero length.
+        .byte   0               # Extended opcode indicator.
+        .byte   0               # LEB length of extended opcode + operands.
+        # Real opcode and operand.
+        .byte   0
+        .byte   9
+        .byte   2               # DW_LNE_set_address
+        .quad   .text
+        # Bogus extended opcode with multibyte LEB length.
+        .byte   0
+        .byte   0x82            # Length of 2 but with additional length byte.
+        .byte   0               # Additional length byte.
+        .byte   0x47            # Unrecognized opcode...
+        .byte   0               # with its 1-byte operand.
+        # Proper end-sequence opcode.
+        .byte   0
+        .byte   1
+        .byte   1               # DW_LNE_end_sequence
+LT_end:
+
+# CHECK:      Line table prologue:
+# CHECK:      version: 4
+# Exact prologue length isn't important but it tells us where to expect the
+# line-number program to start, and we do want to verify those offsets.
+# CHECK-NEXT: prologue_length: 0x00000014
+# CHECK:      0x0000001e: 00 Badly formed extended line op
+# CHECK-NEXT: 0x00000020: 00 DW_LNE_set_address
+# CHECK-NEXT: 0x0000002b: 00 Unrecognized extended op 0x47 length 2
+# CHECK-NEXT: 0x00000030: 00 DW_LNE_end_sequence
+# CHECK-NEXT: 0x0000000000000000 {{.*}} is_stmt end_sequence

llvm/test/MC/ELF/discriminator.s

foo:
     ret
     .size foo, .-foo
 
         .section        .debug_info,"",@progbits
 .L.debug_info_begin0:
         .long   34                      # Length of Unit
         .short  4                       # DWARF version number
         .long   .L.debug_abbrev_begin   # Offset Into Abbrev. Section
-        .byte   8                       # Address Size (in bytes)
+        .byte   4                       # Address Size (in bytes)
         .byte   1                       # Abbrev [1] 0xb:0x1b DW_TAG_compile_unit
         .long   info_string0            # DW_AT_producer
         .short  12                      # DW_AT_language
         .long   info_string1            # DW_AT_name
         .quad   0                       # DW_AT_low_pc
         .long   0                       # DW_AT_stmt_list
         .long   info_string2            # DW_AT_comp_dir
                                         # DW_AT_APPLE_optimized

 
 # CHECK:      Relocations [
 # CHECK:        Section ({{[^ ]+}}) .rel.debug_line {
 # CHECK-NEXT:     0x2D 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
+# DWARF-DUMP: 0x0000000000000000     2      0      1   0             1  is_stmt