[Lldb-commits] [lldb] [lldb/Target] Fix data race in StackFrameList::FetchFramesUpTo (PR #182668)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

<details>
<summary>Changes</summary>

This patch should fix a data race introduced in commit c373d7632ac1 where StackFrameList by releasing m_list_mutex IN `GetFramesUpTo` before calling `FetchFramesUpTo`, which then modifies m_frames without re-acquiring the lock.

Prior to that change, the whole function was guarded by the lock which caused frame providers to deadlock when unwinding more frames from Python. The comment at line 428 says "FetchFramesUpTo will acquire locks as needed" but the base `FetchFramesUpTo` implementation was never updated, only the `SyntheticStackFrameList` override correctly acquires the lock before pushing frames.

This causes a race between:
- Thread A: `GetFrameWithStackID` holds shared_lock, performs binary search on m_frames via `llvm::lower_bound`
- Thread B: `FetchFramesUpTo` modifies m_frames via `push_back` without any lock

The crash manifests as a PAC failure when Thread A dereferences an iterator that was invalidated by Thread B's concurrent push_back().

The fix adds lock scopes around the two m_frames.push_back() calls in the base (unwinder) `StackFrameList::FetchFramesUpTo` implementation, matching the pattern already used in SyntheticStackFrameList::FetchFramesUpTo().

rdar://169982918

---
Full diff: https://github.com/llvm/llvm-project/pull/182668.diff


1 Files Affected:

- (modified) lldb/source/Target/StackFrameList.cpp (+8-2) 


``````````diff
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index 825d538e3815b..290230855ac7f 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -511,7 +511,10 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx,
               m_thread.shared_from_this(), m_frames.size(), idx, reg_ctx_sp,
               cfa, pc, behaves_like_zeroth_frame, nullptr);
           unwind_frame_sp->m_frame_list_id = GetIdentifier();
-          m_frames.push_back(unwind_frame_sp);
+          {
+            std::unique_lock<std::shared_mutex> guard(m_list_mutex);
+            m_frames.push_back(unwind_frame_sp);
+          }
         }
       } else {
         unwind_frame_sp = m_frames.front();
@@ -546,7 +549,10 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx,
       SynthesizeTailCallFrames(*unwind_frame_sp.get());
 
       unwind_frame_sp->m_frame_list_id = GetIdentifier();
-      m_frames.push_back(unwind_frame_sp);
+      {
+        std::unique_lock<std::shared_mutex> guard(m_list_mutex);
+        m_frames.push_back(unwind_frame_sp);
+      }
     }
 
     assert(unwind_frame_sp);

``````````

</details>


https://github.com/llvm/llvm-project/pull/182668