This re-enables the assertion in the PluginInstances destructor that
catches plugins that were not unregistered in their Terminate method. It
also adds a helpful message to quickly identify the plugin.
Fix missing calls to UnregisterPlugin and add an assert in the
PluginManager that ensures all plugins have been unregistered by the
time the plugin manager is destroyed.
The default move constructor wasn't nulling out the callbacks. Combined
with the fact that llvm::sys::DynamicLibrary has no explicit move
constructor and hence library.isValid() still returned true after having
moved-from, we would end up calling plugin_term_callback() when
destroying the moved-from PluginInfo, calling it prematurely.
This PR replaces the Get*CallbackAtIndex pattern in the PluginManager
with returning a snapshot of callbacks that the caller can iterate over
using a range-based for loop. This is a continuation of #184452 which
added thread safety by using snapshots. However, that introduced a bunch
of unnecessary copies which are largely eliminated again by getting the
snapshot once when gather all the callbacks, rather than doing that on
each iteration when querying a plugin for a given index. It also
eliminates the possibility of the snapshot changing underneath you when
iterating over the plugins.
This change was largely mechanical and I used Claude to do the menial
work of updating the signatures and call sites.
In #184273, John pointed out that the PluginManager is currently not
thread safe. While we don't currently provide any guarantees, it seems
desirable to be able to interact safely with the PluginManager from
different threads. For example, we allow dynamically loading plugins
from the command interpreter, which in theory could be coming from
different threads.
LLDB's architecture is heavily centered around plugins. Its primary
purpose is abstraction and modularity, more so than extensibility. For
example, all the in-tree plugins are linked in statically. However, it
is possible to load modules dynamically, though that's mostly aimed at
plugins that built on top of the stable public SB API.
I'm working on support for loading modules dynamically, specifically
in-tree modules that use the LLDB_PLUGIN_DEFINE macro and the
corresponding CMake machinery.
This PR adds support for initializing modules using the symbols
generated by the aforementioned macro. This makes it possible to convert
plugins to shared libraries with minimal changes: is:
- Replace PLUGIN with SHARED in the plugin's CMakeLists.txt.
- Link against libLLDB instead of linking statically against LLVM and
LLDB libraries.
- Re-export all private symbols from libLLDB with
`-DLLDB_EXPORT_ALL_SYMBOLS=ON`.
Avoid directly including `ScriptInterpreterPython.h` in `SBHostOS`, and
instead go through the plugin interface to obtain the scripting path.
This also deprecates the ``SBHostOS::GetLLDBPythonPath`` method in favor
of the more generic GetScriptPath variant.
This revert #181334 and its follow-up PRs (including #181488, #181492,
#181493, #181494 and #181498) as well as Ismail's documentation changes
(#181594, #181717). The original commit causes a test failure in CI
(https://github.com/llvm/llvm-project/issues/181938) but the more I look
at the patch, the more I'm convinced it was not ready to land. It will
be easier to iterate on the feedback by re-landing this than by using
post-commit review.
## Summary
Based on discussion from
[RFC](https://discourse.llvm.org/t/rfc-python-callback-for-source-file-resolution/83545),
this PR adds a new `SymbolLocatorScripted` plugin that allows Python
scripts to implement custom symbol and source file resolution logic.
This enables downstream users to build custom symbol servers, source
file remapping, and build artifact resolution entirely in Python.
### Changes
- Adds `LocateSourceFile()` to the SymbolLocator plugin interface,
called during source path resolution with a fully loaded `ModuleSP`, so
the plugin has access to the module's UUID, file paths, and symbols.
- Adds `SymbolLocatorScripted` plugin that delegates all four
SymbolLocator methods (`LocateExecutableObjectFile`,
`LocateExecutableSymbolFile`, `DownloadObjectAndSymbolFile`,
`LocateSourceFile`) to a user-provided Python class.
- Adds `ScriptedSymbolLocatorPythonInterface` to bridge C++ calls to
Python, with proper GIL management and error handling.
- Results for `LocateSourceFile` are cached per (module UUID, source
file) pair.
- The Python class is configured via: `settings set
plugin.symbol-locator.scripted.script-class module.ClassName`
### Python class interface
```python
class MyLocator:
def __init__(self, exe_ctx, args): ...
def locate_source_file(self, module, original_source_file):
...
def locate_executable_object_file(self, module_spec): ...
def locate_executable_symbol_file(self, module_spec,
default_search_paths): ...
def download_object_and_symbol_file(self, module_spec,
force_lookup, copy_executable): ...
```
### Test plan
```
Added TestScriptedSymbolLocator.py with 3 test cases:
- test_locate_source_file — verifies the locator resolves source
files, receives a valid SBModule with UUID, and remaps paths correctly
- test_locate_source_file_none_fallthrough — verifies returning
None falls through to default LLDB resolution, and that having no script
class set works normally
- test_invalid_script_class — verifies graceful handling of
invalid class names without crashing
```
Co-authored-by: Rahul Reddy Chamala <rachamal@fb.com>
This PR converts the syntax highlighters to plugins. Previously, the
highlighters were part of the Language plugin, using a library shared by
the C-like languages. The Highlighters already had a plugin-like design,
with a clang and default highlighter. This PR takes them out of the
language plugin and into their own highlighter plugin. They are still
accessed thought he HighlightManager.
This change is motivated by #170250. It will allow us to have both a
clang and tree-sitter based highlighter, as well as make it possible to
have a highlighter for a language that doesn't have an upstream language
plugin, like Swift or Rust.
In #168245, I attempted to dump the available settings to Markdown. That
required a full build of LLDB. However, to build the docs, only the swig
wrappers should need to be compiled. The comment was that we should be
able to use the definitions from the TableGen files.
Currently, the property definitions in don't have information about the
path where they will be available. They only contain a `Definition`
which groups properties, so they can be added to
`OptionValueProperties`.
With this PR, I'm adding the path for each property definition. For
example, `symbols.enable-external-lookup` would have `Name =
enable-external-lookup, Path = symbols`. In LLDB itself, we don't need
this path, we only need it for the documentation. To avoid mismatches
between the actual path and the declared one, I added a debug-only check
when a property group is added to a parent
(`OptionValueProperties::AppendProperty`).
The TableGen emitter for the properties now additionally emits
`g_{definition}_properties_def`, which includes both the array of
properties and the expected path. This constant has to be used to
initialize a `OptionValueProperties`.
I couldn't test this for everything (e.g. IntelPT or ProcessKDP), but
the necessary changes are simple: (1) set the `Path` in the TableGen
file, (2) update `initialize` to use `_def`.
This patch introduces a new way to reconstruct the thread stackframe
list.
New `SyntheticFrameProvider` classes can lazy fetch a StackFrame at
index using a provided StackFrameList.
In can either be the real unwinder StackFrameList or we could also chain
SyntheticFrameProviders to each others.
This is the foundation work to implement ScriptedFrameProviders, which
will come in a follow-up patch.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
### Context
Over a year ago, I landed support for 64b Memory ranges in Minidump
(#95312). In this patch we added the Memory64 list stream, which is
effectively a Linked List on disk. The layout is a sixteen byte header
and then however many Memory descriptors.
### The Bug
This is a classic off-by one error, where I added 8 bytes instead of 16
for the header. This caused the first region to start 8 bytes before the
correct RVA, thus shifting all memory reads by 8 bytes. We are correctly
writing all the regions to disk correctly, with no physical corruption
but the RVA is defined wrong, meaning we were incorrectly reading memory
### Why wasn't this caught?
One problem we've had is forcing Minidump to actually use the 64b mode,
it would be a massive waste of resources to have a test that actually
wrote >4.2gb of IO to validate the 64b regions, and so almost all
validation has been manual. As a weakness of manual testing, this issue
is psuedo non-deterministic, as what regions end up in 64b or 32b is
handled greedily and iterated in the order it's laid out in
/proc/pid/maps. We often validated 64b was written correctly by
hexdumping the Minidump itself, which was not corrupted (other than the
BaseRVA)
### Why is this showing up now?
During internal usage, we had a bug report that the Minidump wasn't
displaying values. I was unable to repro the issue, but during my
investigation I saw the variables were in the 64b regions which resulted
in me identifying the bug.
### How do we prevent future regressions?
To prevent regressions, and honestly to save my sanity for figuring out
where 8 bytes magically came from, I've added a new API to
SBSaveCoreOptions.
```SBSaveCoreOptions::GetMemoryRegionsToSave()```
The ability to get the memory regions that we intend to include in the Coredump. I added this so we can compare what we intended to include versus what was actually included. Traditionally we've always had issues comparing regions because Minidump includes `/proc/pid/maps` and it can be difficult to know what memoryregion read failure was a genuine error or just a page that wasn't meant to be included.
We are also leveraging this API to choose the memory regions to be generated, as well as for testing what regions should be bytewise 1:1.
After much debate with @clayborg, I've moved all non-stack memory to the Memory64 List. This list doesn't incur us any meaningful overhead and Greg originally suggested doing this in the original 64b PR. This also means we're exercising the 64b path every single time we save a Minidump, preventing regressions on this feature from slipping through testing in the future.
Snippet produced by [minidump.py](https://github.com/clayborg/scripts)
```
MINIDUMP_MEMORY_LIST:
NumberOfMemoryRanges = 0x00000002
MemoryRanges[0] = [0x00007f61085ff9f0 - 0x00007f6108601000) @ 0x0003f655
MemoryRanges[1] = [0x00007ffe47e50910 - 0x00007ffe47e52000) @ 0x00040c65
MINIDUMP_MEMORY64_LIST:
NumberOfMemoryRanges = 0x000000000000002e
BaseRva = 0x0000000000042669
MemoryRanges[0] = [0x00005584162d8000 - 0x00005584162d9000)
MemoryRanges[1] = [0x00005584162d9000 - 0x00005584162db000)
MemoryRanges[2] = [0x00005584162db000 - 0x00005584162dd000)
MemoryRanges[3] = [0x00005584162dd000 - 0x00005584162ff000)
MemoryRanges[4] = [0x00007f6100000000 - 0x00007f6100021000)
MemoryRanges[5] = [0x00007f6108800000 - 0x00007f6108828000)
MemoryRanges[6] = [0x00007f6108828000 - 0x00007f610899d000)
MemoryRanges[7] = [0x00007f610899d000 - 0x00007f61089f9000)
MemoryRanges[8] = [0x00007f61089f9000 - 0x00007f6108a08000)
MemoryRanges[9] = [0x00007f6108bf5000 - 0x00007f6108bf7000)
```
### Misc
As a part of this fix I had to look at LLDB logs a lot, you'll notice I added `0x` to many of the PRIx64 `LLDB_LOGF`. This is so the user (or I) can directly copy paste the address in the logs instead of adding the hex prefix themselves.
Added some SBSaveCore tests for the new GetMemoryAPI, and Docstrings.
CC: @DavidSpickett, @da-viper @labath because we've been working together on save-core plugins, review it optional and I didn't tag you but figured you'd want to know
This commit adds completion support for the plugin commands. It will try
to complete partial namespaces to the full namespace string. If the
completion input is already a full namespace string then it will add all
the matching plugins in that namespace as completions.
This lets the user complete to the namespace first and then tab-complete
to the next level if desired.
```
(lldb) plugin list a<tab>
Available completions:
abi
architecture
(lldb) plugin list ab<tab>
(lldb) plugin list abi<tab>
(lldb) plugin list abi.<tab>
Available completions:
abi.SysV-arm64
abi.ABIMacOSX_arm64
abi.SysV-arm
...
```
This PR adds an MCP (Model Context Protocol ) server to LLDB. For
motivation and background, please refer to the corresponding RFC:
https://discourse.llvm.org/t/rfc-adding-mcp-support-to-lldb/86798
I implemented this as a new kind of plugin. The idea is that we could
support multiple protocol servers (e.g. if we want to support DAP from
within LLDB). This also introduces a corresponding top-level command
(`protocol-server`) with two subcommands to `start` and `stop` the
server.
```
(lldb) protocol-server start MCP tcp://localhost:1234
MCP server started with connection listeners: connection://[::1]:1234, connection://[127.0.0.1]:1234
```
The MCP sever supports one tool (`lldb_command`) which executes a
command, but can easily be extended with more commands.
In #134418 we added support to list/enable/disable `SystemRuntime` and
`InstrumentationRuntime` plugins. We limited it to those two plugin
types to flesh out the idea with a smaller change.
This PR adds support for the remaining plugin types. We now support all
the plugins that can be registered directly with the plugin manager.
Plugins that are added by loading shared objects are still not
supported.
This commit adds three new commands for managing plugins. The `list`
command will show which plugins are currently registered and their
enabled state. The `enable` and `disable` commands can be used to enable
or disable plugins.
A disabled plugin will not show up to the PluginManager when it iterates
over available plugins of a particular type.
The purpose of these commands is to provide more visibility into
registered plugins and allow users to disable plugins for experimental
perf reasons.
There are a few limitations to the current implementation
1. Only SystemRuntime and InstrumentationRuntime plugins are currently
supported. We can easily extend the existing implementation to support
more types. The scope was limited to these plugins to keep the PR size
manageable.
2. Only "statically" know plugin types are supported (i.e. those managed
by the PluginManager and not from `plugin load`). It is possibly we
could support dynamic plugins as well, but I have not looked into it
yet.
Identical PR to: https://github.com/llvm/llvm-project/pull/134563
Previous PR was approved and landed but broke the build due to bad
merge.
Manually resolve the merge conflict and try to land again.
Co-authored-by: George Hu <georgehuyubo@gmail.com>
This reverts commit 070a4ae2f9bcf6967a7147ed2972f409eaa7d3a6.
Multiple buildbot failures have been reported:
https://github.com/llvm/llvm-project/pull/134563
The build fails with:
lldb/source/Target/Statistics.cpp:75:39: error: use of undeclared
identifier 'num_symbols_loaded'
Adds the new `plugin.cplusplus.display.function-name-format` setting and makes the `${function.name-with-args}` query it for formatting the function name.
One caveat is that the setting can't itself be set to `${function.name-with-args}` because that would cause infinite recursion and blow the stack. I added an XFAILed test-case for it and will address it in a follow-up patch.
https://github.com/llvm/llvm-project/pull/131836
This commit adds support for enabling and disabling plugins by name. The
changes are made generically in the `PluginInstances` class, but
currently we only expose the ability to SystemRuntime plugins. Other
plugins types can be added easily.
We had a few design goals for how disabled plugins should work
1. Plugins that are disabled should still be visible to the system. This
allows us to dynamically enable and disable plugins and report their
state to the user.
2. Plugin order should be stable across disable and enable changes. We
want avoid changing the order of plugin lookup. When a plugin is
re-enabled it should return to its original slot in the creation order.
3. Disabled plugins should not appear in PluginManager operations.
Clients should be able to assume that only enabled plugins will be
returned from the PluginManager.
For the implementation we modify the plugin instance to maintain a bool
of its enabled state. Existing clients external to the Instances class
expect to iterate over only enabled instance so we skip over disabed
instances in the query and snapshot apis. This way the client does not
have to manually check which instances are enabled.
Remove raw access to PluginInstances vector
This commit modifies the PluginInstances class to remove direct access
to the m_instances vector. Instead, we expose a new `GetSnapshot` method
that returns a copy of the current state of the instances vector. All
external iteration over the instances is updated to use the new method.
The motivation for the change is to allow modifying the way we store
instances without having to change all the clients. This is a
preliminary change to allow enabling/disabling of plugins in which case
we want to iterate over only enabled plugins.
We also considered using a custom iterator that wraps the vector
iterator and can skip over disabled instances. That works, but the
iterator code is a bit messy with all template and typedefs to make a
compliant iterator.
This patch removes all of the Set.* methods from Status.
This cleanup is part of a series of patches that make it harder use the
anti-pattern of keeping a long-lives Status object around and updating
it while dropping any errors it contains on the floor.
This patch is largely NFC, the more interesting next steps this enables
is to:
1. remove Status.Clear()
2. assert that Status::operator=() never overwrites an error
3. remove Status::operator=()
Note that step (2) will bring 90% of the benefits for users, and step
(3) will dramatically clean up the error handling code in various
places. In the end my goal is to convert all APIs that are of the form
` ResultTy DoFoo(Status& error)
`
to
` llvm::Expected<ResultTy> DoFoo()
`
How to read this patch?
The interesting changes are in Status.h and Status.cpp, all other
changes are mostly
` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git
grep -l SetErrorString lldb/source)
`
plus the occasional manual cleanup.
Reapply #100443 and #101770. These were originally reverted due to a
test failure and an MSAN failure. I changed the test attribute to
restrict to x86 (following the other existing tests). I could not
reproduce the test or the MSAN failure and no repo steps were provided.
In #100443, Mach-o and Minidump now only call process API's that take a
`SaveCoreOption` as the container for the style and information if a
thread should be included in the core or not. This introduced a bug
where in subsequent method calls we were not honoring the defaults of
both implementations.
~~To solve this I have made a copy of each SaveCoreOptions that is
mutable by the respective plugin. Originally I wanted to leave the
SaveCoreOptions as non const so these default value mutations could be
shown back to the user. Changing that behavior is outside of the scope
of this bugfix, but is context for why we are making a copy.~~
Removed const on the savecoreoptions so defaults can be inspected by the
user
CC: @Michael137
In #98403 I enabled the SBSaveCoreOptions object, which allows users via
the scripting API to define what they want saved into their core file.
As the first option I've added a threadlist, so users can scan and
identify which threads and corresponding stacks they want to save.
In order to support this, I had to add a new method to `Process.h` on
how we identify which threads are to be saved, and I had to change the
book keeping in minidump to ensure we don't double save the stacks.
Important to @jasonmolenda I also changed the MachO coredump to accept
these new APIs.
This patch introduces a new `template` multiword sub-command to the
`scripting` top-level command. As the name suggests, this sub-command
operates on scripting templates, and currently has the ability to
automatically discover the various scripting extensions that lldb
supports.
This was previously reviewed in #97273.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch introduces a new `template` multiword sub-command to the
`scripting` top-level command. As the name suggests, this sub-command
operates on scripting templates, and currently has the ability to
automatically discover the various scripting extensions that lldb
supports.
This was previously reviewed in #97273.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch introduces a new `template` multiword sub-command to the
`scripting` top-level command. As the name suggests, this sub-command
operates on scripting templates, and currently has the ability to
automatically discover the various scripting extensions that lldb
supports.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This PR adds `SBSaveCoreOptions`, which is a container class for options
when LLDB is taking coredumps. For this first iteration this container
just keeps parity with the extant API of `file, style, plugin`. In the
future this options object can be extended to allow users to take a
subset of their core dumps.
This patch replaces uses of StringRef::{starts,ends}with with
StringRef::{starts,ends}_with for consistency with
std::{string,string_view}::{starts,ends}_with in C++20.
I'm planning to deprecate and eventually remove
StringRef::{starts,ends}with.
I've plumbed the LLVM DebugInfoD client into LLDB, and added automatic
downloading of DWP files to the SymbolFileDWARF.cpp plugin. If you have
DEBUGINFOD_URLS set to a space delimited set of web servers, LLDB will
try to use them as a last resort when searching for DWP files. If you do
*not* have that environment variable set, nothing should be changed.
There's also a setting, per @clayborg 's suggestion, that will override
the environment variable, or can be used instead of the environment
variable. The setting is why I also needed to add an API to the
llvm-debuginfod library
### Test Plan:
Suggestions are welcome here. I should probably have some positive and
negative tests, but I wanted to get the diff up for people who have a
clue what they're doing to rip it to pieces before spending too much
time validating the initial implementation.
---------
Co-authored-by: Kevin Frei <freik@meta.com>
Co-authored-by: Alex Langford <nirvashtzero@gmail.com>
This builds on top of the work started in c3a302d to convert
LocateSymbolFile to a SymbolLocator plugin. This commit moves
DownloadObjectAndSymbolFile.
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.
The underlying structures no longer use ConstString so we can remove it
wholesale from PluginManager now.
Differential Revision: https://reviews.llvm.org/D153818
I removed ConstString from OptionValueProperties in 643ba926c1f6, but
there are a few call sites that still create a ConstString as an
argument. I did not catch these initially because ConstString has an
implicit conversion method to StringRef.
Differential Revision: https://reviews.llvm.org/D153673
These probably do not need to be in the ConstString StringPool as they
don't really need any of the advantages that ConstStrings offer.
Lifetime for these things is always static and we never need to perform
comparisons for setting descriptions.
Differential Revision: https://reviews.llvm.org/D148679
