clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
I will let Jason comment on the unwind specifics since this is his area. I
caught a few other things that need to be cleaned up.
================
Comment at: lldb/include/lldb/Target/Unwind.h:40
uint32_t idx;
+ bool behaves_like_zeroth_frame;
----------------
initialize with value just in case.
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:153-155
+ AddressRange addr_range;
+ m_sym_ctx_valid = TryResolveSymbolContextAndAddressRange(
+ m_current_pc, m_sym_ctx, addr_range);
----------------
See comment for RegisterContextLLDB::TryResolveSymbolContextAndAddressRange,
but if we change as suggested this will become:
```
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, nullptr);
```
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:430-432
+ AddressRange addr_range;
+ m_sym_ctx_valid = TryResolveSymbolContextAndAddressRange(
+ m_current_pc, m_sym_ctx, addr_range);
----------------
See comment for RegisterContextLLDB::TryResolveSymbolContextAndAddressRange,
but if we change as suggested this will become:
```
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, nullptr);
```
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:486-487
m_sym_ctx.Clear(false);
- m_sym_ctx_valid = false;
- SymbolContextItem resolve_scope =
- eSymbolContextFunction | eSymbolContextSymbol;
-
- ModuleSP temporary_module_sp = temporary_pc.GetModule();
- if (temporary_module_sp &&
- temporary_module_sp->ResolveSymbolContextForAddress(
- temporary_pc, resolve_scope, m_sym_ctx) &
- resolve_scope) {
- if (m_sym_ctx.GetAddressRange(resolve_scope, 0, false, addr_range))
- m_sym_ctx_valid = true;
- }
+ m_sym_ctx_valid = TryResolveSymbolContextAndAddressRange(
+ temporary_pc, m_sym_ctx, addr_range);
+
----------------
See comment for RegisterContextLLDB::TryResolveSymbolContextAndAddressRange,
but if we change as suggested this will become:
```
m_sym_ctx_valid = m_current_pc.ResolveFunctionScope(m_sym_ctx, &addr_range);
```
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:595-624
+bool RegisterContextLLDB::TryResolveSymbolContextAndAddressRange(
+ lldb_private::Address pc, lldb_private::SymbolContext &sym_ctx,
+ lldb_private::AddressRange &addr_range) {
+ ModuleSP pc_module_sp = pc.GetModule();
+
+ // Can't resolve context without a module.
+ if (!pc_module_sp)
----------------
This function doesn't belong in RegisterContextLLDB. It think
lldb_private::Address would be a better place:
```
/// if "addr_range_ptr" is not NULL, then fill in with the address range of the
function.
bool lldb_private::Address::ResolveFunctionScope(lldb_private::SymbolContext
&sym_ctx, lldb_private::AddressRange *addr_range_ptr) {
constexpr bool resolve_tail_call_address = false;
constexpr SymbolContextItem resolve_scope = eSymbolContextFunction |
eSymbolContextSymbol;
if (!CalculateSymbolContext(&sym_ctx, resolve_scope))
return false;
if (!addr_range_ptr)
return true;
return sym_ctx.GetAddressRange(resolve_scope, 0, false, *addr_range_ptr);
}
```
================
Comment at: lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp:1760
+void RegisterContextLLDB::PropagateTrapHandlerFlag(
+ lldb::UnwindPlanSP unwind_plan) {
+ if (unwind_plan->GetUnwindPlanForSignalTrap() != eLazyBoolYes) {
----------------
JosephTremoulet wrote:
> I'm a bit ambivalent about adding this part -- the downside is that it's not
> concretely helping today, because if an eh_frame record has the 'S' flag in
> its augmentation (which is what `unwind_Plan->GetUnwindPlanForSignalTrap()`
> reports), we'll have already decremented pc and generated unwind plans based
> on the prior function when initializing the register context. But the upside
> is that this connects the dots between the otherwise-unused bit on the unwind
> plan and the frame type, and will be in place should we subsequently add code
> to try the second function's unwind plan as a fallback.
I will let Jason comment on this one.
================
Comment at: lldb/source/Target/StackFrameList.cpp:453
lldb::addr_t cfa = LLDB_INVALID_ADDRESS;
+ bool behaves_like_zeroth_frame;
if (idx == 0) {
----------------
init with value
================
Comment at: lldb/source/Target/StackFrameList.cpp:670
addr_t pc, cfa;
- if (unwinder->GetFrameInfoAtIndex(idx, cfa, pc)) {
+ bool behaves_like_zeroth_frame;
+ if (unwinder->GetFrameInfoAtIndex(idx, cfa, pc,
----------------
init with value
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64993/new/
https://reviews.llvm.org/D64993
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
