clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Glad this is happening. Does this mean we won't see the "bad eh frame" warnings
anymore on linux? See inlined comments.
================
Comment at: include/lldb/Symbol/DWARFCallFrameInfo.h:38
DWARFCallFrameInfo(ObjectFile &objfile, lldb::SectionSP §ion,
lldb::RegisterKind reg_kind, bool is_eh_frame);
----------------
Remove "reg_kind" and "is_eh_frame" and replace with DWARFCallFrameInfo::Type.
See inlined comment below by the CFIVersion enum.
================
Comment at: include/lldb/Symbol/DWARFCallFrameInfo.h:77
enum { CFI_AUG_MAX_SIZE = 8, CFI_HEADER_SIZE = 8 };
+ enum CFIVersion {
+ CFI_VERSION1 = 1, // DWARF v.2
----------------
How about we remove "is_eh_frame" from constructor and make CFIVersion into
CFIType and make it public. We went from 2 flavors (DWARF and EH frame) to now
4, so we should just use an enum to define the exact flavor.
```
enum Type {
EHFrame,
DWARFv2,
DWARFv3,
DWARFv4
};
```
The enum is already in DWARFCallFrameInfo so no need for a prefix.
================
Comment at: source/Symbol/DWARFCallFrameInfo.cpp:306
+ // may ignore these fields?
+ if (!m_is_eh_frame && cie_sp->version >= CFI_VERSION4) {
+ cie_sp->address_size = m_cfi_data.GetU8(&offset);
----------------
Update to use DWARFCallFrameInfo::Type
================
Comment at: source/Symbol/DWARFCallFrameInfo.cpp:315
+ cie_sp->return_addr_reg_num =
+ !m_is_eh_frame && cie_sp->version >= CFI_VERSION3
+ ? static_cast<uint32_t>(m_cfi_data.GetULEB128(&offset))
----------------
Update to use DWARFCallFrameInfo::Type
================
Comment at: source/Symbol/DWARFCallFrameInfo.cpp:485
+ // 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) {
+ auto cie_sp = ParseCIE(current_entry);
----------------
Update to use DWARFCallFrameInfo::Type
================
Comment at: source/Symbol/DWARFCallFrameInfo.cpp:499
+
+ if (!m_is_eh_frame)
+ cie_offset = cie_id;
----------------
Update to use DWARFCallFrameInfo::Type
================
Comment at: source/Symbol/DWARFCallFrameInfo.cpp:567
+ // FDE entries with zeroth cie_offset may occur for debug_frame.
+ assert(!(m_is_eh_frame && 0 == cie_offset) && cie_offset != UINT32_MAX);
----------------
Update to use DWARFCallFrameInfo::Type
================
Comment at: source/Symbol/FuncUnwinders.cpp:219-221
+ // Only supported on x86 architectures where we get debug_frame from the
+ // compiler that describes the prologue instructions perfectly, and sometimes
+ // the epilogue instructions too.
----------------
What compiler suddenly started including complete unwind info in .debug_frame?
They are all supposed, but none ever have. MacOSX x86 and x86_64 has never done
this right. I think this if statement is too inclusive. Can you narrow it down?
I would love to see an example of this in action. I have never seen the x86 PIC
bump code be properly described in any compiler. Are we trying to use this for
unwinding frame zero? I would hope not unless we really and carefully know the
exact compiler that produced the info. It would be preferable marked with an
augmentation code that says "yes, I am really complete everywhere".
================
Comment at: source/Symbol/UnwindTable.cpp:62
+ m_debug_frame_up.reset(
+ new DWARFCallFrameInfo(m_object_file, sect, eRegisterKindDWARF,
false));
+ }
----------------
Update to use DWARFCallFrameInfo::Type
https://reviews.llvm.org/D34613
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
