[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

diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
index b5ec79b..d9a2970 100644 (file)
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -196,17 +196,20 @@ raw_ostream &llvm::dwarf::operator<<(raw_ostream &OS, const UnwindTable &Rows) {
 }
 
 Expected<UnwindTable> 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> 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> 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/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
index fda6d26..94a93ae 100644 (file)
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
+++ b/llvm/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<dwarf::UnwindTable> 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<dwarf::UnwindTable> 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,