If the swap is correct, then wouldn't we also need to swap the variable
list?
On Mon, Nov 14, 2016 at 10:58 AM Jim Ingham <jing...@apple.com> wrote:
>
> > On Nov 13, 2016, at 4:48 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> > I was going through doing some routine StringRef changes and I ran
> across this function:
> >
> > std::lock_guard<std::recursive_mutex> guard(m_mutex);
> > assert(GetStackID() ==
> > prev_frame.GetStackID()); // TODO: remove this after some
> testing
> > m_variable_list_sp = prev_frame.m_variable_list_sp;
> >
> m_variable_list_value_objects.Swap(prev_frame.m_variable_list_value_objects);
> > if (!m_disassembly.GetString().empty()) {
> > m_disassembly.Clear();
> > m_disassembly.GetString().swap(m_disassembly.GetString());
> > }
> >
> > Either I'm crazy or that bolded line is a bug. Is it supposed to be
> prev_frame.m_disassembly.GetString()?
> >
> > What would the implications of this bug be? i.e. how can we write a
> test for this?
> >
> > Also, as a matter of curiosity, why is it swapping? That means it's
> modifying the input frame, when it seems like it really should just be
> modifying the current frame.
>
> What lldb does is store the stack frame list it calculated from a previous
> stop, and copy as much as is relevant into the new stack frame when it
> stops, which will then become the stack frame list that gets used. So this
> is a transfer of information from the older stop's stack frame to the new
> one. Thus the swap.
>
> To be clear, current here means "the stack frame we are calculating from
> this stop" and previous here means "the stack frame from the last stop".
> That's confusing because previous & next also get used for up and down the
> current stack frame list. That's why I always try to use "younger" and
> "older" for ordering in one stack (that and it makes the ordering
> unambiguous.)
>
> So while this is definitely a bug, this is just going to keep the frames
> in the newly calculated stack frame list from taking advantage of any
> disassembly that was done on frames from the previous stop. Since this
> will get created on demand if left empty, it should have no behavioral
> effect. To test this you would have to count the number of times you
> disassembled the code for a given frame. If this were working properly,
> you'd only do it once for the time that frame lived on the stack. With
> this bug you will do it every time you stop and ask for disassembly for
> this frame.
>
> Jim
>
>
> > _______________________________________________
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
