[DebugInfo] Report errors for truncated debug line standard opcode

[jhenderson]

Summary

Standard opcodes usually have ULEB128 arguments, so it is generally not possible to recover from such errors. This patch causes the parser to stop parsing the table in such situations.

Also don't emit the operands or add data to the table if there is an error reading these opcodes.

[JDevlieghere]

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

940–946

A helper function might simplify this code a little bit:

template <typename T>
Optional<T> parseULEB128(DWARFDataExtractor &TableData,
                         DataExtractor::Cursor &Cursor) {
  T Value = TableData.getULEB128(Cursor);
  if (Cursor)
    return Value;
  return {};
}

if (auto Operand = parseULEB128<uint64_t>(TableData, Cursor)) {
  uint64_t AddrOffset =
      State.advanceAddr(*Operand, Opcode, OpcodeOffset);
  if (Verbose)
    *OS << " (" << AddrOffset << ")";
}

I was considering making the helper more generic so it could do both ULEB and SLEB but I don't think that's worth the extra complexity.

[jhenderson]

Address @JDevlieghere's suggestion.

[JDevlieghere]

In D81470#2084604, @jhenderson wrote:

Address @JDevlieghere's suggestion.

I think you might have re-uploaded the old patch?

[jhenderson]

Upload correct diff. Sorry @JDevlieghere!

[JDevlieghere]

In D81470#2087073, @jhenderson wrote:

Upload correct diff. Sorry @JDevlieghere!

No worries! This LGTM.

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

939–940

The lexical block is no longer necessary with the helper.

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

if (Opcode == 0) {
       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, Cursor.tell() - ExtOffset));
       *OffsetPtr = End;
     } else if (Opcode < Prologue.OpcodeBase) {
+      DataExtractor::Cursor Cursor(*OffsetPtr);
       if (Verbose)
         *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.
         if (Verbose) {
           *OS << "\n";
           OS->indent(12);
         }
         if (OS)
           State.Row.dump(*OS);
         State.appendRowToMatrix();
         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(
-              TableData.getULEB128(OffsetPtr), Opcode, OpcodeOffset);
+          uint64_t Operand = TableData.getULEB128(Cursor);
+          if (Cursor) {
+            uint64_t AddrOffset =
+                State.advanceAddr(Operand, Opcode, OpcodeOffset);
             if (Verbose)
               *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 += TableData.getSLEB128(OffsetPtr);
+        {
+          int64_t LineDelta = TableData.getSLEB128(Cursor);
+          if (Cursor) {
+            State.Row.Line += LineDelta;
             if (Verbose)
               *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 = TableData.getULEB128(OffsetPtr);
+        {
+          uint16_t File = TableData.getULEB128(Cursor);
+          if (Cursor) {
+            State.Row.File = File;
             if (Verbose)
               *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 = TableData.getULEB128(OffsetPtr);
+        {
+          uint16_t Column = TableData.getULEB128(Cursor);
+          if (Cursor) {
+            State.Row.Column = Column;
             if (Verbose)
               *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;
         break;
 
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 = TableData.getRelocatedValue(2, OffsetPtr);
+          uint16_t PCOffset =
+              TableData.getRelocatedValue(Cursor, 2);
+          if (Cursor) {
             State.Row.Address.Address += PCOffset;
             if (Verbose)
               *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 = TableData.getULEB128(OffsetPtr);
+        // ISA register of the state machine.
+        {
+          uint8_t Isa = TableData.getULEB128(Cursor);
+          if (Cursor) {
+            State.Row.Isa = Isa;
             if (Verbose)
               *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());
           if (Verbose)
             *OS << "Unrecognized standard opcode";
           uint8_t OpcodeLength = Prologue.StandardOpcodeLengths[Opcode - 1];
-          if (OpcodeLength != 0) {
-            if (Verbose)
-              *OS << format(" (operands: ");
-            for (uint8_t I = 0; I < OpcodeLength; ++I) {
-              uint64_t Value = TableData.getULEB128(OffsetPtr);
-              if (Verbose) {
-                if (I > 0)
-                  *OS << ", ";
-                *OS << format("0x%16.16" PRIx64, Value);
-              }
-            }
-            if (Verbose)
-              *OS << format(")");
+          std::vector<uint64_t> Operands;
+            uint64_t Value = TableData.getULEB128(Cursor);
+            if (!Cursor)
+              break;
+            Operands.push_back(Value);
+          }
+          if (Verbose && !Operands.empty()) {
+            *OS << format(" (operands: ");
+            bool First = true;
+            for (uint64_t Value : Operands) {
+              if (!First)
+              First = false;
           }
         }
         break;
       }
+
+      *OffsetPtr = Cursor.tell();
+
+      // Most standard opcode failures are due to failures to read ULEBs. Bail
+      // out of parsing, since we don't know where to continue reading from as
+      // there is no stated length for such byte sequences.
+      if (!Cursor) {
+        if (Verbose)
+          *OS << "\n\n";
+        return Cursor.takeError();
+      }
     } else {
       // Special Opcodes.
       ParsingState::AddrAndLineDelta Delta =
           State.handleSpecialOpcode(Opcode, OpcodeOffset);
 
       if (Verbose) {
         *OS << "address += " << Delta.Address << ",  line += " << Delta.Line
             << "\n";

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

TEST_F(DebugLineBasicFixture, VerboseOutput) {
   EXPECT_EQ(NextLine(), "            0x0123456789abce53     20     44     33  "
                         "66             0  end_sequence");
   EXPECT_EQ(NextLine(), "");
   EXPECT_EQ(Output.size(), Pos);
 }
 
 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() {
+struct TruncatedOpcodeFixtureBase : public CommonFixture {
+  LineTable &setupTable() {
     LineTable &LT = Gen->addLineTable();
-    // Creating the prologue before adding the opcode ensures that the unit
+
+    // Creating the prologue before adding any opcodes ensures that the unit
     // length does not include the table body.
     DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue();
-    Prologue.TotalLength += BodyLength;
+
+    // Add an unrecognised standard opcode, and adjust prologue properties
+    // accordingly.
+    Prologue.TotalLength += BodyLength + 1;
+    ++Prologue.PrologueLength;
+    ++Prologue.OpcodeBase;
+    Prologue.StandardOpcodeLengths.push_back(2);
     LT.setPrologue(Prologue);
-    LT.addExtendedOpcode(OpcodeLength, Opcode, Operands);
-    generate();
 
+    return LT;
+  }
+
+  void runTest(uint8_t OpcodeValue) {
+    generate();
     DWARFDebugLine::SectionParser Parser(LineData, *Context, CUs, TUs);
     std::string Output;
     raw_string_ostream OS(Output);
     Parser.parseNext(RecordRecoverable, RecordUnrecoverable, &OS,
                      /*Verbose=*/true);
     OS.flush();
 
-    StringRef LinePrefix = "0x0000002e: 00 ";
+    std::string LinePrefix =
+        ("0x0000002f: 0" + Twine::utohexstr(OpcodeValue) + " ").str();
     StringRef OutputRef(Output);
     StringRef OutputToCheck = OutputRef.split(LinePrefix).second;
     // Each extended opcode ends with a new line and then the table ends with an
     // additional blank line.
     EXPECT_EQ((ExpectedOutput + "\n\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;
 };
 
+struct TruncatedStandardOpcodeFixture
+    : public TestWithParam<
+          std::tuple<uint64_t, uint8_t, ValueAndLengths, StringRef, StringRef>>,
+      public TruncatedOpcodeFixtureBase {
+  void SetUp() {
+    std::tie(BodyLength, Opcode, Operands, ExpectedOutput, ExpectedErr) =
+        GetParam();
+  }
+};
+
+struct TruncatedExtendedOpcodeFixture
+    : public TestWithParam<std::tuple<uint64_t, uint64_t, uint8_t,
+                                      ValueAndLengths, StringRef, StringRef>>,
+      public TruncatedOpcodeFixtureBase {
+  void SetUp() {
+    std::tie(BodyLength, OpcodeLength, Opcode, Operands, ExpectedOutput,
+             ExpectedErr) = GetParam();
+  }
+
+  uint64_t OpcodeLength;
+};
+
 TEST_P(TruncatedExtendedOpcodeFixture, ErrorForTruncatedExtendedOpcode) {
   if (!setupGenerator())
     return;
-  runTest();
+  LineTable &LT = setupTable();
+  LT.addExtendedOpcode(OpcodeLength, Opcode, Operands);
+  runTest(0);
+  EXPECT_THAT_ERROR(std::move(Recoverable),
+                    FailedWithMessage(ExpectedErr.str()));
 }
 
 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: "
+                        "unable to decode LEB128 at offset 0x00000030: "
                         "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)"),
+            "unexpected end of data at offset 0x31 while reading [0x31, 0x32)"),
         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)"),
+            "unexpected end of data at offset 0x32 while reading [0x32, 0x3a)"),
         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"),
+                        "no null terminated string at offset 0x32"),
         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: "
+                        "unable to decode LEB128 at offset 0x00000034: "
                         "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: "
+                        "unable to decode LEB128 at offset 0x00000035: "
                         "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: "
+                        "unable to decode LEB128 at offset 0x00000036: "
                         "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")), );
+                        "unable to decode LEB128 at offset 0x00000032: "
+                        "malformed uleb128, extends past end")), );
+
+TEST_P(TruncatedStandardOpcodeFixture, ErrorForTruncatedStandardOpcode) {
+  if (!setupGenerator())
+    return;
+  LineTable &LT = setupTable();
+  LT.addStandardOpcode(Opcode, Operands);
+  runTest(Opcode);
+  EXPECT_THAT_ERROR(std::move(Unrecoverable),
+                    FailedWithMessage(ExpectedErr.str()));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    TruncatedStandardOpcodeParams, TruncatedStandardOpcodeFixture,
+    Values(
+        std::make_tuple(2, DW_LNS_advance_pc,
+                        ValueAndLengths{{0x100, LineTable::ULEB}},
+                        "DW_LNS_advance_pc",
+                        "unable to decode LEB128 at offset 0x00000030: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(2, DW_LNS_advance_line,
+                        ValueAndLengths{{0x200, LineTable::SLEB}},
+                        "DW_LNS_advance_line",
+                        "unable to decode LEB128 at offset 0x00000030: "
+                        "malformed sleb128, extends past end"),
+        std::make_tuple(2, DW_LNS_set_file,
+                        ValueAndLengths{{0x300, LineTable::ULEB}},
+                        "DW_LNS_set_file",
+                        "unable to decode LEB128 at offset 0x00000030: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(2, DW_LNS_set_column,
+                        ValueAndLengths{{0x400, LineTable::ULEB}},
+                        "DW_LNS_set_column",
+                        "unable to decode LEB128 at offset 0x00000030: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(
+            2, DW_LNS_fixed_advance_pc,
+            ValueAndLengths{{0x500, LineTable::Half}},
+            "DW_LNS_fixed_advance_pc",
+            "unexpected end of data at offset 0x31 while reading [0x30, 0x32)"),
+        std::make_tuple(2, DW_LNS_set_isa,
+                        ValueAndLengths{{0x600, LineTable::ULEB}},
+                        "DW_LNS_set_isa",
+                        "unable to decode LEB128 at offset 0x00000030: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(2, 0xd,
+                        ValueAndLengths{{0x700, LineTable::ULEB},
+                                        {0x800, LineTable::ULEB}},
+                        "Unrecognized standard opcode",
+                        "unable to decode LEB128 at offset 0x00000030: "
+                        "malformed uleb128, extends past end"),
+        std::make_tuple(
+            4, 0xd,
+            ValueAndLengths{{0x900, LineTable::ULEB}, {0xa00, LineTable::ULEB}},
+            "Unrecognized standard opcode (operands: 0x0000000000000900)",
+            "unable to decode LEB128 at offset 0x00000032: "
 
 TEST_F(DebugLineBasicFixture, PrintPathsProperly) {
   if (!setupGenerator(5))
     return;
 
   LineTable &LT = Gen->addLineTable();
   DWARFDebugLine::Prologue P = LT.createBasicPrologue();
   P.IncludeDirectories.push_back(