JDevlieghere updated this revision to Diff 170465.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added a comment.
Address Greg's feedback.
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,17 @@
types.Clear();
return 0;
}
+
+void SymbolFile::AssertModuleLock() {
+ // The code below is too expensive to leave enabled in release builds. It's
+ // enabled in debug builds or when the correct macro is set.
+#if defined(LLDB_CONFIGURATION_DEBUG)
+ // 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.
+ assert(std::async(std::launch::async,
+ [this] { return this->GetModuleMutex().try_lock(); })
+ .get() == false &&
+ "Module is not locked");
+#endif
+}
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)
{
+ ASSERT_MODULE_LOCK(this);
TypeSet type_set;
CompileUnit *comp_unit = NULL;
@@ -860,6 +864,7 @@
}
CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+ ASSERT_MODULE_LOCK(this);
CompUnitSP cu_sp;
DWARFDebugInfo *info = DebugInfo();
if (info) {
@@ -872,6 +877,7 @@
Function *SymbolFileDWARF::ParseCompileUnitFunction(const SymbolContext &sc,
const DWARFDIE &die) {
+ ASSERT_MODULE_LOCK(this);
if (die.IsValid()) {
TypeSystem *type_system =
GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
@@ -895,6 +901,7 @@
}
lldb::LanguageType
SymbolFileDWARF::ParseCompileUnitLanguage(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
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) {
+ ASSERT_MODULE_LOCK(this);
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) {
+ ASSERT_MODULE_LOCK(this);
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) {
+ ASSERT_MODULE_LOCK(this);
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) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(sc.comp_unit);
if (dwarf_cu) {
@@ -1037,6 +1048,7 @@
}
bool SymbolFileDWARF::ParseCompileUnitLineTable(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
assert(sc.comp_unit);
if (sc.comp_unit->GetLineTable() != NULL)
return true;
@@ -1122,6 +1134,7 @@
}
bool SymbolFileDWARF::ParseCompileUnitDebugMacros(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
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 {
+ lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
+ if (module_sp)
+ return module_sp->GetMutex();
+ return GetObjectFile()->GetModule()->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) {
+ ASSERT_MODULE_LOCK(this);
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) {
+ ASSERT_MODULE_LOCK(this);
// 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) {
+ ASSERT_MODULE_LOCK(this);
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,14 @@
#include "llvm/ADT/DenseSet.h"
+#include <mutex>
+
+#if defined(LLDB_CONFIGURATION_DEBUG)
+#define ASSERT_MODULE_LOCK(expr) (expr->AssertModuleLock();)
+#else
+#define ASSERT_MODULE_LOCK(expr) ((void)0)
+#endif
+
namespace lldb_private {
class SymbolFile : public PluginInterface {
@@ -93,6 +101,12 @@
virtual uint32_t CalculateAbilities() = 0;
+ //------------------------------------------------------------------
+ /// Symbols file subclasses should override this to return the Module that
+ /// owns the TypeSystem that this symbol file modifies type information in.
+ //------------------------------------------------------------------
+ virtual std::recursive_mutex &GetModuleMutex() const;
+
//------------------------------------------------------------------
/// Initialize the SymbolFile object.
///
@@ -208,6 +222,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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits