SBStackFrame::Disassemble calls StackFrame::Disassemble to do its job.  From 
the looks of it, your goof would cause lldb never to return any instructions 
when asked to disassemble a stack frame, since StackFrame does the work lazily 
and the error was to NOT do the work when the disassembly is empty, rather than 
to ONLY do the work...  It should be straightforward to write a test that 
disassembles a stack frame with the SB API's and see if it gets ANY 
instructions.  You can't do more than that without getting architecture 
specific, but testing that in the future somebody didn't break stack frame 
disassembly altogether should be pretty simple.

Jim



> On Feb 28, 2017, at 10:17 AM, Zachary Turner <ztur...@google.com> wrote:
> 
> I'm not even sure how to exercise this code path.  Granted, the reason it 
> broke at all is because of no test case, but I feel like someone who 
> understands this code should probably prepare a test case for it.  (I know in 
> the past Jason has said that it was notoriously hard to write test cases for 
> disassembler)  
> 
> On Tue, Feb 28, 2017 at 10:13 AM Jim Ingham <jing...@apple.com> wrote:
> No test case?
> 
> Jim
> 
> > On Feb 28, 2017, at 9:59 AM, Zachary Turner via lldb-commits 
> > <lldb-commits@lists.llvm.org> wrote:
> >
> > Author: zturner
> > Date: Tue Feb 28 11:59:59 2017
> > New Revision: 296495
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=296495&view=rev
> > Log:
> > Fix incorrect logic in StackFrame::Disassemble.
> >
> > This had broken as the result of some previous cleanup.
> >
> > Modified:
> >    lldb/trunk/source/Target/StackFrame.cpp
> >
> > Modified: lldb/trunk/source/Target/StackFrame.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrame.cpp?rev=296495&r1=296494&r2=296495&view=diff
> > ==============================================================================
> > --- lldb/trunk/source/Target/StackFrame.cpp (original)
> > +++ lldb/trunk/source/Target/StackFrame.cpp Tue Feb 28 11:59:59 2017
> > @@ -221,18 +221,20 @@ bool StackFrame::ChangePC(addr_t pc) {
> >
> > const char *StackFrame::Disassemble() {
> >   std::lock_guard<std::recursive_mutex> guard(m_mutex);
> > -  if (m_disassembly.Empty())
> > -    return nullptr;
> > -
> > -  ExecutionContext exe_ctx(shared_from_this());
> > -  Target *target = exe_ctx.GetTargetPtr();
> > -  if (target) {
> > -    const char *plugin_name = nullptr;
> > -    const char *flavor = nullptr;
> > -    Disassembler::Disassemble(target->GetDebugger(), 
> > target->GetArchitecture(),
> > -                              plugin_name, flavor, exe_ctx, 0, false, 0, 0,
> > -                              m_disassembly);
> > +  if (m_disassembly.Empty()) {
> > +    ExecutionContext exe_ctx(shared_from_this());
> > +    Target *target = exe_ctx.GetTargetPtr();
> > +    if (target) {
> > +      const char *plugin_name = nullptr;
> > +      const char *flavor = nullptr;
> > +      Disassembler::Disassemble(target->GetDebugger(),
> > +                                target->GetArchitecture(), plugin_name, 
> > flavor,
> > +                                exe_ctx, 0, false, 0, 0, m_disassembly);
> > +    }
> > +    if (m_disassembly.Empty())
> > +      return nullptr;
> >   }
> > +
> >   return m_disassembly.GetData();
> > }
> >
> >
> >
> > _______________________________________________
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to