`cfa_reg_contents` is a local variable. Whatever value we assign there
right before the `return` statement will be lost anyway.
Co-authored-by: Matej Košík <matej.kosik@codasip.com>
Prior to this patch, SBFrame/SBThread methods exhibit racy behavior if
called while the process is running, because they do not lock the
`Process::RetRunLock` mutex. If they did, they would fail, correctly
identifying that the process is not running.
Some methods _attempt_ to protect against this with the pattern:
```
ExecutionContext exe_ctx(m_opaque_sp.get(), lock); // this is a different lock
Process *process = exe_ctx.GetProcessPtr();
if (process) {
Process::StopLocker stop_locker;
if (stop_locker.TryLock(&process->GetRunLock()))
.... do work ...
```
However, this is also racy: the constructor of `ExecutionContext` will
access the frame list, which is something that can only be done once the
process is stopped.
With this patch:
1. The constructor of `ExecutionContext` now expects a `ProcessRunLock`
as an argument. It attempts to lock the run lock, and only fills in
information about frames and threads if the lock can be acquired.
Callers of the constructor are expected to check the lock.
2. All uses of ExecutionContext are adjusted to conform to the above.
3. The SBThread.cpp-defined helper function ResumeNewPlan now expects a
locked ProcessRunLock as _proof_ that the execution is stopped. It will
unlock the mutex prior to resuming the process.
This commit exposes many opportunities for early-returns, but these
would increase the diff of this patch and distract from the important
changes, so we opt not to do it here.
In architectures where pointers may contain metadata, such as arm64e, it
is important to ignore those bits when comparing two different StackIDs,
as the metadata may not be the same even if the pointers are.
This patch is a step towards allowing consumers of pointers to decide
whether they want to keep or remove metadata, as opposed to discarding
metadata at the moment pointers are created. See
https://github.com/llvm/llvm-project/pull/150537.
This was tested running the LLDB test suite on arm64e.
Resolves#141955
- Adds data to breakpoints `Source` object, in order for assembly
breakpoints, which rely on a temporary `sourceReference` value, to be
able to resolve in future sessions like normal path+line breakpoints
- Adds optional `instructions_offset` parameter to `BreakpointResolver`
`PlatformList::Create()` added an item to the list even when
`Platform::Create()` returned `nullptr`. Other methods use these items
without checking, which can lead to a crash. For example:
```
> lldb
(lldb) platform select unknown
error: unable to find a plug-in for the platform named "unknown"
(lldb) file a.out-arm64
PLEASE submit a bug report to...
Stack dump:
0. Program arguments: lldb
1. HandleCommand(command = "file a.out-arm64 ")
...
```
The test for this is artificial as I'm not aware of any upstream targets
that use DW_CFA_val_offset
RegisterContextUnwind::ReadFrameAddress now reports how it's attempting
to obtain the CFA unless all success/failure cases emit logs that
clearly identify the method it was attempting. Previously several of the
existing failure paths emit no message or a message that's
indistinguishable from those on other paths.
The ctor that takes a reference to ExecutionContextRef can be
implemented in terms of the ctor that takes a pointer to that object,
removing code duplication.
This patch uses the "setting changed" callback to clear previously
cached stack frames when
`target.process.disable-language-runtime-unwindplans` is changed. This
is necessary so that changing the setting followed by a `backtrace`
command produces an accurate backtrace.
With this, a user can create a custom command like below in order to
quickly inspect a backtrace created without language plugins.
```
debugger.HandleCommand("settings set target.process.disable-language-runtime-unwindplans true")
debugger.HandleCommand("bt " + command)
debugger.HandleCommand("settings set target.process.disable-language-runtime-unwindplans false")
```
In the future, we may consider implementing this as an option to
`backtrace`. Currently, this process setting is the only way of
affecting the unwinder, and changing the process setting as part of the
backtrace implementation doesn't feel right.
There are no upstream users of this flag, so we cannot write a test for
it here.
Extend support in LLDB for WebAssembly. This PR adds a new Process
plugin (ProcessWasm) that extends ProcessGDBRemote for WebAssembly
targets. It adds support for WebAssembly's memory model with separate
address spaces, and the ability to fetch the call stack from the
WebAssembly runtime.
I have tested this change with the WebAssembly Micro Runtime (WAMR,
https://github.com/bytecodealliance/wasm-micro-runtime) which implements
a GDB debug stub and supports the qWasmCallStack packet.
```
(lldb) process connect --plugin wasm connect://localhost:4567
Process 1 stopped
* thread #1, name = 'nobody', stop reason = trace
frame #0: 0x40000000000001ad
wasm32_args.wasm`main:
-> 0x40000000000001ad <+3>: global.get 0
0x40000000000001b3 <+9>: i32.const 16
0x40000000000001b5 <+11>: i32.sub
0x40000000000001b6 <+12>: local.set 0
(lldb) b add
Breakpoint 1: where = wasm32_args.wasm`add + 28 at test.c:4:12, address = 0x400000000000019c
(lldb) c
Process 1 resuming
Process 1 stopped
* thread #1, name = 'nobody', stop reason = breakpoint 1.1
frame #0: 0x400000000000019c wasm32_args.wasm`add(a=<unavailable>, b=<unavailable>) at test.c:4:12
1 int
2 add(int a, int b)
3 {
-> 4 return a + b;
5 }
6
7 int
(lldb) bt
* thread #1, name = 'nobody', stop reason = breakpoint 1.1
* frame #0: 0x400000000000019c wasm32_args.wasm`add(a=<unavailable>, b=<unavailable>) at test.c:4:12
frame #1: 0x40000000000001e5 wasm32_args.wasm`main at test.c:12:12
frame #2: 0x40000000000001fe wasm32_args.wasm
```
This PR is based on an unmerged patch from Paolo Severini:
https://reviews.llvm.org/D78801. I intentionally stuck to the
foundations to keep this PR small. I have more PRs in the pipeline to
support the other features/packets.
My motivation for supporting Wasm is to support debugging Swift compiled
to WebAssembly:
https://www.swift.org/documentation/articles/wasm-getting-started.html
Protected m_selected_frame_idx with a mutex. To avoid deadlocks, always acquire m_selected_frame_mutex after m_list_mutex. I'm using a recursive mutex because GetSelectedFrameIndex may indirectly call SetSelectedFrame.
This is a continuation of 68fd102, which did the same thing but only for
StopInfo. Using make_shared is both safer and more efficient:
- With make_shared, the object and the control block are allocated
together, which is more efficient.
- With make_shared, the enable_shared_from_this base class is properly
linked to the control block before the constructor finishes, so
shared_from_this() will be safe to use (though still not recommended
during construction).
LanguageType has two kinds of enumerators in it. The first is
DWARF-assigned enumerators which must be consecutive and match DW_LANG
values. The second is the vendor-assigned enumerators which must be
unique and must follow on from the DWARF-assigned values (i.e. the first
one is currently eLanguageTypeMojo + 1) even if that collides with
DWARF-assigned values that lldb is not yet aware of
Only the DWARF-assigned enumerators may be static_cast from DW_LANG
since their values match. The vendor-assigned enumerators must be
explicitly converted since their values do not match. This needs to
handle new languages added to DWARF and not yet implemented in lldb.
This fixes a crash when encountering a DW_LANG value >=
eNumLanguageTypes and wrong behaviour when encountering DW_LANG values
that have not yet been added to LanguageType but happen to coincide with
a vendor-assigned enumerator due to the consecutive values requirement
described above.
Another way to fix the crash is to add the language to LanguageType (and
fill any preceeding gaps in the number space) so that the DW_LANG being
encountered is correctly handled but this just moves the problem to a
new subset of DW_LANG values.
Also fix an unnecessary static-cast from LanguageType to LanguageType.
Target::ReadMemory may or may not read live memory, but whether it did
read from live memory or from the filecache is opaque to callers. Add an
extra out parameter to indicate whether live memory was read or not.
Use std::make_shared to create a StopInfoSP, which inherits from
shared_from_this. It's both the most efficient and safest way to create
these objects:
- With make_shared, the object and the control block are allocated
together, which is more efficient.
- With make_shared, the enable_shared_from_this base class is properly
linked to the control block before the constructor finishes, so
shared_from_this() will be safe to use (though still not recommended
during construction).
Don't make assumptions about the lifetime of the underlying object and
use the shared_ptr to participate in reference counting and extend the
lifetime of the object to the end of the lexical scope.
Deeply nested structs can be noisy, so Apple's LLDB fork sets the
default to `4`:
9c93adbb28/lldb/source/Target/TargetProperties.td (L134-L136)
Thought it would be useful to upstream this. Though happy to pick a
different default or keep it as-is.
This is an odd corner case of the use of scripts loaded from dSYM's - a
macOS only feature, which can load OS Plugins that re-present the thread
state of the program we attach to. If we find out about and load the
dSYM scripts when we discover a target in the course of attaching to it,
we can end up running the OS plugin before we've started up the private
state thread. However, the os_plugin in that case will be running before
we broadcast the stop event to the public event listener. So it should
formally use the private state and not the public state for the Python
code environment.
This patch says that if we have not yet started up the private state
thread, then any thread that is servicing events is doing so on behalf
of the private state machinery, and should see the private state, not
the public state.
Most of the patch is getting a test that will actually reproduce the
error. Only the test `test_python_os_plugin_remote` actually reproduced
the error. In `test_python_os_plugin` we actually do start up the
private state thread before handling the event. `test_python_os_plugin`
is there for completeness sake.
LLDB breakpoint conditions take an expression that's evaluated using the
language of the code where the breakpoint is located. Users have asked
to have an option to tell it to evaluate the expression in a specific
language.
This is feature is especially helpful for Swift, for example for a
condition based on the value in memory at an offset from a register.
Such a condition is pretty difficult to write in Swift, but easy in C.
This PR adds a new argument (-Y) to specify the language of the
condition expression. We can't reuse the current -L option, since you
might want to break on only Swift symbols, but run a C expression there
as per the example above.
rdar://146119507
As a preliminary to making DIL the default implementation for
'frame var', ran check-lldb forcing 'frame var' to always use DIL,
and discovered a few failing tests. This fixes most of them. The only
remaining failing test is TestDAP_evaluate.py, which now passes
a test case that the test says should fail (still investigating this).
Changes in this PR:
- Sets correct VariableSP, as well as returning ValueObjectSP (needed
for several watchpoint tests).
- Updates error messages, when looking up members, to match what the
rest of LLDB expects. Also update appropriate DIL tests to expect the
updated error messages.
- Updates DIL parser to look for and accept "(anonymous namespace)::" at
the front of a variable name.
c72c0b298c13 fixed a race condition in Target::GetExecutableModule. The
patch originally added the lock_guard but I suggested using the locking
ModuleList::Modules() helper instead. That didn't consider that the
fallback would still access the ModuleList without holding the lock.
This patch fixes the remaining issue.
…ess" (#144810)
This relands commit e0933ab5ae4856c4aa188a5ea16716b3a8d0840b. The
original commit was causing the test TestCreateAfterAttach.py to fail on
ARM Ubuntu bots. It's possible that this could've been happening because
the test for wait-attach progress reporting is waiting on a process
named "a.out" which could be too generic as multiple other tests (when
run in parallel on the bots) could also be using processes named
"a.out". This commit changes the wait-attach progress report test to
wait on a unique process name.
Original PR description:
This commit adds a progress report when wait-attaching to a process as
well as a test for this.
Original PR link: https://github.com/llvm/llvm-project/pull/144768
It's not necessary on posix platforms as of #126935 and it's ignored on
windows as of #138896. For both platforms, we have a better way of
inheriting FDs/HANDLEs.
This relands changes in #144424 for adding a count of DWO files
parsed/loaded and the total number of DWO files.
The previous PR was reverted in #145494 due to the newly added unit
tests failing on Windows and MacOS CIs since these platforms don't
support DWO. This change add an additional
`@add_test_categories(["dwo"])` to the new tests to
[skip](cd46354dbd/lldb/packages/Python/lldbsuite/test/test_categories.py (L56))
these tests on Windows/MacOS.
Original PR: #144424
### Testing
Ran unit tests
```
$ bin/lldb-dotest -p TestStats.py llvm-project/lldb/test/API/commands/statistics/basic/
----------------------------------------------------------------------
Ran 24 tests in 211.391s
OK (skipped=3)
```
## Summary
A new `totalLoadedDwoFileCount` and `totalDwoFileCount` counters to
available statisctics when calling "statistics dump".
1. `GetDwoFileCounts ` is created, and returns a pair of ints
representing the number of loaded DWO files and the total number of DWO
files, respectively. An override is implemented for `SymbolFileDWARF`
that loops through each compile unit, and adds to a counter if it's a
DWO unit, and then uses `GetDwoSymbolFile(false)` to check whether the
DWO file was already loaded/parsed.
3. In `Statistics`, use `GetSeparateDebugInfo` to sum up the total
number of loaded/parsed DWO files along with the total number of DWO
files. This is done by checking whether the DWO file was already
successfully `loaded` in the collected DWO data, anding adding to the
`totalLoadedDwoFileCount`, and adding to `totalDwoFileCount` for all CU
units.
## Expected Behavior
- When binaries are compiled with split-dwarf and separate DWO files,
`totalLoadedDwoFileCount` would be the number of loaded DWO files and
`totalDwoFileCount` would be the total count of DWO files.
- When using a DWP file instead of separate DWO files,
`totalLoadedDwoFileCount` would be the number of parsed compile units,
while `totalDwoFileCount` would be the total number of CUs in the DWP
file. This should be similar to the counts we get from loading separate
DWO files rather than only counting whether a single DWP file was
loaded.
- When not using split-dwarf, we expect both `totalDwoFileCount` and
`totalLoadedDwoFileCount` to be 0 since no separate debug info is
loaded.
## Testing
**Manual Testing**
On an internal script that has many DWO files, `statistics dump` was
called before and after a `type lookup` command. The
`totalLoadedDwoFileCount` increased as expected after the `type lookup`.
```
(lldb) statistics dump
{
...
"totalLoadedDwoFileCount": 29,
}
(lldb) type lookup folly::Optional<unsigned int>::Storage
typedef std::conditional<true, folly::Optional<unsigned int>::StorageTriviallyDestructible, folly::Optional<unsigned int>::StorageNonTriviallyDestructible>::type
typedef std::conditional<true, folly::Optional<unsigned int>::StorageTriviallyDestructible, folly::Optional<unsigned int>::StorageNonTriviallyDestructible>::type
...
(lldb) statistics dump
{
...
"totalLoadedDwoFileCount": 2160,
}
```
**Unit test**
Added three unit tests that build with new "third.cpp" and "baz.cpp"
files. For tests with w/ flags `-gsplit-dwarf -gpubnames`, this
generates 2 DWO files. Then, the test incrementally adds breakpoints,
and does a type lookup, and the count should increase for each of these
as new DWO files get loaded to support these.
```
$ bin/lldb-dotest -p TestStats.py ~/llvm-sand/external/llvm-project/lldb/test/API/commands/statistics/basic/
----------------------------------------------------------------------
Ran 20 tests in 211.738s
OK (skipped=3)
```
This is breaking TestCreateAfterAttach.py on Ubuntu:
```
======================================================================
FAIL: test_create_after_attach_dwo (TestCreateAfterAttach.CreateAfterAttachTestCase.test_create_after_attach_dwo)
Test thread creation after process attach.
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1804, in test_method
return attrvalue(self)
^^^^^^^^^^^^^^^
File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py", line 149, in wrapper
return func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py", line 36, in test_create_after_attach
self.runCmd("process attach -p " + str(pid))
File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1005, in runCmd
self.assertTrue(self.res.Succeeded(), msg + output)
AssertionError: False is not true : Command 'process attach -p 1474309' did not return successfully
Error output:
error: attach failed: lost connection
```
on the buildbots for lldb-remote-linux-ubuntu, lldb-arm-ubuntu,
lldb-aarch64-ubuntu, lldb-arm-ubuntu.
Since this PR: https://github.com/llvm/llvm-project/pull/141670/ We
started to override the Platform/Arch for a target if needed. However we
may have already registered locate module callback with the old
platform.
This PR will move the locate module callback to the new Platform
whenever Target changes architecture.
Co-authored-by: George Hu <georgehuyubo@gmail.com>
RegisterContextUnwind::SavedLocationForRegister is around 450 lines that
first find an abstract register location (e.g. "CFA-8") for a register
by looking in the UnwindPlans. Then it evaluates the abstract register
location to create a concrete register location (e.g. "stored at address
0x...", "live in register at frame 0"). There are some complicated cases
in the first half of the method to handle return address register
architectures correctly, in particular.
Looking at the two halves, they're both exactly 226 lines long and
there's little involvement between them except for passing an abstract
register location along.
(there were some parts in the "abstract register location" code that
would set the concrete register location, unnecessarily)
It's also a complex enough method that there are some bits of code that
aren't actually doing anything at this point.
This patch adds a RegisterContextUnwind::GetAbstractRegisterLocation
method, which does the first half, and has a clearly defined return
values.
The code to convert an AbstractRegisterLocation into a
ConcreteRegisterLocation remains in SavedLocationForRegister.
It's a bit of a tricky patch to visually inspect, despite it not
changing functionality, the reorganizations and rewrites make the diff
unreadable. Nearly all the real changes are in the "find the abstract
register location" first half of the method. I think reading the new
code in its new form is the easiest way to inspect this PR. With a
defined interface between the two of what is expected, it's pretty easy
to look at the code and reason about whether it is written correctly.
(whereas before, that was very difficult, for me at least.)
---------
Co-authored-by: Pavel Labath <pavel@labath.sk>
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.
This patch should be mostly obvious, but in one place, this patch
changes:
const auto &it = std::find(...)
to:
auto it = llvm::find(...)
We do not need to bind to a temporary with const ref.
If we're not touching them, we don't need to do anything special to pass
them along -- with one important caveat: due to how cmake arguments
work, the implicitly passed arguments need to be specified before
arguments that we handle.
This isn't particularly nice, but the alternative is enumerating all
arguments that can be used by llvm_add_library and the macros it calls
(it also relies on implicit passing of some arguments to
llvm_process_sources).
This fixes a data race between the main thread and the default event
handler thread. The statusline format option value was protected by a
mutex, but it was returned as a pointer, allowing one thread to access
it while another was modifying it.
Avoid the data race by returning format values by value instead of by
pointer.
This macro does not do what is described. The only thing it actually
does control is whether or not the process's memory cache gets flushed
when writing to an address. It does not override the setting
`target.process.disable-memory-cache`.
Instead of making it work as intended, I chose to remove the macro. I
don't see much value in being able to override the setting with a
preprocessor macro.
A few files of lldb dir & few other files had duplicate headers
included. This patch removes those redundancies.
---------
Co-authored-by: Akash Agrawal <akashag@qti.qualcomm.com>
After some debugging, I found out ProcessELFCore never updates the
platform. I've updated ProcessElfCore to set the arch and platform
before we parse the Notes.
Recently, I was on an issue that generated a large number of Coredumps,
and every time in both LLDB and GDB the signal was just `SIGSEGV`.
This was frustrating because we would expect a `SIGSEGV` to have an
address, or ideally even bounds. After some digging I found the
`si_code` consistently was -6. With some help from
[@cdown](https://github.com/cdown), we found neither LLDB or GDB
supports the si_codes sent from [user
space](https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/siginfo.h#L185).
Excerpted from the sigaction man page.
```
For a regular signal, the following list shows the values which
can be placed in si_code for any signal, along with the reason
that the signal was generated.
```
For which I added all of the si_codes to every Linux signal. Now for the
Coredump that triggered this whole investigation we get the accurate and
now very informative summary.
<img width="524" alt="image"
src="https://github.com/user-attachments/assets/5149f781-ef21-4491-a077-8fac862fbc20"
/>
Additionally from @labath's suggestion to move this to platform and
leverage the existing `getSiginfo()` call on thread, we can now inspect
the siginfo struct itself via `thread siginfo`. Giving us another
towards GDB parity on elf cores.
Basically, disable everything except the eh_frame unwind plan, as that's
the only one which supports this right now. The other plans are working
with now trying the interpret everything in between the function parts
as a part of the function, which is more likely to produce wrong results
than correct ones.
I changed the interface for object file plans, to give the
implementations a chance to implement this correctly, but I haven't
actually converted PECallFrameInfo (its only implementation) to handle
that. (from the looks of things, it should be relatively easy to do, if
it becomes necessary)
I'm also deleting UnwindPlan::GetFirstNonPrologueInsn, as it's not used,
and it doesn't work for discontinuous functions.
This reapplies https://github.com/llvm/llvm-project/pull/138892, which
was reverted in
5fb9dca14a
due to failures on windows.
Windows loads modules from the Process class, and it does that quite
early, and it kinda makes sense which is why I'm moving the clearing
code even earlier.
The original commit message was:
Minidump files contain explicit information about load addresses of
modules, so it can load them itself. This works on other platforms, but
fails on darwin because DynamicLoaderDarwin nukes the loaded module list
on initialization (which happens after the core file plugin has done its
work).
This used to work until
https://github.com/llvm/llvm-project/pull/109477, which enabled the
dynamic loader
plugins for minidump files in order to get them to provide access to
TLS.
Clearing the load list makes sense, but I think we could do it earlier
in the process, so that both Process and DynamicLoader plugins get a
chance to load modules. This patch does that by calling the function
early in the launch/attach/load core flows.
This fixes TestDynamicValue.py:test_from_core_file on darwin.
