[Lldb-commits] [lldb] 3828042 - [lldb] Guard SourceManager::File's line offsets with a mutex (#219424)

[via lldb-commits]

Author: Raphael Isemann
Date: 2026-09-01T13:54:03+01:00
New Revision: 3828042a79ada542ec1dcc7b4217f6a44b4a4617

URL: https://github.com/llvm/llvm-project/commit/3828042a79ada542ec1dcc7b4217f6a44b4a4617
DIFF: https://github.com/llvm/llvm-project/commit/3828042a79ada542ec1dcc7b4217f6a44b4a4617.diff

LOG: [lldb] Guard SourceManager::File's line offsets with a mutex (#219424)

A SourceManager::File is shared by every Target and Process because
Debugger and Process hand out cached instances (see SourceFileCache).

m_offsets is the one member that is computed after the File was
created. CalculateLineOffsets() indexes the file on the first access.
Two threads that read the same source file at the same time therefore
race in the current implementation.

This patch guards m_offsets with a `Guarded` which avoids any potential
races. The shared mutex allows concurrent accesses once the line
offsets were calculated.

assisted-by: claude

Added: 
    

Modified: 
    lldb/include/lldb/Core/SourceManager.h
    lldb/source/Core/SourceManager.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/SourceManager.h b/lldb/include/lldb/Core/SourceManager.h
index 034f171f0a40d..e5b6f313ed2f7 100644
--- a/lldb/include/lldb/Core/SourceManager.h
+++ b/lldb/include/lldb/Core/SourceManager.h
@@ -11,6 +11,7 @@
 
 #include "lldb/Utility/Checksum.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Locked.h"
 #include "lldb/Utility/SupportFile.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-forward.h"
@@ -23,6 +24,7 @@
 #include <map>
 #include <memory>
 #include <optional>
+#include <shared_mutex>
 #include <string>
 #include <vector>
 
@@ -104,7 +106,12 @@ class SourceManager {
     uint32_t m_source_map_mod_id = 0;
     lldb::DataBufferSP m_data_sp;
     typedef std::vector<uint32_t> LineOffsets;
-    LineOffsets m_offsets;
+
+    /// The line offsets for this file.
+    /// This member that is computed after this File was created, so write
+    /// access can happen from several threads..
+    Guarded<LineOffsets, std::shared_mutex> m_offsets;
+
     lldb::DebuggerWP m_debugger_wp;
     lldb::TargetWP m_target_wp;
 

diff  --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp
index 5ffedcc86ca1f..bec772fe584f6 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -611,15 +611,17 @@ uint32_t SourceManager::File::GetLineOffset(uint32_t line) {
     return 0;
 
   if (CalculateLineOffsets(line)) {
-    if (line < m_offsets.size())
-      return m_offsets[line - 1]; // yes we want "line - 1" in the index
+    SharedLocked<const LineOffsets *, std::shared_mutex> offsets =
+        m_offsets.LockShared();
+    if (line < offsets->size())
+      return (*offsets)[line - 1]; // yes we want "line - 1" in the index
   }
   return UINT32_MAX;
 }
 
 uint32_t SourceManager::File::GetNumLines() {
   CalculateLineOffsets();
-  return m_offsets.size();
+  return m_offsets.LockShared()->size();
 }
 
 const char *SourceManager::File::PeekLineData(uint32_t line) {
@@ -669,7 +671,7 @@ bool SourceManager::File::LineIsValid(uint32_t line) {
     return false;
 
   if (CalculateLineOffsets(line))
-    return line < m_offsets.size();
+    return line < m_offsets.LockShared()->size();
   return false;
 }
 
@@ -782,11 +784,22 @@ bool SourceManager::File::CalculateLineOffsets(uint32_t line) {
   line =
       UINT32_MAX; // TODO: take this line out when we support partial indexing
   if (line == UINT32_MAX) {
-    // Already done?
-    if (!m_offsets.empty() && m_offsets[0] == UINT32_MAX)
+    // Already done? Check with just a reader lock first so concurrent reads
+    // of an already-indexed file don't serialize on each other.
+    {
+      SharedLocked<const LineOffsets *, std::shared_mutex> offsets =
+          m_offsets.LockShared();
+      if (!offsets->empty() && (*offsets)[0] == UINT32_MAX)
+        return true;
+    }
+
+    Locked<LineOffsets *, std::shared_mutex> offsets = m_offsets.Lock();
+    // Another thread may have finished indexing while we were waiting for
+    // the writer lock.
+    if (!offsets->empty() && (*offsets)[0] == UINT32_MAX)
       return true;
 
-    if (m_offsets.empty()) {
+    if (offsets->empty()) {
       if (!m_data_sp)
         return false;
 
@@ -798,7 +811,7 @@ bool SourceManager::File::CalculateLineOffsets(uint32_t line) {
 
         // Push a 1 at index zero to indicate the file has been completely
         // indexed.
-        m_offsets.push_back(UINT32_MAX);
+        offsets->push_back(UINT32_MAX);
         const char *s;
         for (s = start; s < end; ++s) {
           char curr_ch = *s;
@@ -810,12 +823,12 @@ bool SourceManager::File::CalculateLineOffsets(uint32_t line) {
                   ++s;
               }
             }
-            m_offsets.push_back(s + 1 - start);
+            offsets->push_back(s + 1 - start);
           }
         }
-        if (!m_offsets.empty()) {
-          if (m_offsets.back() < size_t(end - start))
-            m_offsets.push_back(end - start);
+        if (!offsets->empty()) {
+          if (offsets->back() < size_t(end - start))
+            offsets->push_back(end - start);
         }
         return true;
       }