[DebugInfo] Check for errors when reading data for extended opcode

Previously, if an extended opcode was truncated, it would manifest as an
"unexpected line op length error" which wasn't quite accurate. This
change checks for errors any time data is read whilst parsing an
extended opcode, and reports any errors detected.

Reviewed by: MaskRay, labath, aprantl

Differential Revision: https://reviews.llvm.org/D80797

diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
index 7fb950a..1f30ef4 100644 (file)
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp
@@ -756,17 +756,21 @@ Error DWARFDebugLine::LineTable::parse(
     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 = TableData.getULEB128(OffsetPtr);
-      uint64_t ExtOffset = *OffsetPtr;
+      DataExtractor::Cursor Cursor(*OffsetPtr);
+      uint64_t Len = TableData.getULEB128(Cursor);
+      uint64_t ExtOffset = Cursor.tell();
 
       // Tolerate zero-length; assume length is correct and soldier on.
       if (Len == 0) {
         if (OS)
           *OS << "Badly formed extended line op (length 0)\n";
+        if (!Cursor)
+          RecoverableErrorHandler(Cursor.takeError());
+        *OffsetPtr = Cursor.tell();
         continue;
       }
 
-      uint8_t SubOpcode = TableData.getU8(OffsetPtr);
+      uint8_t SubOpcode = TableData.getU8(Cursor);
       if (OS)
         *OS << LNExtendedString(SubOpcode);
       switch (SubOpcode) {
@@ -820,11 +824,11 @@ Error DWARFDebugLine::LineTable::parse(
                 " of DW_LNE_set_address opcode at offset 0x%8.8" PRIx64
                 " is unsupported",
                 OpcodeAddressSize, ExtOffset));
-            *OffsetPtr += OpcodeAddressSize;
+            TableData.skip(Cursor, OpcodeAddressSize);
           } else {
             TableData.setAddressSize(OpcodeAddressSize);
             State.Row.Address.Address = TableData.getRelocatedAddress(
-                OffsetPtr, &State.Row.Address.SectionIndex);
+                Cursor, &State.Row.Address.SectionIndex);
 
             // Restore the address size if the extractor already had it.
             if (ExtractorAddressSize != 0)
@@ -859,12 +863,12 @@ Error DWARFDebugLine::LineTable::parse(
         // the file register of the state machine.
         {
           FileNameEntry FileEntry;
-          const char *Name = TableData.getCStr(OffsetPtr);
+          const char *Name = TableData.getCStr(Cursor);
           FileEntry.Name =
               DWARFFormValue::createFromPValue(dwarf::DW_FORM_string, Name);
-          FileEntry.DirIdx = TableData.getULEB128(OffsetPtr);
-          FileEntry.ModTime = TableData.getULEB128(OffsetPtr);
-          FileEntry.Length = TableData.getULEB128(OffsetPtr);
+          FileEntry.DirIdx = TableData.getULEB128(Cursor);
+          FileEntry.ModTime = TableData.getULEB128(Cursor);
+          FileEntry.Length = TableData.getULEB128(Cursor);
           Prologue.FileNames.push_back(FileEntry);
           if (OS)
             *OS << " (" << Name << ", dir=" << FileEntry.DirIdx << ", mod_time="
@@ -874,7 +878,7 @@ Error DWARFDebugLine::LineTable::parse(
         break;
 
       case DW_LNE_set_discriminator:
-        State.Row.Discriminator = TableData.getULEB128(OffsetPtr);
+        State.Row.Discriminator = TableData.getULEB128(Cursor);
         if (OS)
           *OS << " (" << State.Row.Discriminator << ")";
         break;
@@ -885,21 +889,23 @@ Error DWARFDebugLine::LineTable::parse(
               << 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;
+        TableData.skip(Cursor, Len - 1);
         break;
       }
       // Make sure the length as recorded in the table and the standard length
       // for the opcode match. If they don't, continue from the end as claimed
-      // by the table.
+      // by the table. Similarly, continue from the claimed end in the event of
+      // a parsing error.
       uint64_t End = ExtOffset + Len;
-      if (*OffsetPtr != End) {
+      if (!Cursor)
+        RecoverableErrorHandler(Cursor.takeError());
+      else if (Cursor.tell() != End)
         RecoverableErrorHandler(createStringError(
             errc::illegal_byte_sequence,
             "unexpected line op length at offset 0x%8.8" PRIx64
             " expected 0x%2.2" PRIx64 " found 0x%2.2" PRIx64,
-            ExtOffset, Len, *OffsetPtr - ExtOffset));
-        *OffsetPtr = End;
-      }
+            ExtOffset, Len, Cursor.tell() - ExtOffset));
+      *OffsetPtr = End;
     } else if (Opcode < Prologue.OpcodeBase) {
       if (OS)
         *OS << LNStandardString(Opcode);

diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test b/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
index 8017b93..e81e6a1 100644 (file)
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug_line_invalid.test
@@ -284,6 +284,6 @@
 # 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:  unexpected end of data at offset 0x419 while reading [0x412, 0x41a)
 # OTHER-NEXT: warning: last sequence in debug line table at offset 0x000003c9 is not terminated
 # ALL-NOT:  warning:

diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
index 7ceb656..3dad185 100644 (file)
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp
@@ -1299,6 +1299,117 @@ TEST_F(DebugLineBasicFixture, ParserPrintsStandardOpcodesWhenRequested) {
   EXPECT_TRUE(InOutput("0x0000003f: 0c DW_LNS_set_isa (66)\n")) << Output;
 }
 
+using ValueAndLengths = std::vector<LineTable::ValueAndLength>;
+
+struct TruncatedExtendedOpcodeFixture
+    : public TestWithParam<std::tuple<uint64_t, uint64_t, uint8_t,
+                                      ValueAndLengths, StringRef, StringRef>>,
+      public CommonFixture {
+  void SetUp() {
+    std::tie(BodyLength, OpcodeLength, Opcode, Operands, ExpectedOutput,
+             ExpectedErr) = GetParam();
+  }
+
+  void runTest() {
+    LineTable &LT = Gen->addLineTable();
+    // Creating the prologue before adding the opcode ensures that the unit
+    // length does not include the table body.
+    DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue();
+    Prologue.TotalLength += BodyLength;
+    LT.setPrologue(Prologue);
+    LT.addExtendedOpcode(OpcodeLength, Opcode, Operands);
+    generate();
+
+    DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
+    std::string Output;
+    raw_string_ostream OS(Output);
+    Parser.parseNext(RecordRecoverable, RecordUnrecoverable, &OS);
+    OS.flush();
+
+    StringRef LinePrefix = "0x0000002e: 00 ";
+    StringRef OutputRef(Output);
+    StringRef OutputToCheck = OutputRef.split(LinePrefix).second;
+    EXPECT_EQ((ExpectedOutput + "\n").str(), OutputToCheck);
+    EXPECT_THAT_ERROR(std::move(Recoverable),
+                      FailedWithMessage(ExpectedErr.str()));
+  }
+
+  uint64_t BodyLength;
+  uint64_t OpcodeLength;
+  uint8_t Opcode;
+  ValueAndLengths Operands;
+  StringRef ExpectedOutput;
+  StringRef ExpectedErr;
+};
+
+TEST_P(TruncatedExtendedOpcodeFixture, ErrorForTruncatedExtendedOpcode) {
+  if (!setupGenerator())
+    return;
+  runTest();
+}
+
+INSTANTIATE_TEST_CASE_P(
+    TruncatedExtendedOpcodeParams, TruncatedExtendedOpcodeFixture,
+    Values(
+        std::make_tuple(1, 1, DW_LNE_end_sequence, ValueAndLengths(),
+                        "Badly formed extended line op (length 0)",
+                        "unable to decode LEB128 at offset 0x0000002f: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(
+            2, 9, DW_LNE_set_address,
+            ValueAndLengths{{0x12345678, LineTable::Quad}},
+            "Unrecognized extended op 0x00 length 9",
+            "unexpected end of data at offset 0x30 while reading [0x30, 0x31)"),
+        std::make_tuple(
+            3, 9, DW_LNE_set_address,
+            ValueAndLengths{{0x12345678, LineTable::Quad}},
+            "DW_LNE_set_address (0x0000000000000000)",
+            "unexpected end of data at offset 0x31 while reading [0x31, 0x39)"),
+        std::make_tuple(3, 5, DW_LNE_define_file,
+                        ValueAndLengths{{'a', LineTable::Byte},
+                                        {'\0', LineTable::Byte},
+                                        {1, LineTable::ULEB},
+                                        {1, LineTable::ULEB},
+                                        {1, LineTable::ULEB}},
+                        "DW_LNE_define_file (, dir=0, "
+                        "mod_time=(0x0000000000000000), length=0)",
+                        "no null terminated string at offset 0x31"),
+        std::make_tuple(5, 5, DW_LNE_define_file,
+                        ValueAndLengths{{'a', LineTable::Byte},
+                                        {'\0', LineTable::Byte},
+                                        {1, LineTable::ULEB},
+                                        {1, LineTable::ULEB},
+                                        {1, LineTable::ULEB}},
+                        "DW_LNE_define_file (a, dir=0, "
+                        "mod_time=(0x0000000000000000), length=0)",
+                        "unable to decode LEB128 at offset 0x00000033: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(6, 5, DW_LNE_define_file,
+                        ValueAndLengths{{'a', LineTable::Byte},
+                                        {'\0', LineTable::Byte},
+                                        {1, LineTable::ULEB},
+                                        {1, LineTable::ULEB},
+                                        {1, LineTable::ULEB}},
+                        "DW_LNE_define_file (a, dir=1, "
+                        "mod_time=(0x0000000000000000), length=0)",
+                        "unable to decode LEB128 at offset 0x00000034: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(7, 5, DW_LNE_define_file,
+                        ValueAndLengths{{'a', LineTable::Byte},
+                                        {'\0', LineTable::Byte},
+                                        {1, LineTable::ULEB},
+                                        {1, LineTable::ULEB},
+                                        {1, LineTable::ULEB}},
+                        "DW_LNE_define_file (a, dir=1, "
+                        "mod_time=(0x0000000000000001), length=0)",
+                        "unable to decode LEB128 at offset 0x00000035: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(3, 2, DW_LNE_set_discriminator,
+                        ValueAndLengths{{1, LineTable::ULEB}},
+                        "DW_LNE_set_discriminator (0)",
+                        "unable to decode LEB128 at offset 0x00000031: "
+                        "malformed uleb128, extends past end")), );
+
 TEST_F(DebugLineBasicFixture, PrintPathsProperly) {
   if (!setupGenerator(5))
     return;