[lldb-dev] Bug in StackFrame::UpdateCurrentFrameFromPreviousFrame

[Zachary Turner via lldb-dev]

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 <jingham at apple.com> wrote:

>
> > On Nov 13, 2016, at 4:48 PM, Zachary Turner via lldb-dev <
> lldb-dev at 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 at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20161114/03a7f692/attachment.html>