From 61d8a362893992253c21f2229257d1d4e85eb823 Mon Sep 17 00:00:00 2001 From: RamNalamothu Date: Sat, 8 May 2021 10:15:49 +0530 Subject: [PATCH] [DebugInfo] UnwindTable::create() should not add empty rows to CFI unwind table UnwindTable::parseRows() may return successfully if the CFIProgram has either no CFI instructions or only DW_CFA_nop instructions and the UnwindRow return argument will be empty. But currently, the callers are not checking for this case which is leading to incorrect dumps in the unwind tables in such cases i.e. CFA=unspecified Reviewed By: clayborg Differential Revision: https://reviews.llvm.org/D101892 --- lib/DebugInfo/DWARF/DWARFDebugFrame.cpp | 29 +++++-- .../DebugInfo/DWARF/DWARFDebugFrameTest.cpp | 78 +++++++++++++++++++ 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp index b5ec79b615d..d9a29702c6c 100644 --- a/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp +++ b/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp @@ -196,17 +196,20 @@ raw_ostream &llvm::dwarf::operator<<(raw_ostream &OS, const UnwindTable &Rows) { } Expected UnwindTable::create(const FDE *Fde) { - UnwindTable UT; - UnwindRow Row; - Row.setAddress(Fde->getInitialLocation()); - UT.EndAddress = Fde->getInitialLocation() + Fde->getAddressRange(); - const CIE *Cie = Fde->getLinkedCIE(); if (Cie == nullptr) return createStringError(errc::invalid_argument, "unable to get CIE for FDE at offset 0x%" PRIx64, Fde->getOffset()); + // Rows will be empty if there are no CFI instructions. + if (Cie->cfis().empty() && Fde->cfis().empty()) + return UnwindTable(); + + UnwindTable UT; + UnwindRow Row; + Row.setAddress(Fde->getInitialLocation()); + UT.EndAddress = Fde->getInitialLocation() + Fde->getAddressRange(); if (Error CieError = UT.parseRows(Cie->cfis(), Row, nullptr)) return std::move(CieError); // We need to save the initial locations of registers from the CIE parsing @@ -214,16 +217,28 @@ Expected UnwindTable::create(const FDE *Fde) { const RegisterLocations InitialLocs = Row.getRegisterLocations(); if (Error FdeError = UT.parseRows(Fde->cfis(), Row, &InitialLocs)) return std::move(FdeError); - UT.Rows.push_back(Row); + // May be all the CFI instructions were DW_CFA_nop amd Row becomes empty. + // Do not add that to the unwind table. + if (Row.getRegisterLocations().hasLocations() || + Row.getCFAValue().getLocation() != UnwindLocation::Unspecified) + UT.Rows.push_back(Row); return UT; } Expected UnwindTable::create(const CIE *Cie) { + // Rows will be empty if there are no CFI instructions. + if (Cie->cfis().empty()) + return UnwindTable(); + UnwindTable UT; UnwindRow Row; if (Error CieError = UT.parseRows(Cie->cfis(), Row, nullptr)) return std::move(CieError); - UT.Rows.push_back(Row); + // May be all the CFI instructions were DW_CFA_nop amd Row becomes empty. + // Do not add that to the unwind table. + if (Row.getRegisterLocations().hasLocations() || + Row.getCFAValue().getLocation() != UnwindLocation::Unspecified) + UT.Rows.push_back(Row); return UT; } diff --git a/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp b/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp index fda6d265f8e..94a93ae0e68 100644 --- a/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp +++ b/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp @@ -447,6 +447,84 @@ TEST(DWARFDebugFrame, RegisterLocations) { expectDumpResult(Locs, ""); } +// Test that empty rows are not added to UnwindTable when +// dwarf::CIE::CFIs or dwarf::FDE::CFIs is empty. +TEST(DWARFDebugFrame, UnwindTableEmptyRows) { + dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/false, + /*Offset=*/0x0, + /*Length=*/0xff); + + // Having an empty instructions list is fine. + EXPECT_THAT_ERROR(parseCFI(TestCIE, {}), Succeeded()); + EXPECT_TRUE(TestCIE.cfis().empty()); + + // Verify dwarf::UnwindTable::create() won't result in errors and + // and empty rows are not added to CIE UnwindTable. + Expected RowsOrErr = dwarf::UnwindTable::create(&TestCIE); + EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded()); + const size_t ExpectedNumOfRows = 0; + EXPECT_EQ(RowsOrErr->size(), ExpectedNumOfRows); + + dwarf::FDE TestFDE(/*IsDWARF64=*/true, + /*Offset=*/0x3333abcdabcd, + /*Length=*/0x4444abcdabcd, + /*CIEPointer=*/0x1111abcdabcd, + /*InitialLocation=*/0x1000, + /*AddressRange=*/0x1000, + /*Cie=*/&TestCIE, + /*LSDAAddress=*/None, + /*Arch=*/Triple::x86_64); + + // Having an empty instructions list is fine. + EXPECT_THAT_ERROR(parseCFI(TestFDE, {}), Succeeded()); + EXPECT_TRUE(TestFDE.cfis().empty()); + + // Verify dwarf::UnwindTable::create() won't result in errors and + // and empty rows are not added to FDE UnwindTable. + RowsOrErr = dwarf::UnwindTable::create(&TestFDE); + EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded()); + EXPECT_EQ(RowsOrErr->size(), ExpectedNumOfRows); +} + +// Test that empty rows are not added to UnwindTable when dwarf::CIE::CFIs +// or dwarf::FDE::CFIs is not empty but has only DW_CFA_nop instructions. +TEST(DWARFDebugFrame, UnwindTableEmptyRows_NOPs) { + dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/false, + /*Offset=*/0x0, + /*Length=*/0xff); + + // Make a CIE that has only DW_CFA_nop instructions. + EXPECT_THAT_ERROR(parseCFI(TestCIE, {dwarf::DW_CFA_nop}), Succeeded()); + EXPECT_TRUE(!TestCIE.cfis().empty()); + + // Verify dwarf::UnwindTable::create() won't result in errors and + // and empty rows are not added to CIE UnwindTable. + Expected RowsOrErr = dwarf::UnwindTable::create(&TestCIE); + EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded()); + const size_t ExpectedNumOfRows = 0; + EXPECT_EQ(RowsOrErr->size(), ExpectedNumOfRows); + + dwarf::FDE TestFDE(/*IsDWARF64=*/true, + /*Offset=*/0x3333abcdabcd, + /*Length=*/0x4444abcdabcd, + /*CIEPointer=*/0x1111abcdabcd, + /*InitialLocation=*/0x1000, + /*AddressRange=*/0x1000, + /*Cie=*/&TestCIE, + /*LSDAAddress=*/None, + /*Arch=*/Triple::x86_64); + + // Make an FDE that has only DW_CFA_nop instructions. + EXPECT_THAT_ERROR(parseCFI(TestFDE, {dwarf::DW_CFA_nop}), Succeeded()); + EXPECT_TRUE(!TestFDE.cfis().empty()); + + // Verify dwarf::UnwindTable::create() won't result in errors and + // and empty rows are not added to FDE UnwindTable. + RowsOrErr = dwarf::UnwindTable::create(&TestFDE); + EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded()); + EXPECT_EQ(RowsOrErr->size(), ExpectedNumOfRows); +} + TEST(DWARFDebugFrame, UnwindTableErrorNonAscendingFDERows) { dwarf::CIE TestCIE = createCIE(/*IsDWARF64=*/false, /*Offset=*/0x0,