[Lldb-commits] [lldb] r344945 - [SymbolFile] Add the module lock where necessary and assert that we own it.
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 22 13:14:36 PDT 2018
Author: jdevlieghere
Date: Mon Oct 22 13:14:36 2018
New Revision: 344945
URL: http://llvm.org/viewvc/llvm-project?rev=344945&view=rev
Log:
[SymbolFile] Add the module lock where necessary and assert that we own it.
As discussed with Greg at the dev meeting, we need to ensure we have the
module lock in the SymbolFile. Usually the symbol file is accessed
through the symbol vendor which ensures that the necessary locks are
taken. However, there are a few methods that are accessed by the
expression parser and were lacking the lock.
This patch adds the locking where necessary and everywhere else asserts
that we actually already own the lock.
Differential revision: https://reviews.llvm.org/D52543
Modified:
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Symbol/SymbolFile.cpp
Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=344945&r1=344944&r2=344945&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Mon Oct 22 13:14:36 2018
@@ -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 {
@@ -94,6 +102,12 @@ public:
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.
///
/// The SymbolFile object with the best set of abilities (detected
@@ -208,6 +222,8 @@ public:
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;
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=344945&r1=344944&r2=344945&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Mon Oct 22 13:14:36 2018
@@ -260,6 +260,9 @@ SymbolFile *SymbolFileDWARF::CreateInsta
}
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 @@ size_t SymbolFileDWARF::GetTypes(SymbolC
uint32_t type_mask, TypeList &type_list)
{
+ ASSERT_MODULE_LOCK(this);
TypeSet type_set;
CompileUnit *comp_unit = NULL;
@@ -860,6 +864,7 @@ uint32_t SymbolFileDWARF::GetNumCompileU
}
CompUnitSP SymbolFileDWARF::ParseCompileUnitAtIndex(uint32_t cu_idx) {
+ ASSERT_MODULE_LOCK(this);
CompUnitSP cu_sp;
DWARFDebugInfo *info = DebugInfo();
if (info) {
@@ -872,6 +877,7 @@ CompUnitSP SymbolFileDWARF::ParseCompile
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 @@ bool SymbolFileDWARF::FixupAddress(Addre
}
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 @@ SymbolFileDWARF::ParseCompileUnitLanguag
}
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 @@ size_t SymbolFileDWARF::ParseCompileUnit
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::ParseCompileUnitSu
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::ParseCompileUnitIs
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 @@ static void ParseDWARFLineTableCallback(
}
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 @@ SymbolFileDWARF::ParseDebugMacros(lldb::
}
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 @@ void SymbolFileDWARF::ParseDeclsForConte
}
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 @@ SymbolFileDWARF *SymbolFileDWARF::GetDWA
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 @@ SymbolFileDWARF::GetDIEFromUID(lldb::use
}
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 @@ CompilerDecl SymbolFileDWARF::GetDeclFor
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 @@ SymbolFileDWARF::GetDeclContextForUID(ll
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 @@ SymbolFileDWARF::GetDeclContextContainin
}
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::HasForwardDeclForC
}
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 @@ uint32_t SymbolFileDWARF::ResolveSymbolC
}
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::ParseTypes(const
}
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::ParseFunctionBlo
}
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::ParseTypes(const
}
size_t SymbolFileDWARF::ParseVariablesForContext(const SymbolContext &sc) {
+ ASSERT_MODULE_LOCK(this);
if (sc.comp_unit != NULL) {
DWARFDebugInfo *info = DebugInfo();
if (info == NULL)
Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h?rev=344945&r1=344944&r2=344945&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h Mon Oct 22 13:14:36 2018
@@ -228,6 +228,8 @@ public:
void PreloadSymbols() override;
+ std::recursive_mutex &GetModuleMutex() const override;
+
//------------------------------------------------------------------
// PluginInterface protocol
//------------------------------------------------------------------
Modified: lldb/trunk/source/Symbol/SymbolFile.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/SymbolFile.cpp?rev=344945&r1=344944&r2=344945&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/SymbolFile.cpp (original)
+++ lldb/trunk/source/Symbol/SymbolFile.cpp Mon Oct 22 13:14:36 2018
@@ -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 @@ size_t SymbolFile::FindTypes(const std::
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
+}
More information about the lldb-commits
mailing list