[Lldb-commits] [lldb] r296495 - Fix incorrect logic in StackFrame::Disassemble.

[Jim Ingham via lldb-commits]

Make that SBFrame::Disassemble...

Jim

> On Feb 28, 2017, at 10:21 AM, Jim Ingham <jingham at apple.com> wrote:
> 
> 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 <zturner at 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 <jingham at apple.com> wrote:
>> No test case?
>> 
>> Jim
>> 
>>> On Feb 28, 2017, at 9:59 AM, Zachary Turner via lldb-commits <lldb-commits at 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 at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>> 
>