[Lldb-commits] [lldb] 430e145 - Clean up PlatformDarwinKernel::GetSharedModule, document (#78652)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Jan 18 23:26:19 PST 2024
Author: Jason Molenda
Date: 2024-01-18T23:26:15-08:00
New Revision: 430e145fc3964dac0bbf355f27616190d403dd83
URL: https://github.com/llvm/llvm-project/commit/430e145fc3964dac0bbf355f27616190d403dd83
DIFF: https://github.com/llvm/llvm-project/commit/430e145fc3964dac0bbf355f27616190d403dd83.diff
LOG: Clean up PlatformDarwinKernel::GetSharedModule, document (#78652)
PlatformDarwinKernel::GetSharedModule, which can find a kernel or kext
from a local filesystem scan, needed a little cleanup. The method which
finds kernels was (1) not looking for the SymbolFileSpec when creating a
Module, and (2) adding that newly created Module to a Target, which
GetSharedModule should not be doing - after auditing many other subclass
implementations of this method, I haven't found any others doing it.
Platform::GetSharedModule didn't have a headerdoc so it took a little
work to piece together the intended behaviors.
This is addressing a bug where
PlatformDarwinKernel::GetSharedModuleKernel would find the ObjectFile
for a kernel, create a Module, and add it to the Target. Then up in
DynamicLoaderDarwinKernel, it would check if the Module had a SymbolFile
FileSpec, and because it did not, it would do its own search for a
binary & dSYM, find them, and then add that to the Target. Now we have
two copies of the Module in the Target, one with a dSYM and the other
without, and only one of them has its load addresses set.
GetSharedModule should not be adding binaries to the Target, and it
should set the SymbolFile FileSpec when it is creating the Module.
rdar://120895951
Added:
Modified:
lldb/include/lldb/Target/Platform.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 129e4565d9ff99..13196ff74d663f 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -288,6 +288,32 @@ class Platform : public PluginInterface {
LocateExecutableScriptingResources(Target *target, Module &module,
Stream &feedback_stream);
+ /// \param[in] module_spec
+ /// The ModuleSpec of a binary to find.
+ ///
+ /// \param[in] process
+ /// A Process.
+ ///
+ /// \param[out] module_sp
+ /// A Module that matches the ModuleSpec, if one is found.
+ ///
+ /// \param[in] module_search_paths_ptr
+ /// Locations to possibly look for a binary that matches the ModuleSpec.
+ ///
+ /// \param[out] old_modules
+ /// Existing Modules in the Process' Target image list which match
+ /// the FileSpec.
+ ///
+ /// \param[out] did_create_ptr
+ /// Optional boolean, nullptr may be passed for this argument.
+ /// If this method is returning a *new* ModuleSP, this
+ /// will be set to true.
+ /// If this method is returning a ModuleSP that is already in the
+ /// Target's image list, it will be false.
+ ///
+ /// \return
+ /// The Status object for any errors found while searching for
+ /// the binary.
virtual Status GetSharedModule(
const ModuleSpec &module_spec, Process *process,
lldb::ModuleSP &module_sp, const FileSpecList *module_search_paths_ptr,
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
index e2839f3285cce4..1f121f5c4e5d0a 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -791,9 +791,10 @@ Status PlatformDarwinKernel::GetSharedModuleKernel(
const ModuleSpec &module_spec, Process *process, ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr,
llvm::SmallVectorImpl<ModuleSP> *old_modules, bool *did_create_ptr) {
- Status error;
- module_sp.reset();
+ assert(module_sp.get() == nullptr);
UpdateKextandKernelsLocalScan();
+ if (did_create_ptr)
+ *did_create_ptr = false;
// First try all kernel binaries that have a dSYM next to them
for (auto possible_kernel : m_kernel_binaries_with_dsyms) {
@@ -803,22 +804,19 @@ Status PlatformDarwinKernel::GetSharedModuleKernel(
module_sp.reset(new Module(kern_spec));
if (module_sp && module_sp->GetObjectFile() &&
module_sp->MatchesModuleSpec(kern_spec)) {
- // module_sp is an actual kernel binary we want to add.
- if (process) {
- const bool notify = false;
- process->GetTarget().GetImages().AppendIfNeeded(module_sp, notify);
- error.Clear();
- return error;
- } else {
- error = ModuleList::GetSharedModule(kern_spec, module_sp, nullptr,
- nullptr, nullptr);
- if (module_sp && module_sp->GetObjectFile() &&
- module_sp->GetObjectFile()->GetType() !=
- ObjectFile::Type::eTypeCoreFile) {
- return error;
- }
- module_sp.reset();
- }
+ // The dSYM is next to the binary (that's the only
+ // way it ends up in the index), but it might be a
+ // .dSYM.yaa that needs to be expanded, don't just
+ // append ".dSYM" to the filename for the SymbolFile.
+ FileSpecList search_paths =
+ process->GetTarget().GetDebugFileSearchPaths();
+ FileSpec dsym_fspec =
+ PluginManager::LocateExecutableSymbolFile(kern_spec, search_paths);
+ if (FileSystem::Instance().Exists(dsym_fspec))
+ module_sp->SetSymbolFileFileSpec(dsym_fspec);
+ if (did_create_ptr)
+ *did_create_ptr = true;
+ return {};
}
}
}
@@ -836,36 +834,18 @@ Status PlatformDarwinKernel::GetSharedModuleKernel(
module_sp.reset(new Module(kern_spec));
if (module_sp && module_sp->GetObjectFile() &&
module_sp->MatchesModuleSpec(kern_spec)) {
- // module_sp is an actual kernel binary we want to add.
- if (process) {
- const bool notify = false;
- process->GetTarget().GetImages().AppendIfNeeded(module_sp, notify);
- error.Clear();
- return error;
- } else {
- error = ModuleList::GetSharedModule(kern_spec, module_sp, nullptr,
- nullptr, nullptr);
- if (module_sp && module_sp->GetObjectFile() &&
- module_sp->GetObjectFile()->GetType() !=
- ObjectFile::Type::eTypeCoreFile) {
- return error;
- }
- module_sp.reset();
- }
+ if (did_create_ptr)
+ *did_create_ptr = true;
+ return {};
}
}
}
- // Give the generic methods, including possibly calling into DebugSymbols
+ // Give the generic methods, including possibly calling into DebugSymbols
// framework on macOS systems, a chance.
- error = PlatformDarwin::GetSharedModule(module_spec, process, module_sp,
- module_search_paths_ptr, old_modules,
- did_create_ptr);
- if (error.Success() && module_sp.get()) {
- return error;
- }
-
- return error;
+ return PlatformDarwin::GetSharedModule(module_spec, process, module_sp,
+ module_search_paths_ptr, old_modules,
+ did_create_ptr);
}
std::vector<lldb_private::FileSpec>
More information about the lldb-commits
mailing list