[DWARFCallFrameInfo] Add Type enum to differentiate eh/debug_frame sections

[labath]

Summary

instead of using a boolean to differentiate between the two section
types, use an enum to make the intent clearer.

I also remove the RegisterKind argument from the constructor, as this
can be deduced from the Type argument.

[clayborg]

Looks good.

[tatyana-krasnukha]

Saying about clear intent, it would be even much more better if class name doesn't start with DWARF ;)

[labath]

In D34681#792602, @tatyana-krasnukha wrote:

Saying about clear intent, it would be even much more better if class name doesn't start with DWARF ;)

That's probably true. :) However, I think I've done enough of renaming of files lately that I will pass on this one for now.

lldb/trunk/include/lldb/Symbol/DWARFCallFrameInfo.h

 // Call Frame Information FDEs.  It stores little information internally.
 // Only two APIs are exported - one to find the high/low pc values
 // of a function given a text address via the information in the
 // eh_frame / debug_frame, and one to generate an UnwindPlan based
 // on the FDE in the eh_frame / debug_frame section.
 
 class DWARFCallFrameInfo {
 public:
-  DWARFCallFrameInfo(ObjectFile &objfile, lldb::SectionSP &section,
-                     lldb::RegisterKind reg_kind, bool is_eh_frame);
+  enum Type { EH, DWARF };
+
+  DWARFCallFrameInfo(ObjectFile &objfile, lldb::SectionSP &section, Type type);
 
   ~DWARFCallFrameInfo() = default;
 
   // Locate an AddressRange that includes the provided Address in this
   // object's eh_frame/debug_info
   // Returns true if a range is found to cover that address.
   bool GetAddressRange(Address addr, AddressRange &range);
 
private:
   // CIE and in FDE).
   // Returns true if the opcode is handled and false otherwise.
   bool HandleCommonDwarfOpcode(uint8_t primary_opcode, uint8_t extended_opcode,
                                int32_t data_align, lldb::offset_t &offset,
                                UnwindPlan::Row &row);
 
   ObjectFile &m_objfile;
   lldb::SectionSP m_section_sp;
-  lldb::RegisterKind m_reg_kind;
-  Flags m_flags;
+  Flags m_flags = 0;
   cie_map_t m_cie_map;
 
   DataExtractor m_cfi_data;
-  bool m_cfi_data_initialized; // only copy the section into the DE once
+  bool m_cfi_data_initialized = false; // only copy the section into the DE once
 
   FDEEntryMap m_fde_index;
-  bool m_fde_index_initialized; // only scan the section for FDEs once
+  bool m_fde_index_initialized = false; // only scan the section for FDEs once
   std::mutex m_fde_index_mutex; // and isolate the thread that does it
 
-  bool m_is_eh_frame;
+  Type m_type;
 
   CIESP
   ParseCIE(const uint32_t cie_offset);
+
+  lldb::RegisterKind GetRegisterKind() const {
+    return m_type == EH ? lldb::eRegisterKindEHFrame : lldb::eRegisterKindDWARF;
+  }
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_DWARFCallFrameInfo_h_

lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

if (text_section_sp && function_starts_data.GetByteSize()) {
       // but it doesn't actually have the eh_frame content.  And if we have a
       // dSYM, we don't need to do any
       // of this fill-in-the-missing-symbols works anyway - the debug info
       // should give us all the functions in
       // the module.
       if (text_section_sp.get() && eh_frame_section_sp.get() &&
           m_type != eTypeDebugInfo) {
         DWARFCallFrameInfo eh_frame(*this, eh_frame_section_sp,
-                                    eRegisterKindEHFrame, true);
+                                    DWARFCallFrameInfo::EH);
         DWARFCallFrameInfo::FunctionAddressAndSizeVector functions;
         eh_frame.GetFunctionAddressAndSizeVector(functions);
         addr_t text_base_addr = text_section_sp->GetFileAddress();
         size_t count = functions.GetSize();
         for (size_t i = 0; i < count; ++i) {
           const DWARFCallFrameInfo::FunctionAddressAndSizeVector::Entry *func =
               functions.GetEntryAtIndex(i);
           if (func) {

lldb/trunk/source/Symbol/DWARFCallFrameInfo.cpp

if (sign_bit & addressValue) {
       uint64_t mask = ~sign_bit + 1;
       addressValue |= mask;
     }
   }
   return baseAddress + addressValue;
 }
 
 DWARFCallFrameInfo::DWARFCallFrameInfo(ObjectFile &objfile,
-                                       SectionSP &section_sp,
-                                       lldb::RegisterKind reg_kind,
-                                       bool is_eh_frame)
-    : m_objfile(objfile), m_section_sp(section_sp),
-      m_reg_kind(reg_kind), // The flavor of registers that the CFI data uses
-                            // (enum RegisterKind)
-      m_flags(), m_cie_map(), m_cfi_data(), m_cfi_data_initialized(false),
-      m_fde_index(), m_fde_index_initialized(false),
-      m_is_eh_frame(is_eh_frame) {}
+                                       SectionSP &section_sp, Type type)
+    : m_objfile(objfile), m_section_sp(section_sp), m_type(type) {}
 
 bool DWARFCallFrameInfo::GetUnwindPlan(Address addr, UnwindPlan &unwind_plan) {
   FDEEntryMap::Entry fde_entry;
 
   // Make sure that the Address we're searching for is the same object file
   // as this DWARFCallFrameInfo, we only store File offsets in m_fde_index.
   ModuleSP module_sp = addr.GetModule();
   if (module_sp.get() == nullptr || module_sp->GetObjectFile() == nullptr ||
DWARFCallFrameInfo::ParseCIE(const dw_offset_t cie_offset) {
   if (is_64bit) {
     length = m_cfi_data.GetU64(&offset);
     cie_id = m_cfi_data.GetU64(&offset);
     end_offset = cie_offset + length + 12;
   } else {
     cie_id = m_cfi_data.GetU32(&offset);
     end_offset = cie_offset + length + 4;
   }
-  if (length > 0 && ((!m_is_eh_frame && cie_id == UINT32_MAX) ||
-                     (m_is_eh_frame && cie_id == 0ul))) {
+  if (length > 0 && ((m_type == DWARF && cie_id == UINT32_MAX) ||
+                     (m_type == EH && cie_id == 0ul))) {
     size_t i;
     //    cie.offset = cie_offset;
     //    cie.length = length;
     //    cie.cieID = cieID;
     cie_sp->ptr_encoding = DW_EH_PE_absptr; // default
     cie_sp->version = m_cfi_data.GetU8(&offset);
     if (cie_sp->version > CFI_VERSION4) {
       Host::SystemLog(Host::eSystemLogError,
if (i == CFI_AUG_MAX_SIZE &&
                       "CIE parse error: CIE augmentation string was too large "
                       "for the fixed sized buffer of %d bytes.\n",
                       CFI_AUG_MAX_SIZE);
       return nullptr;
     }
 
     // m_cfi_data uses address size from target architecture of the process
     // may ignore these fields?
-    if (!m_is_eh_frame && cie_sp->version >= CFI_VERSION4) {
+    if (m_type == DWARF && cie_sp->version >= CFI_VERSION4) {
       cie_sp->address_size = m_cfi_data.GetU8(&offset);
       cie_sp->segment_size = m_cfi_data.GetU8(&offset);
     }
 
     cie_sp->code_align = (uint32_t)m_cfi_data.GetULEB128(&offset);
     cie_sp->data_align = (int32_t)m_cfi_data.GetSLEB128(&offset);
 
     cie_sp->return_addr_reg_num =
-        !m_is_eh_frame && cie_sp->version >= CFI_VERSION3
+        m_type == DWARF && cie_sp->version >= CFI_VERSION3
             ? static_cast<uint32_t>(m_cfi_data.GetULEB128(&offset))
             : m_cfi_data.GetU8(&offset);
 
     if (cie_sp->augmentation[0]) {
       // Get the length of the eh_frame augmentation data
       // which starts with a ULEB128 length in bytes
       const size_t aug_data_len = (size_t)m_cfi_data.GetULEB128(&offset);
       const size_t aug_data_end = offset + aug_data_len;
if (next_entry > m_cfi_data.GetByteSize() + 1) {
       m_fde_index_initialized = true;
       return;
     }
 
     // An FDE entry contains CIE_pointer in debug_frame in same place as cie_id
     // in eh_frame. CIE_pointer is an offset into the .debug_frame section.
     // So, variable cie_offset should be equal to cie_id for debug_frame.
     // FDE entries with cie_id == 0 shouldn't be ignored for it.
-    if ((cie_id == 0 && m_is_eh_frame) || cie_id == UINT32_MAX || len == 0) {
+    if ((cie_id == 0 && m_type == EH) || cie_id == UINT32_MAX || len == 0) {
       auto cie_sp = ParseCIE(current_entry);
       if (!cie_sp) {
         // Cannot parse, the reason is already logged
         m_fde_index.Clear();
         m_fde_index_initialized = true;
         return;
       }
 
       m_cie_map[current_entry] = std::move(cie_sp);
       offset = next_entry;
       continue;
     }
 
-    if (!m_is_eh_frame)
+    if (m_type == DWARF)
       cie_offset = cie_id;
 
     if (cie_offset > m_cfi_data.GetByteSize()) {
       Host::SystemLog(Host::eSystemLogError,
                       "error: Invalid cie offset of 0x%x "
                       "found in cie/fde at 0x%x\n",
                       cie_offset, current_entry);
       // Don't trust anything in this eh_frame section if we find blatantly
bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset,
   if (is_64bit) {
     length = m_cfi_data.GetU64(&offset);
     cie_offset = m_cfi_data.GetU64(&offset);
   } else {
     cie_offset = m_cfi_data.GetU32(&offset);
   }
 
   // FDE entries with zeroth cie_offset may occur for debug_frame.
-  assert(!(m_is_eh_frame && 0 == cie_offset) && cie_offset != UINT32_MAX);
+  assert(!(m_type == EH && 0 == cie_offset) && cie_offset != UINT32_MAX);
 
   // Translate the CIE_id from the eh_frame format, which
   // is relative to the FDE offset, into a __eh_frame section
   // offset
-  if (m_is_eh_frame) {
+  if (m_type == EH) {
     unwind_plan.SetSourceName("eh_frame CFI");
     cie_offset = current_entry + (is_64bit ? 12 : 4) - cie_offset;
     unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo);
   } else {
     unwind_plan.SetSourceName("DWARF CFI");
     // In theory the debug_frame info should be valid at all call sites
     // ("asynchronous unwind info" as it is sometimes called) but in practice
     // gcc et al all emit call frame info for the prologue and call sites, but
bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset,
   uint32_t code_align = cie->code_align;
   int32_t data_align = cie->data_align;
 
   unwind_plan.SetPlanValidAddressRange(range);
   UnwindPlan::Row *cie_initial_row = new UnwindPlan::Row;
   *cie_initial_row = cie->initial_row;
   UnwindPlan::RowSP row(cie_initial_row);
 
-  unwind_plan.SetRegisterKind(m_reg_kind);
+  unwind_plan.SetRegisterKind(GetRegisterKind());
   unwind_plan.SetReturnAddressRegister(cie->return_addr_reg_num);
 
   std::vector<UnwindPlan::RowSP> stack;
 
   UnwindPlan::Row::RegisterLocation reg_location;
   while (m_cfi_data.ValidOffset(offset) && offset < end_offset) {
     uint8_t inst = m_cfi_data.GetU8(&offset);
     uint8_t primary_opcode = inst & 0xC0;

lldb/trunk/source/Symbol/UnwindTable.cpp

void UnwindTable::Initialize() {
   m_initialized = true;
 
   SectionList *sl = m_object_file.GetSectionList();
   if (!sl)
     return;
 
   SectionSP sect = sl->FindSectionByType(eSectionTypeEHFrame, true);
   if (sect.get()) {
-    m_eh_frame_up.reset(new DWARFCallFrameInfo(m_object_file, sect,
-                                               eRegisterKindEHFrame, true));
+    m_eh_frame_up.reset(
+        new DWARFCallFrameInfo(m_object_file, sect, DWARFCallFrameInfo::EH));
   }
 
   sect = sl->FindSectionByType(eSectionTypeDWARFDebugFrame, true);
   if (sect) {
     m_debug_frame_up.reset(
-        new DWARFCallFrameInfo(m_object_file, sect, eRegisterKindDWARF, false));
+        new DWARFCallFrameInfo(m_object_file, sect, DWARFCallFrameInfo::DWARF));
   }
 
   sect = sl->FindSectionByType(eSectionTypeCompactUnwind, true);
   if (sect) {
     m_compact_unwind_up.reset(new CompactUnwindInfo(m_object_file, sect));
   }
 
   sect = sl->FindSectionByType(eSectionTypeARMexidx, true);

lldb/trunk/unittests/Symbol/TestDWARFCallFrameInfo.cpp

public:
   void TearDown() override {
     ObjectFileELF::Terminate();
     HostInfo::Terminate();
   }
 
 protected:
   llvm::SmallString<128> m_inputs_folder;
 
-  void TestBasic(bool eh_frame, llvm::StringRef symbol);
+  void TestBasic(DWARFCallFrameInfo::Type type, llvm::StringRef symbol);
 };
 
 #define ASSERT_NO_ERROR(x)                                                     \
   if (std::error_code ASSERT_NO_ERROR_ec = x) {                                \
     llvm::SmallString<128> MessageStorage;                                     \
     llvm::raw_svector_ostream Message(MessageStorage);                         \
     Message << #x ": did not return errc::success.\n"                          \
             << "error number: " << ASSERT_NO_ERROR_ec.value() << "\n"          \
static UnwindPlan::Row GetExpectedRow2() {
   UnwindPlan::Row row;
   row.SetOffset(4);
   row.GetCFAValue().SetIsRegisterPlusOffset(dwarf_rbp_x86_64, 16);
   row.SetRegisterLocationToAtCFAPlusOffset(dwarf_rip_x86_64, -8, false);
   row.SetRegisterLocationToAtCFAPlusOffset(dwarf_rbp_x86_64, -16, false);
   return row;
 }
 
-void DWARFCallFrameInfoTest::TestBasic(bool eh_frame, llvm::StringRef symbol) {
+void DWARFCallFrameInfoTest::TestBasic(DWARFCallFrameInfo::Type type,
+                                       llvm::StringRef symbol) {
   llvm::SmallString<128> yaml = m_inputs_folder;
   llvm::sys::path::append(yaml, "basic-call-frame-info.yaml");
   llvm::SmallString<128> obj = m_inputs_folder;
 
   ASSERT_NO_ERROR(llvm::sys::fs::createTemporaryFile(
       "basic-call-frame-info-%%%%%%", "obj", obj));
   llvm::FileRemover obj_remover(obj);
 
   const char *args[] = {YAML2OBJ, yaml.c_str(), nullptr};
   llvm::StringRef obj_ref = obj;
   const llvm::StringRef *redirects[] = {nullptr, &obj_ref, nullptr};
   ASSERT_EQ(0, llvm::sys::ExecuteAndWait(YAML2OBJ, args, nullptr, redirects));
 
   uint64_t size;
   ASSERT_NO_ERROR(llvm::sys::fs::file_size(obj, size));
   ASSERT_GT(size, 0u);
 
   auto module_sp = std::make_shared<Module>(ModuleSpec(FileSpec(obj, false)));
   SectionList *list = module_sp->GetSectionList();
   ASSERT_NE(nullptr, list);
 
-  auto section_sp = list->FindSectionByType(
-      eh_frame ? eSectionTypeEHFrame : eSectionTypeDWARFDebugFrame, false);
+  auto section_sp = list->FindSectionByType(type == DWARFCallFrameInfo::EH
+                                                ? eSectionTypeEHFrame
+                                                : eSectionTypeDWARFDebugFrame,
+                                            false);
   ASSERT_NE(nullptr, section_sp);
 
-  DWARFCallFrameInfo cfi(*module_sp->GetObjectFile(), section_sp,
-                         eh_frame ? eRegisterKindEHFrame : eRegisterKindDWARF,
-                         eh_frame);
+  DWARFCallFrameInfo cfi(*module_sp->GetObjectFile(), section_sp, type);
 
   const Symbol *sym = module_sp->FindFirstSymbolWithNameAndType(
       ConstString(symbol), eSymbolTypeAny);
   ASSERT_NE(nullptr, sym);
 
   UnwindPlan plan(eRegisterKindGeneric);
   ASSERT_TRUE(cfi.GetUnwindPlan(sym->GetAddress(), plan));
   ASSERT_EQ(3, plan.GetRowCount());
   EXPECT_EQ(GetExpectedRow0(), *plan.GetRowAtIndex(0));
   EXPECT_EQ(GetExpectedRow1(), *plan.GetRowAtIndex(1));
   EXPECT_EQ(GetExpectedRow2(), *plan.GetRowAtIndex(2));
 }
 
 TEST_F(DWARFCallFrameInfoTest, Basic_dwarf3) {
-  TestBasic(false, "debug_frame3");
+  TestBasic(DWARFCallFrameInfo::DWARF, "debug_frame3");
 }
 
 TEST_F(DWARFCallFrameInfoTest, Basic_dwarf4) {
-  TestBasic(false, "debug_frame4");
+  TestBasic(DWARFCallFrameInfo::DWARF, "debug_frame4");
 }
 
-TEST_F(DWARFCallFrameInfoTest, Basic_eh) { TestBasic(true, "eh_frame"); }
+TEST_F(DWARFCallFrameInfoTest, Basic_eh) {
+  TestBasic(DWARFCallFrameInfo::EH, "eh_frame");
+}