[NFC] Remove UnwindTable dependency on CIE, and FDE #142520
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-debuginfo Author: AmirHossein PashaeeHir (amsen20) ChangesThis PR is in the direction of separating Unwind table from FDE, and CIE that depends on llvm/Object. To separate, first the runtime dependencies have to be removed, which this commit does that. Full diff: https://github.com/llvm/llvm-project/pull/142520.diff 3 Files Affected:
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
index b4b1e49e68a84..6d0285f9ca3b2 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
@@ -310,9 +310,6 @@ class UnwindRow {
raw_ostream &operator<<(raw_ostream &OS, const UnwindRow &Row);
-class CIE;
-class FDE;
-
/// A class that contains all UnwindRow objects for an FDE or a single unwind
/// row for a CIE. To unwind an address the rows, which are sorted by start
/// address, can be searched to find the UnwindRow with the lowest starting
@@ -333,6 +330,12 @@ class UnwindTable {
assert(Index < size());
return Rows[Index];
}
+ void insertRow(const UnwindRow &Row) { Rows.push_back(Row); }
+
+ /// Set the last address that this unwinding table refers to.
+ ///
+ /// This is used when this table is created based on a FDE.
+ void setEndAddress(uint64_t Addr) { EndAddress = Addr; }
/// Dump the UnwindTable to the stream.
///
@@ -351,32 +354,6 @@ class UnwindTable {
void dump(raw_ostream &OS, DIDumpOptions DumpOpts,
unsigned IndentLevel = 0) const;
- /// Create an UnwindTable from a Common Information Entry (CIE).
- ///
- /// \param Cie The Common Information Entry to extract the table from. The
- /// CFIProgram is retrieved from the \a Cie object and used to create the
- /// UnwindTable.
- ///
- /// \returns An error if the DWARF Call Frame Information opcodes have state
- /// machine errors, or a valid UnwindTable otherwise.
- static Expected<UnwindTable> create(const CIE *Cie);
-
- /// Create an UnwindTable from a Frame Descriptor Entry (FDE).
- ///
- /// \param Fde The Frame Descriptor Entry to extract the table from. The
- /// CFIProgram is retrieved from the \a Fde object and used to create the
- /// UnwindTable.
- ///
- /// \returns An error if the DWARF Call Frame Information opcodes have state
- /// machine errors, or a valid UnwindTable otherwise.
- static Expected<UnwindTable> create(const FDE *Fde);
-
-private:
- RowContainer Rows;
- /// The end address when data is extracted from a FDE. This value will be
- /// invalid when a UnwindTable is extracted from a CIE.
- std::optional<uint64_t> EndAddress;
-
/// Parse the information in the CFIProgram and update the CurrRow object
/// that the state machine describes.
///
@@ -394,10 +371,40 @@ class UnwindTable {
/// DW_CFA_restore and DW_CFA_restore_extended opcodes.
Error parseRows(const CFIProgram &CFIP, UnwindRow &CurrRow,
const RegisterLocations *InitialLocs);
+
+private:
+ RowContainer Rows;
+ /// The end address when data is extracted from a FDE. This value will be
+ /// invalid when a UnwindTable is extracted from a CIE.
+ std::optional<uint64_t> EndAddress;
};
raw_ostream &operator<<(raw_ostream &OS, const UnwindTable &Rows);
+class CIE;
+
+/// Create an UnwindTable from a Common Information Entry (CIE).
+///
+/// \param Cie The Common Information Entry to extract the table from. The
+/// CFIProgram is retrieved from the \a Cie object and used to create the
+/// UnwindTable.
+///
+/// \returns An error if the DWARF Call Frame Information opcodes have state
+/// machine errors, or a valid UnwindTable otherwise.
+Expected<UnwindTable> createUnwindTable(const CIE *Cie);
+
+class FDE;
+
+/// Create an UnwindTable from a Frame Descriptor Entry (FDE).
+///
+/// \param Fde The Frame Descriptor Entry to extract the table from. The
+/// CFIProgram is retrieved from the \a Fde object and used to create the
+/// UnwindTable.
+///
+/// \returns An error if the DWARF Call Frame Information opcodes have state
+/// machine errors, or a valid UnwindTable otherwise.
+Expected<UnwindTable> createUnwindTable(const FDE *Fde);
+
/// An entry in either debug_frame or eh_frame. This entry can be a CIE or an
/// FDE.
class FrameEntry {
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
index aecfc4565dbc2..7ae902ca0b09c 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -201,7 +201,7 @@ raw_ostream &llvm::dwarf::operator<<(raw_ostream &OS, const UnwindTable &Rows) {
return OS;
}
-Expected<UnwindTable> UnwindTable::create(const FDE *Fde) {
+Expected<UnwindTable> llvm::dwarf::createUnwindTable(const FDE *Fde) {
const CIE *Cie = Fde->getLinkedCIE();
if (Cie == nullptr)
return createStringError(errc::invalid_argument,
@@ -215,7 +215,7 @@ Expected<UnwindTable> UnwindTable::create(const FDE *Fde) {
UnwindTable UT;
UnwindRow Row;
Row.setAddress(Fde->getInitialLocation());
- UT.EndAddress = Fde->getInitialLocation() + Fde->getAddressRange();
+ UT.setEndAddress(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
@@ -227,11 +227,11 @@ Expected<UnwindTable> UnwindTable::create(const FDE *Fde) {
// Do not add that to the unwind table.
if (Row.getRegisterLocations().hasLocations() ||
Row.getCFAValue().getLocation() != UnwindLocation::Unspecified)
- UT.Rows.push_back(Row);
+ UT.insertRow(Row);
return UT;
}
-Expected<UnwindTable> UnwindTable::create(const CIE *Cie) {
+Expected<UnwindTable> llvm::dwarf::createUnwindTable(const CIE *Cie) {
// Rows will be empty if there are no CFI instructions.
if (Cie->cfis().empty())
return UnwindTable();
@@ -244,7 +244,7 @@ Expected<UnwindTable> UnwindTable::create(const CIE *Cie) {
// Do not add that to the unwind table.
if (Row.getRegisterLocations().hasLocations() ||
Row.getCFAValue().getLocation() != UnwindLocation::Unspecified)
- UT.Rows.push_back(Row);
+ UT.insertRow(Row);
return UT;
}
@@ -603,7 +603,7 @@ void CIE::dump(raw_ostream &OS, DIDumpOptions DumpOpts) const {
CFIs.dump(OS, DumpOpts, /*IndentLevel=*/1, /*InitialLocation=*/{});
OS << "\n";
- if (Expected<UnwindTable> RowsOrErr = UnwindTable::create(this))
+ if (Expected<UnwindTable> RowsOrErr = createUnwindTable(this))
RowsOrErr->dump(OS, DumpOpts, 1);
else {
DumpOpts.RecoverableErrorHandler(joinErrors(
@@ -631,7 +631,7 @@ void FDE::dump(raw_ostream &OS, DIDumpOptions DumpOpts) const {
CFIs.dump(OS, DumpOpts, /*IndentLevel=*/1, InitialLocation);
OS << "\n";
- if (Expected<UnwindTable> RowsOrErr = UnwindTable::create(this))
+ if (Expected<UnwindTable> RowsOrErr = createUnwindTable(this))
RowsOrErr->dump(OS, DumpOpts, 1);
else {
DumpOpts.RecoverableErrorHandler(joinErrors(
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
index 2be656547c92e..2f91b1f86f6dd 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
@@ -465,9 +465,9 @@ TEST(DWARFDebugFrame, UnwindTableEmptyRows) {
EXPECT_THAT_ERROR(parseCFI(TestCIE, {}), Succeeded());
EXPECT_TRUE(TestCIE.cfis().empty());
- // Verify dwarf::UnwindTable::create() won't result in errors and
+ // Verify dwarf::createUnwindTable() won't result in errors and
// and empty rows are not added to CIE UnwindTable.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestCIE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestCIE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const size_t ExpectedNumOfRows = 0;
EXPECT_EQ(RowsOrErr->size(), ExpectedNumOfRows);
@@ -486,9 +486,9 @@ TEST(DWARFDebugFrame, UnwindTableEmptyRows) {
EXPECT_THAT_ERROR(parseCFI(TestFDE, {}), Succeeded());
EXPECT_TRUE(TestFDE.cfis().empty());
- // Verify dwarf::UnwindTable::create() won't result in errors and
+ // Verify dwarf::createUnwindTable() won't result in errors and
// and empty rows are not added to FDE UnwindTable.
- RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
EXPECT_EQ(RowsOrErr->size(), ExpectedNumOfRows);
}
@@ -504,9 +504,9 @@ TEST(DWARFDebugFrame, UnwindTableEmptyRows_NOPs) {
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
+ // Verify dwarf::createUnwindTable() won't result in errors and
// and empty rows are not added to CIE UnwindTable.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestCIE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestCIE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const size_t ExpectedNumOfRows = 0;
EXPECT_EQ(RowsOrErr->size(), ExpectedNumOfRows);
@@ -525,9 +525,9 @@ TEST(DWARFDebugFrame, UnwindTableEmptyRows_NOPs) {
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
+ // Verify dwarf::createUnwindTable() won't result in errors and
// and empty rows are not added to FDE UnwindTable.
- RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
EXPECT_EQ(RowsOrErr->size(), ExpectedNumOfRows);
}
@@ -567,7 +567,7 @@ TEST(DWARFDebugFrame, UnwindTableErrorNonAscendingFDERows) {
Succeeded());
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(),
FailedWithMessage("DW_CFA_set_loc with adrress 0x1000 which"
" must be greater than the current row "
@@ -603,7 +603,7 @@ TEST(DWARFDebugFrame, UnwindTableError_DW_CFA_restore_state) {
Succeeded());
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(),
FailedWithMessage("DW_CFA_restore_state without a matching "
"previous DW_CFA_remember_state"));
@@ -639,7 +639,7 @@ TEST(DWARFDebugFrame, UnwindTableError_DW_CFA_GNU_window_save) {
Succeeded());
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(),
FailedWithMessage("DW_CFA opcode 0x2d is not supported for "
"architecture x86_64"));
@@ -674,7 +674,7 @@ TEST(DWARFDebugFrame, UnwindTableError_DW_CFA_def_cfa_offset) {
Succeeded());
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(),
FailedWithMessage("DW_CFA_def_cfa_offset found when CFA "
"rule was not RegPlusOffset"));
@@ -709,7 +709,7 @@ TEST(DWARFDebugFrame, UnwindTableDefCFAOffsetSFCFAError) {
Succeeded());
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(),
FailedWithMessage("DW_CFA_def_cfa_offset_sf found when CFA "
"rule was not RegPlusOffset"));
@@ -745,7 +745,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_def_cfa_register) {
EXPECT_THAT_ERROR(parseCFI(TestFDE, {}), Succeeded());
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -817,7 +817,7 @@ TEST(DWARFDebugFrame, UnwindTableRowPushingOpcodes) {
Reg, dwarf::UnwindLocation::createIsRegisterPlusOffset(InReg, 0));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
ASSERT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 6u);
@@ -892,7 +892,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_restore) {
Reg, dwarf::UnwindLocation::createIsRegisterPlusOffset(InReg, 0));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 2u);
@@ -955,7 +955,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_restore_extended) {
Reg, dwarf::UnwindLocation::createIsRegisterPlusOffset(InReg, 0));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 2u);
@@ -1016,7 +1016,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_offset) {
Reg3, dwarf::UnwindLocation::createAtCFAPlusOffset(8));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -1068,7 +1068,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_val_offset) {
Reg2, dwarf::UnwindLocation::createIsCFAPlusOffset(8));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -1113,7 +1113,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_nop) {
Reg1, dwarf::UnwindLocation::createAtCFAPlusOffset(-8));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -1203,7 +1203,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_remember_state) {
Reg3, dwarf::UnwindLocation::createAtCFAPlusOffset(-24));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 5u);
@@ -1270,7 +1270,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_undefined) {
dwarf::UnwindLocation::createUndefined());
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -1314,7 +1314,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_same_value) {
VerifyLocs.setRegisterLocation(Reg1, dwarf::UnwindLocation::createSame());
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -1360,7 +1360,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_register) {
Reg, dwarf::UnwindLocation::createIsRegisterPlusOffset(InReg, 0));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -1412,7 +1412,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_expression) {
Reg, dwarf::UnwindLocation::createAtDWARFExpression(Expr));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -1464,7 +1464,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_val_expression) {
Reg, dwarf::UnwindLocation::createIsDWARFExpression(Expr));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 1u);
@@ -1527,7 +1527,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_def_cfa) {
Reg, dwarf::UnwindLocation::createIsRegisterPlusOffset(InReg, 0));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 5u);
@@ -1625,7 +1625,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_LLVM_def_aspace_cfa) {
Reg, dwarf::UnwindLocation::createIsRegisterPlusOffset(InReg, 0));
// Verify we catch state machine error.
- Expected<dwarf::UnwindTable> RowsOrErr = dwarf::UnwindTable::create(&TestFDE);
+ Expected<dwarf::UnwindTable> RowsOrErr = dwarf::createUnwindTable(&TestFDE);
EXPECT_THAT_ERROR(RowsOrErr.takeError(), Succeeded());
const dwarf::UnwindTable &Rows = RowsOrErr.get();
EXPECT_EQ(Rows.size(), 5u);
|
…bject) For creating new UnwindTable, two static methods was implemented inside it, to create an instance of it from a CIE or FDE. This static methods are moved out of the class as a library functions.
amsen20
force-pushed
the
resolve-unwind-table-deps
branch
from
9544206 to
055492d
Compare
|
Rebased it |
There was a problem hiding this comment.
The old code created an immutable object, which is usually preferable from a design perspective. Would it be better to add a constructor to pass a pre-filled RowContainer instead of a public method that fills the object row by row and which, technically, allows the object to be modified after creation?
| /// Set the last address that this unwinding table refers to. | ||
| /// | ||
| /// This is used when this table is created based on a FDE. | ||
| void setEndAddress(uint64_t Addr) { EndAddress = Addr; } | ||
|
|
There was a problem hiding this comment.
It looks like EndAddress is never used and can be dropped, so you don't need to add a method to set it.
The method So I believe |
Sorry I mistakenly closed it, re-opened it. |
I probably don't see that far, and my review is solely for this (and #142521) change. For now,
A standalone function, yes, that is what I meant. Can we give it a shot? |
Done, please check. |
| LLVM_ABI Expected<UnwindTable::RowContainer> | ||
| parseRows(const CFIProgram &CFIP, UnwindRow &CurrRow, | ||
| const RegisterLocations *InitialLocs); |
There was a problem hiding this comment.
Do you expect any users for this function besides createUnwindTable()? If not, it would be better to move it to the cpp file and make it static. In this case, I would also pass RowContainer as a reference to avoid creating temporary objects.
There was a problem hiding this comment.
Not if we don't look further than the next PR. But I'm going to use it in the analysis.
I can move it there, and when I got to the RFC RPs, move it back.
There was a problem hiding this comment.
No, moving it back and forth is pointless.
Co-authored-by: Igor Kudrin <igor.kudrin@gmail.com>
|
@amsen20 Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
This PR is in the direction of separating Unwind table from FDE, and CIE that depends on llvm/Object. To separate, first the runtime dependencies have to be removed, which this commit does that. This is similar to [1](llvm#140096), [2](llvm#139175), and [3](llvm#139326).
This PR is in the direction of separating Unwind table from FDE, and CIE that depends on llvm/Object. To separate, first the runtime dependencies have to be removed, which this commit does that. This is similar to [1](llvm#140096), [2](llvm#139175), and [3](llvm#139326).
This PR is in the direction of separating Unwind table from FDE, and CIE that depends on llvm/Object.
To separate, first the runtime dependencies have to be removed, which this commit does that.
This is similar to 1, 2, and 3.