Skip to content

Commit fe6983a

Browse files
committed
[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 1271cde, when non-byte operands were used as arguments for extended or standard opcodes. Reviewed by: dblaikie Differential Revision: https://reviews.llvm.org/D73962
1 parent 9dc84e9 commit fe6983a

File tree

4 files changed

+142
-12
lines changed

4 files changed

+142
-12
lines changed

llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -665,22 +665,36 @@ Error DWARFDebugLine::LineTable::parse(
665665
// from the size of the operand.
666666
{
667667
uint8_t ExtractorAddressSize = DebugLineData.getAddressSize();
668-
if (ExtractorAddressSize != Len - 1 && ExtractorAddressSize != 0)
668+
uint64_t OpcodeAddressSize = Len - 1;
669+
if (ExtractorAddressSize != OpcodeAddressSize &&
670+
ExtractorAddressSize != 0)
669671
RecoverableErrorHandler(createStringError(
670672
errc::invalid_argument,
671673
"mismatching address size at offset 0x%8.8" PRIx64
672674
" expected 0x%2.2" PRIx8 " found 0x%2.2" PRIx64,
673675
ExtOffset, ExtractorAddressSize, Len - 1));
674676

675677
// Assume that the line table is correct and temporarily override the
676-
// address size.
677-
DebugLineData.setAddressSize(Len - 1);
678-
State.Row.Address.Address = DebugLineData.getRelocatedAddress(
679-
OffsetPtr, &State.Row.Address.SectionIndex);
680-
681-
// Restore the address size if the extractor already had it.
682-
if (ExtractorAddressSize != 0)
683-
DebugLineData.setAddressSize(ExtractorAddressSize);
678+
// address size. If the size is unsupported, give up trying to read
679+
// the address and continue to the next opcode.
680+
if (OpcodeAddressSize != 1 && OpcodeAddressSize != 2 &&
681+
OpcodeAddressSize != 4 && OpcodeAddressSize != 8) {
682+
RecoverableErrorHandler(createStringError(
683+
errc::invalid_argument,
684+
"address size 0x%2.2" PRIx64
685+
" of DW_LNE_set_address opcode at offset 0x%8.8" PRIx64
686+
" is unsupported",
687+
OpcodeAddressSize, ExtOffset));
688+
*OffsetPtr += OpcodeAddressSize;
689+
} else {
690+
DebugLineData.setAddressSize(OpcodeAddressSize);
691+
State.Row.Address.Address = DebugLineData.getRelocatedAddress(
692+
OffsetPtr, &State.Row.Address.SectionIndex);
693+
694+
// Restore the address size if the extractor already had it.
695+
if (ExtractorAddressSize != 0)
696+
DebugLineData.setAddressSize(ExtractorAddressSize);
697+
}
684698

685699
if (OS)
686700
*OS << format(" (0x%16.16" PRIx64 ")", State.Row.Address.Address);

llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,100 @@ TEST_F(DebugLineBasicFixture,
640640
EXPECT_EQ((*ExpectedLineTable)->Rows[1].Address.Address, Addr2);
641641
}
642642

643+
TEST_F(DebugLineBasicFixture,
644+
ErrorForUnsupportedAddressSizeInSetAddressLength) {
645+
// Use DWARF v4, and 0 for data extractor address size so that the address
646+
// size is derived from the opcode length.
647+
if (!setupGenerator(4, 0))
648+
return;
649+
650+
LineTable &LT = Gen->addLineTable();
651+
// 4 == length of the extended opcode, i.e. 1 for the opcode itself and 3 for
652+
// the Half (2) + Byte (1) operand, representing the unsupported address size.
653+
LT.addExtendedOpcode(4, DW_LNE_set_address,
654+
{{0x1234, LineTable::Half}, {0x56, LineTable::Byte}});
655+
LT.addStandardOpcode(DW_LNS_copy, {});
656+
// Special opcode to ensure the address has changed between the first and last
657+
// row in the sequence. Without this, the sequence will not be recorded.
658+
LT.addByte(0xaa);
659+
LT.addExtendedOpcode(1, DW_LNE_end_sequence, {});
660+
661+
generate();
662+
663+
auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
664+
nullptr, RecordRecoverable);
665+
checkError(
666+
"address size 0x03 of DW_LNE_set_address opcode at offset 0x00000030 is "
667+
"unsupported",
668+
std::move(Recoverable));
669+
ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
670+
ASSERT_EQ((*ExpectedLineTable)->Rows.size(), 3u);
671+
EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u);
672+
// Show that the set address opcode is ignored in this case.
673+
EXPECT_EQ((*ExpectedLineTable)->Rows[0].Address.Address, 0);
674+
}
675+
676+
TEST_F(DebugLineBasicFixture, ErrorForAddressSizeGreaterThanByteSize) {
677+
// Use DWARF v4, and 0 for data extractor address size so that the address
678+
// size is derived from the opcode length.
679+
if (!setupGenerator(4, 0))
680+
return;
681+
682+
LineTable &LT = Gen->addLineTable();
683+
// Specifically use an operand size that has a trailing byte of a supported
684+
// size (8), so that any potential truncation would result in a valid size.
685+
std::vector<LineTable::ValueAndLength> Operands(0x108);
686+
LT.addExtendedOpcode(Operands.size() + 1, DW_LNE_set_address, Operands);
687+
LT.addExtendedOpcode(1, DW_LNE_end_sequence, {});
688+
689+
generate();
690+
691+
auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
692+
nullptr, RecordRecoverable);
693+
checkError(
694+
"address size 0x108 of DW_LNE_set_address opcode at offset 0x00000031 is "
695+
"unsupported",
696+
std::move(Recoverable));
697+
ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
698+
}
699+
700+
TEST_F(DebugLineBasicFixture, ErrorForUnsupportedAddressSizeDefinedInHeader) {
701+
// Use 0 for data extractor address size so that it does not clash with the
702+
// header address size.
703+
if (!setupGenerator(5, 0))
704+
return;
705+
706+
LineTable &LT = Gen->addLineTable();
707+
// AddressSize + 1 == length of the extended opcode, i.e. 1 for the opcode
708+
// itself and 9 for the Quad (8) + Byte (1) operand representing the
709+
// unsupported address size.
710+
uint8_t AddressSize = 9;
711+
LT.addExtendedOpcode(AddressSize + 1, DW_LNE_set_address,
712+
{{0x12345678, LineTable::Quad}, {0, LineTable::Byte}});
713+
LT.addStandardOpcode(DW_LNS_copy, {});
714+
// Special opcode to ensure the address has changed between the first and last
715+
// row in the sequence. Without this, the sequence will not be recorded.
716+
LT.addByte(0xaa);
717+
LT.addExtendedOpcode(1, DW_LNE_end_sequence, {});
718+
DWARFDebugLine::Prologue Prologue = LT.createBasicPrologue();
719+
Prologue.FormParams.AddrSize = AddressSize;
720+
LT.setPrologue(Prologue);
721+
722+
generate();
723+
724+
auto ExpectedLineTable = Line.getOrParseLineTable(LineData, 0, *Context,
725+
nullptr, RecordRecoverable);
726+
checkError(
727+
"address size 0x09 of DW_LNE_set_address opcode at offset 0x00000038 is "
728+
"unsupported",
729+
std::move(Recoverable));
730+
ASSERT_THAT_EXPECTED(ExpectedLineTable, Succeeded());
731+
ASSERT_EQ((*ExpectedLineTable)->Rows.size(), 3u);
732+
EXPECT_EQ((*ExpectedLineTable)->Sequences.size(), 1u);
733+
// Show that the set address opcode is ignored in this case.
734+
EXPECT_EQ((*ExpectedLineTable)->Rows[0].Address.Address, 0);
735+
}
736+
643737
TEST_F(DebugLineBasicFixture, CallbackUsedForUnterminatedSequence) {
644738
if (!setupGenerator())
645739
return;

llvm/unittests/DebugInfo/DWARF/DwarfGenerator.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "llvm/MC/MCSubtargetInfo.h"
2828
#include "llvm/MC/MCTargetOptionsCommandFlags.inc"
2929
#include "llvm/PassAnalysisSupport.h"
30+
#include "llvm/Support/LEB128.h"
3031
#include "llvm/Support/TargetRegistry.h"
3132
#include "llvm/Support/raw_ostream.h"
3233
#include "llvm/Target/TargetLoweringObjectFile.h"
@@ -175,7 +176,7 @@ DWARFDebugLine::Prologue dwarfgen::LineTable::createBasicPrologue() const {
175176
P.TotalLength += 4;
176177
P.FormParams.Format = DWARF64;
177178
}
178-
P.TotalLength += Contents.size();
179+
P.TotalLength += getContentsSize();
179180
P.FormParams.Version = Version;
180181
P.MinInstLength = 1;
181182
P.MaxOpsPerInst = 1;
@@ -259,6 +260,24 @@ void dwarfgen::LineTable::writeData(ArrayRef<ValueAndLength> Data,
259260
}
260261
}
261262

263+
size_t dwarfgen::LineTable::getContentsSize() const {
264+
size_t Size = 0;
265+
for (auto Entry : Contents) {
266+
switch (Entry.Length) {
267+
case ULEB:
268+
Size += getULEB128Size(Entry.Value);
269+
break;
270+
case SLEB:
271+
Size += getSLEB128Size(Entry.Value);
272+
break;
273+
default:
274+
Size += Entry.Length;
275+
break;
276+
}
277+
}
278+
return Size;
279+
}
280+
262281
MCSymbol *dwarfgen::LineTable::writeDefaultPrologue(AsmPrinter &Asm) const {
263282
MCSymbol *UnitStart = Asm.createTempSymbol("line_unit_start");
264283
MCSymbol *UnitEnd = Asm.createTempSymbol("line_unit_end");

llvm/unittests/DebugInfo/DWARF/DwarfGenerator.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ class LineTable {
171171
enum ValueLength { Byte = 1, Half = 2, Long = 4, Quad = 8, ULEB, SLEB };
172172

173173
struct ValueAndLength {
174-
uint64_t Value;
175-
ValueLength Length;
174+
uint64_t Value = 0;
175+
ValueLength Length = Byte;
176176
};
177177

178178
LineTable(uint16_t Version, dwarf::DwarfFormat Format, uint8_t AddrSize,
@@ -214,6 +214,9 @@ class LineTable {
214214
void writeProloguePayload(const DWARFDebugLine::Prologue &Prologue,
215215
AsmPrinter &Asm) const;
216216

217+
// Calculate the number of bytes the Contents will take up.
218+
size_t getContentsSize() const;
219+
217220
llvm::Optional<DWARFDebugLine::Prologue> Prologue;
218221
std::vector<ValueAndLength> CustomPrologue;
219222
std::vector<ValueAndLength> Contents;

0 commit comments

Comments
 (0)