[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

llvm-svn: 344945
This commit is contained in:
Jonas Devlieghere 2018-10-22 20:14:36 +00:00
parent 7e4edbdd1b
commit 4f78c4f67b
4 changed files with 81 additions and 4 deletions

View File

@ -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 @@ 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.
///
@ -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;

View File

@ -260,6 +260,9 @@ SymbolFile *SymbolFileDWARF::CreateInstance(ObjectFile *obj_file) {
}
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(SymbolContextScope *sc_scope,
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::GetNumCompileUnits() {
}
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::ParseCompileUnitAtIndex(uint32_t cu_idx) {
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(Address &addr) {
}
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::ParseCompileUnitLanguage(const SymbolContext &sc) {
}
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::ParseCompileUnitFunctions(const SymbolContext &sc) {
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::ParseCompileUnitSupportFiles(
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::ParseCompileUnitIsOptimized(
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(dw_offset_t offset,
}
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::offset_t *offset) {
}
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::ParseDeclsForContext(CompilerDeclContext decl_ctx) {
}
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::GetDWARFForUID(lldb::user_id_t uid) {
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::user_id_t uid) {
}
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::GetDeclForUID(lldb::user_id_t type_uid) {
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(lldb::user_id_t type_uid) {
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::GetDeclContextContainingUID(lldb::user_id_t type_uid) {
}
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::HasForwardDeclForClangType(
}
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::ResolveSymbolContext(const FileSpec &file_spec,
}
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 SymbolContext &sc,
}
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::ParseFunctionBlocks(const SymbolContext &sc) {
}
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 SymbolContext &sc) {
}
size_t SymbolFileDWARF::ParseVariablesForContext(const SymbolContext &sc) {
ASSERT_MODULE_LOCK(this);
if (sc.comp_unit != NULL) {
DWARFDebugInfo *info = DebugInfo();
if (info == NULL)

View File

@ -228,6 +228,8 @@ public:
void PreloadSymbols() override;
std::recursive_mutex &GetModuleMutex() const override;
//------------------------------------------------------------------
// PluginInterface protocol
//------------------------------------------------------------------

View File

@ -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::vector<CompilerContext> &context,
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
}