This patch adds `get_priority()` support to synthetic frame providers to
enable priority-based selection when multiple providers match a thread.
This is the first step toward supporting frame provider chaining for
visualizing coroutines, Swift async tasks, and et al.
Priority ordering follows Unix nice convention where lower numbers
indicate higher priority (0 = highest). Providers without explicit
priority return `std::nullopt`, which maps to UINT32_MAX (lowest
priority), ensuring backward compatibility with existing providers.
The implementation adds `GetPriority()` as a virtual method to
`SyntheticFrameProvider` base class, implements it through the scripting
interface hierarchy (`ScriptedFrameProviderInterface` and
`ScriptedFrameProviderPythonInterface`), and updates
`Thread::GetStackFrameList()` to sort applicable providers by priority
before attempting to load them.
Python frame providers can now specify priority:
```python
@staticmethod
def get_priority():
return 10 # Or return None for default priority.
```
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
When you run an expression and the result has a dynamic type that is
different from the expression's static result type, we print the result
variable using the dynamic type, but at present when you use the result
variable in an expression later on, we only give you access to the
static type. For instance:
```
(lldb) expr MakeADerivedReportABase()
(Derived *) $0 = 0x00000001007e93e0
(lldb) expr $0->method_from_derived()
^
error: no member named 'method_from_derived' in 'Base'
(lldb)
```
The static return type of that function is `Base *`, but we printed that
the result was a `Derived *` and then only used the `Base *` part of it
in subsequent expressions. That's not very helpful, and forces you to
guess and then cast the result types to their dynamic type in order to
be able to access the full type you were returned, which is
inconvenient.
This patch makes lldb retain the dynamic type of the result variable
(and ditto for persistent result variables).
It also adds more testing of expression result variables with various
types of dynamic values, to ensure we can access both the ivars and
methods of the type we print the result as.
Scripted frames that materialize Python functions or other non-native
code are PC-less by design, meaning they don't have valid program
counter values. Previously, these frames would display invalid addresses
(`0xffffffffffffffff`) in backtrace output.
This patch updates `FormatEntity` to detect and suppress invalid address
display for PC-less frames, adds fallback to frame methods when symbol
context is unavailable, and modifies `StackFrame::GetSymbolContext` to
skip PC-based symbol resolution for invalid addresses.
The changes enable PC-less frames to display cleanly with proper
function names, file paths, and line numbers, and allow for source
display of foreign sources (like Python). Includes comprehensive test
coverage demonstrating frames pointing to Python source files.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This method puts strings into the ConstString pool and vends them as
llvm::StringRefs. Most of the uses only require a `std::string` or a
`const char *`. This can be achieved without wasting memory.
Someone came up with a clever idea for a new breakpoint type, but we
couldn't figure out how to add it in an ergonomic way because
`breakpoint set` has used up all the short-option characters. And even
if they did find a way to add it, the help for `break set` is so
confusing - because of the way it is implemented - that very few people
would detect the addition.
The basic problem is that `break set` distinguishes amongst the
fundamental breakpoint types it offers by which "required option" you
provide. If you pass a `-a` you are setting an address breakpoint, if
`-n`, `-F`, etc. a symbol name based breakpoint. And so forth. That is
however pretty hard to discern from the option grouping printing from
`help break set`. `break set` also suffers from the problem that it uses
common options in different ways depending on which "required" option is
present, which makes documenting the various behaviors difficult. And as
we run out of single letters it makes extending it difficult to
impossible.
This PR fixes that situation by adding a new command for adding
breakpoints - `break add`. The new command specifies the "breakpoint
types" as subcommands of `break add` rather than distinguishing them by
their being one of the "required" options the way `break set` does. That
both makes it much clearer what the breakpoint types actually are, and
means that the option set can be dedicated to that particular breakpoint
type, and so the help for each is less cluttered, and can be documented
properly for each usage.
Instead of trying to parse the meaning of:
```
(lldb) help break set
Sets a breakpoint or set of breakpoints in the executable.
Syntax: breakpoint set <cmd-options>
Command Options Usage:
breakpoint set [-DHd] -l <linenum> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-u <column>] [-f <filename>] [-m <boolean>] [-s <shlib-name>] [-K <boolean>]
breakpoint set [-DHd] -a <address-expression> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-N <breakpoint-name>] [-s <shlib-name>]
breakpoint set [-DHd] -n <function-name> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-f <filename>] [-L <source-language>] [-s <shlib-name>] [-K <boolean>]
breakpoint set [-DHd] -F <fullname> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-f <filename>] [-L <source-language>] [-s <shlib-name>] [-K <boolean>]
breakpoint set [-DHd] -S <selector> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-f <filename>] [-L <source-language>] [-s <shlib-name>] [-K <boolean>]
breakpoint set [-DHd] -M <method> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-f <filename>] [-L <source-language>] [-s <shlib-name>] [-K <boolean>]
breakpoint set [-DHd] -r <regular-expression> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-f <filename>] [-L <source-language>] [-s <shlib-name>] [-K <boolean>]
breakpoint set [-DHd] -b <function-name> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-f <filename>] [-L <source-language>] [-s <shlib-name>] [-K <boolean>]
breakpoint set [-ADHd] -p <regular-expression> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-N <breakpoint-name>] [-f <filename>] [-m <boolean>] [-s <shlib-name>] [-X <function-name>]
breakpoint set [-DHd] -E <source-language> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-N <breakpoint-name>] [-h <boolean>] [-w <boolean>]
breakpoint set [-DHd] -P <python-class> [-k <none>] [-v <none>] [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-N <breakpoint-name>] [-f <filename>] [-s <shlib-name>]
breakpoint set [-DHd] -y <linespec> [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-R <address>] [-N <breakpoint-name>] [-m <boolean>] [-s <shlib-name>] [-K <boolean>]
```
We instead offer:
```
(lldb) help break add
Commands to add breakpoints of various types
Syntax: breakpoint add
Access the breakpoint search kernels built into lldb. Along with specifying the
search kernel, each breakpoint add operation can specify a common set of
"reaction" options for each breakpoint. The reaction options can also be
modified after breakpoint creation using the "breakpoint modify" command.
The following subcommands are supported:
address -- Add breakpoints by raw address
exception -- Add breakpoints on language exceptions. If no language is specified, break on exceptions for all supported languages
file -- Add breakpoints on lines in specified source files
name -- Add breakpoints matching function or symbol names
pattern -- Add breakpoints matching patterns in the source text Expects 'raw' input (see 'help raw-input'.)
scripted -- Add breakpoints using a scripted breakpoint resolver.
For more help on any particular subcommand, type 'help <command> <subcommand>'.
```
The individual subcommand helps are also easier to read. They are still
a little too verbose because they all repeat the options for the
`reactions`. A general fix for our help system would be when a command
incorporates an OptionGroup whole into the command options, help would
show the option group name - which you could separately look up. But
even without that:
```
(lldb) help br a a
Add breakpoints by raw address
Syntax: breakpoint add address <cmd-options> <address> [<address> [...]]
Command Options Usage:
breakpoint add address [-DHde] [-G <boolean>] [-C <command>] [-c <expr>] [-Y <source-language>] [-i <count>] [-o <boolean>] [-q <queue-name>] [-t <thread-id>] [-x <thread-index>] [-T <thread-name>] [-N <breakpoint-name>] [-s <shlib-name>] <address> [<address> [...]]
-C <command> ( --command <command> )
A command to run when the breakpoint is hit, can be provided more than once, the commands will be run in left-to-right order.
-D ( --dummy-breakpoints )
Act on Dummy breakpoints - i.e. breakpoints set before a file is provided, which prime new targets.
-G <boolean> ( --auto-continue <boolean> )
The breakpoint will auto-continue after running its commands.
-H ( --hardware )
Require the breakpoint to use hardware breakpoints.
-N <breakpoint-name> ( --breakpoint-name <breakpoint-name> )
Adds this name to the list of names for this breakpoint. Can be specified more than once.
-T <thread-name> ( --thread-name <thread-name> )
The breakpoint stops only for the thread whose thread name matches this argument.
-Y <source-language> ( --condition-language <source-language> )
Specifies the Language to use when executing the breakpoint's condition expression.
-c <expr> ( --condition <expr> )
The breakpoint stops only if this condition expression evaluates to true.
-d ( --disable )
Disable the breakpoint.
-e ( --enable )
Enable the breakpoint.
-i <count> ( --ignore-count <count> )
Set the number of times this breakpoint is skipped before stopping.
-o <boolean> ( --one-shot <boolean> )
The breakpoint is deleted the first time it stop causes a stop.
-q <queue-name> ( --queue-name <queue-name> )
The breakpoint stops only for threads in the queue whose name is given by this argument.
-s <shlib-name> ( --shlib <shlib-name> )
Set the breakpoint at an address relative to sections in this shared library.
-t <thread-id> ( --thread-id <thread-id> )
The breakpoint stops only for the thread whose TID matches this argument. The token 'current' resolves to the current thread's ID.
-x <thread-index> ( --thread-index <thread-index> )
The breakpoint stops only for the thread whose index matches this argument.
This command takes options and free-form arguments. If your arguments resemble option specifiers (i.e., they start with a - or --), you must use ' --
' between the end of the command options and the beginning of the arguments.
```
is pretty readable.
When the target is being created, the target list acquires the mutex for
the duration of the target creation process. However if a module
callback is enabled and is being called in parallel there exists an
opportunity to deadlock if the callback calls into targetlist. I've
created a minimum repro
[here](https://gist.github.com/Jlalond/2557e06fa09825f338eca08b1d21884f).
```
command script import dead-lock-example (from above gist)
...
target create a.out
[hangs]
```
This looks like a straight forward fix, where `CreateTargetInternal`
doesn't access any state directly, and instead calls methods which they
themselves are thread-safe. So I've moved the lock to when we update the
list with the created target. I'm not sure if this is a comprehensive
fix, but it does fix my above example and in my (albeit limited)
testing, doesn't cause any strange change in behavior.
Suppose two threads are performing the exact same step out plan. They
will both have an internal breakpoint set at their parent frame. Now
supposed both of those breakpoints are in the same address (i.e. the
same BreakpointSite).
At the end of `ThreadPlanStepOut::DoPlanExplainsStop`, we see this:
```
// If there was only one owner, then we're done. But if we also hit
// some user breakpoint on our way out, we should mark ourselves as
// done, but also not claim to explain the stop, since it is more
// important to report the user breakpoint than the step out
// completion.
if (site_sp->GetNumberOfConstituents() == 1)
return true;
```
In other words, the plan looks at the name number of constituents of the
site to decide whether it explains the stop, the logic being that a
_user_ might have put a breakpoint there. However, the implementation is
not correct; in particular, it will fail in the situation described
above. We should only care about non-internal breakpoints that would
stop for the current thread.
It is tricky to test this, as it depends on the timing of threads, but I
was able to consistently reproduce the issue with a swift program using
concurrency.
rdar://165481473
This change makes StackFrame methods virtual to enable subclass
overrides and introduces BorrowedStackFrame, a wrapper that presents an
existing StackFrame with a different frame index.
This enables creating synthetic frame views or renumbering frames
without copying the underlying frame data, which is useful for frame
manipulation scenarios.
This also adds a new borrowed-info format entity to show what was the
original frame index of the borrowed frame.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This change adds tracking of the StackFrameList that produced each frame
by storing a weak pointer (m_frame_list_wp) in both `StackFrame` and
`ExecutionContextRef`.
When resolving frames through `ExecutionContextRef::GetFrameSP`, the
code now first attempts to use the remembered frame list instead of
immediately calling `Thread::GetStackFrameList`. This breaks circular
dependencies that can occur during frame provider initialization, where
creating a frame provider might trigger `ExecutionContext` resolution,
which would then call back into `Thread::GetStackFrameList()`, creating
an infinite loop.
The `StackFrameList` now sets m_frame_list_wp on every frame it creates,
and a new virtual method `GetOriginatingStackFrameList` allows frames to
expose their originating list.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
Both `Target::ReadSignedIntegerFromMemory()` and
`Process::ReadSignedIntegerFromMemory()` internally created an unsigned
scalar, so extending the value later did not duplicate the sign bit.
## Summary:
This change introduces a `DAPSessionManager` to enable multiple DAP
sessions to share debugger instances when needed, for things like child
process debugging and some scripting hooks that create dynamically new
targets.
Changes include:
- Add `DAPSessionManager` singleton to track and coordinate all active DAP
sessions
- Support attaching to an existing target via its globally unique target
ID (targetId parameter)
- Share debugger instances across sessions when new targets are created
dynamically
- Refactor event thread management to allow sharing event threads
between sessions and move event thread and event thread handlers to `EventHelpers`
- Add `eBroadcastBitNewTargetCreated` event to notify when new targets are
created
- Extract session names from target creation events
- Defer debugger initialization from 'initialize' request to
'launch'/'attach' requests. The only time the debugger is used currently
in between its creation in `InitializeRequestHandler` and the `Launch`
or `Attach` requests is during the `TelemetryDispatcher` destruction
call at the end of the `DAP::HandleObject` call, so this is safe.
This enables scenarios when new targets are created dynamically so that
the debug adapter can automatically start a new debug session for the
spawned target while sharing the debugger instance.
## Tests:
The refactoring maintains backward compatibility. All existing DAP test
cases pass.
Also added a few basic unit tests for DAPSessionManager
```
>> ninja DAPTests
>> ./tools/lldb/unittests/DAP/DAPTests
>>./bin/llvm-lit -v ../llvm-project/lldb/test/API/tools/lldb-dap/
```
show information about the signal when the user presses `process handle
<unix-signal>` i.e
```sh
(lldb) process handle SIGWINCH
NAME PASS STOP NOTIFY DESCRIPTION
=========== ===== ===== ====== ===================
SIGWINCH true false false window size changes
```
Wanted to use the existing `GetSignalDescription` but it is expected
behaviour to return the signal name if no signal code is passed. It is
used in stop info.
65c895dfe0/lldb/source/Target/StopInfo.cpp (L1192-L1195)
In C++17, static constexpr members are implicitly inline, so they no
longer require an out-of-line definition.
Identified with readability-redundant-declaration.
This patch fixes and eliminates the possibility of SupportFileSP ever
being nullptr. The support file was originally treated like a value
type, but became a polymorphic type and therefore has to be stored and
passed around as a pointer.
To avoid having all the callers check the validity of the pointer, I
introduced the invariant that SupportFileSP is never null and always
default constructed. However, without enforcement at the type level,
that's fragile and indeed, we already identified two crashes where
someone accidentally broke that invariant.
This PR introduces a NonNullSharedPtr to prevent that. NonNullSharedPtr
is a smart pointer wrapper around std::shared_ptr that guarantees the
pointer is never null. If default-constructed, it creates a
default-constructed instance of the contained type. Note that I'm using
private inheritance because you shouldn't inherit from standard library
classes due to the lack of virtual destructor. So while the new
abstraction looks like a `std::shared_ptr`, it is in fact **not** a
shared pointer. Given that our destructor is trivial, we could use
public inheritance, but currently there's no need for it.
rdar://164989579
Another attempt at resolving the deadlock issue @GeorgeHuyubo discovered
(his previous
[attempt](https://github.com/llvm/llvm-project/pull/160225)).
This change can be summarized as the following:
* Plumb through a boolean flag to force no preload in
`GetOrCreateModules` all the way through to `LoadModuleAtAddress`.
* Parallelize `Module::PreloadSymbols` separately from
`Target::GetOrCreateModule` and its caller `LoadModuleAtAddress` (this
is what avoids the deadlock).
These changes roughly maintain the performance characteristics of the
previous implementation of parallel module loading. Testing on targets
with between 5000 and 14000 modules, I saw similar numbers as before,
often more than 10% faster in the new implementation across multiple
trials for these massive targets. I think it's because we have less lock
contention with this approach.
# The deadlock
See [bt.txt](https://github.com/user-attachments/files/22524471/bt.txt)
for a sample backtrace of LLDB when the deadlock occurs.
As @GeorgeHuyubo explains in his
[PR](https://github.com/llvm/llvm-project/pull/160225), the deadlock
occurs from an ABBA deadlock that happens when a thread context-switches
out of `Module::PreloadSymbols`, goes into `Target::GetOrCreateModule`
for another module, possibly entering this block:
```
if (!module_sp) {
// The platform is responsible for finding and caching an appropriate
// module in the shared module cache.
if (m_platform_sp) {
error = m_platform_sp->GetSharedModule(
module_spec, m_process_sp.get(), module_sp, &search_paths,
&old_modules, &did_create_module);
} else {
error = Status::FromErrorString("no platform is currently set");
}
}
```
`Module::PreloadSymbols` holds a module-level mutex, and then
`GetSharedModule` *attempts* to hold the mutex of the global shared
`ModuleList`. So, this thread holds the module mutex, and waits on the
global shared `ModuleList` mutex.
A competing thread may execute `Target::GetOrCreateModule`, enter the
same block as above, grabbing the global shared `ModuleList` mutex.
Then, in `ModuleList::GetSharedModule`, we eventually call
`ModuleList::FindModules` which eventually waits for the `Module` mutex
held by the first thread (via `Module::GetUUID`). Thus, we deadlock.
## Reproducing the deadlock
It might be worth noting that I've never been able to observe this
deadlock issue during live debugging (e.g. launching or attaching to
processes), however we were able to consistently reproduce this issue
with coredumps when using the following settings:
```
(lldb) settings set target.parallel-module-load true
(lldb) settings set target.preload-symbols true
(lldb) settings set symbols.load-on-demand false
(lldb) target create --core /some/core/file/here
# deadlock happens
```
## How this change avoids this deadlock
This change avoids concurrent executions of `Module::PreloadSymbols`
with `Target::GetOrCreateModule` by waiting until after the
`Target::GetOrCreateModule` executions to run `Module::PreloadSymbols`
in parallel. This avoids the ordering of holding a Module lock *then*
the ModuleList lock, as `Target::GetOrCreateModule` executions maintain
the ordering of the shared ModuleList lock first (from what I've read
and tested).
## Why not read-write lock?
Some feedback in https://github.com/llvm/llvm-project/pull/160225 was to
modify mutexes used in these components with read-write locks. This
might be a good idea overall, but I don't think it would *easily*
resolve this specific deadlock. `Module::PreloadSymbols` would probably
need a write lock to Module, so even if we had a read lock in
`Module::GetUUID` we would still contend. Maybe the `ModuleList` lock
could be a read lock that converts to a write lock if it chooses to
update the module, but it seems likely that some thread would try to
update the shared module list and then the write lock would contend
again.
Perhaps with deeper architectural changes, we could fix this issue?
# Other attempts
One downside of this approach (and the former approach of parallel
module loading) is that each DYLD would need to implement this pattern
themselves. With @clayborg's help, I looked at a few other approaches:
* In `Target::GetOrCreateModule`, backgrounding the
`Module::PreloadSymbols` call by adding it directly to the thread pool
via `Debugger::GetThreadPool().async()`. This required adding a lock to
`Module::SetLoadAddress` (probably should be one there already) since
`ObjectFileELF::SetLoadAddress` is not thread-safe (updates sections).
Unfortunately, during execution, this causes the preload symbols to run
synchronously with `Target::GetOrCreateModule`, preventing us from truly
parallelizing the execution.
* In `Module::PreloadSymbols`, backgrounding the `symtab` and `sym_file`
`PreloadSymbols` calls individually, but similar issues as the above.
* Passing a callback function like
https://github.com/swiftlang/llvm-project/pull/10746 instead of the
boolean I use in this change. It's functionally the same change IMO,
with some design tradeoffs:
* Pro: the caller doesn't need to explicitly call
`Module::PreloadSymbols` itself, and can instead call whatever function
is passed into the callback.
* Con: the caller needs to delay the execution of the callback such that
it occurs after the `GetOrCreateModule` logic, otherwise we run into the
same issue. I thought this would be trickier for the caller, requiring
some kinda condition variable or otherwise storing the calls to execute
afterwards.
# Test Plan:
```
ninja check-lldb
```
---------
Co-authored-by: Tom Yang <toyang@fb.com>
This patch extends ScriptedFrame to work with real (non-scripted)
threads,
enabling frame providers to synthesize frames for native processes.
Previously, ScriptedFrame only worked within
ScriptedProcess/ScriptedThread
contexts. This patch decouples ScriptedFrame from ScriptedThread,
allowing
users to augment or replace stack frames in real debugging sessions for
use
cases like custom calling conventions, reconstructing corrupted frames
from
core files, or adding diagnostic frames.
Key changes:
- ScriptedFrame::Create() now accepts ThreadSP instead of requiring
ScriptedThread, extracting architecture from the target triple rather
than ScriptedProcess.arch
- Added SBTarget::RegisterScriptedFrameProvider() and
ClearScriptedFrameProvider() APIs, with Target storing a
SyntheticFrameProviderDescriptor template for new threads
- Added "target frame-provider register/clear" commands for CLI access
- Thread class gains LoadScriptedFrameProvider(),
ClearScriptedFrameProvider(),
and GetFrameProvider() methods for per-thread frame provider management
- New SyntheticStackFrameList overrides FetchFramesUpTo() to lazily
provide
frames from either the frame provider or the real stack
This enables practical use of the SyntheticFrameProvider infrastructure
in
real debugging workflows.
rdar://161834688
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
We're iterating over the stop hooks so if one of them changes the stop
hook list by deleting itself or another stop hook, that invalidates our
iterator.
I chose to fix this by making a copy of the stop hook list and iterating
over that. That's a cheap operation since this is just an array of
shared pointers. But more importantly doing it this way means that if on
a stop where one stop hook deletes another, the deleted hook will always
get a chance to run. If you iterated over the list itself, then whether
that to be deleted hook gets to run would be dependent on whether it was
before or after the deleting stop hook, which would be confusing.
We weren't setting `m_should_detach` when going through the
`DoConnectRemote` code path. This meant that when you would attaches to
a remote process with `gdb-remote <port>` and use Ctrl+D, it would kill
the process instead of detach from it.
rdar://156111423
Main executables were bypassing the locate module callback that shared
libraries use, preventing custom symbol file location logic from working
consistently.
This PR fix this by
* Adding target context to ModuleSpec
* Leveraging that context to use target search path and platform's
locate module callback in ModuleList::GetSharedModule
This ensures both main executables and shared libraries get the same
callback treatment for symbol file resolution.
---------
Co-authored-by: George Hu <hyubo@meta.com>
Co-authored-by: George Hu <georgehuyubo@gmail.com>
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>
This avoids unintentional comparisons between `SourceLanguage` and
`LanguageType`.
Also marks `operator bool` explicit so we don't implicitly convert to
bool.
https://github.com/llvm/llvm-project/pull/165996 is adding a Clang
dependency to Target because we're moving some of the functionality of
the VerboseTrapFrameRecognizer into libClang. To avoid adding this
dependency this patch moves VerboseTrapFrameRecognizer into the
CPPLanguageRuntime. Most of the frame recognizers already live in the
various runtime plugins.
An alternative discussed was to create a common `CLanguageRuntime` whose
currently sole responsibility was to register the
`VerboseTrapFrameRecognizer` and `AssertStackFrameRecognizer`. The main
issue I ran into here was frame recognizers aren't uniqued in the
target. Currently this only manifests when re-running a target, which
re-triggers all the recognizer registration (added a test with a FIXME
for this). If we had a common `CLanguageRuntime` that
`CPPLanguageRuntime` and `ObjCLanguageRuntime` inherited from, I didn't
find a great way to avoid registering the recognizer multiple times. We
can't just call_once on it because we do want the recognisers to be
re-registered for new targets in the same debugger session. If the
recognisers were stored in something like a UniqueVector in the Target,
then we wouldn't have that issue. But currently that's not the case, and
it would take a bit of refactoring to de-dupe the recognisers.
There may very well be solutions I haven't considered, but all the
things I've tried so far I wasn't very happy with. So in the end I just
moved this to the C++ runtime for now in order to unblock
https://github.com/llvm/llvm-project/pull/165996.
The C++ language runtime is always available (even for C targets) if the
C++ language plugin is available. Which it should also be unless someone
is using an LLDB with the C++ plugin compiled out. But at that point
numerous things wouldn't work when even debugging just C.
This adds a virtual method that allows `InstrumentationRuntime` sub
classes to match against all modules rather than just a library that
matches a particular regex. When the implementation returns true
`GetPatternForRuntimeLibrary()` is ignored and all modules are iterated
over. The default implementation returns false which was the previous
behavior which uses `GetPatternForRuntimeLibrary()` to only match a
particular runtime library.
The intended use case here is for implementing an
`InstrumentationRuntime` where the runtime library of interest can have
multiple implementations and whose name is not known ahead of time.
The concrete use case here is for a `InstrumentationRuntime` plugin for
implementations of the `-fbounds-safety` soft-trap runtime which can
have multiple different implementations and so the module containing the
runtime functions isn't known ahead of time. This plug-in will be
upstreamed as part of the process of upstreaming `-fbounds-safety`.
An alternative to this would be for the `GetPatternForRuntimeLibrary()`
function to return a regex that matches everything. While that
technically works this new API more clearly indicates in the intent. We
probably also save a little perf by not executing the regex match for
every loaded module but I have not measured this.
rdar://163230807
This commit aggregates the following changes:
1. Fix the format (i.e., indentation) when printing stop hooks via `target
stop-hook list`.
2. Add `IndentScope Stream::MakeIndentScope()` to make managing (and restoring!)
of the indentation level on `Stream` instances more ergonomic and less error
prone.
3. Simplify printing of stop hooks using the new `IndentScope`.
### Summary
This PR refactors breakpoint event notification in LLDB to centralize
and eliminate code duplication. It creates a unified method in the
`Target` class for sending breakpoint change events. The new methods
check if listeners exist before broadcasting events
### Test
<img width="1532" height="76" alt="Screenshot 2025-10-23 at 12 49 31 PM"
src="https://github.com/user-attachments/assets/6d6a6da6-9684-463c-aeeb-90663cdbd077"
/>
---------
Co-authored-by: Piyush Jaiswal <piyushjais@meta.com>
Introduce the concept of internal stop hooks.
These are similar to LLDB's internal breakpoints:
LLDB itself will add them and users of LLDB will
not be able to add or remove them.
This change adds the following 3
independently-useful concepts:
* Maintain a list of internal stop hooks that will be populated by LLDB
and cannot be added to or removed from by users. They are managed in a
separate list in `Target::m_internal_stop_hooks`.
* `StopHookKind:CodeBased` and `StopHookCoded` represent a stop hook
defined by a C++ code callback (instead of command line expressions or a
Python class).
* Stop hooks that do not print any output can now also suppress the
printing of their header and description when they are hit via
`StopHook::GetSuppressOutput`.
Combining these 3 concepts we can model "internal
stop hooks" which serve the same function as
LLDB's internal breakpoints: executing built-in,
LLDB-defined behavior, leveraging the existing
mechanism of stop hooks.
This change also simplifies
`Target::RunStopHooks`. We already have to
materialize a new list for combining internal and
user stop hooks. Filter and only add active hooks to this list to avoid
the need for "isActive?"
checks later on.
This commit introduces a base-class implementation for a method that
reads memory from multiple ranges at once. This implementation simply
calls the underlying `ReadMemoryFromInferior` method on each requested
range, intentionally bypassing the memory caching mechanism (though this
may be easily changed in the future).
`Process` implementations that can be perform this operation more
efficiently - e.g. with the MultiMemPacket described in [1] - are
expected to override this method.
As an example, this commit changes AppleObjCClassDescriptorV2 to use the
new API.
Note about the API
------------------
In the RFC, we discussed having the API return some kind of class
`ReadMemoryRangesResult`. However, while writing such a class, it became
clear that it was merely wrapping a vector, without providing anything
useful. For example, this class:
```
struct ReadMemoryRangesResult {
ReadMemoryRangesResult(
llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ranges)
: ranges(std::move(ranges)) {}
llvm::ArrayRef<llvm::MutableArrayRef<uint8_t>> getRanges() const {
return ranges;
}
private:
llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ranges;
};
```
As can be seen in the added test and in the added use-case
(AppleObjCClassDescriptorV2), users of this API will just iterate over
the vector of memory buffers. So they want a return type that can be
iterated over, and the vector seems more natural than creating a new
class and defining iterators for it.
Likewise, in the RFC, we discussed wrapping the result into an
`Expected`. Upon experimenting with the code, this feels like it limits
what the API is able to do as the base class implementation never needs
to fail the entire result, it's the individual reads that may fail and
this is expressed through a zero-length result. Any derived classes
overriding `ReadMemoryRanges` should also never produce a top level
failure: if they did, they can just fall back to the base class
implementation, which would produce a better result.
The choice of having the caller allocate a buffer and pass it to
`Process::ReadMemoryRanges` is done mostly to follow conventions already
done in the Process class.
[1]:
https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/
StopInfoBreakpoint keeps a BreakpointLocationCollection for all the
breakpoint locations at the BreakpointSite that was hit. It is also
lives through the time a given thread is stopped, so there are plenty of
opportunities for one of the owning breakpoints to get deleted.
But BreakpointLocations don't keep their owner Breakpoints alive, so if
the BreakpointLocationCollection can live past when some code gets a
chance to delete an owner breakpoint, and then you ask that location for
some breakpoint information, it will access freed memory.
This wasn't a problem before PR #158128 because the StopInfoBreakpoint
just kept the BreakpointSite that was hit, and when you asked it
questions, it relooked up that list. That was not great, however,
because if you hit breakpoints 5 & 6, deleted 5 and then asked which
breakpoints got hit, you would just get 6. For that and other reasons
that PR changed to storing a BreakpointLocationCollection of the
breakpoints that were hit. That's better from a UI perspective but
caused this potential problem.
I fix it by adding a variant of the BreakpointLocationCollection that
also holds onto a shared pointer to the Breakpoints that own the
locations that were hit, thus keeping them alive till the
StopInfoBreakpoint goes away.
This fixed the ASAN assertion. I also added a test that works harder to
cause trouble by deleting breakpoints during a stop.
Specially when dealing with different address spaces, CFAs could start
from addresses like 0. For instance, Nvidia GPUs have instructions to
read from local memory that use 0-based offsets and stack memory can be
referenced by these offsets rather than global addresses. Note that ABIs
could already specify what they consider to be valid CFA values but this
was never used in these parts of the unwinder code. For most ABIs, this
makes the validation more strict, as they already used to discard 0s and
then checked for alignment which would discard 1s. There a few
exceptions where 0s were possible and this makes it less strict, like
the RISCV and ARC ABIs.
@jasonmolenda Would you be the appropriate reviewer for this?
Also cc. @clayborg @walter-erquinigo
This patch adds the notion of "Facade" locations which can be reported
from a ScriptedResolver instead of the actual underlying breakpoint
location for the breakpoint. Also add a "was_hit" method to the scripted
resolver that allows the breakpoint to say which of these "Facade"
locations was hit, and "get_location_description" to provide a
description for the facade locations.
I apologize in advance for the size of the patch. Almost all of what's
here was necessary to (a) make the feature testable and (b) not break
any of the current behavior.
The motivation for this feature is given in the "Providing Facade
Locations" section that I added to the python-reference.rst so I won't
repeat it here.
rdar://152112327
When a thread reaches a breakpoint at the return address set by
`ThreadPlanStepOut`, `ThreadPlanStepOut::ShouldStop()` calls
`ThreadPlanShouldStopHere::InvokeShouldStopHereCallback()`, and if it
returns `false`, `ThreadPlanShouldStopHere::QueueStepOutFromHerePlan()`
is called to queue a new plan to skip the corresponding range. Once the
new plan finishes, `ThreadPlanStepOut::ShouldStop()` should recheck the
stop condition; however, there is no code path in the method that sets
`done` to `true`. Before #126838, if `done` was `false`, the method checked
if a suitable frame had been reached. After the patch, the check is only
performed at a breakpoint; thus, the execution continues.
This patch causes `ThreadPlanStepOut::ShouldStop()` to recheck the stop
condition when `m_step_out_further_plan_sp` completes.
Depends on https://github.com/llvm/llvm-project/pull/162048
This makes sure we also include the version number in the description.
For `C++17`, this would, e.g., now return `"C++17"` instead of `"ISO
C++"`.
This fixes a potential use after free where
ModuleList::RemoveSharedModuleIfOrphaned ->
SharedModuleList::RemoveIfOrphaned -> SharedModuleList::RemoveFromMap
would potentially dereference a freed pointer. This fixes it by not
calling ModuleList::RemoveSharedModuleIfOrphaned at all if the pointer
was just freed.
The ToDwarfSourceLanguage function incorrectly excluded languages that
equal eLanguageTypeLastStandardLanguage. The comparison used `<` instead
of `<=`, causing the last standard language to fall through to the
default case and return std::nullopt.
This broke language plugins that use eLanguageTypeLastStandardLanguage
(currently Mojo at 0x0033) as their language code, preventing proper
DWARF language conversion and breaking REPL functionality.
The fix changes the comparison from `<` to `<=` to include the last
standard language in the automatic conversion to DWARF source language.
This is a regression from commit
7f51a2a47d2e706d04855b0e41690ebafa2b3238 which introduced the
ToDwarfSourceLanguage function.
### Summary
Add support for unique target ids per Target instance. This is needed
for upcoming changes to allow debugger instances to be shared across
separate DAP instances for child process debugging. We want the IDE to
be able to attach to existing targets in an already runny lldb-dap
session, and having a unique ID per target would make that easier.
Each Target instance will have its own unique id, and uses a
function-local counter in `TargetList::CreateTargetInternal` to assign
incremental unique ids.
### Tests
Added several unit tests to test basic functionality, uniqueness of
targets, and target deletion doesn't affect the uniqueness.
```
bin/lldb-dotest -p TestDebuggerAPI
```
The intention for this API is to be used when presenting language names
to the user, e.g., in expression evaluation diagnostics or LLDB errors.
Most uses of `GetNameForLanguageType` can be probably replaced with
`GetDisplayNameForLanguageType`, but that's out of scope of this PR.
This uses `llvm::dwarf::LanguageDescription` under the hood, so we would
lose the version numbers in the names. If we deem those to be important,
we could switch to an implementation that hardcodes a list of
user-friendly names with version numbers included.
The intention is to use it from
https://github.com/llvm/llvm-project/pull/161688
Depends on https://github.com/llvm/llvm-project/pull/161804
Currently we don't benefit from the user-friendly names that
`LanguageDescription` returns because we would always use
`Language::GetNameForLanguageType`. I'm not aware of a situation where
`GetDescription` should prefer the non-human readable form of the name
with. This patch removes the call to `GetNameForLanguageType`.
`LanguageDescription` already handles languages that it doesn't know
about. For those it would return `Unknown`. The LLDB language types
should all be available via DWARF. If there are languages that don't map
cleanly between `lldb::LanguageType` and `DW_LANG`, then we should add
explicit support for that in the `SourceLanguage::SourceLanguage`
constructor.
This patch adds a load core time, right now we don't have much insight
into the performance of load core, especially for large coredumps. To
start collecting information on this I've added some minor
instrumentation code to measure the two call sites of `LoadCore`.
I've also added a test to validate the new metric is output in
statistics dump
Currently, we always pass the "selected" execution context to the
statusline. When handling a process or thread event, we can be more
precise, and build an execution context from the event data. This PR
also adopts the new `StoppedExecutionContext` that was recently
introduced.
Based on testing on processors that use pointer metadata, and with all
the work done to delay calls to FixDataAddress, this is no longer
necessary.
Note that, with debugserver in particular, this is an NFC change: the
code path here is for frame zero, and debugserver will strip metadata
when reading fp from frame zero anyway.
The first call, in InitializeNonZerothFrame, is inside a logging branch.
For that, it's better to log the real value instead of the fixed one.
The second call, inside RegisterContextUnwind::ReadFrameAddress, is
computing an address which is then passed to
ReadRegisterValueFromMemory, which boils down to a Process::ReadMemory,
which fixes the address if it wants to. The current variable names are
misleading, making readers believe it is the cfa value itself that is
being passed to Fix*Address; that's not the case. This commit renames
the variable to make this abundantly clear.
[lldb] Track CFA pointer metadata in StackID
In this commit:
9c8e71644227 [lldb] Make StackID call Fix{Code,Data} pointers (#152796)
We made StackID keep track of the CFA without any pointer metadata in
it. This is necessary when comparing two StackIDs to determine which one
is "younger".
However, the CFA inside StackIDs is also used in other contexts through
the method StackID::GetCallFrameAddress. One notable case is
DWARFExpression: the computation of `DW_OP_call_frame_address` is done
using StackID. This feeds into many other places, e.g. expression
evaluation may require the address of a variable that is computed from
the CFA; to access the variable without faulting, we may need to
preserve the pointer metadata. As such, StackID must be able to provide
both versions of the CFA.
In the spirit of allowing consumers of pointers to decide what to do
with pointer metadata, this patch changes StackID to store both versions
of the cfa pointer. Two getter methods are provided, and all call sites
except DWARFExpression preserve their existing behavior (stripped
pointer). Other alternatives were considered:
* Just store the raw pointer. This would require changing the
comparisong operator `<` to also receive a Process, as the comparison
requires stripped pointers. It wasn't clear if all call-sites had a
non-null process, whereas we know we have a process when creating a
StackID.
* Store a weak pointer to the process inside the class, and then strip
metadata as needed. This would require a `weak_ptr::lock` in many
operations of LLDB, and it felt wasteful. It also prevents stripping
of the pointer if the process has gone away.
This patch also changes RegisterContextUnwind::ReadFrameAddress, which
is the method computing the CFA fed into StackID, to also preserve the
signature pointers.
This is something the StopInfo class manages, so it should be allowed to
compute this rather than having SBThread do so. This code just moves the
computation to methods in StopInfo. It is mostly NFC. The one change
that I actually had to adjust the tests for was a couple of tests that
were asking for the UnixSignal stop info data by asking for the data at
index 1. GetStopInfoDataCount returns 1 and we don't do 1 based indexing
so the test code was clearly wrong. But I don't think it makes sense to
perpetuate handing out the value regardless of what index you pass us.
