Skip to content

Commit 61bfc70

Browse files
JosephTremouletJosephTremoulet
committed
[lldb] GetSharedModule: Collect old modules in SmallVector
The various GetSharedModule methods have an optional out parameter for the old module when a file has changed or been replaced, which the Target uses to keep its module list current/correct. We've been using a single ModuleSP to track "the" old module, and this change switches to using a SmallVector of ModuleSP, which has a couple benefits: - There are multiple codepaths which may discover an old module, and this centralizes the code for how to handle multiples in one place, in the Target code. With the single ModuleSP, each place that may discover an old module is responsible for how it handles multiples, and the current code is inconsistent (some code paths drop the first old module, others drop the second). - The API will be more natural for identifying old modules in routines that work on sets, like ModuleList::ReplaceEquivalent (which I plan on updating to report old module(s) in a subsequent change to fix a bug). I'm not convinced we can ever actually run into the case that multiple old modules are found in the same GetOrCreateModule call, but I think this change makes sense regardless, in light of the above. When an old module is reported, Target::GetOrCreateModule calls m_images.ReplaceModule, which doesn't allow multiple "old" modules; the new code calls ReplaceModule for the first "old" module, and for any subsequent old modules it logs the event and calls m_images.Remove. Reviewed By: jingham Differential Revision: https://reviews.llvm.org/D89156
1 parent 7aac3a9 commit 61bfc70

15 files changed

+176
-122
lines changed

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -443,12 +443,11 @@ class ModuleList {
443443

444444
static bool ModuleIsInCache(const Module *module_ptr);
445445

446-
static Status GetSharedModule(const ModuleSpec &module_spec,
447-
lldb::ModuleSP &module_sp,
448-
const FileSpecList *module_search_paths_ptr,
449-
lldb::ModuleSP *old_module_sp_ptr,
450-
bool *did_create_ptr,
451-
bool always_create = false);
446+
static Status
447+
GetSharedModule(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
448+
const FileSpecList *module_search_paths_ptr,
449+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
450+
bool *did_create_ptr, bool always_create = false);
452451

453452
static bool RemoveSharedModule(lldb::ModuleSP &module_sp);
454453

‎lldb/include/lldb/Target/Platform.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,11 +301,10 @@ class Platform : public PluginInterface {
301301
LocateExecutableScriptingResources(Target *target, Module &module,
302302
Stream *feedback_stream);
303303

304-
virtual Status GetSharedModule(const ModuleSpec &module_spec,
305-
Process *process, lldb::ModuleSP &module_sp,
306-
const FileSpecList *module_search_paths_ptr,
307-
lldb::ModuleSP *old_module_sp_ptr,
308-
bool *did_create_ptr);
304+
virtual Status GetSharedModule(
305+
const ModuleSpec &module_spec, Process *process,
306+
lldb::ModuleSP &module_sp, const FileSpecList *module_search_paths_ptr,
307+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, bool *did_create_ptr);
309308

310309
virtual bool GetModuleSpec(const FileSpec &module_file_spec,
311310
const ArchSpec &arch, ModuleSpec &module_spec);

‎lldb/source/Core/ModuleList.cpp

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -741,11 +741,11 @@ size_t ModuleList::RemoveOrphanSharedModules(bool mandatory) {
741741
return GetSharedModuleList().RemoveOrphans(mandatory);
742742
}
743743

744-
Status ModuleList::GetSharedModule(const ModuleSpec &module_spec,
745-
ModuleSP &module_sp,
746-
const FileSpecList *module_search_paths_ptr,
747-
ModuleSP *old_module_sp_ptr,
748-
bool *did_create_ptr, bool always_create) {
744+
Status
745+
ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
746+
const FileSpecList *module_search_paths_ptr,
747+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
748+
bool *did_create_ptr, bool always_create) {
749749
ModuleList &shared_module_list = GetSharedModuleList();
750750
std::lock_guard<std::recursive_mutex> guard(
751751
shared_module_list.m_modules_mutex);
@@ -757,8 +757,6 @@ Status ModuleList::GetSharedModule(const ModuleSpec &module_spec,
757757

758758
if (did_create_ptr)
759759
*did_create_ptr = false;
760-
if (old_module_sp_ptr)
761-
old_module_sp_ptr->reset();
762760

763761
const UUID *uuid_ptr = module_spec.GetUUIDPtr();
764762
const FileSpec &module_file_spec = module_spec.GetFileSpec();
@@ -779,8 +777,8 @@ Status ModuleList::GetSharedModule(const ModuleSpec &module_spec,
779777

780778
// Make sure the file for the module hasn't been modified
781779
if (module_sp->FileHasChanged()) {
782-
if (old_module_sp_ptr && !*old_module_sp_ptr)
783-
*old_module_sp_ptr = module_sp;
780+
if (old_modules)
781+
old_modules->push_back(module_sp);
784782

785783
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_MODULES));
786784
if (log != nullptr)
@@ -934,8 +932,8 @@ Status ModuleList::GetSharedModule(const ModuleSpec &module_spec,
934932
located_binary_modulespec.GetFileSpec());
935933
if (file_spec_mod_time != llvm::sys::TimePoint<>()) {
936934
if (file_spec_mod_time != module_sp->GetModificationTime()) {
937-
if (old_module_sp_ptr)
938-
*old_module_sp_ptr = module_sp;
935+
if (old_modules)
936+
old_modules->push_back(module_sp);
939937
shared_module_list.Remove(module_sp);
940938
module_sp.reset();
941939
}

‎lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,8 @@ Status PlatformAppleSimulator::GetSymbolFile(const FileSpec &platform_file,
460460

461461
Status PlatformAppleSimulator::GetSharedModule(
462462
const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
463-
const FileSpecList *module_search_paths_ptr, ModuleSP *old_module_sp_ptr,
464-
bool *did_create_ptr) {
463+
const FileSpecList *module_search_paths_ptr,
464+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, bool *did_create_ptr) {
465465
// For iOS/tvOS/watchOS, the SDK files are all cached locally on the
466466
// host system. So first we ask for the file in the cached SDK, then
467467
// we attempt to get a shared module for the right architecture with
@@ -476,9 +476,9 @@ Status PlatformAppleSimulator::GetSharedModule(
476476
module_search_paths_ptr);
477477
} else {
478478
const bool always_create = false;
479-
error = ModuleList::GetSharedModule(
480-
module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
481-
did_create_ptr, always_create);
479+
error = ModuleList::GetSharedModule(module_spec, module_sp,
480+
module_search_paths_ptr, old_modules,
481+
did_create_ptr, always_create);
482482
}
483483
if (module_sp)
484484
module_sp->SetPlatformFileSpec(platform_file);

‎lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class PlatformAppleSimulator : public PlatformDarwin {
7575
GetSharedModule(const lldb_private::ModuleSpec &module_spec,
7676
lldb_private::Process *process, lldb::ModuleSP &module_sp,
7777
const lldb_private::FileSpecList *module_search_paths_ptr,
78-
lldb::ModuleSP *old_module_sp_ptr,
78+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
7979
bool *did_create_ptr) override;
8080

8181
uint32_t

‎lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ BringInRemoteFile(Platform *platform,
221221
lldb_private::Status PlatformDarwin::GetSharedModuleWithLocalCache(
222222
const lldb_private::ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
223223
const lldb_private::FileSpecList *module_search_paths_ptr,
224-
lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr) {
224+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, bool *did_create_ptr) {
225225

226226
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM));
227227
LLDB_LOGF(log,
@@ -254,15 +254,15 @@ lldb_private::Status PlatformDarwin::GetSharedModuleWithLocalCache(
254254
ModuleSpec shared_cache_spec(module_spec.GetFileSpec(), image_info.uuid,
255255
image_info.data_sp);
256256
err = ModuleList::GetSharedModule(shared_cache_spec, module_sp,
257-
module_search_paths_ptr,
258-
old_module_sp_ptr, did_create_ptr);
257+
module_search_paths_ptr, old_modules,
258+
did_create_ptr);
259259
if (module_sp)
260260
return err;
261261
}
262262
}
263263

264264
err = ModuleList::GetSharedModule(module_spec, module_sp,
265-
module_search_paths_ptr, old_module_sp_ptr,
265+
module_search_paths_ptr, old_modules,
266266
did_create_ptr);
267267
if (module_sp)
268268
return err;
@@ -365,8 +365,8 @@ lldb_private::Status PlatformDarwin::GetSharedModuleWithLocalCache(
365365

366366
Status PlatformDarwin::GetSharedModule(
367367
const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
368-
const FileSpecList *module_search_paths_ptr, ModuleSP *old_module_sp_ptr,
369-
bool *did_create_ptr) {
368+
const FileSpecList *module_search_paths_ptr,
369+
llvm::SmallVectorImpl<ModuleSP> *old_modules, bool *did_create_ptr) {
370370
Status error;
371371
module_sp.reset();
372372

@@ -375,16 +375,16 @@ Status PlatformDarwin::GetSharedModule(
375375
// module first.
376376
if (m_remote_platform_sp) {
377377
error = m_remote_platform_sp->GetSharedModule(
378-
module_spec, process, module_sp, module_search_paths_ptr,
379-
old_module_sp_ptr, did_create_ptr);
378+
module_spec, process, module_sp, module_search_paths_ptr, old_modules,
379+
did_create_ptr);
380380
}
381381
}
382382

383383
if (!module_sp) {
384384
// Fall back to the local platform and find the file locally
385385
error = Platform::GetSharedModule(module_spec, process, module_sp,
386-
module_search_paths_ptr,
387-
old_module_sp_ptr, did_create_ptr);
386+
module_search_paths_ptr, old_modules,
387+
did_create_ptr);
388388

389389
const FileSpec &platform_file = module_spec.GetFileSpec();
390390
if (!module_sp && module_search_paths_ptr && platform_file) {
@@ -397,7 +397,7 @@ Status PlatformDarwin::GetSharedModule(
397397
new_module_spec.GetFileSpec() = bundle_directory;
398398
if (Host::ResolveExecutableInBundle(new_module_spec.GetFileSpec())) {
399399
Status new_error(Platform::GetSharedModule(
400-
new_module_spec, process, module_sp, nullptr, old_module_sp_ptr,
400+
new_module_spec, process, module_sp, nullptr, old_modules,
401401
did_create_ptr));
402402

403403
if (module_sp)
@@ -424,8 +424,8 @@ Status PlatformDarwin::GetSharedModule(
424424
ModuleSpec new_module_spec(module_spec);
425425
new_module_spec.GetFileSpec() = new_file_spec;
426426
Status new_error(Platform::GetSharedModule(
427-
new_module_spec, process, module_sp, nullptr,
428-
old_module_sp_ptr, did_create_ptr));
427+
new_module_spec, process, module_sp, nullptr, old_modules,
428+
did_create_ptr));
429429

430430
if (module_sp) {
431431
module_sp->SetPlatformFileSpec(new_file_spec);
@@ -1727,8 +1727,8 @@ PlatformDarwin::LaunchProcess(lldb_private::ProcessLaunchInfo &launch_info) {
17271727

17281728
lldb_private::Status PlatformDarwin::FindBundleBinaryInExecSearchPaths(
17291729
const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
1730-
const FileSpecList *module_search_paths_ptr, ModuleSP *old_module_sp_ptr,
1731-
bool *did_create_ptr) {
1730+
const FileSpecList *module_search_paths_ptr,
1731+
llvm::SmallVectorImpl<ModuleSP> *old_modules, bool *did_create_ptr) {
17321732
const FileSpec &platform_file = module_spec.GetFileSpec();
17331733
// See if the file is present in any of the module_search_paths_ptr
17341734
// directories.
@@ -1785,9 +1785,9 @@ lldb_private::Status PlatformDarwin::FindBundleBinaryInExecSearchPaths(
17851785
if (FileSystem::Instance().Exists(path_to_try)) {
17861786
ModuleSpec new_module_spec(module_spec);
17871787
new_module_spec.GetFileSpec() = path_to_try;
1788-
Status new_error(Platform::GetSharedModule(
1789-
new_module_spec, process, module_sp, nullptr, old_module_sp_ptr,
1790-
did_create_ptr));
1788+
Status new_error(
1789+
Platform::GetSharedModule(new_module_spec, process, module_sp,
1790+
nullptr, old_modules, did_create_ptr));
17911791

17921792
if (module_sp) {
17931793
module_sp->SetPlatformFileSpec(path_to_try);

‎lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class PlatformDarwin : public PlatformPOSIX {
4747
GetSharedModule(const lldb_private::ModuleSpec &module_spec,
4848
lldb_private::Process *process, lldb::ModuleSP &module_sp,
4949
const lldb_private::FileSpecList *module_search_paths_ptr,
50-
lldb::ModuleSP *old_module_sp_ptr,
50+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
5151
bool *did_create_ptr) override;
5252

5353
size_t GetSoftwareBreakpointTrapOpcode(
@@ -138,7 +138,7 @@ class PlatformDarwin : public PlatformPOSIX {
138138
virtual lldb_private::Status GetSharedModuleWithLocalCache(
139139
const lldb_private::ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
140140
const lldb_private::FileSpecList *module_search_paths_ptr,
141-
lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
141+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, bool *did_create_ptr);
142142

143143
struct SDKEnumeratorInfo {
144144
lldb_private::FileSpec found_path;
@@ -164,7 +164,7 @@ class PlatformDarwin : public PlatformPOSIX {
164164
const lldb_private::ModuleSpec &module_spec,
165165
lldb_private::Process *process, lldb::ModuleSP &module_sp,
166166
const lldb_private::FileSpecList *module_search_paths_ptr,
167-
lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
167+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, bool *did_create_ptr);
168168

169169
static std::string FindComponentInPath(llvm::StringRef path,
170170
llvm::StringRef component);

‎lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -727,8 +727,8 @@ PlatformDarwinKernel::GetDWARFBinaryInDSYMBundle(FileSpec dsym_bundle) {
727727

728728
Status PlatformDarwinKernel::GetSharedModule(
729729
const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
730-
const FileSpecList *module_search_paths_ptr, ModuleSP *old_module_sp_ptr,
731-
bool *did_create_ptr) {
730+
const FileSpecList *module_search_paths_ptr,
731+
llvm::SmallVectorImpl<ModuleSP> *old_modules, bool *did_create_ptr) {
732732
Status error;
733733
module_sp.reset();
734734
const FileSpec &platform_file = module_spec.GetFileSpec();
@@ -740,26 +740,26 @@ Status PlatformDarwinKernel::GetSharedModule(
740740
if (!kext_bundle_id.empty() && module_spec.GetUUID().IsValid()) {
741741
if (kext_bundle_id == "mach_kernel") {
742742
return GetSharedModuleKernel(module_spec, process, module_sp,
743-
module_search_paths_ptr, old_module_sp_ptr,
743+
module_search_paths_ptr, old_modules,
744744
did_create_ptr);
745745
} else {
746746
return GetSharedModuleKext(module_spec, process, module_sp,
747-
module_search_paths_ptr, old_module_sp_ptr,
747+
module_search_paths_ptr, old_modules,
748748
did_create_ptr);
749749
}
750750
} else {
751751
// Give the generic methods, including possibly calling into DebugSymbols
752752
// framework on macOS systems, a chance.
753753
return PlatformDarwin::GetSharedModule(module_spec, process, module_sp,
754-
module_search_paths_ptr,
755-
old_module_sp_ptr, did_create_ptr);
754+
module_search_paths_ptr, old_modules,
755+
did_create_ptr);
756756
}
757757
}
758758

759759
Status PlatformDarwinKernel::GetSharedModuleKext(
760760
const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
761-
const FileSpecList *module_search_paths_ptr, ModuleSP *old_module_sp_ptr,
762-
bool *did_create_ptr) {
761+
const FileSpecList *module_search_paths_ptr,
762+
llvm::SmallVectorImpl<ModuleSP> *old_modules, bool *did_create_ptr) {
763763
Status error;
764764
module_sp.reset();
765765
const FileSpec &platform_file = module_spec.GetFileSpec();
@@ -785,8 +785,8 @@ Status PlatformDarwinKernel::GetSharedModuleKext(
785785
// Give the generic methods, including possibly calling into DebugSymbols
786786
// framework on macOS systems, a chance.
787787
error = PlatformDarwin::GetSharedModule(module_spec, process, module_sp,
788-
module_search_paths_ptr,
789-
old_module_sp_ptr, did_create_ptr);
788+
module_search_paths_ptr, old_modules,
789+
did_create_ptr);
790790
if (error.Success() && module_sp.get()) {
791791
return error;
792792
}
@@ -811,8 +811,8 @@ Status PlatformDarwinKernel::GetSharedModuleKext(
811811

812812
Status PlatformDarwinKernel::GetSharedModuleKernel(
813813
const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
814-
const FileSpecList *module_search_paths_ptr, ModuleSP *old_module_sp_ptr,
815-
bool *did_create_ptr) {
814+
const FileSpecList *module_search_paths_ptr,
815+
llvm::SmallVectorImpl<ModuleSP> *old_modules, bool *did_create_ptr) {
816816
Status error;
817817
module_sp.reset();
818818

@@ -878,8 +878,8 @@ Status PlatformDarwinKernel::GetSharedModuleKernel(
878878
// Give the generic methods, including possibly calling into DebugSymbols
879879
// framework on macOS systems, a chance.
880880
error = PlatformDarwin::GetSharedModule(module_spec, process, module_sp,
881-
module_search_paths_ptr,
882-
old_module_sp_ptr, did_create_ptr);
881+
module_search_paths_ptr, old_modules,
882+
did_create_ptr);
883883
if (error.Success() && module_sp.get()) {
884884
return error;
885885
}

‎lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class PlatformDarwinKernel : public PlatformDarwin {
5757
GetSharedModule(const lldb_private::ModuleSpec &module_spec,
5858
lldb_private::Process *process, lldb::ModuleSP &module_sp,
5959
const lldb_private::FileSpecList *module_search_paths_ptr,
60-
lldb::ModuleSP *old_module_sp_ptr,
60+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
6161
bool *did_create_ptr) override;
6262

6363
bool GetSupportedArchitectureAtIndex(uint32_t idx,
@@ -143,13 +143,14 @@ class PlatformDarwinKernel : public PlatformDarwin {
143143
GetSharedModuleKext(const lldb_private::ModuleSpec &module_spec,
144144
lldb_private::Process *process, lldb::ModuleSP &module_sp,
145145
const lldb_private::FileSpecList *module_search_paths_ptr,
146-
lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
146+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
147+
bool *did_create_ptr);
147148

148149
lldb_private::Status GetSharedModuleKernel(
149150
const lldb_private::ModuleSpec &module_spec,
150151
lldb_private::Process *process, lldb::ModuleSP &module_sp,
151152
const lldb_private::FileSpecList *module_search_paths_ptr,
152-
lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr);
153+
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, bool *did_create_ptr);
153154

154155
lldb_private::Status
155156
ExamineKextForMatchingUUID(const lldb_private::FileSpec &kext_bundle_path,

0 commit comments

Comments
 (0)