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.
`SBType::GetBasicType` fails on typedefs to primitive types. The docs
for `GetBasicType` state:
```
Returns the BasicType value that is most appropriate to this type
```
But, e.g., for `uint64_t` this would currently return
`eBasicTypeInvalid`.
`TypeSystemClang::GetBasicTypeEnumeration` (which is what
`SBType::GetBasicType` uses) doesn't see through typedefs. Inside LLDB
we almost always call `GetBasicTypeEnumeration` on the canonical type.
In the cases we don't I suspect those were just subtle bugs. This patch
gets the canonical type inside of `GetBasicTypeEnumeration` instead.
rdar://155829208
### 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
Since https://github.com/llvm/llvm-project/pull/148691 enabled
exceptions when compiling the tests, this test has been failing.
Much like was noted there, one of the variables disappeared
from the debug info. Giving it a non-zero size and initialising
it fixed that.
This PR adds type summaries for
`std::{string,wstring,u8string,u16string,u32string}` from the MSVC STL.
See https://github.com/llvm/llvm-project/issues/24834 for the MSVC STL
issue.
The following changes were made:
- `dotest.py` now detects if the MSVC STL is available. It does so by
looking at the target triple, which is an additional argument passed
from Lit. It specifically checks for `windows-msvc` to not match on
`windows-gnu` (i.e. MinGW/Cygwin).
- (The main part): Added support for summarizing `std::(w)string` from
MSVC's STL. Because the type names from the libstdc++ (pre C++ 11)
string types are the same as on MSVC's STL, `CXXCompositeSummaryFormat`
is used with two entries, one for MSVC's STL and one for libstdc++.
With MSVC's STL, `std::u{8,16,32}string` is also handled. These aren't
handled for libstdc++, so I put them in `LoadMsvcStlFormatters`.
Add a class property for the version string. This allows you to use
access the version string through `lldb.SBDebugger.version` instead of
having to call `lldb.SBDebugger.GetVersionString()`.
The problem was in calling GetLoadAddress on a value in the error state,
where `ValueObject::GetLoadAddress` could end up accessing the
uninitialized "address type" by-ref return value from `GetAddressOf`.
This probably happened because each function expected the other to
initialize it.
We can guarantee initialization by turning this into a proper return
value.
I've added a test, but it only (reliably) crashes if lldb is built with
ubsan.
The architectures provided to skipIf / expectedFail are regular
expressions (v. _match_decorator_property() in decorators.py
so on Darwin systems "arm64" would match the skips for "arm" (32-bit
Linux). Update these to "arm$" to prevent this, and also update
three tests (TestBuiltinFormats.py, TestCrossDSOTailCalls.py,
TestCrossObjectTailCalls.py) that were skipped for arm64 via this
behavior, and need to be skipped or they will fail.
This was moviated by the new TestDynamicValue.py test which has
an expected-fail for arm, but the test was passing on arm64 Darwin
resulting in failure for the CIs.
* Changes the default synthetic symbol names to contain their file
address
This is a new PR after the first PR (#137512) was reverted because it
didn't update the way unnamed symbols were searched in the symbol table,
which relied on the index being in the name.
This time also added extra test to make sure the symbol is found as
expected
The motivation here is being (un)able to treat pointer values as an
array consistently. This works for pointers to simple/scalar values, but
for aggregates, we get a very surprising result:
- GetChildAtIndex(x, ??, true) returns the `x` child of the zeroth array
member (the one you get by dereferencing the pointer/array) for all `x`
which are smaller than the number of children of that value.
- for other values of `x`, we get `v[x]`, where `v` is treated like a
(C) pointer
This patch reimagines this interface so that the value of `true` always
treats (pointer and array) values as pointers. For `false`, we always
dereference pointers, while in the case of arrays, we only return the
values as far as the array bounds will allow.
This has the potential to break existing code, but I have a suspicion
that code was already broken to begin with, which is why I think this
would be better than introducing a new API and keeping the old (and
surprising) behavior. If our own test coverage is any indication,
breakage should be minimal.
test_common is force-included into every compilation, which causes
problems when we're compiling assembly code, as we were in #138805.
This avoids that as we can include the header only when it's needed.
My current internal work requires some sensitivity to IO usage. I had a
work around to calculate the expected size of a Minidump, but I've added
this PR so an automated system could look at the expected size of an
LLDB generated Minidump and then choose if it has the space or wants to
generate it.
There are some prerequisites to calculating the correct size, so I have
the API take a reference for an SBError, I originally tried to return an
SBError and instead take a uint64_t reference, but this made the API
very difficult to use in python.
Added a test case as well.
`ValueObject::AddressOf()` used to return address as a value which has
it's own address, allowing to do `value.AddressOf().AddressOf()`.
This patch makes the return address a simple const value.
stop-hooks are supposed to trigger every time the process stops, but as
initially implemented they would only fire when control was returned to
the user. So for instance when a process was launched the stop hook
would only trigger when the process hit a breakpoint or crashed.
However, it would be really useful to be able to trigger a stop hook
when lldb first gains control over the process. One way to do that would
be to implement general "target lifecycle events" and then send process
created events that users could bind actions to.
OTOH, extending the stop hooks to fire when lldb first gains control
over the process is a pretty natural extension to the notion of a stop
hook. So this patch takes the shorter route to that ability by making
stop-hooks fire when lldb first gains control over the process.
I also added the ability to specify whether to trigger the stop hook "on
gaining control". I'm on the fence about whether to set the default to
be "trigger on gaining control" or "don't trigger on gaining control".
Since I think it's a generally useful feature, I've set the default to
"trigger on gaining control".
Original in https://github.com/llvm/llvm-project/pull/134626 was
written as if it was "this or this" but it's "this and this".
So the test ran on AArch64 Linux, because Linux is not Windows.
Split out the Windows check to fix that.
When you call the `SBTarget::ReadInstructions` with flavor from lldb
crashes. This is because the wrong order of the `DisassemblyBytes`
constructor this fixes that
---------
Signed-off-by: Ebuka Ezike <yerimyah1@gmail.com>
Expose u target API mutex through the SB API. This is motivated by
lldb-dap, which is built on top of the SB API and needs a way to execute
a series of SB API calls in an atomic manner (see #131242).
We can solve this problem by either introducing an additional layer of
locking at the DAP level or by exposing the existing locking at the SB
API level. This patch implements the second approach.
This was discussed in an RFC on Discourse [0]. The original
implementation exposed a move-only lock rather than a mutex [1] which
doesn't work well with SWIG 4.0 [2]. This implement the alternative
solution of exposing the mutex rather than the lock. The SBMutex
conforms to the BasicLockable requirement [3] (which is why the methods
are called `lock` and `unlock` rather than Lock and Unlock) so it can be
used as `std::lock_guard<lldb::SBMutex>` and
`std::unique_lock<lldb::SBMutex>`.
[0]: https://discourse.llvm.org/t/rfc-exposing-the-target-api-lock-through-the-sb-api/85215/6
[1]: https://github.com/llvm/llvm-project/pull/131404
[2]: https://discourse.llvm.org/t/rfc-bumping-the-minimum-swig-version-to-4-1-0/85377/9
[3]: https://en.cppreference.com/w/cpp/named_req/BasicLockable
Remove support for coalescing progress reports in LLDB. This
functionality was motivated by Xcode, which wanted to listen for less
frequent, aggregated progress events at the cost of losing some detail.
See the original RFC [1] for more details. Since then, they've
reevaluated this trade-off and opted to listen for the regular, full
fidelity progress events and do any post processing on their end.
rdar://146425487
This patch adds a finalize method which destroys the underlying RAII
SBProgress. My primary motivation for this is so I can write better
tests that are non-flaky, but after discussing with @clayborg in my DAP
message improvement patch (#124648) this is probably an essential API
despite that I originally argued it wasn't.
I am using VSCode with the official vscode-lldb extension. When I try to
list the breakpoints in the debug console get the message:
```
br list
can't evaluate expressions when the process is running.
```
I know that this is wrong and you need to use
```
`br list
(lldb) br list
No breakpoints currently set.
```
but the error message is misleading. I cleaned up the code and now the
error message is
```
br list
sbframe object is not valid.
```
which is still not perfect, but at least it's not misleading.
In #125132, Michael pointed out that there are now two tests with the
same name:
./lldb/test/API/api/command-return-object/TestSBCommandReturnObject.py
./lldb/test/API/python_api/commandreturnobject/TestSBCommandReturnObject.py
The WatchAddress API includes a flag to indicate if watchpoint should be
for read, modify or both. This API uses 2 booleans, but the 'modify'
flag was ignored and WatchAddress unconditionally watched write
(actually modify).
We now only watch for modify when the modify flag is true.
---
The included test fails prior to this patch and succeeds after. That is
previously specifying `False` for `modify` would still stop on _write_,
but after the patch correctly only stops on _read_
Xcode uses a pseudoterminal for the debugger console.
- The upside of this apporach is that it means that it can rely on
LLDB's IOHandlers for multiline and script input.
- The downside of this approach is that the command output is printed to
the PTY and you don't get a SBCommandReturnObject. Adrian added support
for inline diagnostics (#110901) and we'd like to access those from the
IDE.
This patch adds support for registering a callback in the command
interpreter that gives access to the `(SB)CommandReturnObject` right
before it will be printed. The callback implementation can choose
whether it likes to handle printing the result or defer to lldb. If the
callback indicated it handled the result, the command interpreter will
skip printing the result.
We considered a few other alternatives to solve this problem:
- The most obvious one is using `HandleCommand`, which returns a
`SBCommandReturnObject`. The problem with this approach is the multiline
input mentioned above. We would need a way to tell the IDE that it
should expect multiline input, which isn't known until LLDB starts
handling the command.
- To address the multiline issue,we considered exposing (some of the)
IOHandler machinery through the SB API. To solve this particular issue,
that would require reimplementing a ton of logic that already exists
today in the CommandInterpeter. Furthermore that seems like overkill
compared to the proposed solution.
rdar://141254310
As suggested in #125006. Depending on which PR lands first, I'll update
`TestCommandInterepterPrintCallback.py` to check that the
`CommandReturnObject` passed to the callback has the correct command.
Recently I've been working on a lot of internal Python tooling, and in
certain cases I want to report async to the script over DAP. Progress.h
already handles this, so I've exposed Progress via the SB API so Python
scripts can also update progress objects.
I actually have no idea how to test this, so I just wrote a [toy command
to test
it](https://gist.github.com/Jlalond/48d85e75a91f7a137e3142e6a13d0947)
I also copied the first section of the extensive Progress.h class
documentation to the docstrings.
This patch adds the ability to get a thread at a give index, based on
insertion order, for SBSaveCore Options. This is primarily to benefit
scripts using SBSaveCore, and remove the need to have both options and a
second collection if your script is tracking what threads need to be
saved. Such as if you want to collect the source of all the threads to
be saved after the Core is generated.
Introduces a `member` property to `SBValue`. This property provides pythonic access to a
value's members, by name. The expression `value.member["name"]` will be an alternate
form form of writing `value.GetChildMemberWithName("name")`.
This PR fixes a simple SWIG issue with SBMemoryRegionInfoList not being
iterable out-of-the-box. This is mostly because of limitations to the
`lldb_iter` function, which doesn't allow for specifying arguments to
the size / iter functions passed.
Before:
```
(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> for region in lldb.process.GetMemoryRegions():
... print(region)
...
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/opt/llvm/stable/Toolchains/llvm-sand.xctoolchain/usr/lib/python3.10/site-packages/lldb/__init__.py", line 114, in lldb_iter
yield elem(i)
TypeError: SBMemoryRegionInfoList.GetMemoryRegionAtIndex() missing 1 required positional argument: 'region_info'
```
After:
```
(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
>>> for region in lldb.process.GetMemoryRegions():
... print(region)
...
[0x0000000000200000-0x00000000002cf000 R--]
[0x00000000002cf000-0x0000000000597000 R-X]
[0x0000000000597000-0x00000000005ad000 R--]
[0x00000000005ad000-0x00000000005b1000 RW-]
[0x00000000005b1000-0x0000000000b68000 RW-]
[0x000000007fff7000-0x000000008fff7000 RW-]
[0x000002008fff7000-0x000010007fff8000 RW-]
[0x0000503000000000-0x0000503000010000 RW-]
[0x0000503e00000000-0x0000503e00010000 RW-]
[0x0000504000000000-0x0000504000010000 RW-]
[0x0000504e00000000-0x0000504e00010000 RW-]
[0x000050d000000000-0x000050d000010000 RW-]
[0x000050de00000000-0x000050de00010000 RW-]
[0x000050e000000000-0x000050e000010000 RW-]
[0x000050ee00000000-0x000050ee00010000 RW-]
[0x0000511000000000-0x0000511000010000 RW-]
[0x0000511e00000000-0x0000511e00010000 RW-]
[0x0000513000000000-0x0000513000010000 RW-]
...
```
The tests were using the variable directly to get the dwarf version used
for the test. That's only the overridden value, and won't be set if
we're using the compiler default. I also put a comment by the variable
to make sure people don't make the same mistake in the future.
When `FileAction` opens file with write access, it doesn't clear the
file nor append to the end of the file if it already exists. Instead, it
writes from cursor index 0.
For example, by using the settings `target.output-path` and
`target.error-path`, lldb will redirect process stdout/stderr to files.
It then calls this function to write to the files which the above
symptoms appear.
## Test
- Added unit test checking the file flags
- Added 2 api tests checking
- File content overwritten if the file path already exists
- Stdout and stderr redirection to the same file doesn't change its
behavior
A memory region can be relatively large. Searching for a value in the
entire region is time-consuming, especially when running tests against a
remote target, because the memory data is transferred in small chunks
over a relatively slow GDB Remote Protocol. The patch limits the address
range to be searched to 2K, which seems sufficient for these tests. In
my setup, for local runs, these tests now take half the time they did
before the patch. For a remote target, the improvement is even more
significant.
Line ending policies were changed in the parent, dccebddb3b80. To make
it easier to resolve downstream merge conflicts after line-ending
policies are adjusted this is a separate whitespace-only commit. If you
have merge conflicts as a result, you can simply `git add --renormalize
-u && git merge --continue` or `git add --renormalize -u && git rebase
--continue` - depending on your workflow.
`SBDebugger().Create()` returns a debugger with only the host platform
in its platform list. If the test suite is running for a remote
platform, it should be explicitly added and selected in the new debugger
created within the test, otherwise, the test will fail because the host
platform may not be able to launch the built binary.
The function should use the by-ref SBError argument instead of creating
a new one. This code has been here since ~forever, and was probably
copied from methods which return an SBError result (where one needs to
create a local variable).
This fix is based on a problem with cxx_compiler and cxx_linker macros
on Windows.
There was an issue with compiler detection in paths containing "icc". In
such case, Makefile.rules thought it was provided with icc compiler.
To solve that, utilities detection has been rewritten in Python.
The last element of compiler's path is separated, taking into account
the platform path delimiter, and compiler type is extracted, with regard
of possible cross-toolchain prefix.
---------
Co-authored-by: Pavel Labath <pavel@labath.sk>
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.
