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
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to