Skip to content

Commit f2e0585

Browse files
JDevlieghereJDevlieghere
committed
[lldb] Access the ModuleList through iterators where possible (NFC)
Replace uses of GetModuleAtIndexUnlocked and GetModulePointerAtIndexUnlocked with the ModuleIterable and ModuleIterableNoLocking where applicable. Differential revision: https://reviews.llvm.org/D94271
1 parent b934160 commit f2e0585

File tree

16 files changed

+114
-220
lines changed

16 files changed

+114
-220
lines changed

‎lldb/include/lldb/Core/ModuleList.h

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -237,20 +237,6 @@ class ModuleList {
237237
/// \see ModuleList::GetSize()
238238
Module *GetModulePointerAtIndex(size_t idx) const;
239239

240-
/// Get the module pointer for the module at index \a idx without acquiring
241-
/// the ModuleList mutex. This MUST already have been acquired with
242-
/// ModuleList::GetMutex and locked for this call to be safe.
243-
///
244-
/// \param[in] idx
245-
/// An index into this module collection.
246-
///
247-
/// \return
248-
/// A pointer to a Module which can by nullptr if \a idx is out
249-
/// of range.
250-
///
251-
/// \see ModuleList::GetSize()
252-
Module *GetModulePointerAtIndexUnlocked(size_t idx) const;
253-
254240
/// Find compile units by partial or full path.
255241
///
256242
/// Finds all compile units that match \a path in all of the modules and
@@ -491,11 +477,13 @@ class ModuleList {
491477
typedef LockingAdaptedIterable<collection, lldb::ModuleSP, vector_adapter,
492478
std::recursive_mutex>
493479
ModuleIterable;
494-
ModuleIterable Modules() { return ModuleIterable(m_modules, GetMutex()); }
480+
ModuleIterable Modules() const {
481+
return ModuleIterable(m_modules, GetMutex());
482+
}
495483

496484
typedef AdaptedIterable<collection, lldb::ModuleSP, vector_adapter>
497485
ModuleIterableNoLocking;
498-
ModuleIterableNoLocking ModulesNoLocking() {
486+
ModuleIterableNoLocking ModulesNoLocking() const {
499487
return ModuleIterableNoLocking(m_modules);
500488
}
501489
};

‎lldb/include/lldb/Utility/Iterable.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ template <typename C, typename E, E (*A)(typename C::const_iterator &),
170170
typename MutexType>
171171
class LockingAdaptedIterable : public AdaptedIterable<C, E, A> {
172172
public:
173-
LockingAdaptedIterable(C &container, MutexType &mutex)
173+
LockingAdaptedIterable(const C &container, MutexType &mutex)
174174
: AdaptedIterable<C, E, A>(container), m_mutex(&mutex) {
175175
m_mutex->lock();
176176
}

‎lldb/source/Breakpoint/Breakpoint.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,6 @@ void Breakpoint::ModulesChanged(ModuleList &module_list, bool load,
508508
"delete_locations: %i\n",
509509
module_list.GetSize(), load, delete_locations);
510510

511-
std::lock_guard<std::recursive_mutex> guard(module_list.GetMutex());
512511
if (load) {
513512
// The logic for handling new modules is:
514513
// 1) If the filter rejects this module, then skip it. 2) Run through the
@@ -525,7 +524,7 @@ void Breakpoint::ModulesChanged(ModuleList &module_list, bool load,
525524
// them after the locations pass. Have to do it this way because resolving
526525
// breakpoints will add new locations potentially.
527526

528-
for (ModuleSP module_sp : module_list.ModulesNoLocking()) {
527+
for (ModuleSP module_sp : module_list.Modules()) {
529528
bool seen = false;
530529
if (!m_filter_sp->ModulePasses(module_sp))
531530
continue;
@@ -589,9 +588,7 @@ void Breakpoint::ModulesChanged(ModuleList &module_list, bool load,
589588
else
590589
removed_locations_event = nullptr;
591590

592-
size_t num_modules = module_list.GetSize();
593-
for (size_t i = 0; i < num_modules; i++) {
594-
ModuleSP module_sp(module_list.GetModuleAtIndexUnlocked(i));
591+
for (ModuleSP module_sp : module_list.Modules()) {
595592
if (m_filter_sp->ModulePasses(module_sp)) {
596593
size_t loc_idx = 0;
597594
size_t num_locations = m_locations.GetSize();

‎lldb/source/Commands/CommandObjectTarget.cpp

Lines changed: 73 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,31 +1399,30 @@ static void DumpBasename(Stream &strm, const FileSpec *file_spec_ptr,
13991399
}
14001400

14011401
static size_t DumpModuleObjfileHeaders(Stream &strm, ModuleList &module_list) {
1402-
size_t num_dumped = 0;
14031402
std::lock_guard<std::recursive_mutex> guard(module_list.GetMutex());
14041403
const size_t num_modules = module_list.GetSize();
1405-
if (num_modules > 0) {
1406-
strm.Printf("Dumping headers for %" PRIu64 " module(s).\n",
1407-
static_cast<uint64_t>(num_modules));
1408-
strm.IndentMore();
1409-
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
1410-
Module *module = module_list.GetModulePointerAtIndexUnlocked(image_idx);
1411-
if (module) {
1412-
if (num_dumped++ > 0) {
1413-
strm.EOL();
1414-
strm.EOL();
1415-
}
1416-
ObjectFile *objfile = module->GetObjectFile();
1417-
if (objfile)
1418-
objfile->Dump(&strm);
1419-
else {
1420-
strm.Format("No object file for module: {0:F}\n",
1421-
module->GetFileSpec());
1422-
}
1404+
if (num_modules == 0)
1405+
return 0;
1406+
1407+
size_t num_dumped = 0;
1408+
strm.Format("Dumping headers for {0} module(s).\n", num_modules);
1409+
strm.IndentMore();
1410+
for (ModuleSP module_sp : module_list.ModulesNoLocking()) {
1411+
if (module_sp) {
1412+
if (num_dumped++ > 0) {
1413+
strm.EOL();
1414+
strm.EOL();
1415+
}
1416+
ObjectFile *objfile = module_sp->GetObjectFile();
1417+
if (objfile)
1418+
objfile->Dump(&strm);
1419+
else {
1420+
strm.Format("No object file for module: {0:F}\n",
1421+
module_sp->GetFileSpec());
14231422
}
14241423
}
1425-
strm.IndentLess();
14261424
}
1425+
strm.IndentLess();
14271426
return num_dumped;
14281427
}
14291428

@@ -2025,25 +2024,23 @@ class CommandObjectTargetModulesDumpSymtab
20252024

20262025
if (command.GetArgumentCount() == 0) {
20272026
// Dump all sections for all modules images
2028-
std::lock_guard<std::recursive_mutex> guard(
2029-
target->GetImages().GetMutex());
2030-
const size_t num_modules = target->GetImages().GetSize();
2027+
const ModuleList &module_list = target->GetImages();
2028+
std::lock_guard<std::recursive_mutex> guard(module_list.GetMutex());
2029+
const size_t num_modules = module_list.GetSize();
20312030
if (num_modules > 0) {
2032-
result.GetOutputStream().Printf("Dumping symbol table for %" PRIu64
2033-
" modules.\n",
2034-
(uint64_t)num_modules);
2035-
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
2031+
result.GetOutputStream().Format(
2032+
"Dumping symbol table for {0} modules.\n", num_modules);
2033+
for (ModuleSP module_sp : module_list.ModulesNoLocking()) {
20362034
if (num_dumped > 0) {
20372035
result.GetOutputStream().EOL();
20382036
result.GetOutputStream().EOL();
20392037
}
20402038
if (m_interpreter.WasInterrupted())
20412039
break;
20422040
num_dumped++;
2043-
DumpModuleSymtab(
2044-
m_interpreter, result.GetOutputStream(),
2045-
target->GetImages().GetModulePointerAtIndexUnlocked(image_idx),
2046-
m_options.m_sort_order, name_preference);
2041+
DumpModuleSymtab(m_interpreter, result.GetOutputStream(),
2042+
module_sp.get(), m_options.m_sort_order,
2043+
name_preference);
20472044
}
20482045
} else {
20492046
result.AppendError("the target has no associated executable images");
@@ -2060,18 +2057,18 @@ class CommandObjectTargetModulesDumpSymtab
20602057
const size_t num_matches =
20612058
FindModulesByName(target, arg_cstr, module_list, true);
20622059
if (num_matches > 0) {
2063-
for (size_t i = 0; i < num_matches; ++i) {
2064-
Module *module = module_list.GetModulePointerAtIndex(i);
2065-
if (module) {
2060+
for (ModuleSP module_sp : module_list.Modules()) {
2061+
if (module_sp) {
20662062
if (num_dumped > 0) {
20672063
result.GetOutputStream().EOL();
20682064
result.GetOutputStream().EOL();
20692065
}
20702066
if (m_interpreter.WasInterrupted())
20712067
break;
20722068
num_dumped++;
2073-
DumpModuleSymtab(m_interpreter, result.GetOutputStream(), module,
2074-
m_options.m_sort_order, name_preference);
2069+
DumpModuleSymtab(m_interpreter, result.GetOutputStream(),
2070+
module_sp.get(), m_options.m_sort_order,
2071+
name_preference);
20752072
}
20762073
}
20772074
} else
@@ -2120,23 +2117,22 @@ class CommandObjectTargetModulesDumpSections
21202117
if (command.GetArgumentCount() == 0) {
21212118
// Dump all sections for all modules images
21222119
const size_t num_modules = target->GetImages().GetSize();
2123-
if (num_modules > 0) {
2124-
result.GetOutputStream().Printf("Dumping sections for %" PRIu64
2125-
" modules.\n",
2126-
(uint64_t)num_modules);
2127-
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
2128-
if (m_interpreter.WasInterrupted())
2129-
break;
2130-
num_dumped++;
2131-
DumpModuleSections(
2132-
m_interpreter, result.GetOutputStream(),
2133-
target->GetImages().GetModulePointerAtIndex(image_idx));
2134-
}
2135-
} else {
2120+
if (num_modules == 0) {
21362121
result.AppendError("the target has no associated executable images");
21372122
result.SetStatus(eReturnStatusFailed);
21382123
return false;
21392124
}
2125+
2126+
result.GetOutputStream().Format("Dumping sections for {0} modules.\n",
2127+
num_modules);
2128+
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
2129+
if (m_interpreter.WasInterrupted())
2130+
break;
2131+
num_dumped++;
2132+
DumpModuleSections(
2133+
m_interpreter, result.GetOutputStream(),
2134+
target->GetImages().GetModulePointerAtIndex(image_idx));
2135+
}
21402136
} else {
21412137
// Dump specified images (by basename or fullpath)
21422138
const char *arg_cstr;
@@ -2198,7 +2194,8 @@ class CommandObjectTargetModulesDumpClangAST
21982194
bool DoExecute(Args &command, CommandReturnObject &result) override {
21992195
Target *target = &GetSelectedTarget();
22002196

2201-
const size_t num_modules = target->GetImages().GetSize();
2197+
const ModuleList &module_list = target->GetImages();
2198+
const size_t num_modules = module_list.GetSize();
22022199
if (num_modules == 0) {
22032200
result.AppendError("the target has no associated executable images");
22042201
result.SetStatus(eReturnStatusFailed);
@@ -2207,14 +2204,12 @@ class CommandObjectTargetModulesDumpClangAST
22072204

22082205
if (command.GetArgumentCount() == 0) {
22092206
// Dump all ASTs for all modules images
2210-
result.GetOutputStream().Printf("Dumping clang ast for %" PRIu64
2211-
" modules.\n",
2212-
(uint64_t)num_modules);
2213-
for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
2207+
result.GetOutputStream().Format("Dumping clang ast for {0} modules.\n",
2208+
num_modules);
2209+
for (ModuleSP module_sp : module_list.ModulesNoLocking()) {
22142210
if (m_interpreter.WasInterrupted())
22152211
break;
2216-
Module *m = target->GetImages().GetModulePointerAtIndex(image_idx);
2217-
if (SymbolFile *sf = m->GetSymbolFile())
2212+
if (SymbolFile *sf = module_sp->GetSymbolFile())
22182213
sf->DumpClangAST(result.GetOutputStream());
22192214
}
22202215
result.SetStatus(eReturnStatusSuccessFinishResult);
@@ -2279,23 +2274,19 @@ class CommandObjectTargetModulesDumpSymfile
22792274
const ModuleList &target_modules = target->GetImages();
22802275
std::lock_guard<std::recursive_mutex> guard(target_modules.GetMutex());
22812276
const size_t num_modules = target_modules.GetSize();
2282-
if (num_modules > 0) {
2283-
result.GetOutputStream().Printf("Dumping debug symbols for %" PRIu64
2284-
" modules.\n",
2285-
(uint64_t)num_modules);
2286-
for (uint32_t image_idx = 0; image_idx < num_modules; ++image_idx) {
2287-
if (m_interpreter.WasInterrupted())
2288-
break;
2289-
if (DumpModuleSymbolFile(
2290-
result.GetOutputStream(),
2291-
target_modules.GetModulePointerAtIndexUnlocked(image_idx)))
2292-
num_dumped++;
2293-
}
2294-
} else {
2277+
if (num_modules == 0) {
22952278
result.AppendError("the target has no associated executable images");
22962279
result.SetStatus(eReturnStatusFailed);
22972280
return false;
22982281
}
2282+
result.GetOutputStream().Format(
2283+
"Dumping debug symbols for {0} modules.\n", num_modules);
2284+
for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
2285+
if (m_interpreter.WasInterrupted())
2286+
break;
2287+
if (DumpModuleSymbolFile(result.GetOutputStream(), module_sp.get()))
2288+
num_dumped++;
2289+
}
22992290
} else {
23002291
// Dump specified images (by basename or fullpath)
23012292
const char *arg_cstr;
@@ -2373,15 +2364,13 @@ class CommandObjectTargetModulesDumpLineTable
23732364

23742365
const ModuleList &target_modules = target->GetImages();
23752366
std::lock_guard<std::recursive_mutex> guard(target_modules.GetMutex());
2376-
const size_t num_modules = target_modules.GetSize();
2377-
if (num_modules > 0) {
2367+
if (target_modules.GetSize() > 0) {
23782368
uint32_t num_dumped = 0;
2379-
for (uint32_t i = 0; i < num_modules; ++i) {
2369+
for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
23802370
if (m_interpreter.WasInterrupted())
23812371
break;
23822372
if (DumpCompileUnitLineTable(
2383-
m_interpreter, result.GetOutputStream(),
2384-
target_modules.GetModulePointerAtIndexUnlocked(i),
2373+
m_interpreter, result.GetOutputStream(), module_sp.get(),
23852374
file_spec,
23862375
m_options.m_verbose ? eDescriptionLevelFull
23872376
: eDescriptionLevelBrief))
@@ -3903,25 +3892,20 @@ class CommandObjectTargetModulesLookup : public CommandObjectParsed {
39033892

39043893
const ModuleList &target_modules = target->GetImages();
39053894
std::lock_guard<std::recursive_mutex> guard(target_modules.GetMutex());
3906-
const size_t num_modules = target_modules.GetSize();
3907-
if (num_modules > 0) {
3908-
for (i = 0; i < num_modules && !syntax_error; ++i) {
3909-
Module *module_pointer =
3910-
target_modules.GetModulePointerAtIndexUnlocked(i);
3911-
3912-
if (module_pointer != current_module.get() &&
3913-
LookupInModule(m_interpreter,
3914-
target_modules.GetModulePointerAtIndexUnlocked(i),
3915-
result, syntax_error)) {
3916-
result.GetOutputStream().EOL();
3917-
num_successful_lookups++;
3918-
}
3919-
}
3920-
} else {
3895+
if (target_modules.GetSize() == 0) {
39213896
result.AppendError("the target has no associated executable images");
39223897
result.SetStatus(eReturnStatusFailed);
39233898
return false;
39243899
}
3900+
3901+
for (ModuleSP module_sp : target_modules.ModulesNoLocking()) {
3902+
if (module_sp != current_module &&
3903+
LookupInModule(m_interpreter, module_sp.get(), result,
3904+
syntax_error)) {
3905+
result.GetOutputStream().EOL();
3906+
num_successful_lookups++;
3907+
}
3908+
}
39253909
} else {
39263910
// Dump specified images (by basename or fullpath)
39273911
const char *arg_cstr;

‎lldb/source/Core/ModuleList.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -346,10 +346,6 @@ void ModuleList::ClearImpl(bool use_notifier) {
346346

347347
Module *ModuleList::GetModulePointerAtIndex(size_t idx) const {
348348
std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
349-
return GetModulePointerAtIndexUnlocked(idx);
350-
}
351-
352-
Module *ModuleList::GetModulePointerAtIndexUnlocked(size_t idx) const {
353349
if (idx < m_modules.size())
354350
return m_modules[idx].get();
355351
return nullptr;

0 commit comments

Comments
 (0)