jasonmolenda added inline comments.
================
Comment at: lldb/source/Target/RegisterContextUnwind.cpp:642-643
m_full_unwind_plan_sp = GetFullUnwindPlanForFrame();
int valid_offset = -1;
if (IsUnwindPlanValidForCurrentPC(m_full_unwind_plan_sp, valid_offset)) {
+ active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset(
----------------
clayborg wrote:
> jasonmolenda wrote:
> > clayborg wrote:
> > > remove "valid_offset" variable and also remove it from the second
> > > argument to IsUnwindPlanValidForCurrentPC as it isn't used anywhere now
> > > and is just dead code.
> > hm, this merits consideration more widely; I looked at all callers to
> > IsUnwindPlanValidForCurrentPC and none of them use the valid_offset that it
> > provides; they are all merely checking that the offset is covered by the
> > UnwindPlan's byte size. I suspect none of these are actually necessary; we
> > picked the unwind plan based on symbol name so you'd need a "symbol" that
> > has a large byte size, but an UnwindPlan that was sourced from some input
> > that limited the byte size range covered. idk, it might be possible tho,
> > especially in a stripped binary.
> >
> > I'll change all of the callers to not use the returned valid_offset for now.
> If it is actually being used somewhere, then that is fine. My search of the
> code showed 3 locations, and this location was the only one that was using
> it, but now it isn't. Are there other locations I missed?
I wrote that poorly. I meant to say that no one was using the valid_offset
returned value, so I would change the method to not return it. I uploaded an
updated patch showing what I mean here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124957/new/
https://reviews.llvm.org/D124957
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
