[Lldb-commits] [lldb] [LLDB] Fix potential data race in Symtab initialization (PR #192753)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

<details>
<summary>Changes</summary>

Claude pointed out to me that Symtab::FindFunctionSymbols doesn't lock the mutex before checking m_name_indexes_computed and recomputing it. On top of that all the initialization flags are bitfields, which makes any unguarded concurrent accesses UB. Changing them to bools should no longer be necessary after introducing a lock, but several of the public methods trust that their caller holds the lock so I'm opting to remove this footgun just in case.

rdar://174988238

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


2 Files Affected:

- (modified) lldb/include/lldb/Symbol/Symtab.h (+6-4) 
- (modified) lldb/source/Symbol/Symtab.cpp (+1) 


``````````diff
diff --git a/lldb/include/lldb/Symbol/Symtab.h b/lldb/include/lldb/Symbol/Symtab.h
index 57627d2dde7d2..9c7173c1153a3 100644
--- a/lldb/include/lldb/Symbol/Symtab.h
+++ b/lldb/include/lldb/Symbol/Symtab.h
@@ -277,10 +277,12 @@ class Symtab {
   /// Maps function names to symbol indices (grouped by FunctionNameTypes)
   std::map<lldb::FunctionNameType, UniqueCStringMap<uint32_t>>
       m_name_to_symbol_indices;
-  mutable std::recursive_mutex
-      m_mutex; // Provide thread safety for this symbol table
-  bool m_file_addr_to_index_computed : 1, m_name_indexes_computed : 1,
-    m_loaded_from_cache : 1, m_saved_to_cache : 1;
+  /// Provide thread safety for this symbol table.
+  mutable std::recursive_mutex m_mutex;
+  bool m_file_addr_to_index_computed = false;
+  bool m_name_indexes_computed = false;
+  bool m_loaded_from_cache = false;
+  bool m_saved_to_cache = false;
 
 private:
   UniqueCStringMap<uint32_t> &
diff --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index c7cd900f41cbb..57a07e3d70c2f 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -1126,6 +1126,7 @@ void Symtab::FindFunctionSymbols(ConstString name, uint32_t name_type_mask,
     }
   }
 
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   if (!m_name_indexes_computed)
     InitNameIndexes();
 

``````````

</details>


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