[llvm] 7cf9934 - [DWARF] Store CFA value on DW_CFA_remember_state

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 4 13:51:19 PST 2023


Author: Alexis Engelke
Date: 2023-01-04T13:51:14-08:00
New Revision: 7cf99347d4f4341b2f8a53eaaed6f0e137b30869

URL: https://github.com/llvm/llvm-project/commit/7cf99347d4f4341b2f8a53eaaed6f0e137b30869
DIFF: https://github.com/llvm/llvm-project/commit/7cf99347d4f4341b2f8a53eaaed6f0e137b30869.diff

LOG: [DWARF] Store CFA value on DW_CFA_remember_state

Previously, CFA_remember_state stored only the register locations but ignored the CFA value. This needs also to be remembered and restored for correct behavior. The problem occurs, e.g., on functions with multiple epilogues, where the CFA value after the first epilogue is becomes wrong.

Reviewed By: #debug-info, MaskRay

Differential Revision: https://reviews.llvm.org/D140338

Added: 
    

Modified: 
    llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
    llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
index 1b71f4b7dab70..aae1668c1639c 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
@@ -514,7 +514,8 @@ CFIProgram::Instruction::getOperandAsSigned(const CFIProgram &CFIP,
 
 Error UnwindTable::parseRows(const CFIProgram &CFIP, UnwindRow &Row,
                              const RegisterLocations *InitialLocs) {
-  std::vector<RegisterLocations> RegisterStates;
+  // State consists of CFA value and register locations.
+  std::vector<std::pair<UnwindLocation, RegisterLocations>> States;
   for (const CFIProgram::Instruction &Inst : CFIP) {
     switch (Inst.Opcode) {
     case dwarf::DW_CFA_set_loc: {
@@ -597,16 +598,18 @@ Error UnwindTable::parseRows(const CFIProgram &CFIP, UnwindRow &Row,
       break;
 
     case dwarf::DW_CFA_remember_state:
-      RegisterStates.push_back(Row.getRegisterLocations());
+      States.push_back(
+          std::make_pair(Row.getCFAValue(), Row.getRegisterLocations()));
       break;
 
     case dwarf::DW_CFA_restore_state:
-      if (RegisterStates.empty())
+      if (States.empty())
         return createStringError(errc::invalid_argument,
                                  "DW_CFA_restore_state without a matching "
                                  "previous DW_CFA_remember_state");
-      Row.getRegisterLocations() = RegisterStates.back();
-      RegisterStates.pop_back();
+      Row.getCFAValue() = States.back().first;
+      Row.getRegisterLocations() = States.back().second;
+      States.pop_back();
       break;
 
     case dwarf::DW_CFA_GNU_window_save:

diff  --git a/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
index ebfba7c1ee6d8..d96553174536e 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
@@ -1143,16 +1143,22 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_remember_state) {
 
   // Make a CIE that has a valid CFA definition and a single register unwind
   // rule for register that we will verify is in all of the pushed rows.
-  EXPECT_THAT_ERROR(parseCFI(TestCIE, {dwarf::DW_CFA_def_cfa, 12, 32}),
+  constexpr uint8_t CFAOff1 = 32;
+  constexpr uint8_t CFAOff2 = 16;
+  constexpr uint8_t Reg1 = 14;
+  constexpr uint8_t Reg2 = 15;
+  constexpr uint8_t Reg3 = 16;
+
+  EXPECT_THAT_ERROR(parseCFI(TestCIE, {dwarf::DW_CFA_def_cfa, 12, CFAOff1}),
                     Succeeded());
 
   // Make a FDE with DWARF call frame instruction opcodes that encodes the
   // follwing rows:
-  // 0x1000: CFA=reg12+32: Reg1=[CFA-8]
-  // 0x1004: CFA=reg12+32: Reg1=[CFA-8] Reg2=[CFA-16]
-  // 0x1008: CFA=reg12+32: Reg1=[CFA-8] Reg2=[CFA-16] Reg3=[CFA-24]
-  // 0x100C: CFA=reg12+32: Reg1=[CFA-8] Reg2=[CFA-16]
-  // 0x1010: CFA=reg12+32: Reg1=[CFA-8]
+  // 0x1000: CFA=reg12+CFAOff1: Reg1=[CFA-8]
+  // 0x1004: CFA=reg12+CFAOff1: Reg1=[CFA-8] Reg2=[CFA-16]
+  // 0x1008: CFA=reg12+CFAOff2: Reg1=[CFA-8] Reg2=[CFA-16] Reg3=[CFA-24]
+  // 0x100C: CFA=reg12+CFAOff1: Reg1=[CFA-8] Reg2=[CFA-16]
+  // 0x1010: CFA=reg12+CFAOff1: Reg1=[CFA-8]
   // This state machine will:
   //  - set Reg1 location
   //  - push a row (from DW_CFA_advance_loc)
@@ -1160,6 +1166,7 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_remember_state) {
   //  - set Reg2 location
   //  - push a row (from DW_CFA_advance_loc)
   //  - remember the state
+  //  - set CFA offset to CFAOff2
   //  - set Reg3 location
   //  - push a row (from DW_CFA_advance_loc)
   //  - remember the state where Reg1 and Reg2 were set
@@ -1167,14 +1174,12 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_remember_state) {
   //  - remember the state where only Reg1 was set
   //  - push a row (automatically at the end of instruction parsing)
   // Then we verify that all registers are correct in all generated rows.
-  constexpr uint8_t Reg1 = 14;
-  constexpr uint8_t Reg2 = 15;
-  constexpr uint8_t Reg3 = 16;
   EXPECT_THAT_ERROR(
       parseCFI(TestFDE,
                {dwarf::DW_CFA_offset | Reg1, 1, dwarf::DW_CFA_advance_loc | 4,
                 dwarf::DW_CFA_remember_state, dwarf::DW_CFA_offset | Reg2, 2,
                 dwarf::DW_CFA_advance_loc | 4, dwarf::DW_CFA_remember_state,
+                dwarf::DW_CFA_def_cfa_offset, CFAOff2,
                 dwarf::DW_CFA_offset | Reg3, 3, dwarf::DW_CFA_advance_loc | 4,
                 dwarf::DW_CFA_restore_state, dwarf::DW_CFA_advance_loc | 4,
                 dwarf::DW_CFA_restore_state}),
@@ -1206,18 +1211,28 @@ TEST(DWARFDebugFrame, UnwindTable_DW_CFA_remember_state) {
   const dwarf::UnwindTable &Rows = RowsOrErr.get();
   EXPECT_EQ(Rows.size(), 5u);
   EXPECT_EQ(Rows[0].getAddress(), 0x1000u);
+  EXPECT_EQ(Rows[0].getCFAValue(),
+            dwarf::UnwindLocation::createIsRegisterPlusOffset(12, CFAOff1));
   EXPECT_EQ(Rows[0].getRegisterLocations(), VerifyLocs1);
 
   EXPECT_EQ(Rows[1].getAddress(), 0x1004u);
+  EXPECT_EQ(Rows[1].getCFAValue(),
+            dwarf::UnwindLocation::createIsRegisterPlusOffset(12, CFAOff1));
   EXPECT_EQ(Rows[1].getRegisterLocations(), VerifyLocs2);
 
   EXPECT_EQ(Rows[2].getAddress(), 0x1008u);
+  EXPECT_EQ(Rows[2].getCFAValue(),
+            dwarf::UnwindLocation::createIsRegisterPlusOffset(12, CFAOff2));
   EXPECT_EQ(Rows[2].getRegisterLocations(), VerifyLocs3);
 
   EXPECT_EQ(Rows[3].getAddress(), 0x100Cu);
+  EXPECT_EQ(Rows[3].getCFAValue(),
+            dwarf::UnwindLocation::createIsRegisterPlusOffset(12, CFAOff1));
   EXPECT_EQ(Rows[3].getRegisterLocations(), VerifyLocs2);
 
   EXPECT_EQ(Rows[4].getAddress(), 0x1010u);
+  EXPECT_EQ(Rows[4].getCFAValue(),
+            dwarf::UnwindLocation::createIsRegisterPlusOffset(12, CFAOff1));
   EXPECT_EQ(Rows[4].getRegisterLocations(), VerifyLocs1);
 }
 


        


More information about the llvm-commits mailing list