[DebugInfo] Add use of truncating data extractor to debug line parsing

[jhenderson]

Summary

This will ensure that nothing can ever start parsing data from a future sequence and part-read data will be returned as 0 instead.

Depends on D80795.

[labath]

Looks good too. (and reminds me I should really get back to finishing my patch for parsing the prologue).

llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test

251

Ideally, I think we should not print this opcode, as we weren't able to read all of it, but I think this is an improvement nonetheless.

[jhenderson]

llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test

251

Yep, I have a follow-up in the works already to do exactly that (and similar things for standard opcodes at some point).

[jhenderson]

In D80796#2065818, @labath wrote:

Looks good too. (and reminds me I should really get back to finishing my patch for parsing the prologue).

By the way, could you refresh me on what that patch will be doing? I've got a few more patches in related areas I want to work on, but I don't want to tread on your toes, if you are already part way through doing one or more of these changes!

[labath]

In D80796#2066013, @jhenderson wrote:

In D80796#2065818, @labath wrote:

Looks good too. (and reminds me I should really get back to finishing my patch for parsing the prologue).

By the way, could you refresh me on what that patch will be doing? I've got a few more patches in related areas I want to work on, but I don't want to tread on your toes, if you are already part way through doing one or more of these changes!

It's D77557 -- it does something similar to this patch, but handles the line table prologue. The patch had a bunch of dependencies, and was never my top priority so it got buried on my stack.. I have a bit of time now, so let me see what it takes to revive it...

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

RecoverableErrorHandler(
                           "line table program with offset 0x%8.8" PRIx64
                           " has length 0x%8.8" PRIx64 " but only 0x%8.8" PRIx64
                           " bytes are available",
                           DebugLineOffset, ProgramLength, BytesRemaining));
     // Continue by capping the length at the number of remaining bytes.
     ProgramLength = BytesRemaining;
   }
 
+  // Create a DataExtractor which can only see the data up to the end of the
+  // table, to prevent reading past the end.
   const uint64_t EndOffset = DebugLineOffset + ProgramLength;
+  DWARFDataExtractor TableData(DebugLineData, EndOffset);
 
   // See if we should tell the data extractor the address size.
-  if (DebugLineData.getAddressSize() == 0)
-    DebugLineData.setAddressSize(Prologue.getAddressSize());
+  if (TableData.getAddressSize() == 0)
+    TableData.setAddressSize(Prologue.getAddressSize());
   else
     assert(Prologue.getAddressSize() == 0 ||
-           Prologue.getAddressSize() == DebugLineData.getAddressSize());
+           Prologue.getAddressSize() == TableData.getAddressSize());
 
   ParsingState State(this, DebugLineOffset, RecoverableErrorHandler);
 
   *OffsetPtr = DebugLineOffset + Prologue.getLength();
   while (*OffsetPtr < EndOffset) {
     if (OS)
       *OS << format("0x%08.08" PRIx64 ": ", *OffsetPtr);
 
     uint64_t OpcodeOffset = *OffsetPtr;
-    uint8_t Opcode = DebugLineData.getU8(OffsetPtr);
+    uint8_t Opcode = TableData.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
-      uint64_t Len = DebugLineData.getULEB128(OffsetPtr);
+      uint64_t Len = TableData.getULEB128(OffsetPtr);
       uint64_t ExtOffset = *OffsetPtr;
 
       // Tolerate zero-length; assume length is correct and soldier on.
       if (Len == 0) {
         if (OS)
           *OS << "Badly formed extended line op (length 0)\n";
         continue;
       }
 
-      uint8_t SubOpcode = DebugLineData.getU8(OffsetPtr);
+      uint8_t SubOpcode = TableData.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
         // state-machine registers. Then reset the registers to the initial
         // values specified above. Every statement program sequence must end
if (Opcode == 0) {
         // machine. Set the address register to the value given by the
         // relocatable address. All of the other statement program opcodes
         // that affect the address register add a delta to it. This instruction
         // stores a relocatable value into it instead.
         //
         // Make sure the extractor knows the address size.  If not, infer it
         // from the size of the operand.
         {
-          uint8_t ExtractorAddressSize = DebugLineData.getAddressSize();
+          uint8_t ExtractorAddressSize = TableData.getAddressSize();
           uint64_t OpcodeAddressSize = Len - 1;
           if (ExtractorAddressSize != OpcodeAddressSize &&
               ExtractorAddressSize != 0)
             RecoverableErrorHandler(createStringError(
                 errc::invalid_argument,
                 "mismatching address size at offset 0x%8.8" PRIx64
                 " expected 0x%2.2" PRIx8 " found 0x%2.2" PRIx64,
                 ExtOffset, ExtractorAddressSize, Len - 1));
 
           // Assume that the line table is correct and temporarily override the
           // address size. If the size is unsupported, give up trying to read
           // the address and continue to the next opcode.
           if (OpcodeAddressSize != 1 && OpcodeAddressSize != 2 &&
               OpcodeAddressSize != 4 && OpcodeAddressSize != 8) {
             RecoverableErrorHandler(createStringError(
                 errc::invalid_argument,
                 "address size 0x%2.2" PRIx64
                 " of DW_LNE_set_address opcode at offset 0x%8.8" PRIx64
                 " is unsupported",
                 OpcodeAddressSize, ExtOffset));
             *OffsetPtr += OpcodeAddressSize;
           } else {
-            DebugLineData.setAddressSize(OpcodeAddressSize);
-            State.Row.Address.Address = DebugLineData.getRelocatedAddress(
+            TableData.setAddressSize(OpcodeAddressSize);
+            State.Row.Address.Address = TableData.getRelocatedAddress(
                 OffsetPtr, &State.Row.Address.SectionIndex);
 
             // Restore the address size if the extractor already had it.
             if (ExtractorAddressSize != 0)
-              DebugLineData.setAddressSize(ExtractorAddressSize);
+              TableData.setAddressSize(ExtractorAddressSize);
           }
 
           if (OS)
             *OS << format(" (0x%16.16" PRIx64 ")", State.Row.Address.Address);
         }
         break;
 
       case DW_LNE_define_file:
if (Opcode == 0) {
         // ignored for file names that represent full path names.
         //
         // The files are numbered, starting at 1, in the order in which they
         // appear; the names in the prologue come before names defined by
         // the DW_LNE_define_file instruction. These numbers are used in the
         // the file register of the state machine.
         {
           FileNameEntry FileEntry;
-          const char *Name = DebugLineData.getCStr(OffsetPtr);
+          const char *Name = TableData.getCStr(OffsetPtr);
           FileEntry.Name =
               DWARFFormValue::createFromPValue(dwarf::DW_FORM_string, Name);
-          FileEntry.DirIdx = DebugLineData.getULEB128(OffsetPtr);
-          FileEntry.ModTime = DebugLineData.getULEB128(OffsetPtr);
-          FileEntry.Length = DebugLineData.getULEB128(OffsetPtr);
+          FileEntry.DirIdx = TableData.getULEB128(OffsetPtr);
+          FileEntry.ModTime = TableData.getULEB128(OffsetPtr);
+          FileEntry.Length = TableData.getULEB128(OffsetPtr);
           Prologue.FileNames.push_back(FileEntry);
           if (OS)
             *OS << " (" << Name << ", dir=" << FileEntry.DirIdx << ", mod_time="
                 << format("(0x%16.16" PRIx64 ")", FileEntry.ModTime)
                 << ", length=" << FileEntry.Length << ")";
         }
         break;
 
       case DW_LNE_set_discriminator:
-        State.Row.Discriminator = DebugLineData.getULEB128(OffsetPtr);
+        State.Row.Discriminator = TableData.getULEB128(OffsetPtr);
         if (OS)
           *OS << " (" << State.Row.Discriminator << ")";
         break;
 
       default:
         if (OS)
           *OS << format("Unrecognized extended op 0x%02.02" PRIx8, SubOpcode)
               << format(" length %" PRIx64, Len);
if (Opcode == 0) {
         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 = State.advanceAddr(
-              DebugLineData.getULEB128(OffsetPtr), Opcode, OpcodeOffset);
+              TableData.getULEB128(OffsetPtr), Opcode, OpcodeOffset);
           if (OS)
             *OS << " (" << AddrOffset << ")";
         }
         break;
 
       case DW_LNS_advance_line:
         // Takes a single signed LEB128 operand and adds that value to
         // the line register of the state machine.
-        State.Row.Line += DebugLineData.getSLEB128(OffsetPtr);
+        State.Row.Line += TableData.getSLEB128(OffsetPtr);
         if (OS)
           *OS << " (" << State.Row.Line << ")";
         break;
 
       case DW_LNS_set_file:
         // Takes a single unsigned LEB128 operand and stores it in the file
         // register of the state machine.
-        State.Row.File = DebugLineData.getULEB128(OffsetPtr);
+        State.Row.File = TableData.getULEB128(OffsetPtr);
         if (OS)
           *OS << " (" << State.Row.File << ")";
         break;
 
       case DW_LNS_set_column:
         // Takes a single unsigned LEB128 operand and stores it in the
         // column register of the state machine.
-        State.Row.Column = DebugLineData.getULEB128(OffsetPtr);
+        State.Row.Column = TableData.getULEB128(OffsetPtr);
         if (OS)
           *OS << " (" << State.Row.Column << ")";
         break;
 
       case DW_LNS_negate_stmt:
         // Takes no arguments. Set the is_stmt register of the state
         // machine to the logical negation of its current value.
         State.Row.IsStmt = !State.Row.IsStmt;
if (Opcode == 0) {
         // is the only extended opcode that takes an argument that is not
         // a variable length number. The motivation for DW_LNS_fixed_advance_pc
         // is this: existing assemblers cannot emit DW_LNS_advance_pc or
         // special opcodes because they cannot encode LEB128 numbers or
         // judge when the computation of a special opcode overflows and
         // requires the use of DW_LNS_advance_pc. Such assemblers, however,
         // can use DW_LNS_fixed_advance_pc instead, sacrificing compression.
         {
-          uint16_t PCOffset = DebugLineData.getRelocatedValue(2, OffsetPtr);
+          uint16_t PCOffset = TableData.getRelocatedValue(2, OffsetPtr);
           State.Row.Address.Address += PCOffset;
           if (OS)
             *OS
                 << format(" (0x%4.4" PRIx16 ")", PCOffset);
         }
         break;
 
       case DW_LNS_set_prologue_end:
         // Takes no arguments. Set the prologue_end register of the
         // state machine to true
         State.Row.PrologueEnd = true;
         break;
 
       case DW_LNS_set_epilogue_begin:
         // Takes no arguments. Set the basic_block register of the
         // state machine to true
         State.Row.EpilogueBegin = true;
         break;
 
       case DW_LNS_set_isa:
         // Takes a single unsigned LEB128 operand and stores it in the
         // column register of the state machine.
-        State.Row.Isa = DebugLineData.getULEB128(OffsetPtr);
+        State.Row.Isa = TableData.getULEB128(OffsetPtr);
         if (OS)
           *OS << " (" << (uint64_t)State.Row.Isa << ")";
         break;
 
       default:
         // Handle any unknown standard opcodes here. We know the lengths
         // of such opcodes because they are specified in the prologue
         // as a multiple of LEB128 operands for each opcode.
         {
           assert(Opcode - 1U < Prologue.StandardOpcodeLengths.size());
           uint8_t OpcodeLength = Prologue.StandardOpcodeLengths[Opcode - 1];
           for (uint8_t I = 0; I < OpcodeLength; ++I) {
-            uint64_t Value = DebugLineData.getULEB128(OffsetPtr);
+            uint64_t Value = TableData.getULEB128(OffsetPtr);
             if (OS)
               *OS << format("Skipping ULEB128 value: 0x%16.16" PRIx64 ")\n",
                             Value);
           }
         }
         break;
       }
     } else {

llvm/test/tools/llvm-dwarfdump/X86/Inputs/debug_line_malformed.s

 .asciz  "foo.c"         # File table
 .byte   1, 2, 3
 .Lunterminated_files_prologue_end:
 .byte   0, 9, 2         # DW_LNE_set_address
 .quad   0xababcdcdefef0909
 .byte   0, 1, 1         # DW_LNE_end_sequence
 .Lunterminated_files_end:
 
+# Opcode extends past the end of the table, as claimed by the unit length field.
+.long   .Lextended_past_end_end - .Lextended_past_end_start # Length of Unit
+.Lextended_past_end_start:
+.short  4               # DWARF version number
+.long   .Lprologue_extended_past_end_end-.Lprologue_extended_past_end_start # Length of Prologue
+.Lprologue_extended_past_end_start:
+.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
+.asciz "dir1"           # Include table
+.asciz "dir2"
+.byte   0
+.asciz "file1"          # File table
+.byte   0, 0, 0
+.asciz "file2"
+.byte   1, 0, 0
+.byte   0
+.Lprologue_extended_past_end_end:
+.byte   0, 9, 2         # DW_LNE_set_address
+.quad   0xfeedfeed
+.byte   1               # DW_LNS_copy
+.byte   0, 9, 2         # DW_LNE_set_address
+.long   0xf001f000      # Truncated address (should be 8 bytes)
+.byte   0xf0, 0, 1
+.Lextended_past_end_end:
+
 # Trailing good section.
 .long   .Lunit_good_end - .Lunit_good_start # Length of Unit (DWARF-32 format)
 .Lunit_good_start:
 .short  4               # DWARF version number
 .long   .Lprologue_good_end-.Lprologue_good_start # Length of Prologue
 .Lprologue_good_start:
 .byte   1               # Minimum Instruction Length
 .byte   1               # Maximum Operations per Instruction

llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test

 # RUN: FileCheck %s --input-file=%t-malformed-verbose.err --check-prefixes=ALL,OTHER
 
 ## We should still produce warnings for malformed tables after the specified unit.
 # RUN: llvm-dwarfdump -debug-line=0 %t-malformed.o 2> %t-malformed-off-first.err \
 # RUN:   | FileCheck %s --check-prefixes=FIRST,NOLATER
 # RUN: FileCheck %s --input-file=%t-malformed-off-first.err --check-prefix=ALL
 
 ## Don't stop looking for the later unit if non-fatal issues are found.
-# RUN: llvm-dwarfdump -debug-line=0x3c9 %t-malformed.o 2> %t-malformed-off-last.err \
+# RUN: llvm-dwarfdump -debug-line=0x419 %t-malformed.o 2> %t-malformed-off-last.err \
 # RUN:   | FileCheck %s --check-prefix=LAST --implicit-check-not='debug_line[{{.*}}]'
 # RUN: FileCheck %s --input-file=%t-malformed-off-last.err --check-prefix=ALL
 
 # FIRST:        debug_line[0x00000000]
 # VERBOSE:      DW_LNE_set_address (0x000000000badbeef)
 # VERBOSE-NEXT: DW_LNE_end_sequence
 # FIRST:        0x000000000badbeef {{.*}} end_sequence
 # NOFIRST-NOT:  debug_line[0x00000000]

 # NONFATAL-NEXT:            name: "foo.c"
 # NONFATAL-NEXT:       dir_index: 1
 # NONFATAL-NEXT:        mod_time: 0x00000002
 # NONFATAL-NEXT:          length: 0x00000003
 # NONFATAL-NOT:  file_names
 # VERBOSE:       DW_LNE_set_address (0xababcdcdefef0909)
 # VERBOSE-NEXT:  DW_LNE_end_sequence
 
-# LAST:          debug_line[0x000003c9]
+## Table with extended opcode that overruns table end.
+# NONFATAL:      debug_line[0x000003c9]
+# NONFATAL-NEXT: Line table prologue
+# VERBOSE:       DW_LNE_set_address (0x00000000feedfeed)
+# VERBOSE-NEXT:  DW_LNS_copy
+# VERBOSE:       DW_LNE_set_address (0x0000000000000000)
+
+# LAST:          debug_line[0x00000419]
 # VERBOSE:       DW_LNE_set_address (0x00000000cafebabe)
 # VERBOSE-NEXT:  DW_LNE_end_sequence
 
 # ALL-NOT:  warning:
 # ALL:      warning: parsing line table prologue at offset 0x00000048: unsupported version 0
 # ALL-NEXT: warning: parsing line table prologue at offset 0x0000004e: unsupported version 1
 # ALL-NEXT: warning: parsing line table prologue at 0x00000054 found an invalid directory or file table description at 0x00000073
 # ALL-NEXT: warning: failed to parse entry content descriptions because no path was found

 # ALL-NEXT: warning: failed to parse file entry because the MD5 hash is invalid
 # ALL-NEXT: warning: parsing line table prologue at 0x000002ec found an invalid directory or file table description at 0x00000315
 # ALL-NEXT: warning: failed to parse directory entry because skipping the form value failed.
 # ALL-NEXT: warning: parsing line table prologue at offset 0x00000332 found opcode base of 0. Assuming no standard opcodes
 # ALL-NEXT: warning: parsing line table prologue at 0x00000361 found an invalid directory or file table description at 0x00000383
 # ALL-NEXT: warning: include directories table was not null terminated before the end of the prologue
 # ALL-NEXT: warning: parsing line table prologue at 0x00000390 found an invalid directory or file table description at 0x000003bf
 # ALL-NEXT: warning: file names table was not null terminated before the end of the prologue
+# OTHER-NEXT: warning: unexpected line op length at offset 0x00000411 expected 0x09 found 0x01
+# OTHER-NEXT: warning: last sequence in debug line table at offset 0x000003c9 is not terminated
 # ALL-NOT:  warning: