clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
You changed the API to return a pointer which can be NULL so we either need to
check all returns for this or change the API to return a reference to a default
constructed UnwindTable. I like the latter option, but I will leave that up to
you. Since we are moving functionality into symbol files, we might want to opt
for the latter to allow us to populate the UnwindTable as needed from any
source when ever that source becomes available (like adding a symbol file to a
module after the fact).
================
Comment at: include/lldb/Core/Module.h:709
+ //------------------------------------------------------------------
+ UnwindTable *GetUnwindTable();
+
----------------
I would vote to return a "UnwindTable&" and make a const and non const version
of this. Otherwise we have to NULL check any call to this API.
================
Comment at: include/lldb/Core/Module.h:1108-1110
+ llvm::Optional<UnwindTable> m_unwind_table; /// < Table of FuncUnwinders
+ /// objects created for this
+ /// Module's functions
----------------
Why not just make this an instance to avoid having to null check all accessor
calls?:
```
UnwindTable m_unwind_table;
```
================
Comment at: source/Commands/CommandObjectTarget.cpp:3536-3537
FuncUnwindersSP func_unwinders_sp(
- sc.module_sp->GetObjectFile()
- ->GetUnwindTable()
- .GetUncachedFuncUnwindersContainingAddress(start_addr, sc));
+ sc.module_sp->GetUnwindTable()
+ ->GetUncachedFuncUnwindersContainingAddress(start_addr, sc));
if (!func_unwinders_sp)
----------------
Either NULL check or change the API?
================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2865-2866
- DWARFCallFrameInfo *eh_frame = GetUnwindTable().GetEHFrameInfo();
- if (eh_frame) {
+ if (DWARFCallFrameInfo *eh_frame =
+ GetModule()->GetUnwindTable()->GetEHFrameInfo()) {
if (m_symtab_ap == nullptr)
----------------
Either NULL check or change the API?
================
Comment at: source/Plugins/Process/Utility/RegisterContextLLDB.cpp:242-243
func_unwinders_sp =
- pc_module_sp->GetObjectFile()
- ->GetUnwindTable()
- .GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx);
+ pc_module_sp->GetUnwindTable()->GetFuncUnwindersContainingAddress(
+ m_current_pc, m_sym_ctx);
}
----------------
Either NULL check or change the API?
================
Comment at: source/Plugins/Process/Utility/RegisterContextLLDB.cpp:674-675
FuncUnwindersSP func_unwinders_sp(
- pc_module_sp->GetObjectFile()
- ->GetUnwindTable()
- .GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx));
+ pc_module_sp->GetUnwindTable()->GetFuncUnwindersContainingAddress(
+ m_current_pc, m_sym_ctx));
if (!func_unwinders_sp)
----------------
Either NULL check or change the API?
================
Comment at: source/Plugins/Process/Utility/RegisterContextLLDB.cpp:776-777
func_unwinders_sp =
- pc_module_sp->GetObjectFile()
- ->GetUnwindTable()
- .GetFuncUnwindersContainingAddress(m_current_pc, m_sym_ctx);
+ pc_module_sp->GetUnwindTable()->GetFuncUnwindersContainingAddress(
+ m_current_pc, m_sym_ctx);
}
----------------
Either NULL check or change the API?
================
Comment at: source/Plugins/Process/Utility/RegisterContextLLDB.cpp:795
DWARFCallFrameInfo *eh_frame =
- pc_module_sp->GetObjectFile()->GetUnwindTable().GetEHFrameInfo();
+ pc_module_sp->GetUnwindTable()->GetEHFrameInfo();
if (eh_frame) {
----------------
Either NULL check or change the API?
================
Comment at: source/Plugins/Process/Utility/RegisterContextLLDB.cpp:805
ArmUnwindInfo *arm_exidx =
- pc_module_sp->GetObjectFile()->GetUnwindTable().GetArmUnwindInfo();
+ pc_module_sp->GetUnwindTable()->GetArmUnwindInfo();
if (arm_exidx) {
----------------
Either NULL check or change the API?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58129/new/
https://reviews.llvm.org/D58129
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
