[DebugInfo] Use Cursor to detect errors in debug line prologue parser

[jhenderson]

Summary

Previously, the debug line parser would keep attempting to read data even if it had run out of data to read. This meant errors in parsing would often end up being reported as something else, such as an unknown version or malformed directory/filename table. This patch fixes the issues by using the Cursor API to capture errors.

[labath]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

404–407

Maybe DebugLineData.getU8(Cursor, StandardOpcodeLengths.begin(), OpcodeBase-1)

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

404–407

That didn't quite work. I think the signature requires either a SmallVector or a pre-sized vector and .data(). I'll make some changes for this in a separate change.

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

404–407

I looked at this again, and it is trivial to change the function signature of the getU8 function to allow any vector type, via templating it, but having done that, I realised there was a behaviour change, which I don't believe to be beneficial, so I haven't adopted this suggestion.

Without using this function, those operand lengths that could be read were printed. With the function, if the whole array can't be read, none of it is. I think the old behaviour is more useful, since it allows users to read a slightly broken prologue to see what could be salvaged from it.

[labath]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

404–407

Sounds good.

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

 
 Error DWARFDebugLine::Prologue::parse(
     DWARFDataExtractor DebugLineData, uint64_t *OffsetPtr,
     function_ref<void(Error)> RecoverableErrorHandler, const DWARFContext &Ctx,
     const DWARFUnit *U) {
   const uint64_t PrologueOffset = *OffsetPtr;
 
   clear();
-  Error Err = Error::success();
+  DataExtractor::Cursor Cursor(*OffsetPtr);
   std::tie(TotalLength, FormParams.Format) =
-      DebugLineData.getInitialLength(OffsetPtr, &Err);
-  if (Err)
-    return createStringError(
-        errc::invalid_argument,
-        "parsing line table prologue at offset 0x%8.8" PRIx64 ": %s",
-        PrologueOffset, toString(std::move(Err)).c_str());
+      DebugLineData.getInitialLength(Cursor);
 
-  DebugLineData = DWARFDataExtractor(DebugLineData, *OffsetPtr + TotalLength);
-  FormParams.Version = DebugLineData.getU16(OffsetPtr);
-  if (!versionIsSupported(getVersion()))
+  DebugLineData =
+      DWARFDataExtractor(DebugLineData, Cursor.tell() + TotalLength);
+  FormParams.Version = DebugLineData.getU16(Cursor);
+  if (Cursor && !versionIsSupported(getVersion())) {
     // Treat this error as unrecoverable - we cannot be sure what any of
     // the data represents including the length field, so cannot skip it or make
     // any reasonable assumptions.
+    *OffsetPtr = Cursor.tell();
     return createStringError(
         errc::not_supported,
         "parsing line table prologue at offset 0x%8.8" PRIx64
         ": unsupported version %" PRIu16,
         PrologueOffset, getVersion());
+  }
 
   if (getVersion() >= 5) {
-    FormParams.AddrSize = DebugLineData.getU8(OffsetPtr);
-    assert((DebugLineData.getAddressSize() == 0 ||
+    FormParams.AddrSize = DebugLineData.getU8(Cursor);
+    assert((!Cursor || DebugLineData.getAddressSize() == 0 ||
             DebugLineData.getAddressSize() == getAddressSize()) &&
            "Line table header and data extractor disagree");
-    SegSelectorSize = DebugLineData.getU8(OffsetPtr);
+    SegSelectorSize = DebugLineData.getU8(Cursor);
   }
 
   PrologueLength =
-      DebugLineData.getRelocatedValue(sizeofPrologueLength(), OffsetPtr);
-  const uint64_t EndPrologueOffset = PrologueLength + *OffsetPtr;
+      DebugLineData.getRelocatedValue(Cursor, sizeofPrologueLength());
+  const uint64_t EndPrologueOffset = PrologueLength + Cursor.tell();
   DebugLineData = DWARFDataExtractor(DebugLineData, EndPrologueOffset);
-  MinInstLength = DebugLineData.getU8(OffsetPtr);
+  MinInstLength = DebugLineData.getU8(Cursor);
   if (getVersion() >= 4)
-    MaxOpsPerInst = DebugLineData.getU8(OffsetPtr);
-  DefaultIsStmt = DebugLineData.getU8(OffsetPtr);
-  LineBase = DebugLineData.getU8(OffsetPtr);
-  LineRange = DebugLineData.getU8(OffsetPtr);
-  OpcodeBase = DebugLineData.getU8(OffsetPtr);
+    MaxOpsPerInst = DebugLineData.getU8(Cursor);
+  DefaultIsStmt = DebugLineData.getU8(Cursor);
+  LineBase = DebugLineData.getU8(Cursor);
+  LineRange = DebugLineData.getU8(Cursor);
+  OpcodeBase = DebugLineData.getU8(Cursor);
 
-  if (OpcodeBase == 0) {
+  if (Cursor && OpcodeBase == 0) {
     // If the opcode base is 0, we cannot read the standard opcode lengths (of
     // which there are supposed to be one fewer than the opcode base). Assume
     // there are no standard opcodes and continue parsing.
     RecoverableErrorHandler(createStringError(
         errc::invalid_argument,
         "parsing line table prologue at offset 0x%8.8" PRIx64
         " found opcode base of 0. Assuming no standard opcodes",
         PrologueOffset));
-  } else {
+  } else if (Cursor) {
     StandardOpcodeLengths.reserve(OpcodeBase - 1);
     for (uint32_t I = 1; I < OpcodeBase; ++I) {
-      uint8_t OpLen = DebugLineData.getU8(OffsetPtr);
+      uint8_t OpLen = DebugLineData.getU8(Cursor);
       StandardOpcodeLengths.push_back(OpLen);
     }
   }
 
+  *OffsetPtr = Cursor.tell();
+  // A corrupt file name or directory table does not prevent interpretation of
+  // the main line program, so check the cursor state now so that its errors can
+  // be handled separately.
+  if (!Cursor)
+    return createStringError(
+        errc::invalid_argument,
+        "parsing line table prologue at offset 0x%8.8" PRIx64 ": %s",
+        PrologueOffset, toString(Cursor.takeError()).c_str());
+
   Error E =
       getVersion() >= 5
           ? parseV5DirFileTables(DebugLineData, OffsetPtr, FormParams, Ctx, U,
                                  ContentTypes, IncludeDirectories, FileNames)
           : parseV2DirFileTables(DebugLineData, OffsetPtr, ContentTypes,
                                  IncludeDirectories, FileNames);
   if (E) {
     RecoverableErrorHandler(joinErrors(

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

 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      prologue_length: 0x00000030
 # VERBOSE:       DW_LNE_set_address (0x00000000deadfade)
 # VERBOSE-NEXT:  DW_LNS_copy
 # MORE-ERR:      warning: last sequence in debug line table at offset 0x0000016b is not terminated
 
 ## Very short prologue length for V5 (ends during parameters).
 # NONFATAL:      debug_line[0x000001b1]
-# SOME-ERR-NEXT: warning: parsing line table prologue at 0x000001b1 found an invalid directory or file table description at 0x000001cd
-# SOME-ERR-NEXT: warning: failed to parse entry content descriptors: unexpected end of data at offset 0x1cd while reading [0x1cd, 0x1ce)
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      standard_opcode_lengths[DW_LNS_set_prologue_end] = 1
 # NONFATAL-NEXT: standard_opcode_lengths[DW_LNS_set_epilogue_begin] = 0
-# NONFATAL-NOT:  include_directories
-# VERBOSE:       DW_LNE_set_address (0x0000babb1ebabb1e)
-# VERBOSE-NEXT:  DW_LNE_end_sequence
+# NONFATAL-NEXT: standard_opcode_lengths[DW_LNS_set_isa] = 0
+# NONFATAL-EMPTY:
+# SOME-ERR-NEXT: warning: parsing line table prologue at offset 0x000001b1: unexpected end of data at offset 0x1cd while reading [0x1cd, 0x1ce)
 
 ## V5 prologue ends during file table.
-# NONFATAL:      debug_line[0x000001db]
+# NONFATAL-NEXT: debug_line[0x000001db]
 # SOME-ERR-NEXT: warning: parsing line table prologue at 0x000001db found an invalid directory or file table description at 0x00000206
 # SOME-ERR-NEXT: warning: failed to parse entry content descriptors: unable to decode LEB128 at offset 0x00000206: malformed uleb128, extends past end
 # NONFATAL-NEXT: Line table prologue
 # NONFATAL:      include_directories[  0] = "/tmp"
 # NONFATAL-NOT:  file_names
 # VERBOSE:       DW_LNE_set_address (0x00000ab4acadab4a)
 # VERBOSE-NEXT:  DW_LNE_end_sequence
 

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

EXPECT_EQ(NextLine(),
             "  0  basic_block prologue_end epilogue_begin");
   EXPECT_EQ(NextLine(), "0x00000079: 00 DW_LNE_end_sequence");
   EXPECT_EQ(NextLine(), "            0x0123456789abce53     20     44     33  "
                         "66             0  end_sequence");
   EXPECT_EQ(NextLine(), "");
   EXPECT_EQ(Output.size(), Pos);
 }
 
+struct TruncatedPrologueFixture
+    : public TestWithParam<
+          std::tuple<uint64_t, uint64_t, uint16_t, DwarfFormat, StringRef>>,
+      public CommonFixture {
+  void SetUp() {
+    std::tie(Length, ExpectedOffset, Version, Format, ExpectedErr) = GetParam();
+  }
+
+  uint64_t Length;
+  uint64_t ExpectedOffset;
+  uint16_t Version;
+  DwarfFormat Format;
+  StringRef ExpectedErr;
+};
+
+TEST_P(TruncatedPrologueFixture, ErrorForTruncatedPrologue) {
+  if (!setupGenerator(Version))
+    return;
+
+  LineTable &Padding = Gen->addLineTable();
+  // Add some padding to show that a non-zero offset is handled correctly.
+  Padding.setCustomPrologue({{0, LineTable::Byte}});
+
+  // Add a table with only two standard opcodes - we don't need to test the full
+  // set.
+  LineTable &Table = Gen->addLineTable(Format);
+  DWARFDebugLine::Prologue InputPrologue = Table.createBasicPrologue();
+  InputPrologue.OpcodeBase = 3;
+  InputPrologue.StandardOpcodeLengths.resize(2);
+  Table.setPrologue(InputPrologue);
+
+  generate();
+  // Truncate the data extractor to the specified length.
+  LineData = DWARFDataExtractor(LineData, Length);
+
+  DWARFDebugLine::Prologue Prologue;
+  uint64_t Offset = 1;
+  Error Err = Prologue.parse(LineData, &Offset, RecordRecoverable, *Context);
+
+  EXPECT_THAT_ERROR(std::move(Err), FailedWithMessage(ExpectedErr.str()));
+  EXPECT_EQ(Offset, ExpectedOffset);
+}
+
+INSTANTIATE_TEST_CASE_P(
+    TruncatedPrologueParams, TruncatedPrologueFixture,
+    Values(
+        // Truncated length:
+        std::make_tuple(
+            4, 1, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x4 while reading [0x1, 0x5)"),
+        std::make_tuple(
+            4, 1, 4, DWARF64,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x4 while reading [0x1, 0x5)"),
+        std::make_tuple(
+            0xc, 1, 4, DWARF64,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0xc while reading [0x5, 0xd)"),
+        // Truncated version:
+        std::make_tuple(
+            6, 5, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x6 while reading [0x5, 0x7)"),
+        // Truncated address size:
+        std::make_tuple(
+            7, 7, 5, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x7 while reading [0x7, 0x8)"),
+        // Truncated segment selector size:
+        std::make_tuple(
+            8, 8, 5, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x8 while reading [0x8, 0x9)"),
+        // Truncated prologue length:
+        std::make_tuple(
+            0xa, 7, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0xa while reading [0x7, 0xb)"),
+        std::make_tuple(
+            0x16, 0xf, 4, DWARF64,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x16 while reading [0xf, 0x17)"),
+        // Truncated min instruction length:
+        std::make_tuple(
+            0xb, 0xb, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0xb while reading [0xb, 0xc)"),
+        // Truncated max ops per inst:
+        std::make_tuple(
+            0xc, 0xc, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0xc while reading [0xc, 0xd)"),
+        // Truncated default is stmt:
+        std::make_tuple(
+            0xd, 0xd, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0xd while reading [0xd, 0xe)"),
+        // Truncated line base:
+        std::make_tuple(
+            0xe, 0xe, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0xe while reading [0xe, 0xf)"),
+        // Truncated line range:
+        std::make_tuple(
+            0xf, 0xf, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0xf while reading [0xf, 0x10)"),
+        // Truncated opcode base:
+        std::make_tuple(
+            0x10, 0x10, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x10 while reading [0x10, 0x11)"),
+        // Truncated first standard opcode:
+        std::make_tuple(
+            0x11, 0x11, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x11 while reading [0x11, 0x12)"),
+        // Truncated second standard opcode:
+        std::make_tuple(
+            0x12, 0x12, 4, DWARF32,
+            "parsing line table prologue at offset 0x00000001: unexpected end "
+            "of data at offset 0x12 while reading [0x12, 0x13)")), );
+
 using ValueAndLengths = std::vector<LineTable::ValueAndLength>;
 
 struct TruncatedOpcodeFixtureBase : public CommonFixture {
   LineTable &setupTable() {
     LineTable &LT = Gen->addLineTable();
 
     // Creating the prologue before adding any opcodes ensures that the unit
     // length does not include the table body.