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

Reply via email to