ReadProcessMemory will not perform the read if part of the memory is
unreadable (and even though the API has a `number_of_bytes_read`
argument). To make this work, I explicitly inspect the memory region
being read and only read the accessible part.
Previously, we were returning an error if we couldn't read the whole
region. This doesn't matter most of the time, because lldb caches memory
reads, and in that process it aligns them to cache line boundaries. As
(LLDB) cache lines are smaller than pages, the reads are unlikely to
cross page boundaries.
Nonetheless, this can cause a problem for large reads (which bypass the
cache), where we're unable to read anything even if just a single byte
of the memory is unreadable. This patch fixes the lldb-server to do
that, and also changes the linux implementation, to reuse any partial
results it got from the process_vm_readv call (to avoid having to
re-read everything again using ptrace, only to find that it stopped at
the same place).
This matches debugserver behavior. It is also consistent with the gdb
remote protocol documentation, but -- notably -- not with actual
gdbserver behavior (which returns errors instead of partial results). We
filed a
[clarification
bug](https://sourceware.org/bugzilla/show_bug.cgi?id=24751) several
years ago. Though we did not really reach a conclusion there, I think
this is the most logical behavior.
The associated test does not currently pass on windows, because the
windows memory read APIs don't support partial reads (I have a WIP patch
to work around that).
This patch extends TypeQuery matching to support anonymous namespaces. A
new flag is added to control the behavior. In the "strict" mode, the
query must match the type exactly -- all anonymous namespaces included.
The dynamic type resolver in the itanium abi (the motivating use case
for this) uses this flag, as it queries using the name from the
demangles, which includes anonymous namespaces.
This ensures we don't confuse a type with a same-named type in an
anonymous namespace. However, this does *not* ensure we don't confuse
two types in anonymous namespacs (in different CUs). To resolve this, we
would need to use a completely different lookup algorithm, which
probably also requires a DWARF extension.
In the "lax" mode (the default), the anonymous namespaces in the query
are optional, and this allows one search for the type using the usual
language rules (`::A` matches `::(anonymous namespace)::A`).
This patch also changes the type context computation algorithm in
DWARFDIE, so that it includes anonymous namespace information. This
causes a slight change in behavior: the algorithm previously stopped
computing the context after encountering an anonymous namespace, which
caused the outer namespaces to be ignored. This meant that a type like
`NS::(anonymous namespace)::A` would be (incorrectly) recognized as
`::A`). This can cause code depending on the old behavior to misbehave.
The fix is to specify all the enclosing namespaces in the query, or use
a non-exact match.
This patch addresses a bug where `cs`/`fs` and other segmentation flags
were being identified as having a type of `32b` and `64b` for `rflags`.
In that case the register value was returning the fail value `0xF...`
and this was corrupting some minidumps. Here we just read it as a 64b
value and truncate it.
In addition to that fix, I added comparing the registers from the live
process to the loaded core for the generic minidump test. Prior only
being ARM register tests. This explains why this was not detected
before.
At least for our Windows on Arm machine compiling with clang-cl,
it has inverted which variables get a `::` prefix.
Would not surprise me if msvc does the opposite so feel free to
revert if these tests fail for you.
This patch removes all of the Set.* methods from Status.
This cleanup is part of a series of patches that make it harder use the
anti-pattern of keeping a long-lives Status object around and updating
it while dropping any errors it contains on the floor.
This patch is largely NFC, the more interesting next steps this enables
is to:
1. remove Status.Clear()
2. assert that Status::operator=() never overwrites an error
3. remove Status::operator=()
Note that step (2) will bring 90% of the benefits for users, and step
(3) will dramatically clean up the error handling code in various
places. In the end my goal is to convert all APIs that are of the form
` ResultTy DoFoo(Status& error)
`
to
` llvm::Expected<ResultTy> DoFoo()
`
How to read this patch?
The interesting changes are in Status.h and Status.cpp, all other
changes are mostly
` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git
grep -l SetErrorString lldb/source)
`
plus the occasional manual cleanup.
With this commit, we also hide the implementation details of
`std::invoke`. To do so, the `LibCXXFrameRecognizer` got a couple more
regular expressions.
The regular expression passed into `AddRecognizer` became problematic,
as it was evaluated on the demangled name. Those names also included
result types for C++ symbols. For `std::__invoke` the return type is a
huge `decltype(...)`, making the regular expresison really hard to
write.
Instead, I added support to `AddRecognizer` for matching on the
demangled names without result type and argument types.
By hiding the implementation details of `invoke`, also the back traces
for `std::function` become even nicer, because `std::function` is using
`__invoke` internally.
Co-authored-by: Adrian Prantl <aprantl@apple.com>
This patch adds the option to specify specific memory ranges to be
included in a given core file. The current implementation lets user
specified ranges either be in addition to a certain save style, or
independent of them via the newly added custom enum.
To achieve being inclusive of save style, I've moved from a std::vector
of ranges to a RangeDataVector, and to join overlapping ranges to
prevent duplication of memory ranges in the core file.
As a non function bonus, when SBSavecore was initially created, the
header was included in the lldb-private interfaces, and I've fixed that
and moved it the forward declare as an oversight. CC @bulbazord in case
we need to include that into swift.
TargetProperties.td had a few settings listed as signed integral values,
but the Target.cpp methods reading those values were reading them as
unsigned. e.g. target.max-memory-read-size, some accesses of
target.max-children-count, still today, previously
target.max-string-summary-length.
After Jonas' change to use templates to read these values in
https://reviews.llvm.org/D149774, when the code tried to fetch these
values, we'd eventually end up calling OptionValue::GetAsUInt64 which
checks that the value is actually a UInt64 before returning it; finding
that it was an SInt64, it would drop the user setting and return the
default value. This manifested as a bug that target.max-memory-read-size
is never used for memory read.
target.max-children-count is less straightforward, where one read of
that setting was fetching it as an int64_t, the other as a uint64_t.
I suspect all of these settings were originally marked as SInt64 so a
user could do -1 for "infinite", getting it static_cast to a UINT64_MAX
value along the way. I can't find any documentation for this behavior,
but it seems like something Greg would have done. We've partially lost
that behavior already via
https://github.com/llvm/llvm-project/pull/72233 for
target.max-string-summary-length, and this further removes it.
We're still fetching UInt64's and returning them as uint32_t's but I'm
not overly pressed about someone setting a count/size limit over 4GB.
I added a simple API test for the memory read setting limit.
Previously, when output like `"hello\nworld\n"` was produced by lldb (or
the process) the message would be sent as a single Output event. By
being a single event this causes VS Code to treat this as a single
message in the console when handling displaying and filtering in the
Debug Console.
Instead, with these changes we send each line as its own event. This
results in VS Code representing each line of output from lldb-dap as an
individual output message.
Resolves#105444
VS Code requests the `instruction` stepping granularity if the assembly
view is currently focused. By implementing `StepGranularity`, we can
hence properly single-step through assembly code.
This PR is in response to a bug my coworker @mbucko discovered where on
MacOS Minidumps were being created where the 64b memory regions were
readable, but were not being listed in
`SBProcess.GetMemoryRegionList()`. This went unnoticed in #95312 due to
all the linux testing including /proc/pid maps. On MacOS generated dumps
(or any dump without access to /proc/pid) we would fail to properly map
Memory Regions due to there being two independent methods for 32b and
64b mapping.
In this PR I addressed this minor bug and merged the methods, but in
order to add test coverage required additions to `obj2yaml` and
`yaml2obj` which make up the bulk of this patch.
Lastly, there are some non-required changes such as the addition of the
`Memory64ListHeader` type, to make writing/reading the header section of
the Memory64List easier.
This commit takes advantage of the recently introduced
`SBFrame::IsHidden` to show those hidden frames as "subtle" frames in
the UI. E.g., VS Code hides those stack frames by default, and renders
them as grayed out frames, in case the user decides to show them in the
stack trace
Compilers and language runtimes often use helper functions that are
fundamentally uninteresting when debugging anything but the
compiler/runtime itself. This patch introduces a user-extensible
mechanism that allows for these frames to be hidden from backtraces and
automatically skipped over when navigating the stack with `up` and
`down`.
This does not affect the numbering of frames, so `f <N>` will still
provide access to the hidden frames. The `bt` output will also print a
hint that frames have been hidden.
My primary motivation for this feature is to hide thunks in the Swift
programming language, but I'm including an example recognizer for
`std::function::operator()` that I wished for myself many times while
debugging LLDB.
rdar://126629381
Example output. (Yes, my proof-of-concept recognizer could hide even
more frames if we had a method that returned the function name without
the return type or I used something that isn't based off regex, but it's
really only meant as an example).
before:
```
(lldb) thread backtrace --filtered=false
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x0000000100001f04 a.out`foo(x=1, y=1) at main.cpp:4:10
frame #1: 0x0000000100003a00 a.out`decltype(std::declval<int (*&)(int, int)>()(std::declval<int>(), std::declval<int>())) std::__1::__invoke[abi:se200000]<int (*&)(int, int), int, int>(__f=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:149:25
frame #2: 0x000000010000399c a.out`int std::__1::__invoke_void_return_wrapper<int, false>::__call[abi:se200000]<int (*&)(int, int), int, int>(__args=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:216:12
frame #3: 0x0000000100003968 a.out`std::__1::__function::__alloc_func<int (*)(int, int), std::__1::allocator<int (*)(int, int)>, int (int, int)>::operator()[abi:se200000](this=0x000000016fdff280, __arg=0x000000016fdff224, __arg=0x000000016fdff220) at function.h:171:12
frame #4: 0x00000001000026bc a.out`std::__1::__function::__func<int (*)(int, int), std::__1::allocator<int (*)(int, int)>, int (int, int)>::operator()(this=0x000000016fdff278, __arg=0x000000016fdff224, __arg=0x000000016fdff220) at function.h:313:10
frame #5: 0x0000000100003c38 a.out`std::__1::__function::__value_func<int (int, int)>::operator()[abi:se200000](this=0x000000016fdff278, __args=0x000000016fdff224, __args=0x000000016fdff220) const at function.h:430:12
frame #6: 0x0000000100002038 a.out`std::__1::function<int (int, int)>::operator()(this= Function = foo(int, int) , __arg=1, __arg=1) const at function.h:989:10
frame #7: 0x0000000100001f64 a.out`main(argc=1, argv=0x000000016fdff4f8) at main.cpp:9:10
frame #8: 0x0000000183cdf154 dyld`start + 2476
(lldb)
```
after
```
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x0000000100001f04 a.out`foo(x=1, y=1) at main.cpp:4:10
frame #1: 0x0000000100003a00 a.out`decltype(std::declval<int (*&)(int, int)>()(std::declval<int>(), std::declval<int>())) std::__1::__invoke[abi:se200000]<int (*&)(int, int), int, int>(__f=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:149:25
frame #2: 0x000000010000399c a.out`int std::__1::__invoke_void_return_wrapper<int, false>::__call[abi:se200000]<int (*&)(int, int), int, int>(__args=0x000000016fdff280, __args=0x000000016fdff224, __args=0x000000016fdff220) at invoke.h:216:12
frame #6: 0x0000000100002038 a.out`std::__1::function<int (int, int)>::operator()(this= Function = foo(int, int) , __arg=1, __arg=1) const at function.h:989:10
frame #7: 0x0000000100001f64 a.out`main(argc=1, argv=0x000000016fdff4f8) at main.cpp:9:10
frame #8: 0x0000000183cdf154 dyld`start + 2476
Note: Some frames were hidden by frame recognizers
```
This test causes the assert in clang CodeGen and python crashes with the
error code 0x80000003. See #105019 for more details. Note the similar
test lldb/test/API/lang/c/bitfields/TestBitfields.py is already disabled
on Windows.
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.
This fixes https://github.com/llvm/llvm-project/issues/56125 and
https://github.com/vadimcn/codelldb/issues/666, as well as the
downstream issue in our binary ninja debugger:
https://github.com/Vector35/debugger/issues/535
Basically, lldb does not claim to support the `swbreak` packet so the
gdbserver would not use it. As a result, the gdbserver always sends the
unmodified program counter value which, on systems like x86, causes the
program counter to be off-by-one (or otherwise wrong). For reference,
the lldb-server always sends the modified program counter value so it
works perfectly with lldb.
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Stop-Reply-Packets.html#swbreak-stop-reason
No new code is added to add support `swbreak`, since the way lldb works
already expects the remote to have adjusted the program counter. The
change just lets the gdbserver know that lldb supports it, so that it
will send the adjusted program counter.
To test this PR, you can use lldb to connect to a gdbserver running on
e.g., Ubuntu 22.04, and see the program counter is off-by-one without
the patch. With the patch, things work as expected
This patch is a follow-up to #97263 that fix ambigous abbreviated
command resolution.
When multiple commands are resolved, instead of failing to pick a
command to
run, this patch changes to resolution logic to check if there is a
single
alias match and if so, it will run the alias instead of the other
matches.
This has as a side-effect that we don't need to make aliases for every
substring of aliases to support abbrivated alias resolution.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also valid JSON type (integer, float, string,
bool, array, null).
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
@walter-erquinigo found the the [PR with testing and a fix for
DebugInfoD](https://github.com/llvm/llvm-project/pull/98344) caused an
issue when working with stripped binaries.
The issue is that when you're working with split-dwarf, there are *3*
possible files: The stripped binary the user is debugging, the
"only-keep-debug" *or* unstripped binary, plus the `.dwp` file. The
debuginfod plugin should provide the unstripped/OKD binary. However, if
the debuginfod plugin fails, the default symbol locator plugin will just
return the stripped binary, which doesn't help. So, to address that, the
SymbolVendorELF code checks to see if the SymbolLocator's
ExecutableObjectFile request returned the same file, and bails if that's
the case. You can see the specific diff as the second commit in the PR.
I'm investigating adding a test: I can't quite get a simple repro, and
I'm unwilling to make any additional changes to Makefile.rules to this
diff, for Pavlovian reasons.
This PR introduces a new `ThreadPlanSingleThreadTimeout` that will be
used to address potential deadlock during single-thread stepping.
While debugging a target with a non-trivial number of threads (around
5000 threads in one example target), we noticed that a simple step over
can take as long as 10 seconds. Enabling single-thread stepping mode
significantly reduces the stepping time to around 3 seconds. However,
this can introduce deadlock if we try to step over a method that depends
on other threads to release a lock.
To address this issue, we introduce a new
`ThreadPlanSingleThreadTimeout` that can be controlled by the
`target.process.thread.single-thread-plan-timeout` setting during
single-thread stepping mode. The concept involves counting the elapsed
time since the last internal stop to detect overall stepping progress.
Once a timeout occurs, we assume the target is not making progress due
to a potential deadlock, as mentioned above. We then send a new async
interrupt, resume all threads, and `ThreadPlanSingleThreadTimeout`
completes its task.
To support this design, the major changes made in this PR are:
1. `ThreadPlanSingleThreadTimeout` is popped during every internal stop
and reset (re-pushed) to the top of the stack (as a leaf node) during
resume. This is achieved by always returning `true` from
`ThreadPlanSingleThreadTimeout::DoPlanExplainsStop()` and
`ThreadPlanSingleThreadTimeout::MischiefManaged()`.
2. A new thread-specific async interrupt stop is introduced, which can
be detected/consumed by `ThreadPlanSingleThreadTimeout`.
3. The clearing of branch breakpoints in the range thread plan has been
moved from `DoPlanExplainsStop()` to `ShouldStop()`, as it is not
guaranteed that it will be called.
The detailed design is discussed in the RFC below:
[https://discourse.llvm.org/t/improve-single-thread-stepping/74599](https://discourse.llvm.org/t/improve-single-thread-stepping/74599)
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
This makes use of the changes introduced in D134604, in order to
instantiate alias templates witn a final sugared substitution.
This comes at no additional relevant cost.
Since we don't track / unique them in specializations, we wouldn't be
able to resugar them later anyway.
Differential Revision: https://reviews.llvm.org/D136565
In #100443, Mach-o and Minidump now only call process API's that take a
`SaveCoreOption` as the container for the style and information if a
thread should be included in the core or not. This introduced a bug
where in subsequent method calls we were not honoring the defaults of
both implementations.
~~To solve this I have made a copy of each SaveCoreOptions that is
mutable by the respective plugin. Originally I wanted to leave the
SaveCoreOptions as non const so these default value mutations could be
shown back to the user. Changing that behavior is outside of the scope
of this bugfix, but is context for why we are making a copy.~~
Removed const on the savecoreoptions so defaults can be inspected by the
user
CC: @Michael137
In #98403 I enabled the SBSaveCoreOptions object, which allows users via
the scripting API to define what they want saved into their core file.
As the first option I've added a threadlist, so users can scan and
identify which threads and corresponding stacks they want to save.
In order to support this, I had to add a new method to `Process.h` on
how we identify which threads are to be saved, and I had to change the
book keeping in minidump to ensure we don't double save the stacks.
Important to @jasonmolenda I also changed the MachO coredump to accept
these new APIs.
This is used by various system routines (the capabilities checker and
dyld to name a few) to add extra color to an abort. This patch adds a
frame recognizer so people can easily see the details, and also adds the
information to the ExtendedCrashInformation dictionary.
I also had to rework how the dictionary is held; previously it was
created on demand, but that was inconvenient since it meant all the
entries had to be produced at that same time. That didn't work for the
recognizer.
This PR splits the test assertion that verifies we're on the correct
line and have the correct value of `val` to make the error message more
clear. At present it just shows `Assertion error: True != False`
Co-authored-by: kendal <kendal@thebrowser.company>
