[Lldb-commits] [lldb] d2f3b60 - [NFC] Don't bother with unstripped binary w/ dSYM, don't DebugSymbols twice
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Mon May 16 15:30:51 PDT 2022
Author: Jason Molenda
Date: 2022-05-16T15:30:39-07:00
New Revision: d2f3b6020fbfa2dd56ebd03c942acda961c421e2
URL: https://github.com/llvm/llvm-project/commit/d2f3b6020fbfa2dd56ebd03c942acda961c421e2
DIFF: https://github.com/llvm/llvm-project/commit/d2f3b6020fbfa2dd56ebd03c942acda961c421e2.diff
LOG: [NFC] Don't bother with unstripped binary w/ dSYM, don't DebugSymbols twice
This patch addresses two perf issues when we find a dSYM on macOS
after calling into the DebugSymbols framework. First, when we have
a local (probably stripped) binaary, we find the dSYM and we may
be told about the location of the symbol rich binary (probably
unstripped) which may be on a remote filesystem. We don't need the
unstripped binary, use the local binary we already have.
Second, after we've found the path to the dSYM, save that in the Module
so we don't call into DebugSymbols a second time later on to
rediscover it. If the user has a DBGShellCommands set, we need to
exec that process twice, serially, which can add up.
Differential Revision: https://reviews.llvm.org/D125616
rdar://84576917
Added:
Modified:
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
index d9f4174b19a3..b99ade50d857 100644
--- a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -149,6 +149,12 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
ObjectFile::FindPlugin(module_sp, &dsym_fspec, 0,
FileSystem::Instance().GetByteSize(dsym_fspec),
dsym_file_data_sp, dsym_file_data_offset);
+ // Important to save the dSYM FileSpec so we don't call
+ // Symbols::LocateExecutableSymbolFile a second time while trying to
+ // add the symbol ObjectFile to this Module.
+ if (dsym_objfile_sp && !module_sp->GetSymbolFileFileSpec()) {
+ module_sp->SetSymbolFileFileSpec(dsym_fspec);
+ }
if (UUIDsMatch(module_sp.get(), dsym_objfile_sp.get(), feedback_strm)) {
// We need a XML parser if we hope to parse a plist...
if (XMLDocument::XMLEnabled()) {
diff --git a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
index d905a8ed88e3..4dc42cf01716 100644
--- a/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ b/lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -18,9 +18,11 @@
#include "Host/macosx/cfcpp/CFCData.h"
#include "Host/macosx/cfcpp/CFCReleaser.h"
#include "Host/macosx/cfcpp/CFCString.h"
+#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleList.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Host/Host.h"
+#include "lldb/Host/HostInfo.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBuffer.h"
@@ -108,12 +110,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
if (dsym_url.get()) {
if (::CFURLGetFileSystemRepresentation(
dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
- if (log) {
- LLDB_LOGF(log,
- "DebugSymbols framework returned dSYM path of %s for "
- "UUID %s -- looking for the dSYM",
- path, uuid->GetAsString().c_str());
- }
+ LLDB_LOGF(log,
+ "DebugSymbols framework returned dSYM path of %s for "
+ "UUID %s -- looking for the dSYM",
+ path, uuid->GetAsString().c_str());
FileSpec dsym_filespec(path);
if (path[0] == '~')
FileSystem::Instance().Resolve(dsym_filespec);
@@ -147,16 +147,54 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
uuid_dict = static_cast<CFDictionaryRef>(
::CFDictionaryGetValue(dict.get(), uuid_cfstr.get()));
}
- if (uuid_dict) {
+
+ // Check to see if we have the file on the local filesystem.
+ if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+ ModuleSpec exe_spec;
+ exe_spec.GetFileSpec() = module_spec.GetFileSpec();
+ exe_spec.GetUUID() = module_spec.GetUUID();
+ ModuleSP module_sp;
+ module_sp.reset(new Module(exe_spec));
+ if (module_sp && module_sp->GetObjectFile() &&
+ module_sp->MatchesModuleSpec(exe_spec)) {
+ success = true;
+ return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
+ LLDB_LOGF(log, "using original binary filepath %s for UUID %s",
+ module_spec.GetFileSpec().GetPath().c_str(),
+ uuid->GetAsString().c_str());
+ ++items_found;
+ }
+ }
+
+ // Check if the requested image is in our shared cache.
+ if (!success) {
+ SharedCacheImageInfo image_info = HostInfo::GetSharedCacheImageInfo(
+ module_spec.GetFileSpec().GetPath());
+
+ // If we found it and it has the correct UUID, let's proceed with
+ // creating a module from the memory contents.
+ if (image_info.uuid && (!module_spec.GetUUID() ||
+ module_spec.GetUUID() == image_info.uuid)) {
+ success = true;
+ return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
+ LLDB_LOGF(log,
+ "using binary from shared cache for filepath %s for "
+ "UUID %s",
+ module_spec.GetFileSpec().GetPath().c_str(),
+ uuid->GetAsString().c_str());
+ ++items_found;
+ }
+ }
+
+ // Use the DBGSymbolRichExecutable filepath if present
+ if (!success && uuid_dict) {
CFStringRef exec_cf_path =
static_cast<CFStringRef>(::CFDictionaryGetValue(
uuid_dict, CFSTR("DBGSymbolRichExecutable")));
if (exec_cf_path && ::CFStringGetFileSystemRepresentation(
exec_cf_path, path, sizeof(path))) {
- if (log) {
- LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
- path, uuid->GetAsString().c_str());
- }
+ LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
+ path, uuid->GetAsString().c_str());
++items_found;
FileSpec exec_filespec(path);
if (path[0] == '~')
@@ -168,20 +206,17 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
}
}
+ // Look next to the dSYM for the binary file.
if (!success) {
- // No dictionary, check near the dSYM bundle for an executable that
- // matches...
if (::CFURLGetFileSystemRepresentation(
dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
char *dsym_extension_pos = ::strstr(path, ".dSYM");
if (dsym_extension_pos) {
*dsym_extension_pos = '\0';
- if (log) {
- LLDB_LOGF(log,
- "Looking for executable binary next to dSYM "
- "bundle with name with name %s",
- path);
- }
+ LLDB_LOGF(log,
+ "Looking for executable binary next to dSYM "
+ "bundle with name with name %s",
+ path);
FileSpec file_spec(path);
FileSystem::Instance().Resolve(file_spec);
ModuleSpecList module_specs;
@@ -208,12 +243,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
{
++items_found;
return_module_spec.GetFileSpec() = bundle_exe_file_spec;
- if (log) {
- LLDB_LOGF(log,
- "Executable binary %s next to dSYM is "
- "compatible; using",
- path);
- }
+ LLDB_LOGF(log,
+ "Executable binary %s next to dSYM is "
+ "compatible; using",
+ path);
}
}
}
@@ -238,12 +271,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
{
++items_found;
return_module_spec.GetFileSpec() = file_spec;
- if (log) {
- LLDB_LOGF(log,
- "Executable binary %s next to dSYM is "
- "compatible; using",
- path);
- }
+ LLDB_LOGF(log,
+ "Executable binary %s next to dSYM is "
+ "compatible; using",
+ path);
}
break;
}
@@ -321,11 +352,9 @@ static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict,
if (CFCString::FileSystemRepresentation(cf_str, str)) {
module_spec.GetFileSpec().SetFile(str.c_str(), FileSpec::Style::native);
FileSystem::Instance().Resolve(module_spec.GetFileSpec());
- if (log) {
- LLDB_LOGF(log,
- "From dsymForUUID plist: Symbol rich executable is at '%s'",
- str.c_str());
- }
+ LLDB_LOGF(log,
+ "From dsymForUUID plist: Symbol rich executable is at '%s'",
+ str.c_str());
}
}
@@ -337,10 +366,7 @@ static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict,
FileSpec::Style::native);
FileSystem::Instance().Resolve(module_spec.GetFileSpec());
success = true;
- if (log) {
- LLDB_LOGF(log, "From dsymForUUID plist: dSYM is at '%s'",
- str.c_str());
- }
+ LLDB_LOGF(log, "From dsymForUUID plist: dSYM is at '%s'", str.c_str());
}
}
@@ -640,14 +666,12 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
}
}
} else {
- if (log) {
- if (!uuid_str.empty())
- LLDB_LOGF(log, "Called %s on %s, no matches",
- g_dsym_for_uuid_exe_path, uuid_str.c_str());
- else if (file_path[0] != '\0')
- LLDB_LOGF(log, "Called %s on %s, no matches",
- g_dsym_for_uuid_exe_path, file_path);
- }
+ if (!uuid_str.empty())
+ LLDB_LOGF(log, "Called %s on %s, no matches",
+ g_dsym_for_uuid_exe_path, uuid_str.c_str());
+ else if (file_path[0] != '\0')
+ LLDB_LOGF(log, "Called %s on %s, no matches",
+ g_dsym_for_uuid_exe_path, file_path);
}
}
}
More information about the lldb-commits
mailing list