https://github.com/yln updated https://github.com/llvm/llvm-project/pull/154247
>From b92dee9ba04d1baabab08ab96fcef7d860d3b47e Mon Sep 17 00:00:00 2001 From: Julian Lettner <jlett...@apple.com> Date: Mon, 18 Aug 2025 12:37:24 -0700 Subject: [PATCH 1/2] [lldb] Introduce TracePCType enum Introduce and adopt `TracePCType` enum to replace `pcs_are_call_addresses` bool to make handling of history back traces more clear and allow for extension of behavior. This change is a mechanical refactoring and preservers current behavior: ``` pcs_are_call_addresses: false -> TracePCType::Returns (default) true -> TracePCType::Calls ``` rdar://157596927 --- lldb/include/lldb/lldb-private-enumerations.h | 2 ++ .../InstrumentationRuntimeMainThreadChecker.cpp | 6 +++--- .../UBSan/InstrumentationRuntimeUBSan.cpp | 6 +++--- .../Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp | 4 ++-- lldb/source/Plugins/Process/Utility/HistoryThread.cpp | 6 ++---- lldb/source/Plugins/Process/Utility/HistoryThread.h | 6 +++--- lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp | 7 +++---- lldb/source/Plugins/Process/Utility/HistoryUnwind.h | 6 ++---- .../Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp | 6 +++--- 9 files changed, 23 insertions(+), 26 deletions(-) diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h index 98c1e956bf8f7..604945011e5d5 100644 --- a/lldb/include/lldb/lldb-private-enumerations.h +++ b/lldb/include/lldb/lldb-private-enumerations.h @@ -248,6 +248,8 @@ enum class IterationAction { Stop, }; +enum class TracePCType { Returns, ReturnsNoZerothFrame, Calls }; + inline std::string GetStatDescription(lldb_private::StatisticKind K) { switch (K) { case StatisticKind::ExpressionSuccessful: diff --git a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp index e67e60b4a3957..3c817f9171d7b 100644 --- a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp +++ b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp @@ -266,9 +266,9 @@ InstrumentationRuntimeMainThreadChecker::GetBacktracesFromExtendedStopInfo( // We gather symbolication addresses above, so no need for HistoryThread to // try to infer the call addresses. - bool pcs_are_call_addresses = true; - ThreadSP new_thread_sp = std::make_shared<HistoryThread>( - *process_sp, tid, PCs, pcs_are_call_addresses); + auto pc_type = TracePCType::Calls; + ThreadSP new_thread_sp = + std::make_shared<HistoryThread>(*process_sp, tid, PCs, pc_type); // Save this in the Process' ExtendedThreadList so a strong pointer retains // the object diff --git a/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp b/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp index c2db3540a797b..57a705715f1a0 100644 --- a/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp +++ b/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp @@ -324,9 +324,9 @@ InstrumentationRuntimeUBSan::GetBacktracesFromExtendedStopInfo( // We gather symbolication addresses above, so no need for HistoryThread to // try to infer the call addresses. - bool pcs_are_call_addresses = true; - ThreadSP new_thread_sp = std::make_shared<HistoryThread>( - *process_sp, tid, PCs, pcs_are_call_addresses); + auto pc_type = TracePCType::Calls; + ThreadSP new_thread_sp = + std::make_shared<HistoryThread>(*process_sp, tid, PCs, pc_type); std::string stop_reason_description = GetStopReasonDescription(info); new_thread_sp->SetName(stop_reason_description.c_str()); diff --git a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp index afaaa57b09587..8c8c48d17e56d 100644 --- a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp +++ b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp @@ -131,9 +131,9 @@ static void CreateHistoryThreadFromValueObject(ProcessSP process_sp, // 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 different line. - bool pcs_are_call_addresses = true; + auto pc_type = TracePCType::Calls; HistoryThread *history_thread = - new HistoryThread(*process_sp, tid, pcs, pcs_are_call_addresses); + new HistoryThread(*process_sp, tid, pcs, pc_type); 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 bc06757c806a9..b5d2395f978a8 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryThread.cpp +++ b/lldb/source/Plugins/Process/Utility/HistoryThread.cpp @@ -26,14 +26,12 @@ using namespace lldb_private; // Constructor HistoryThread::HistoryThread(lldb_private::Process &process, lldb::tid_t tid, - std::vector<lldb::addr_t> pcs, - bool pcs_are_call_addresses) + std::vector<lldb::addr_t> pcs, TracePCType pc_type) : 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 = - std::make_unique<HistoryUnwind>(*this, pcs, pcs_are_call_addresses); + m_unwinder_up = std::make_unique<HistoryUnwind>(*this, pcs, pc_type); Log *log = GetLog(LLDBLog::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 a66e0f2d4207c..2f6c48518672a 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryThread.h +++ b/lldb/source/Plugins/Process/Utility/HistoryThread.h @@ -27,14 +27,14 @@ namespace lldb_private { /// process execution /// /// This subclass of Thread is used to provide a backtrace from earlier in -/// process execution. It is given a backtrace list of pc addresses and it -/// will create stack frames for them. +/// process execution. It is given a backtrace list of pcs (return or call +/// addresses) and it will create stack frames for them. class HistoryThread : public lldb_private::Thread { public: HistoryThread(lldb_private::Process &process, lldb::tid_t tid, std::vector<lldb::addr_t> pcs, - bool pcs_are_call_addresses = false); + TracePCType pc_type = TracePCType::Returns); ~HistoryThread() override; diff --git a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp index 7749dc6f5d514..34ee9054f4e17 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp +++ b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp @@ -24,9 +24,8 @@ using namespace lldb_private; // Constructor 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) {} + TracePCType pc_type) + : Unwind(thread), m_pcs(pcs), m_pc_type(pc_type) {} // Destructor @@ -61,7 +60,7 @@ 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]; - if (m_pcs_are_call_addresses) + if (m_pc_type == TracePCType::Calls) behaves_like_zeroth_frame = true; else behaves_like_zeroth_frame = (frame_idx == 0); diff --git a/lldb/source/Plugins/Process/Utility/HistoryUnwind.h b/lldb/source/Plugins/Process/Utility/HistoryUnwind.h index cb72b5d0a1764..95d1716f39776 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryUnwind.h +++ b/lldb/source/Plugins/Process/Utility/HistoryUnwind.h @@ -19,7 +19,7 @@ namespace lldb_private { class HistoryUnwind : public lldb_private::Unwind { public: HistoryUnwind(Thread &thread, std::vector<lldb::addr_t> pcs, - bool pcs_are_call_addresses = false); + TracePCType pc_type = TracePCType::Returns); ~HistoryUnwind() override; @@ -36,9 +36,7 @@ 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; + TracePCType m_pc_type; }; } // namespace lldb_private diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp index b23f64210cc80..a20f04b78492c 100644 --- a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp +++ b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp @@ -544,9 +544,9 @@ ThreadSP SystemRuntimeMacOSX::GetExtendedBacktraceThread(ThreadSP real_thread, if (!thread_extended_info->ForEach(extract_frame_pc)) return {}; - originating_thread_sp = - std::make_shared<HistoryThread>(*m_process, real_thread->GetIndexID(), - app_specific_backtrace_pcs, true); + originating_thread_sp = std::make_shared<HistoryThread>( + *m_process, real_thread->GetIndexID(), app_specific_backtrace_pcs, + TracePCType::Calls); originating_thread_sp->SetQueueName(type.AsCString()); } return originating_thread_sp; >From d9157e0b3cbfac0e14bc2ad5597c567ab68afaa8 Mon Sep 17 00:00:00 2001 From: Julian Lettner <jlett...@apple.com> Date: Mon, 18 Aug 2025 18:01:32 -0700 Subject: [PATCH 2/2] [lldb] Fix source line annotations for libsanitizers When providing allocation and deallocation traces, the ASan compiler-rt runtime already provides call addresses (`TracePCType::Calls`). On Darwin, system sanitizers (libsanitizers) provides return address. It also discards a few non-user frames at the top of the stack, because these internal libmalloc/libsanitizers stack frames do not provide any value when diagnosing memory errors. Introduce and add handling for `TracePCType::ReturnsNoZerothFrame` to cover this case and enable libsanitizers traces line-level testing. rdar://157596927 --- lldb/include/lldb/lldb-private-enumerations.h | 16 ++++++++- .../MemoryHistory/asan/MemoryHistoryASan.cpp | 34 ++++++++++++------- .../Plugins/Process/Utility/HistoryUnwind.cpp | 16 ++++++--- .../functionalities/asan/TestMemoryHistory.py | 14 ++++---- 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h index 604945011e5d5..9589214466693 100644 --- a/lldb/include/lldb/lldb-private-enumerations.h +++ b/lldb/include/lldb/lldb-private-enumerations.h @@ -248,7 +248,21 @@ enum class IterationAction { Stop, }; -enum class TracePCType { Returns, ReturnsNoZerothFrame, Calls }; +/// Specifies the type of PCs when creating a `HistoryThread`. +/// - `Returns` - Usually, when LLDB unwinds the stack or we retrieve a stack +/// trace via `backtrace()` we are collecting return addresses (except for the +/// topmost frame which is the actual PC). LLDB then maps these return +/// addresses back to call addresses to give accurate source line annotations. +/// - `ReturnsNoZerothFrame` - Some trace providers (e.g., libsanitizers traces) +/// collect return addresses but prune the topmost frames, so we should skip +/// the special treatment of frame 0. +/// - `Calls` - Other trace providers (e.g., ASan compiler-rt runtime) already +/// perform this mapping, so we need to prevent LLDB from doing it again. +enum class TracePCType { + Returns, ///< PCs are return addresses, except for topmost frame. + ReturnsNoZerothFrame, ///< All PCs are return addresses. + Calls ///< PCs are call addresses. +}; inline std::string GetStatDescription(lldb_private::StatisticKind K) { switch (K) { diff --git a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp index 8c8c48d17e56d..a4c18099f495c 100644 --- a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp +++ b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp @@ -91,11 +91,9 @@ const char *memory_history_asan_command_format = t; )"; -static void CreateHistoryThreadFromValueObject(ProcessSP process_sp, - ValueObjectSP return_value_sp, - const char *type, - const char *thread_name, - HistoryThreads &result) { +static void CreateHistoryThreadFromValueObject( + ProcessSP process_sp, ValueObjectSP return_value_sp, TracePCType pc_type, + const char *type, const char *thread_name, HistoryThreads &result) { std::string count_path = "." + std::string(type) + "_count"; std::string tid_path = "." + std::string(type) + "_tid"; std::string trace_path = "." + std::string(type) + "_trace"; @@ -128,10 +126,6 @@ static void CreateHistoryThreadFromValueObject(ProcessSP process_sp, pcs.push_back(pc); } - // 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 different line. - auto pc_type = TracePCType::Calls; HistoryThread *history_thread = new HistoryThread(*process_sp, tid, pcs, pc_type); ThreadSP new_thread_sp(history_thread); @@ -176,10 +170,24 @@ HistoryThreads MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address) { options.SetAutoApplyFixIts(false); options.SetLanguage(eLanguageTypeObjC_plus_plus); + // The ASan compiler-rt runtime already massages the return addresses into + // call addresses, so we don't want LLDB's unwinder to try to locate the + // previous instruction again as this might lead to us reporting a different + // line. + auto pc_type = TracePCType::Calls; + if (auto m = GetPreferredAsanModule(process_sp->GetTarget())) { SymbolContextList sc_list; sc_list.Append(SymbolContext(std::move(m))); options.SetPreferredSymbolContexts(std::move(sc_list)); + } else if (process_sp->GetTarget() + .GetArchitecture() + .GetTriple() + .isOSDarwin()) { + // Darwin, but not ASan compiler-rt implies libsanitizers which collects + // return addresses. It also discards a few non-user frames at the top of + // the stack. + pc_type = TracePCType::ReturnsNoZerothFrame; } ExpressionResults expr_result = UserExpression::Evaluate( @@ -197,10 +205,10 @@ HistoryThreads MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address) { if (!return_value_sp) return result; - CreateHistoryThreadFromValueObject(process_sp, return_value_sp, "free", - "Memory deallocated by", result); - CreateHistoryThreadFromValueObject(process_sp, return_value_sp, "alloc", - "Memory allocated by", result); + CreateHistoryThreadFromValueObject(process_sp, return_value_sp, pc_type, + "free", "Memory deallocated by", result); + CreateHistoryThreadFromValueObject(process_sp, return_value_sp, pc_type, + "alloc", "Memory allocated by", result); return result; } diff --git a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp index 34ee9054f4e17..3754e4320eaf5 100644 --- a/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp +++ b/lldb/source/Plugins/Process/Utility/HistoryUnwind.cpp @@ -51,6 +51,17 @@ HistoryUnwind::DoCreateRegisterContextForFrame(StackFrame *frame) { return rctx; } +static bool BehavesLikeZerothFrame(TracePCType pc_type, uint32_t frame_idx) { + switch (pc_type) { + case TracePCType::Returns: + return (frame_idx == 0); + case TracePCType::ReturnsNoZerothFrame: + return false; + case TracePCType::Calls: + return true; + } +} + bool HistoryUnwind::DoGetFrameInfoAtIndex(uint32_t frame_idx, lldb::addr_t &cfa, lldb::addr_t &pc, bool &behaves_like_zeroth_frame) { @@ -60,10 +71,7 @@ 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]; - if (m_pc_type == TracePCType::Calls) - behaves_like_zeroth_frame = true; - else - behaves_like_zeroth_frame = (frame_idx == 0); + behaves_like_zeroth_frame = BehavesLikeZerothFrame(m_pc_type, frame_idx); return true; } return false; diff --git a/lldb/test/API/functionalities/asan/TestMemoryHistory.py b/lldb/test/API/functionalities/asan/TestMemoryHistory.py index 1568140b355dc..66f6e3e7502c1 100644 --- a/lldb/test/API/functionalities/asan/TestMemoryHistory.py +++ b/lldb/test/API/functionalities/asan/TestMemoryHistory.py @@ -41,18 +41,16 @@ def setUp(self): self.line_free = line_number("main.c", "// free line") self.line_breakpoint = line_number("main.c", "// break line") - # Test line numbers: rdar://126237493 - # for libsanitizers and remove `skip_line_numbers` parameter - def check_traces(self, skip_line_numbers=False): + def check_traces(self): self.expect( "memory history 'pointer'", substrs=[ "Memory deallocated by Thread", "a.out`f2", - "main.c" if skip_line_numbers else f"main.c:{self.line_free}", + f"main.c:{self.line_free}", "Memory allocated by Thread", "a.out`f1", - "main.c" if skip_line_numbers else f"main.c:{self.line_malloc}", + f"main.c:{self.line_malloc}", ], ) @@ -76,7 +74,7 @@ def libsanitizers_traces_tests(self): self.runCmd("env SanitizersAllocationTraces=all") self.run_to_breakpoint(target) - self.check_traces(skip_line_numbers=True) + self.check_traces() def libsanitizers_asan_tests(self): target = self.createTestTarget() @@ -84,7 +82,7 @@ def libsanitizers_asan_tests(self): self.runCmd("env SanitizersAddress=1 MallocSanitizerZone=1") self.run_to_breakpoint(target) - self.check_traces(skip_line_numbers=True) + self.check_traces() self.runCmd("continue") @@ -94,7 +92,7 @@ def libsanitizers_asan_tests(self): "Process should be stopped due to ASan report", substrs=["stopped", "stop reason = Use of deallocated memory"], ) - self.check_traces(skip_line_numbers=True) + self.check_traces() # do the same using SB API process = self.dbg.GetSelectedTarget().process _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits