Skip to content

Commit

Permalink
Revert one part of Ashok Thirumurthi's patch from September 16, r190812.
Browse files Browse the repository at this point in the history
In those set of patches, Ashok changed Module::ResolveSymbolContextForAddress
so that if it failed to find a symbol for a pc, it could back up
the pc value by 1 and re-search for a symbol.

His change to RegisterContextLLDB.cpp partially duplicates that
behavior but it also removes the separate case where we find a
Symbol for the pc address but it's the wrong symbol -- we need to
handle this as well as the lookup-by-pc-finds-no-symbol case.

The most obvious fallout from this regression was that lldb on
Mac OS X couldn't backtrace past __assert_rtn() which tail-calls
abort().  e.g.

(lldb) bt
* thread #1: tid = 0x5d6ea1, 0x00007fff8ee80866 libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x00007fff8ee80866 libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff8eb5835c libsystem_pthread.dylib`pthread_kill + 92
    frame #2: 0x00007fff8852ab1a libsystem_c.dylib`abort + 125
    frame #3: 0x00007fff884f49bf libsystem_c.dylib`__assert_rtn + 321
    frame #4: 0x0000000100000f2c a.out`main + 124

(lldb) dis -c 3 -s 0x7fff884f49b3
libsystem_c.dylib`__assert_rtn + 309:
   0x7fff884f49b3:  movq   %rax, -0x11b96242(%rip)   ; gCRAnnotations + 8
   0x7fff884f49ba:  callq  0x7fff8854fd2c            ; symbol stub for: abort

libsystem_c.dylib`basename:
   0x7fff884f49bf:  pushq  %rbp
(lldb)

in this case, __assert_rtn() is immediately followed by basename() and
the changes in r190812 didn't back up the pc value to get the correct
function name / unwind info.

<rdar://problem/15367233>
  • Loading branch information
jasonmolenda committed Dec 19, 2013
1 parent ce7d72c commit cd32987
Showing 1 changed file with 23 additions and 6 deletions.
29 changes: 23 additions & 6 deletions lldb/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,18 +393,35 @@ RegisterContextLLDB::InitializeNonZerothFrame()
if (m_sym_ctx_valid == false)
decr_pc_and_recompute_addr_range = true;

// Or if we're in the middle of the stack (and not "above" an asynchronous event like sigtramp), and
// our "current" pc is the start of a function or our "current" pc is one past the end of a function...
// Or if we're in the middle of the stack (and not "above" an asynchronous event like sigtramp),
// and our "current" pc is the start of a function...
if (m_sym_ctx_valid
&& GetNextFrame()->m_frame_type != eSigtrampFrame
&& GetNextFrame()->m_frame_type != eDebuggerFrame
&& addr_range.GetBaseAddress().IsValid()
&& addr_range.GetBaseAddress().GetSection() == m_current_pc.GetSection())
&& addr_range.GetBaseAddress().GetSection() == m_current_pc.GetSection()
&& addr_range.GetBaseAddress().GetOffset() == m_current_pc.GetOffset())
{
if (addr_range.GetBaseAddress().GetOffset() == m_current_pc.GetOffset() ||
addr_range.GetBaseAddress().GetOffset() + addr_range.GetByteSize() == m_current_pc.GetOffset())
decr_pc_and_recompute_addr_range = true;
}

// We need to back up the pc by 1 byte and re-search for the Symbol to handle the case where the "saved pc"
// value is pointing to the next function, e.g. if a function ends with a CALL instruction.
// FIXME this may need to be an architectural-dependent behavior; if so we'll need to add a member function
// to the ABI plugin and consult that.
if (decr_pc_and_recompute_addr_range)
{
Address temporary_pc(m_current_pc);
temporary_pc.SetOffset(m_current_pc.GetOffset() - 1);
m_sym_ctx.Clear(false);
m_sym_ctx_valid = false;
if ((pc_module_sp->ResolveSymbolContextForAddress (temporary_pc, eSymbolContextFunction| eSymbolContextSymbol, m_sym_ctx) & eSymbolContextSymbol) == eSymbolContextSymbol)
{
m_sym_ctx_valid = true;
}
if (!m_sym_ctx.GetAddressRange (eSymbolContextFunction | eSymbolContextSymbol, 0, false, addr_range))
{
decr_pc_and_recompute_addr_range = true;
m_sym_ctx_valid = false;
}
}

Expand Down

0 comments on commit cd32987

Please sign in to comment.