Skip to content

Commit 86ef699

Browse files
authored
[lldb] progressive progress reporting for darwin kernel/firmware (#98845)
When doing firmware/kernel debugging, it is frequent that binaries and debug info need to be retrieved / downloaded, and the lack of progress reports made for a poor experience, with lldb seemingly hung while downloading things over the network. This PR adds progress reports to the critical sites for these use cases.
1 parent c736ca8 commit 86ef699

File tree

4 files changed

+69
-32
lines changed

4 files changed

+69
-32
lines changed

lldb/source/Core/DynamicLoader.cpp

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "lldb/Core/ModuleList.h"
1414
#include "lldb/Core/ModuleSpec.h"
1515
#include "lldb/Core/PluginManager.h"
16+
#include "lldb/Core/Progress.h"
1617
#include "lldb/Core/Section.h"
1718
#include "lldb/Symbol/ObjectFile.h"
1819
#include "lldb/Target/MemoryRegionInfo.h"
@@ -195,20 +196,37 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
195196
Target &target = process->GetTarget();
196197
Status error;
197198

199+
StreamString prog_str;
200+
if (!name.empty()) {
201+
prog_str << name.str() << " ";
202+
}
203+
if (uuid.IsValid())
204+
prog_str << uuid.GetAsString();
205+
if (value_is_offset == 0 && value != LLDB_INVALID_ADDRESS) {
206+
prog_str << "at 0x";
207+
prog_str.PutHex64(value);
208+
}
209+
198210
if (!uuid.IsValid() && !value_is_offset) {
199211
memory_module_sp = ReadUnnamedMemoryModule(process, value, name);
200212

201-
if (memory_module_sp)
213+
if (memory_module_sp) {
202214
uuid = memory_module_sp->GetUUID();
215+
if (uuid.IsValid()) {
216+
prog_str << " ";
217+
prog_str << uuid.GetAsString();
218+
}
219+
}
203220
}
204221
ModuleSpec module_spec;
205222
module_spec.GetUUID() = uuid;
206223
FileSpec name_filespec(name);
207-
if (FileSystem::Instance().Exists(name_filespec))
208-
module_spec.GetFileSpec() = name_filespec;
209224

210225
if (uuid.IsValid()) {
226+
Progress progress("Locating binary", prog_str.GetString().str());
227+
211228
// Has lldb already seen a module with this UUID?
229+
// Or have external lookup enabled in DebugSymbols on macOS.
212230
if (!module_sp)
213231
error = ModuleList::GetSharedModule(module_spec, module_sp, nullptr,
214232
nullptr, nullptr);

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "lldb/Core/Module.h"
1414
#include "lldb/Core/ModuleSpec.h"
1515
#include "lldb/Core/PluginManager.h"
16+
#include "lldb/Core/Progress.h"
1617
#include "lldb/Core/Section.h"
1718
#include "lldb/Interpreter/OptionValueProperties.h"
1819
#include "lldb/Symbol/ObjectFile.h"
@@ -714,7 +715,7 @@ void DynamicLoaderDarwinKernel::KextImageInfo::SetIsKernel(bool is_kernel) {
714715
}
715716

716717
bool DynamicLoaderDarwinKernel::KextImageInfo::LoadImageUsingMemoryModule(
717-
Process *process) {
718+
Process *process, Progress *progress) {
718719
Log *log = GetLog(LLDBLog::DynamicLoader);
719720
if (IsLoaded())
720721
return true;
@@ -757,11 +758,37 @@ bool DynamicLoaderDarwinKernel::KextImageInfo::LoadImageUsingMemoryModule(
757758
const ModuleList &target_images = target.GetImages();
758759
m_module_sp = target_images.FindModule(m_uuid);
759760

761+
StreamString prog_str;
762+
// 'mach_kernel' is a fake name we make up to find kernels
763+
// that were located by the local filesystem scan.
764+
if (GetName() != "mach_kernel")
765+
prog_str << GetName() << " ";
766+
if (GetUUID().IsValid())
767+
prog_str << GetUUID().GetAsString() << " ";
768+
if (GetLoadAddress() != LLDB_INVALID_ADDRESS) {
769+
prog_str << "at 0x";
770+
prog_str.PutHex64(GetLoadAddress());
771+
}
772+
773+
std::unique_ptr<Progress> progress_up;
774+
if (progress)
775+
progress->Increment(1, prog_str.GetString().str());
776+
else {
777+
if (IsKernel())
778+
progress_up = std::make_unique<Progress>("Loading kernel",
779+
prog_str.GetString().str());
780+
else
781+
progress_up = std::make_unique<Progress>("Loading kext",
782+
prog_str.GetString().str());
783+
}
784+
760785
// Search for the kext on the local filesystem via the UUID
761786
if (!m_module_sp && m_uuid.IsValid()) {
762787
ModuleSpec module_spec;
763788
module_spec.GetUUID() = m_uuid;
764-
module_spec.GetArchitecture() = target.GetArchitecture();
789+
if (!m_uuid.IsValid())
790+
module_spec.GetArchitecture() = target.GetArchitecture();
791+
module_spec.GetFileSpec() = FileSpec(m_name);
765792

766793
// If the current platform is PlatformDarwinKernel, create a ModuleSpec
767794
// with the filename set to be the bundle ID for this kext, e.g.
@@ -770,17 +797,9 @@ bool DynamicLoaderDarwinKernel::KextImageInfo::LoadImageUsingMemoryModule(
770797
// system.
771798
PlatformSP platform_sp(target.GetPlatform());
772799
if (platform_sp) {
773-
static ConstString g_platform_name(
774-
PlatformDarwinKernel::GetPluginNameStatic());
775-
if (platform_sp->GetPluginName() == g_platform_name.GetStringRef()) {
776-
ModuleSpec kext_bundle_module_spec(module_spec);
777-
FileSpec kext_filespec(m_name.c_str());
778-
FileSpecList search_paths = target.GetExecutableSearchPaths();
779-
kext_bundle_module_spec.GetFileSpec() = kext_filespec;
780-
platform_sp->GetSharedModule(kext_bundle_module_spec, process,
781-
m_module_sp, &search_paths, nullptr,
782-
nullptr);
783-
}
800+
FileSpecList search_paths = target.GetExecutableSearchPaths();
801+
platform_sp->GetSharedModule(module_spec, process, m_module_sp,
802+
&search_paths, nullptr, nullptr);
784803
}
785804

786805
// Ask the Target to find this file on the local system, if possible.
@@ -1058,12 +1077,9 @@ void DynamicLoaderDarwinKernel::LoadKernelModuleIfNeeded() {
10581077
}
10591078
}
10601079
}
1061-
1062-
if (m_kernel.GetLoadAddress() != LLDB_INVALID_ADDRESS) {
1063-
if (!m_kernel.LoadImageUsingMemoryModule(m_process)) {
1080+
if (m_kernel.GetLoadAddress() != LLDB_INVALID_ADDRESS)
1081+
if (!m_kernel.LoadImageUsingMemoryModule(m_process))
10641082
m_kernel.LoadImageAtFileAddress(m_process);
1065-
}
1066-
}
10671083

10681084
// The operating system plugin gets loaded and initialized in
10691085
// LoadImageUsingMemoryModule when we discover the kernel dSYM. For a core
@@ -1347,19 +1363,18 @@ bool DynamicLoaderDarwinKernel::ParseKextSummaries(
13471363
std::vector<std::pair<std::string, UUID>> kexts_failed_to_load;
13481364
if (number_of_new_kexts_being_added > 0) {
13491365
ModuleList loaded_module_list;
1366+
Progress progress("Loading kext", "", number_of_new_kexts_being_added);
13501367

13511368
const uint32_t num_of_new_kexts = kext_summaries.size();
13521369
for (uint32_t new_kext = 0; new_kext < num_of_new_kexts; new_kext++) {
13531370
if (to_be_added[new_kext]) {
13541371
KextImageInfo &image_info = kext_summaries[new_kext];
1355-
bool kext_successfully_added = true;
13561372
if (load_kexts) {
1357-
if (!image_info.LoadImageUsingMemoryModule(m_process)) {
1373+
if (!image_info.LoadImageUsingMemoryModule(m_process, &progress)) {
13581374
kexts_failed_to_load.push_back(std::pair<std::string, UUID>(
13591375
kext_summaries[new_kext].GetName(),
13601376
kext_summaries[new_kext].GetUUID()));
13611377
image_info.LoadImageAtFileAddress(m_process);
1362-
kext_successfully_added = false;
13631378
}
13641379
}
13651380

@@ -1369,13 +1384,6 @@ bool DynamicLoaderDarwinKernel::ParseKextSummaries(
13691384
m_process->GetStopID() == image_info.GetProcessStopId())
13701385
loaded_module_list.AppendIfNeeded(image_info.GetModule());
13711386

1372-
if (load_kexts) {
1373-
if (kext_successfully_added)
1374-
s.Printf(".");
1375-
else
1376-
s.Printf("-");
1377-
}
1378-
13791387
if (log)
13801388
kext_summaries[new_kext].PutToLog(log);
13811389
}

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "lldb/Host/SafeMachO.h"
1818

19+
#include "lldb/Core/Progress.h"
1920
#include "lldb/Target/DynamicLoader.h"
2021
#include "lldb/Target/Process.h"
2122
#include "lldb/Utility/FileSpec.h"
@@ -137,7 +138,8 @@ class DynamicLoaderDarwinKernel : public lldb_private::DynamicLoader {
137138

138139
bool LoadImageAtFileAddress(lldb_private::Process *process);
139140

140-
bool LoadImageUsingMemoryModule(lldb_private::Process *process);
141+
bool LoadImageUsingMemoryModule(lldb_private::Process *process,
142+
lldb_private::Progress *progress = nullptr);
141143

142144
bool IsLoaded() { return m_load_process_stop_id != UINT32_MAX; }
143145

lldb/source/Target/Process.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "lldb/Core/Module.h"
2222
#include "lldb/Core/ModuleSpec.h"
2323
#include "lldb/Core/PluginManager.h"
24+
#include "lldb/Core/Progress.h"
2425
#include "lldb/Expression/DiagnosticManager.h"
2526
#include "lldb/Expression/DynamicCheckerFunctions.h"
2627
#include "lldb/Expression/UserExpression.h"
@@ -2550,6 +2551,14 @@ ModuleSP Process::ReadModuleFromMemory(const FileSpec &file_spec,
25502551
ModuleSP module_sp(new Module(file_spec, ArchSpec()));
25512552
if (module_sp) {
25522553
Status error;
2554+
std::unique_ptr<Progress> progress_up;
2555+
// Reading an ObjectFile from a local corefile is very fast,
2556+
// only print a progress update if we're reading from a
2557+
// live session which might go over gdb remote serial protocol.
2558+
if (IsLiveDebugSession())
2559+
progress_up = std::make_unique<Progress>(
2560+
"Reading binary from memory", file_spec.GetFilename().GetString());
2561+
25532562
ObjectFile *objfile = module_sp->GetMemoryObjectFile(
25542563
shared_from_this(), header_addr, error, size_to_read);
25552564
if (objfile)

0 commit comments

Comments
 (0)