Author: Fred Riss Date: 2020-03-18T13:18:02-07:00 New Revision: b40ee7ff1b16982b39582eee04ca82cac5f3d154
URL: https://github.com/llvm/llvm-project/commit/b40ee7ff1b16982b39582eee04ca82cac5f3d154 DIFF: https://github.com/llvm/llvm-project/commit/b40ee7ff1b16982b39582eee04ca82cac5f3d154.diff LOG: [lldb/MemoryHistoryAsan] Fix address resolution for recorded backtraces Summary: The memory history plugin for Asan creates a HistoryThread with the recorded PC values provided by the Asan runtime. In other cases, thoses PCs are gathered by LLDB directly. The PCs returned by the Asan runtime are the PCs of the calls in the backtrace, not the return addresses you would normally get when unwinding the stack (look for a call to GetPreviousIntructionPc in AsanGetStack). When the above addresses are passed to the unwinder, it will subtract 1 from each address of the non zero frames because it treats them as return addresses. This can lead to the final report referencing the wrong line. This patch fixes this issue by threading a flag through HistoryThread and HistoryUnwinder that tells them to treat every frame like the first one. The Asan MemoryHistory plugin can then use this flag. This fixes running TestMemoryHistory on arm64 devices, although it's hard to guarantee that the test will continue to exhibit the boundary condition that triggers this bug. Reviewers: jasonmolenda, kubamracek Subscribers: kristof.beyls, danielkiss, lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D76341 Added: Modified: lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp lldb/source/Plugins/Process/Utility/HistoryThread.cpp lldb/source/Plugins/Process/Utility/HistoryThread.h lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp lldb/source/Plugins/Process/Utility/HistoryUnwind.h Removed: ################################################################################ diff --git a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp index 4b9da8f76fd2..333113a0b17a 100644 --- a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp +++ b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp @@ -138,7 +138,12 @@ static void CreateHistoryThreadFromValueObject(ProcessSP process_sp, pcs.push_back(pc); } - HistoryThread *history_thread = new HistoryThread(*process_sp, tid, pcs); + // The ASAN runtime already massages the return addresses into call + // addresses, we don't want LLDB's unwinder to try to locate the previous + // instruction again as this might lead to us reporting a diff erent line. + bool pcs_are_call_addresses = true; + HistoryThread *history_thread = + new HistoryThread(*process_sp, tid, pcs, pcs_are_call_addresses); ThreadSP new_thread_sp(history_thread); std::ostringstream thread_name_with_number; thread_name_with_number << thread_name << " Thread " << tid; diff --git a/lldb/source/Plugins/Process/Utility/HistoryThread.cpp b/lldb/source/Plugins/Process/Utility/HistoryThread.cpp index 815883d9e2f6..0649cd2f07de 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryThread.cpp +++ b/lldb/source/Plugins/Process/Utility/HistoryThread.cpp @@ -25,12 +25,13 @@ using namespace lldb_private; // Constructor HistoryThread::HistoryThread(lldb_private::Process &process, lldb::tid_t tid, - std::vector<lldb::addr_t> pcs) + std::vector<lldb::addr_t> pcs, + bool pcs_are_call_addresses) : Thread(process, tid, true), m_framelist_mutex(), m_framelist(), m_pcs(pcs), m_extended_unwind_token(LLDB_INVALID_ADDRESS), m_queue_name(), m_thread_name(), m_originating_unique_thread_id(tid), m_queue_id(LLDB_INVALID_QUEUE_ID) { - m_unwinder_up.reset(new HistoryUnwind(*this, pcs)); + m_unwinder_up.reset(new HistoryUnwind(*this, pcs, pcs_are_call_addresses)); Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT)); LLDB_LOGF(log, "%p HistoryThread::HistoryThread", static_cast<void *>(this)); } diff --git a/lldb/source/Plugins/Process/Utility/HistoryThread.h b/lldb/source/Plugins/Process/Utility/HistoryThread.h index 434cf6af7197..a66e0f2d4207 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryThread.h +++ b/lldb/source/Plugins/Process/Utility/HistoryThread.h @@ -33,7 +33,8 @@ namespace lldb_private { class HistoryThread : public lldb_private::Thread { public: HistoryThread(lldb_private::Process &process, lldb::tid_t tid, - std::vector<lldb::addr_t> pcs); + std::vector<lldb::addr_t> pcs, + bool pcs_are_call_addresses = false); ~HistoryThread() override; diff --git a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp index 93fcde72bf99..9b9522955de9 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp +++ b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp @@ -23,8 +23,10 @@ using namespace lldb_private; // Constructor -HistoryUnwind::HistoryUnwind(Thread &thread, std::vector<lldb::addr_t> pcs) - : Unwind(thread), m_pcs(pcs) {} +HistoryUnwind::HistoryUnwind(Thread &thread, std::vector<lldb::addr_t> pcs, + bool pcs_are_call_addresses) + : Unwind(thread), m_pcs(pcs), + m_pcs_are_call_addresses(pcs_are_call_addresses) {} // Destructor @@ -59,7 +61,10 @@ bool HistoryUnwind::DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa, if (frame_idx < m_pcs.size()) { cfa = frame_idx; pc = m_pcs[frame_idx]; - behaves_like_zeroth_frame = (frame_idx == 0); + if (m_pcs_are_call_addresses) + behaves_like_zeroth_frame = true; + else + behaves_like_zeroth_frame = (frame_idx == 0); return true; } return false; diff --git a/lldb/source/Plugins/Process/Utility/HistoryUnwind.h b/lldb/source/Plugins/Process/Utility/HistoryUnwind.h index b15abd924475..cb72b5d0a176 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryUnwind.h +++ b/lldb/source/Plugins/Process/Utility/HistoryUnwind.h @@ -18,7 +18,8 @@ namespace lldb_private { class HistoryUnwind : public lldb_private::Unwind { public: - HistoryUnwind(Thread &thread, std::vector<lldb::addr_t> pcs); + HistoryUnwind(Thread &thread, std::vector<lldb::addr_t> pcs, + bool pcs_are_call_addresses = false); ~HistoryUnwind() override; @@ -35,6 +36,9 @@ class HistoryUnwind : public lldb_private::Unwind { private: std::vector<lldb::addr_t> m_pcs; + /// This boolean indicates that the PCs in the non-0 frames are call + /// addresses and not return addresses. + bool m_pcs_are_call_addresses; }; } // namespace lldb_private _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
