[DebugInfo] Error if unsupported address size detected in line table

Prior to this patch, if a DW_LNE_set_address opcode was parsed with an
address size (i.e. with a length after the opcode) of anything other 1,
2, 4, or 8, an llvm_unreachable would be hit, as the data extractor does
not support other values. This patch introduces a new error check that
verifies the address size is one of the supported sizes, in common with
other places within the DWARF parsing.

This patch also fixes calculation of a generated line table's size in
unit tests. One of the tests in this patch highlighted a bug introduced
in 1271cde4745, when non-byte operands were used as arguments for
extended or standard opcodes.

Reviewed by: dblaikie

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

lib/DebugInfo/DWARF/DWARFDebugLine.cpp

@@ -665,7 +665,9 @@ Error DWARFDebugLine::LineTable::parse(
         // from the size of the operand.
         {
           uint8_t ExtractorAddressSize = DebugLineData.getAddressSize();
-          if (ExtractorAddressSize != Len - 1 && ExtractorAddressSize != 0)
+          uint64_t OpcodeAddressSize = Len - 1;
+          if (ExtractorAddressSize != OpcodeAddressSize &&
+              ExtractorAddressSize != 0)
             RecoverableErrorHandler(createStringError(
                 errc::invalid_argument,
                 "mismatching address size at offset 0x%8.8" PRIx64
@@ -673,14 +675,26 @@ Error DWARFDebugLine::LineTable::parse(
                 ExtOffset, ExtractorAddressSize, Len - 1));
 
           // Assume that the line table is correct and temporarily override the
-          // address size.
-          DebugLineData.setAddressSize(Len - 1);
-          State.Row.Address.Address = DebugLineData.getRelocatedAddress(
-              OffsetPtr, &State.Row.Address.SectionIndex);
+          // 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(
+                OffsetPtr, &State.Row.Address.SectionIndex);
 
-          // Restore the address size if the extractor already had it.
-          if (ExtractorAddressSize != 0)
-            DebugLineData.setAddressSize(ExtractorAddressSize);
+            // Restore the address size if the extractor already had it.
+            if (ExtractorAddressSize != 0)
+              DebugLineData.setAddressSize(ExtractorAddressSize);
+          }
 
           if (OS)
             *OS << format(" (0x%16.16" PRIx64 ")", State.Row.Address.Address);

unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

@@ -640,6 +640,100 @@ TEST_F(DebugLineBasicFixture,
   EXPECT_EQ((*ExpectedLineTable)->Rows[1].Address.Address, Addr2);
 }
 
+TEST_F(DebugLineBasicFixture,
+       ErrorForUnsupportedAddressSizeInSetAddressLength) {
+  // Use DWARF v4, and 0 for data extractor address size so that the address
+  // size is derived from the opcode length.
+  if (!setupGenerator(4, 0))
+    return;
+
+  LineTable &LT = Gen->addLineTable();
+  // 4 == length of the extended opcode, i.e. 1 for the opcode itself and 3 for
+  // the Half (2) + Byte (1) operand, representing the unsupported address size.
+  LT.addExtendedOpcode(4, DW_LNE_set_address,
+                       {{0x1234, LineTable::Half}, {0x56, LineTable::Byte}});
+  LT.addStandardOpcode(DW_LNS_copy, {});
+  // Special opcode to ensure the address has changed between the first and last
+  // row in the sequence. Without this, the sequence will not be recorded.
+  LT.addByte(0xaa);
+  LT.addExtendedOpcode(1, DW_LNE_end_sequence, {});
+
+  generate();
+
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+                                                    nullptr, RecordRecoverable);
+  checkError(
+      "address size 0x03 of DW_LNE_set_address opcode at offset 0x00000030 is "
+      "unsupported",
+      std::move(Recoverable));
+  ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+  ASSERT_EQ((*ExpectedLineTable)->Rows.size(), 3u);
+  EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u);
+  // Show that the set address opcode is ignored in this case.
+  EXPECT_EQ((*ExpectedLineTable)->Rows[0].Address.Address, 0);
+}
+
+TEST_F(DebugLineBasicFixture, ErrorForAddressSizeGreaterThanByteSize) {
+  // Use DWARF v4, and 0 for data extractor address size so that the address
+  // size is derived from the opcode length.
+  if (!setupGenerator(4, 0))
+    return;
+
+  LineTable &LT = Gen->addLineTable();
+  // Specifically use an operand size that has a trailing byte of a supported
+  // size (8), so that any potential truncation would result in a valid size.
+  std::vector<LineTable::ValueAndLength> Operands(0x108);
+  LT.addExtendedOpcode(Operands.size() + 1, DW_LNE_set_address, Operands);
+  LT.addExtendedOpcode(1, DW_LNE_end_sequence, {});
+
+  generate();
+
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+                                                    nullptr, RecordRecoverable);
+  checkError(
+      "address size 0x108 of DW_LNE_set_address opcode at offset 0x00000031 is "
+      "unsupported",
+      std::move(Recoverable));
+  ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+}
+
+TEST_F(DebugLineBasicFixture, ErrorForUnsupportedAddressSizeDefinedInHeader) {
+  // Use 0 for data extractor address size so that it does not clash with the
+  // header address size.
+  if (!setupGenerator(5, 0))
+    return;
+
+  LineTable &LT = Gen->addLineTable();
+  // AddressSize + 1 == length of the extended opcode, i.e. 1 for the opcode
+  // itself and 9 for the Quad (8) + Byte (1) operand representing the
+  // unsupported address size.
+  uint8_t AddressSize = 9;
+  LT.addExtendedOpcode(AddressSize + 1, DW_LNE_set_address,
+                       {{0x12345678, LineTable::Quad}, {0, LineTable::Byte}});
+  LT.addStandardOpcode(DW_LNS_copy, {});
+  // Special opcode to ensure the address has changed between the first and last
+  // row in the sequence. Without this, the sequence will not be recorded.
+  LT.addByte(0xaa);
+  LT.addExtendedOpcode(1, DW_LNE_end_sequence, {});
+  DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue();
+  Prologue.FormParams.AddrSize = AddressSize;
+  LT.setPrologue(Prologue);
+
+  generate();
+
+  auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
+                                                    nullptr, RecordRecoverable);
+  checkError(
+      "address size 0x09 of DW_LNE_set_address opcode at offset 0x00000038 is "
+      "unsupported",
+      std::move(Recoverable));
+  ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
+  ASSERT_EQ((*ExpectedLineTable)->Rows.size(), 3u);
+  EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u);
+  // Show that the set address opcode is ignored in this case.
+  EXPECT_EQ((*ExpectedLineTable)->Rows[0].Address.Address, 0);
+}
+
 TEST_F(DebugLineBasicFixture, CallbackUsedForUnterminatedSequence) {
   if (!setupGenerator())
     return;

unittests/DebugInfo/DWARF/DwarfGenerator.cpp

@@ -27,6 +27,7 @@
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptionsCommandFlags.inc"
 #include "llvm/PassAnalysisSupport.h"
+#include "llvm/Support/LEB128.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetLoweringObjectFile.h"
@@ -175,7 +176,7 @@ DWARFDebugLine::Prologue dwarfgen::LineTable::createBasicPrologue() const {
     P.TotalLength += 4;
     P.FormParams.Format = DWARF64;
   }
-  P.TotalLength += Contents.size();
+  P.TotalLength += getContentsSize();
   P.FormParams.Version = Version;
   P.MinInstLength = 1;
   P.MaxOpsPerInst = 1;
@@ -259,6 +260,24 @@ void dwarfgen::LineTable::writeData(ArrayRef<ValueAndLength> Data,
   }
 }
 
+size_t dwarfgen::LineTable::getContentsSize() const {
+  size_t Size = 0;
+  for (auto Entry : Contents) {
+    switch (Entry.Length) {
+    case ULEB:
+      Size += getULEB128Size(Entry.Value);
+      break;
+    case SLEB:
+      Size += getSLEB128Size(Entry.Value);
+      break;
+    default:
+      Size += Entry.Length;
+      break;
+    }
+  }
+  return Size;
+}
+
 MCSymbol *dwarfgen::LineTable::writeDefaultPrologue(AsmPrinter &Asm) const {
   MCSymbol *UnitStart = Asm.createTempSymbol("line_unit_start");
   MCSymbol *UnitEnd = Asm.createTempSymbol("line_unit_end");

unittests/DebugInfo/DWARF/DwarfGenerator.h

@@ -171,8 +171,8 @@ public:
   enum ValueLength { Byte = 1, Half = 2, Long = 4, Quad = 8, ULEB, SLEB };
 
   struct ValueAndLength {
-    uint64_t Value;
-    ValueLength Length;
+    uint64_t Value = 0;
+    ValueLength Length = Byte;
   };
 
   LineTable(uint16_t Version, dwarf::DwarfFormat Format, uint8_t AddrSize,
@@ -214,6 +214,9 @@ private:
   void writeProloguePayload(const DWARFDebugLine::Prologue &Prologue,
                             AsmPrinter &Asm) const;
 
+  // Calculate the number of bytes the Contents will take up.
+  size_t getContentsSize() const;
+
   llvm::Optional<DWARFDebugLine::Prologue> Prologue;
   std::vector<ValueAndLength> CustomPrologue;
   std::vector<ValueAndLength> Contents;