[lldb] Convert LocateSymbolFile into a plugin #71151
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JDevlieghere
requested review from
bulbazord,
clayborg,
mysterymath and
jasonmolenda
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesThis commit contains the initial scaffolding to convert the functionality currently implemented in LocateSymbolFile to a plugin architecture. The plugin approach allows us to easily add new ways to find plugins. The current implementation abuses the host OS to use the DebugSymbols framework on macOS. With the plugin approach, we retain all the benefits (including the ability to compile it out) with a much cleaner level of abstraction. To limit the scope of this patch, I've only converted LocateExecutableObjectFile. The goal is to do the same for the remaining LocateSymbolFile functions and eventually remove the file. To make reviewing easier, that will done as follow-up patches. Patch is 32.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71151.diff 18 Files Affected:
diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h
index 24d8e28cf686ebd..feaa351b71fa2df 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -345,6 +345,19 @@ class PluginManager {
static SymbolVendorCreateInstance
GetSymbolVendorCreateCallbackAtIndex(uint32_t idx);
+ // SymbolLocator
+ static bool RegisterPlugin(llvm::StringRef name, llvm::StringRef description,
+ SymbolLocatorCreateInstance create_callback,
+ SymbolLocatorLocateExecutableObjectFile
+ locate_executable_object_file = nullptr);
+
+ static bool UnregisterPlugin(SymbolLocatorCreateInstance create_callback);
+
+ static SymbolLocatorCreateInstance
+ GetSymbolLocatorCreateCallbackAtIndex(uint32_t idx);
+
+ static ModuleSpec LocateExecutableObjectFile(const ModuleSpec &module_spec);
+
// Trace
static bool RegisterPlugin(
llvm::StringRef name, llvm::StringRef description,
diff --git a/lldb/include/lldb/Symbol/LocateSymbolFile.h b/lldb/include/lldb/Symbol/LocateSymbolFile.h
index 0bf65559096766a..8cd62cd3f73e706 100644
--- a/lldb/include/lldb/Symbol/LocateSymbolFile.h
+++ b/lldb/include/lldb/Symbol/LocateSymbolFile.h
@@ -24,12 +24,6 @@ class UUID;
class Symbols {
public:
- // Locate the executable file given a module specification.
- //
- // Locating the file should happen only on the local computer or using the
- // current computers global settings.
- static ModuleSpec LocateExecutableObjectFile(const ModuleSpec &module_spec);
-
// Locate the symbol file given a module specification.
//
// Locating the file should happen only on the local computer or using the
diff --git a/lldb/include/lldb/Symbol/SymbolLocator.h b/lldb/include/lldb/Symbol/SymbolLocator.h
new file mode 100644
index 000000000000000..353d93246e47f24
--- /dev/null
+++ b/lldb/include/lldb/Symbol/SymbolLocator.h
@@ -0,0 +1,23 @@
+//===-- SymbolLocator.h -----------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SYMBOL_SYMBOLFILELOCATOR_H
+#define LLDB_SYMBOL_SYMBOLFILELOCATOR_H
+
+#include "lldb/Core/PluginInterface.h"
+
+namespace lldb_private {
+
+class SymbolLocator : public PluginInterface {
+public:
+ SymbolLocator() = default;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_SYMBOL_SYMBOLFILELOCATOR_H
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index aa099d4abc3b09f..f66b2ad4362f926 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -217,6 +217,7 @@ class SymbolContextScope;
class SymbolContextSpecifier;
class SymbolFile;
class SymbolFileType;
+class SymbolLocator;
class SymbolVendor;
class Symtab;
class SyntheticChildren;
diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h
index 2313723b167829a..a22a3e9792b318d 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -89,6 +89,9 @@ typedef SymbolVendor *(*SymbolVendorCreateInstance)(
const lldb::ModuleSP &module_sp,
lldb_private::Stream
*feedback_strm); // Module can be NULL for default system symbol vendor
+typedef SymbolLocator *(*SymbolLocatorCreateInstance)();
+typedef std::optional<ModuleSpec> (*SymbolLocatorLocateExecutableObjectFile)(
+ const ModuleSpec &module_spec);
typedef bool (*BreakpointHitCallback)(void *baton,
StoppointCallbackContext *context,
lldb::user_id_t break_id,
diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp
index ce6a84e4e02269b..7751fd70dedd730 100644
--- a/lldb/source/Core/DynamicLoader.cpp
+++ b/lldb/source/Core/DynamicLoader.cpp
@@ -221,7 +221,7 @@ ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress(
module_spec.GetSymbolFileSpec() =
Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
ModuleSpec objfile_module_spec =
- Symbols::LocateExecutableObjectFile(module_spec);
+ PluginManager::LocateExecutableObjectFile(module_spec);
module_spec.GetFileSpec() = objfile_module_spec.GetFileSpec();
if (FileSystem::Instance().Exists(module_spec.GetFileSpec()) &&
FileSystem::Instance().Exists(module_spec.GetSymbolFileSpec())) {
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 3650df762231d54..519145bba49559c 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -9,6 +9,7 @@
#include "lldb/Core/ModuleList.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Interpreter/OptionValueFileSpec.h"
#include "lldb/Interpreter/OptionValueFileSpecList.h"
@@ -906,7 +907,7 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
// Fixup the incoming path in case the path points to a valid file, yet the
// arch or UUID (if one was passed in) don't match.
ModuleSpec located_binary_modulespec =
- Symbols::LocateExecutableObjectFile(module_spec);
+ PluginManager::LocateExecutableObjectFile(module_spec);
// Don't look for the file if it appears to be the same one we already
// checked for above...
diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp
index ac274112e53dff9..85e3f1cdcf12438 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -1081,6 +1081,59 @@ PluginManager::GetSymbolVendorCreateCallbackAtIndex(uint32_t idx) {
return GetSymbolVendorInstances().GetCallbackAtIndex(idx);
}
+#pragma mark SymbolLocator
+
+struct SymbolLocatorInstance
+ : public PluginInstance<SymbolLocatorCreateInstance> {
+ SymbolLocatorInstance(
+ llvm::StringRef name, llvm::StringRef description,
+ CallbackType create_callback,
+ SymbolLocatorLocateExecutableObjectFile locate_executable_object_file)
+ : PluginInstance<SymbolLocatorCreateInstance>(name, description,
+ create_callback),
+ locate_executable_object_file(locate_executable_object_file) {}
+
+ SymbolLocatorLocateExecutableObjectFile locate_executable_object_file;
+};
+typedef PluginInstances<SymbolLocatorInstance> SymbolLocatorInstances;
+
+static SymbolLocatorInstances &GetSymbolLocatorInstances() {
+ static SymbolLocatorInstances g_instances;
+ return g_instances;
+}
+
+bool PluginManager::RegisterPlugin(
+ llvm::StringRef name, llvm::StringRef description,
+ SymbolLocatorCreateInstance create_callback,
+ SymbolLocatorLocateExecutableObjectFile locate_executable_object_file) {
+ return GetSymbolLocatorInstances().RegisterPlugin(
+ name, description, create_callback, locate_executable_object_file);
+}
+
+bool PluginManager::UnregisterPlugin(
+ SymbolLocatorCreateInstance create_callback) {
+ return GetSymbolLocatorInstances().UnregisterPlugin(create_callback);
+}
+
+SymbolLocatorCreateInstance
+PluginManager::GetSymbolLocatorCreateCallbackAtIndex(uint32_t idx) {
+ return GetSymbolLocatorInstances().GetCallbackAtIndex(idx);
+}
+
+ModuleSpec
+PluginManager::LocateExecutableObjectFile(const ModuleSpec &module_spec) {
+ auto &instances = GetSymbolLocatorInstances().GetInstances();
+ for (auto &instance : instances) {
+ if (instance.locate_executable_object_file) {
+ std::optional<ModuleSpec> result =
+ instance.locate_executable_object_file(module_spec);
+ if (result)
+ return *result;
+ }
+ }
+ return {};
+}
+
#pragma mark Trace
struct TraceInstance
diff --git a/lldb/source/Plugins/CMakeLists.txt b/lldb/source/Plugins/CMakeLists.txt
index 63f2c5b685a89d7..854f589f45ae0b0 100644
--- a/lldb/source/Plugins/CMakeLists.txt
+++ b/lldb/source/Plugins/CMakeLists.txt
@@ -20,6 +20,7 @@ add_subdirectory(ScriptInterpreter)
add_subdirectory(StructuredData)
add_subdirectory(SymbolFile)
add_subdirectory(SystemRuntime)
+add_subdirectory(SymbolLocator)
add_subdirectory(SymbolVendor)
add_subdirectory(Trace)
add_subdirectory(TraceExporter)
diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index 0d1caf4d7318b72..93d8e9bf75f52ea 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -282,7 +282,7 @@ Status ProcessKDP::DoConnectRemote(llvm::StringRef remote_url) {
search_paths);
if (module_spec.GetSymbolFileSpec()) {
ModuleSpec executable_module_spec =
- Symbols::LocateExecutableObjectFile(module_spec);
+ PluginManager::LocateExecutableObjectFile(module_spec);
if (FileSystem::Instance().Exists(
executable_module_spec.GetFileSpec())) {
module_spec.GetFileSpec() =
diff --git a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
new file mode 100644
index 000000000000000..74abecd79694902
--- /dev/null
+++ b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
@@ -0,0 +1,4 @@
+add_subdirectory(Default)
+if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
+ add_subdirectory(DebugSymbols)
+endif()
diff --git a/lldb/source/Plugins/SymbolLocator/DebugSymbols/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/DebugSymbols/CMakeLists.txt
new file mode 100644
index 000000000000000..40f9cc1d44e2d39
--- /dev/null
+++ b/lldb/source/Plugins/SymbolLocator/DebugSymbols/CMakeLists.txt
@@ -0,0 +1,8 @@
+add_lldb_library(lldbPluginSymbolLocatorDebugSymbols PLUGIN
+ SymbolLocatorDebugSymbols.cpp
+
+ LINK_LIBS
+ lldbCore
+ lldbHost
+ lldbSymbol
+ )
diff --git a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
new file mode 100644
index 000000000000000..96ae876e8088b65
--- /dev/null
+++ b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -0,0 +1,328 @@
+//===-- SymbolLocatorDebugSymbols.cpp
+//----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SymbolLocatorDebugSymbols.h"
+
+#include "Plugins/ObjectFile/wasm/ObjectFileWasm.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleList.h"
+#include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Progress.h"
+#include "lldb/Core/Section.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/LocateSymbolFile.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/Utility/Timer.h"
+#include "lldb/Utility/UUID.h"
+
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/ThreadPool.h"
+
+#include "Host/macosx/cfcpp/CFCBundle.h"
+#include "Host/macosx/cfcpp/CFCData.h"
+#include "Host/macosx/cfcpp/CFCReleaser.h"
+#include "Host/macosx/cfcpp/CFCString.h"
+
+#include "mach/machine.h"
+
+#include <CoreFoundation/CoreFoundation.h>
+
+#include <cstring>
+#include <dirent.h>
+#include <dlfcn.h>
+#include <optional>
+#include <pwd.h>
+
+using namespace lldb;
+using namespace lldb_private;
+
+static CFURLRef (*g_dlsym_DBGCopyFullDSYMURLForUUID)(
+ CFUUIDRef uuid, CFURLRef exec_url) = nullptr;
+static CFDictionaryRef (*g_dlsym_DBGCopyDSYMPropertyLists)(CFURLRef dsym_url) =
+ nullptr;
+
+LLDB_PLUGIN_DEFINE(SymbolLocatorDebugSymbols)
+
+SymbolLocatorDebugSymbols::SymbolLocatorDebugSymbols() : SymbolLocator() {}
+
+void SymbolLocatorDebugSymbols::Initialize() {
+ PluginManager::RegisterPlugin(GetPluginNameStatic(),
+ GetPluginDescriptionStatic(), CreateInstance,
+ LocateExecutableObjectFile);
+}
+
+void SymbolLocatorDebugSymbols::Terminate() {
+ PluginManager::UnregisterPlugin(CreateInstance);
+}
+
+llvm::StringRef SymbolLocatorDebugSymbols::GetPluginDescriptionStatic() {
+ return "DebugSymbols symbol locator.";
+}
+
+SymbolLocator *SymbolLocatorDebugSymbols::CreateInstance() {
+ return new SymbolLocatorDebugSymbols();
+}
+
+std::optional<ModuleSpec> SymbolLocatorDebugSymbols::LocateExecutableObjectFile(
+ const ModuleSpec &module_spec) {
+ Log *log = GetLog(LLDBLog::Host);
+ if (!ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup()) {
+ LLDB_LOGF(log, "Spotlight lookup for .dSYM bundles is disabled.");
+ return {};
+ }
+ ModuleSpec return_module_spec;
+ return_module_spec = module_spec;
+ return_module_spec.GetFileSpec().Clear();
+ return_module_spec.GetSymbolFileSpec().Clear();
+
+ const UUID *uuid = module_spec.GetUUIDPtr();
+ const ArchSpec *arch = module_spec.GetArchitecturePtr();
+
+ int items_found = 0;
+
+ if (g_dlsym_DBGCopyFullDSYMURLForUUID == nullptr ||
+ g_dlsym_DBGCopyDSYMPropertyLists == nullptr) {
+ void *handle = dlopen(
+ "/System/Library/PrivateFrameworks/DebugSymbols.framework/DebugSymbols",
+ RTLD_LAZY | RTLD_LOCAL);
+ if (handle) {
+ g_dlsym_DBGCopyFullDSYMURLForUUID =
+ (CFURLRef(*)(CFUUIDRef, CFURLRef))dlsym(handle,
+ "DBGCopyFullDSYMURLForUUID");
+ g_dlsym_DBGCopyDSYMPropertyLists = (CFDictionaryRef(*)(CFURLRef))dlsym(
+ handle, "DBGCopyDSYMPropertyLists");
+ }
+ }
+
+ if (g_dlsym_DBGCopyFullDSYMURLForUUID == nullptr ||
+ g_dlsym_DBGCopyDSYMPropertyLists == nullptr) {
+ return {};
+ }
+
+ if (uuid && uuid->IsValid()) {
+ // Try and locate the dSYM file using DebugSymbols first
+ llvm::ArrayRef<uint8_t> module_uuid = uuid->GetBytes();
+ if (module_uuid.size() == 16) {
+ CFCReleaser<CFUUIDRef> module_uuid_ref(::CFUUIDCreateWithBytes(
+ NULL, module_uuid[0], module_uuid[1], module_uuid[2], module_uuid[3],
+ module_uuid[4], module_uuid[5], module_uuid[6], module_uuid[7],
+ module_uuid[8], module_uuid[9], module_uuid[10], module_uuid[11],
+ module_uuid[12], module_uuid[13], module_uuid[14], module_uuid[15]));
+
+ if (module_uuid_ref.get()) {
+ CFCReleaser<CFURLRef> exec_url;
+ const FileSpec *exec_fspec = module_spec.GetFileSpecPtr();
+ if (exec_fspec) {
+ char exec_cf_path[PATH_MAX];
+ if (exec_fspec->GetPath(exec_cf_path, sizeof(exec_cf_path)))
+ exec_url.reset(::CFURLCreateFromFileSystemRepresentation(
+ NULL, (const UInt8 *)exec_cf_path, strlen(exec_cf_path),
+ FALSE));
+ }
+
+ CFCReleaser<CFURLRef> dsym_url(g_dlsym_DBGCopyFullDSYMURLForUUID(
+ module_uuid_ref.get(), exec_url.get()));
+ char path[PATH_MAX];
+
+ if (dsym_url.get()) {
+ if (::CFURLGetFileSystemRepresentation(
+ dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
+ 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);
+
+ if (FileSystem::Instance().IsDirectory(dsym_filespec)) {
+ dsym_filespec =
+ Symbols::FindSymbolFileInBundle(dsym_filespec, uuid, arch);
+ ++items_found;
+ } else {
+ ++items_found;
+ }
+ return_module_spec.GetSymbolFileSpec() = dsym_filespec;
+ }
+
+ bool success = false;
+ if (log) {
+ if (::CFURLGetFileSystemRepresentation(
+ dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
+ LLDB_LOGF(log,
+ "DebugSymbols framework returned dSYM path of %s for "
+ "UUID %s -- looking for an exec file",
+ path, uuid->GetAsString().c_str());
+ }
+ }
+
+ CFCReleaser<CFDictionaryRef> dict(
+ g_dlsym_DBGCopyDSYMPropertyLists(dsym_url.get()));
+ CFDictionaryRef uuid_dict = NULL;
+ if (dict.get()) {
+ CFCString uuid_cfstr(uuid->GetAsString().c_str());
+ uuid_dict = static_cast<CFDictionaryRef>(
+ ::CFDictionaryGetValue(dict.get(), uuid_cfstr.get()));
+ }
+
+ // 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))) {
+ 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] == '~')
+ FileSystem::Instance().Resolve(exec_filespec);
+ if (FileSystem::Instance().Exists(exec_filespec)) {
+ success = true;
+ return_module_spec.GetFileSpec() = exec_filespec;
+ }
+ }
+ }
+
+ // Look next to the dSYM for the binary file.
+ if (!success) {
+ if (::CFURLGetFileSystemRepresent...
[truncated]
|
bulbazord
approved these changes
Nov 3, 2023
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef LLDB_SYMBOL_SYMBOLFILELOCATOR_H |
There was a problem hiding this comment.
Include guard doesn't match file name
Comment on lines
1
to
2
| //===-- SymbolLocatorDebugSymbols.cpp | ||
| //----------------------------------------------===// |
Comment on lines
1
to
2
| //===-- SymbolLocatorDebugSymbols.h ----------------------------------*- C++ | ||
| //-*-===// |
Comment on lines
1
to
2
| //===-- SymbolLocatorDefault.cpp | ||
| //----------------------------------------------===// |
This commit contains the initial scaffolding to convert the functionality currently implemented in LocateSymbolFile to a plugin architecture. The plugin approach allows us to easily add new ways to find symbols and fixes some issues with the current implementation. For instance, currently we (ab)use the host OS to include support for querying the DebugSymbols framework on macOS. The plugin approach retains all the benefits (including the ability to compile this out on other platforms) while maintaining a higher level of separation with the platform independent code. To limit the scope of this patch, I've only converted a single function: LocateExecutableObjectFile. Future commits will convert the remaining LocateSymbolFile functions and eventually remove LocateSymbolFile. To make reviewing easier, that will done as follow-ups.
JDevlieghere
force-pushed
the
symbol-locator-plugin
branch
from
e7d1373 to
08eb0a2
Compare
kevinfrei
reviewed
Nov 7, 2023
| @@ -0,0 +1,4 @@ | |||
| add_subdirectory(Default) | |||
| if (CMAKE_SYSTEM_NAME MATCHES "Darwin") | |||
| add_subdirectory(DebugSymbols) | |||
There was a problem hiding this comment.
I'm switching my previous PR to be a "Debuginfod" SymbolLocator plugin, and I think this plugin name is far too general. @clayborg tells me that "DebugSymbols.framework" is a thing on MacOS, but maybe rename this "DebugSymbolsApple" or "MacOSDebugSymbols" or pretty much anything that would indicate to the random developer that this stuff is for (mac|i(Pad)|tv)OS/Apple/Darwin/Sherlock/dSYM symbol lookup.
JDevlieghere
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 8, 2024
This commit contains the initial scaffolding to convert the
functionality currently implemented in LocateSymbolFile to a plugin
architecture. The plugin approach allows us to easily add new ways to
find symbols and fixes some issues with the current implementation.
For instance, currently we (ab)use the host OS to include support for
querying the DebugSymbols framework on macOS. The plugin approach
retains all the benefits (including the ability to compile this out on
other platforms) while maintaining a higher level of separation with the
platform independent code.
To limit the scope of this patch, I've only converted a single function:
LocateExecutableObjectFile. Future commits will convert the remaining
LocateSymbolFile functions and eventually remove LocateSymbolFile. To
make reviewing easier, that will done as follow-ups.
(cherry picked from commit c3a302d)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit contains the initial scaffolding to convert the functionality currently implemented in LocateSymbolFile to a plugin architecture. The plugin approach allows us to easily add new ways to find plugins.
The current implementation abuses the host OS to use the DebugSymbols framework on macOS. With the plugin approach, we retain all the benefits (including the ability to compile it out) with a much cleaner level of abstraction.
To limit the scope of this patch, I've only converted LocateExecutableObjectFile. The goal is to do the same for the remaining LocateSymbolFile functions and eventually remove the file. To make reviewing easier, that will done as follow-up patches.