[lldb] Lock accesses to PathMappingLists's internals

This class is not safe in multithreaded code. It's possible for one
thread to modify a PathMappingList's `m_pair` vector while another
thread is iterating over it, effectively invalidating the iterator and
potentially leading to crashes or other difficult-to-diagnose bugs.

rdar://107695786

Differential Revision: https://reviews.llvm.org/D148380

diff --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h
index 447fb43..2834522 100644 (file)
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -13,6 +13,7 @@
 #include "lldb/Utility/Status.h"
 #include "llvm/Support/JSON.h"
 #include <map>
+#include <mutex>
 #include <optional>
 #include <vector>
 
@@ -50,9 +51,15 @@ public:
 
   llvm::json::Value ToJSON();
 
-  bool IsEmpty() const { return m_pairs.empty(); }
+  bool IsEmpty() const {
+    std::lock_guard<std::recursive_mutex> lock(m_mutex);
+    return m_pairs.empty();
+  }
 
-  size_t GetSize() const { return m_pairs.size(); }
+  size_t GetSize() const {
+    std::lock_guard<std::recursive_mutex> lock(m_mutex);
+    return m_pairs.size();
+  }
 
   bool GetPathsAtIndex(uint32_t idx, ConstString &path,
                        ConstString &new_path) const;
@@ -128,9 +135,13 @@ public:
 
   uint32_t FindIndexForPath(llvm::StringRef path) const;
 
-  uint32_t GetModificationID() const { return m_mod_id; }
+  uint32_t GetModificationID() const {
+    std::lock_guard<std::recursive_mutex> lock(m_mutex);
+    return m_mod_id;
+  }
 
 protected:
+  mutable std::recursive_mutex m_mutex;
   typedef std::pair<ConstString, ConstString> pair;
   typedef std::vector<pair> collection;
   typedef collection::iterator iterator;

diff --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp
index 13bb50b..4efc0bf 100644 (file)
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -48,6 +48,7 @@ PathMappingList::PathMappingList(const PathMappingList &rhs)
 
 const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
   if (this != &rhs) {
+    std::scoped_lock locks(m_mutex, rhs.m_mutex);
     m_pairs = rhs.m_pairs;
     m_callback = nullptr;
     m_callback_baton = nullptr;
@@ -60,6 +61,7 @@ PathMappingList::~PathMappingList() = default;
 
 void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
                              bool notify) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   ++m_mod_id;
   m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
   if (notify && m_callback)
@@ -67,6 +69,7 @@ void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
 }
 
 void PathMappingList::Append(const PathMappingList &rhs, bool notify) {
+  std::scoped_lock locks(m_mutex, rhs.m_mutex);
   ++m_mod_id;
   if (!rhs.m_pairs.empty()) {
     const_iterator pos, end = rhs.m_pairs.end();
@@ -81,6 +84,7 @@ bool PathMappingList::AppendUnique(llvm::StringRef path,
                                    llvm::StringRef replacement, bool notify) {
   auto normalized_path = NormalizePath(path);
   auto normalized_replacement = NormalizePath(replacement);
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   for (const auto &pair : m_pairs) {
     if (pair.first.GetStringRef().equals(normalized_path) &&
         pair.second.GetStringRef().equals(normalized_replacement))
@@ -92,6 +96,7 @@ bool PathMappingList::AppendUnique(llvm::StringRef path,
 
 void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
                              uint32_t index, bool notify) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   ++m_mod_id;
   iterator insert_iter;
   if (index >= m_pairs.size())
@@ -106,6 +111,7 @@ void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
 
 bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement,
                               uint32_t index, bool notify) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   if (index >= m_pairs.size())
     return false;
   ++m_mod_id;
@@ -116,6 +122,7 @@ bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement,
 }
 
 bool PathMappingList::Remove(size_t index, bool notify) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   if (index >= m_pairs.size())
     return false;
 
@@ -130,6 +137,7 @@ bool PathMappingList::Remove(size_t index, bool notify) {
 // For clients which do not need the pair index dumped, pass a pair_index >= 0
 // to only dump the indicated pair.
 void PathMappingList::Dump(Stream *s, int pair_index) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   unsigned int numPairs = m_pairs.size();
 
   if (pair_index < 0) {
@@ -147,6 +155,7 @@ void PathMappingList::Dump(Stream *s, int pair_index) {
 
 llvm::json::Value PathMappingList::ToJSON() {
   llvm::json::Array entries;
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   for (const auto &pair : m_pairs) {
     llvm::json::Array entry{pair.first.GetStringRef().str(),
                             pair.second.GetStringRef().str()};
@@ -156,6 +165,7 @@ llvm::json::Value PathMappingList::ToJSON() {
 }
 
 void PathMappingList::Clear(bool notify) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   if (!m_pairs.empty())
     ++m_mod_id;
   m_pairs.clear();
@@ -186,6 +196,7 @@ static void AppendPathComponents(FileSpec &path, llvm::StringRef components,
 
 std::optional<FileSpec> PathMappingList::RemapPath(llvm::StringRef mapping_path,
                                                    bool only_if_exists) const {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   if (m_pairs.empty() || mapping_path.empty())
     return {};
   LazyBool path_is_relative = eLazyBoolCalculate;
@@ -224,6 +235,7 @@ std::optional<llvm::StringRef>
 PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const {
   std::string path = file.GetPath();
   llvm::StringRef path_ref(path);
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   for (const auto &it : m_pairs) {
     llvm::StringRef removed_prefix = it.second.GetStringRef();
     if (!path_ref.consume_front(it.second.GetStringRef()))
@@ -252,6 +264,7 @@ PathMappingList::FindFile(const FileSpec &orig_spec) const {
 
 bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef new_path,
                               bool notify) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   uint32_t idx = FindIndexForPath(path);
   if (idx < m_pairs.size()) {
     ++m_mod_id;
@@ -264,6 +277,7 @@ bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef new_path,
 }
 
 bool PathMappingList::Remove(ConstString path, bool notify) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   iterator pos = FindIteratorForPath(path);
   if (pos != m_pairs.end()) {
     ++m_mod_id;
@@ -277,6 +291,7 @@ bool PathMappingList::Remove(ConstString path, bool notify) {
 
 PathMappingList::const_iterator
 PathMappingList::FindIteratorForPath(ConstString path) const {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   const_iterator pos;
   const_iterator begin = m_pairs.begin();
   const_iterator end = m_pairs.end();
@@ -290,6 +305,7 @@ PathMappingList::FindIteratorForPath(ConstString path) const {
 
 PathMappingList::iterator
 PathMappingList::FindIteratorForPath(ConstString path) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   iterator pos;
   iterator begin = m_pairs.begin();
   iterator end = m_pairs.end();
@@ -303,6 +319,7 @@ PathMappingList::FindIteratorForPath(ConstString path) {
 
 bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path,
                                       ConstString &new_path) const {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   if (idx < m_pairs.size()) {
     path = m_pairs[idx].first;
     new_path = m_pairs[idx].second;
@@ -313,6 +330,7 @@ bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path,
 
 uint32_t PathMappingList::FindIndexForPath(llvm::StringRef orig_path) const {
   const ConstString path = ConstString(NormalizePath(orig_path));
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   const_iterator pos;
   const_iterator begin = m_pairs.begin();
   const_iterator end = m_pairs.end();