JDevlieghere updated this revision to Diff 170242.
JDevlieghere retitled this revision from "[DWARFSymbolFile] Adds the module 
lock to all virtual methods from SymbolFile" to "[DWARFSymbolFile] Add the 
module lock where necessary and assert that we own it.".
JDevlieghere edited the summary of this revision.

https://reviews.llvm.org/D52543

Files:
  include/lldb/Symbol/SymbolFile.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/SymbolFile.cpp

Index: source/Symbol/SymbolFile.cpp
===================================================================
--- source/Symbol/SymbolFile.cpp
+++ source/Symbol/SymbolFile.cpp
@@ -19,12 +19,18 @@
 #include "lldb/Utility/StreamString.h"
 #include "lldb/lldb-private.h"
 
+#include <future>
+
 using namespace lldb_private;
 
 void SymbolFile::PreloadSymbols() {
   // No-op for most implementations.
 }
 
+std::recursive_mutex &SymbolFile::GetModuleMutex() const {
+  return GetObjectFile()->GetModule()->GetMutex();
+}
+
 SymbolFile *SymbolFile::FindPlugin(ObjectFile *obj_file) {
   std::unique_ptr<SymbolFile> best_symfile_ap;
   if (obj_file != nullptr) {
@@ -150,3 +156,13 @@
     types.Clear();
   return 0;
 }
+
+void SymbolFile::AssertModuleLock() {
+  // We assert that we have to module lock by trying to acquire the lock from a
+  // different thread. Note that we must abort if the result is true to
+  // guarantee correctness.
+  lldbassert(std::async(std::launch::async,
+                        [this] { return this->GetModuleMutex().try_lock(); })
+                     .get() == false &&
+             "Module is not locked");
+}
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -228,6 +228,8 @@
 
   void PreloadSymbols() override;
 
+  std::recursive_mutex &GetModuleMutex() const override;
+
   //------------------------------------------------------------------
   // PluginInterface protocol
   //------------------------------------------------------------------
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -260,6 +260,9 @@
 }
 
 TypeList *SymbolFileDWARF::GetTypeList() {
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
   if (debug_map_symfile)
     return debug_map_symfile->GetTypeList();
@@ -341,6 +344,7 @@
                                  uint32_t type_mask, TypeList &type_list)
 
 {
+  AssertModuleLock();
   TypeSet type_set;
 
   CompileUnit *comp_unit = NULL;
@@ -860,6 +864,7 @@
 }
 
 CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+  AssertModuleLock();
   CompUnitSP cu_sp;
   DWARFDebugInfo *info = DebugInfo();
   if (info) {
@@ -872,6 +877,7 @@
 
 Function *SymbolFileDWARF::ParseCompileUnitFunction(const SymbolContext &sc,
                                                     const DWARFDIE &die) {
+  AssertModuleLock();
   if (die.IsValid()) {
     TypeSystem *type_system =
         GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
@@ -895,6 +901,7 @@
 }
 lldb::LanguageType
 SymbolFileDWARF::ParseCompileUnitLanguage(const SymbolContext &sc) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu)
@@ -904,6 +911,7 @@
 }
 
 size_t SymbolFileDWARF::ParseCompileUnitFunctions(const SymbolContext &sc) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   size_t functions_added = 0;
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -926,6 +934,7 @@
 
 bool SymbolFileDWARF::ParseCompileUnitSupportFiles(
     const SymbolContext &sc, FileSpecList &support_files) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu) {
@@ -951,6 +960,7 @@
 
 bool SymbolFileDWARF::ParseCompileUnitIsOptimized(
     const lldb_private::SymbolContext &sc) {
+  AssertModuleLock();
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu)
     return dwarf_cu->GetIsOptimized();
@@ -960,6 +970,7 @@
 bool SymbolFileDWARF::ParseImportedModules(
     const lldb_private::SymbolContext &sc,
     std::vector<lldb_private::ConstString> &imported_modules) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
   if (dwarf_cu) {
@@ -1037,6 +1048,7 @@
 }
 
 bool SymbolFileDWARF::ParseCompileUnitLineTable(const SymbolContext &sc) {
+  AssertModuleLock();
   assert(sc.comp_unit);
   if (sc.comp_unit->GetLineTable() != NULL)
     return true;
@@ -1122,6 +1134,7 @@
 }
 
 bool SymbolFileDWARF::ParseCompileUnitDebugMacros(const SymbolContext &sc) {
+  AssertModuleLock();
   assert(sc.comp_unit);
 
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -1301,6 +1314,9 @@
 }
 
 SymbolFileDWARF *SymbolFileDWARF::GetDWARFForUID(lldb::user_id_t uid) {
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
   // must make sure we use the correct DWARF file when resolving things. On
   // MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
@@ -1317,6 +1333,9 @@
 
 DWARFDIE
 SymbolFileDWARF::GetDIEFromUID(lldb::user_id_t uid) {
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
   // must make sure we use the correct DWARF file when resolving things. On
   // MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
@@ -1331,6 +1350,9 @@
 }
 
 CompilerDecl SymbolFileDWARF::GetDeclForUID(lldb::user_id_t type_uid) {
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Anytime we have a lldb::user_id_t, we must get the DIE by calling
   // SymbolFileDWARF::GetDIEFromUID(). See comments inside the
   // SymbolFileDWARF::GetDIEFromUID() for details.
@@ -1342,6 +1364,9 @@
 
 CompilerDeclContext
 SymbolFileDWARF::GetDeclContextForUID(lldb::user_id_t type_uid) {
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Anytime we have a lldb::user_id_t, we must get the DIE by calling
   // SymbolFileDWARF::GetDIEFromUID(). See comments inside the
   // SymbolFileDWARF::GetDIEFromUID() for details.
@@ -1353,6 +1378,9 @@
 
 CompilerDeclContext
 SymbolFileDWARF::GetDeclContextContainingUID(lldb::user_id_t type_uid) {
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Anytime we have a lldb::user_id_t, we must get the DIE by calling
   // SymbolFileDWARF::GetDIEFromUID(). See comments inside the
   // SymbolFileDWARF::GetDIEFromUID() for details.
@@ -1363,6 +1391,9 @@
 }
 
 Type *SymbolFileDWARF::ResolveTypeUID(lldb::user_id_t type_uid) {
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Anytime we have a lldb::user_id_t, we must get the DIE by calling
   // SymbolFileDWARF::GetDIEFromUID(). See comments inside the
   // SymbolFileDWARF::GetDIEFromUID() for details.
@@ -1438,8 +1469,7 @@
 }
 
 bool SymbolFileDWARF::CompleteType(CompilerType &compiler_type) {
-  std::lock_guard<std::recursive_mutex> guard(
-      GetObjectFile()->GetModule()->GetMutex());
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
 
   ClangASTContext *clang_type_system =
       llvm::dyn_cast_or_null<ClangASTContext>(compiler_type.GetTypeSystem());
@@ -1984,11 +2014,17 @@
 }
 
 void SymbolFileDWARF::PreloadSymbols() {
-  std::lock_guard<std::recursive_mutex> guard(
-      GetObjectFile()->GetModule()->GetMutex());
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   m_index->Preload();
 }
 
+std::recursive_mutex &SymbolFileDWARF::GetModuleMutex() const {
+  if (m_debug_map_module_wp.expired())
+    return GetObjectFile()->GetModule()->GetMutex();
+  lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
+  return module_sp->GetMutex();
+}
+
 bool SymbolFileDWARF::DeclContextMatchesThisSymbolFile(
     const lldb_private::CompilerDeclContext *decl_ctx) {
   if (decl_ctx == nullptr || !decl_ctx->IsValid()) {
@@ -3075,6 +3111,7 @@
 }
 
 size_t SymbolFileDWARF::ParseFunctionBlocks(const SymbolContext &sc) {
+  AssertModuleLock();
   assert(sc.comp_unit && sc.function);
   size_t functions_added = 0;
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
@@ -3091,6 +3128,7 @@
 }
 
 size_t SymbolFileDWARF::ParseTypes(const SymbolContext &sc) {
+  AssertModuleLock();
   // At least a compile unit must be valid
   assert(sc.comp_unit);
   size_t types_added = 0;
@@ -3114,6 +3152,7 @@
 }
 
 size_t SymbolFileDWARF::ParseVariablesForContext(const SymbolContext &sc) {
+  AssertModuleLock();
   if (sc.comp_unit != NULL) {
     DWARFDebugInfo *info = DebugInfo();
     if (info == NULL)
Index: include/lldb/Symbol/SymbolFile.h
===================================================================
--- include/lldb/Symbol/SymbolFile.h
+++ include/lldb/Symbol/SymbolFile.h
@@ -20,6 +20,8 @@
 
 #include "llvm/ADT/DenseSet.h"
 
+#include <mutex>
+
 namespace lldb_private {
 
 class SymbolFile : public PluginInterface {
@@ -93,6 +95,8 @@
 
   virtual uint32_t CalculateAbilities() = 0;
 
+  virtual std::recursive_mutex &GetModuleMutex() const;
+
   //------------------------------------------------------------------
   /// Initialize the SymbolFile object.
   ///
@@ -208,6 +212,8 @@
   virtual void Dump(Stream &s) {}
 
 protected:
+  void AssertModuleLock();
+
   ObjectFile *m_obj_file; // The object file that symbols can be extracted from.
   uint32_t m_abilities;
   bool m_calculated_abilities;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to