clayborg added inline comments.
================
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)
----------------
JosephTremoulet wrote:
> clayborg wrote:
> > 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);
> >
> > }
> > ```
> Sure, will update. Just to double-check a couple points:
>
> # I'll actually want to pass `&addr_range` at all three callsites, rather
> than passing `nullptr` at two of them, correct? (at the callsite on 154 to
> preserve the `addr_range` previously defined on 175, and at the callsite on
> 431to preserve the `addr_range` previously defined on 470)
>
> # I see you removed the `resolve_scope & resolved_scope` check. Am I
> correct that that was intentional and it's redundant with the checks in
> `GetAddressRange` that require `function` or `symbol` to be non-null, or is
> there something more subtle going on here?
>
> Thanks.
For 1) above, it seems like you were making a new local variable just so you
could pass it. If it is actually used somewhere, then yes, do include it. It
seemed like you were just making a new local for no reason.
For 2) above: You can restore the resolve_scope & resolved_scope if needed.
Can't remember if we will return eSymbolContextModule if we fail to find
eSymbolContextFunction or eSymbolContextSymbol. So problably safest to restore
that so we don't change functionality
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64993/new/
https://reviews.llvm.org/D64993
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
