From fe6983a75ae08dc63e2068f521670562ad77c599 Mon Sep 17 00:00:00 2001 From: James Henderson Date: Mon, 3 Feb 2020 16:43:03 +0000 Subject: [PATCH] [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 --- llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp | 32 +++++--- .../DebugInfo/DWARF/DWARFDebugLineTest.cpp | 94 ++++++++++++++++++++++ llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp | 21 ++++- llvm/unittests/DebugInfo/DWARF/DwarfGenerator.h | 7 +- 4 files changed, 142 insertions(+), 12 deletions(-) diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp index 7bdc72a..436318b 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp +++ b/llvm/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); - - // Restore the address size if the extractor already had it. - if (ExtractorAddressSize != 0) - DebugLineData.setAddressSize(ExtractorAddressSize); + // 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); + } if (OS) *OS << format(" (0x%16.16" PRIx64 ")", State.Row.Address.Address); diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp index d6d9691..0552b5a 100644 --- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp +++ b/llvm/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 < = 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 < = 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 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 < = 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; diff --git a/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp b/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp index f53fbde..f4e407e 100644 --- a/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp +++ b/llvm/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 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"); diff --git a/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.h b/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.h index 2f95e29..9ba8eab 100644 --- a/llvm/unittests/DebugInfo/DWARF/DwarfGenerator.h +++ b/llvm/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 Prologue; std::vector CustomPrologue; std::vector Contents; -- 2.7.4