When terminating the debugger, we wait for all background tasks to
complete. Given that there's no way to interrupt those treads, this can
take a while. When that happens, the debugger appears to hang at exit.
The above situation is unfortunately not uncommon when background
downloading of dSYMs is enabled (`symbols.auto-download background`).
Even when calling dsymForUUID with a reasonable timeout, it can take a
while to complete.
This patch improves the user experience by printing a message from the
driver when it takes more than one (1) second to terminate the debugger.
Currently, calls to Host::SystemLog print to stderr on all host
platforms except Darwin. This severely limits its value on the command
line, where we don't want to overload the user with log messages. Switch
to using the syslog function on POSIX systems to send messages to the
system logger instead of stdout.
On Darwin systems this sends the log message to os_log, which matches
what we do today. Nevertheless I kept the current implementation that
uses os_log directly as it gives us more freedom.
I'm not sure if there's an equivalent on Windows, so I kept the existing
behavior of logging to stderr.
According to the git log (d9442afba1bd6), this test has never been
enabled/disabled, it was checked in without being called anywhere. But
it passes and it is useful, so this commit enables it.
This patch should address some register parsing issue in the legacy
report format.
rdar://107210149
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch addresses an oversight in `ProcessEventDataTest::SetUp`
unittest to ensure the Debugger is initialized properly.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
We do not run `pexpect` based tests on Windows, but there are still cases where those tests run `import pexpect` outside of the scope where the test is skipped. By moving the import statement to a different scope, those tests can run even when `pexpect` truly isn't installed.
Tangentially related: TestSTTYBeforeAndAfter.py is using a manual `@expectedFailureAll` for windows instead of the common `@skipIfWindows`. If `pexepect` is generally expected to not be available, we should not bother running the test at all.
Adds a test-case for debugging a program with a
pch chain, that is, the main executable depends
on a pch that itself included another pch.
Currently clang doesn't emit the sekeleton CUs
required for LLDB to track all types on the pch chain. Thus this test is
XFAILed for now.
Currently, for x86 and x86_64 triples, "+sse" and "+sse2" are appended
to `Features` vector of `TargetOptions` unconditionally. This vector is
later reset in `TargetInfo::CreateTargetInfo` and filled using info from
`FeaturesAsWritten` vector, so previous modifications of the `Features`
vector have no effect. For x86_64 triple, we append "sse2"
unconditionally in `X86TargetInfo::initFeatureMap`, so despite the
`Features` vector reset, we still have the desired sse features enabled.
The corresponding code in `X86TargetInfo::initFeatureMap` is marked as
FIXME, so we should not probably rely on it and should set desired
features properly in `ClangExpressionParser`.
This patch changes the vector the features are appended to from
`Features` to `FeaturesAsWritten`. It's not reset later and is used to
compute resulting `Features` vector.
The help output for `thread backtrace` specifies that you can pass -1 to
`--count` to display all the frames.
```
-c <count> ( --count <count> )
How many frames to display (-1 for all)
```
However, that doesn't work:
```
(lldb) thread backtrace --count -1
error: invalid integer value for option 'c'
```
The problem is that we store the option value as an unsigned and the
code to parse the string correctly rejects it. There's two ways to fix
this:
1. Make `m_count` a signed value so that it accepts negative values and
appease the parser. The function that prints the frames takes an
unsigned so a negative value will just become a really large positive
value, which is what the current implementation relies on.
2. Keep `m_count` unsigned and instead use 0 the magic value to show all
frames. I don't really see a point in not showing any frames at all,
plus that's already broken (`error: error displaying backtrace for
thread: "0x0001"`).
This patch implements (2) and at the same time improve the error
reporting so that we print the invalid value when we cannot parse it.
rdar://123881767
This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track
of these reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.
This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.
This patch adds support to sort the symbol table by size. The command
already supports sorting and it already reports sizes. Sorting by size
helps diagnosing size issues.
rdar://123788375
Pavel added an extension to lldb's gdb remote serial protocol that
allows the debug stub to append an error message (ascii hex encoded)
after an error response packet Exx. This was added in 2017 in
https://reviews.llvm.org/D34945 . lldb sends the
QErrorStringInPacketSupported packet and then the remote stub may add
these error strings.
debugserver has two bugs in its use of extended error messages: the
vAttach family would send the extended error string without checking if
the mode had been enabled. And qLaunchSuccess would not properly format
its error response packet (missing the hex digits, did not asciihex
encode the string).
There is also a bug in the HandlePacket_D (detach) packet where the
error packets did not include hex digits, but this one does not append
an error string.
I'm adding a new RNBRemote::SendErrorPacket() and routing all error
packet returns though this one method. It takes an optional second
string which is the longer error message; it now handles appending it to
the Exx response or not, depending on the QErrorStringInPacketSupported
state. I updated all packets to send their errors via this method.
This reverts commit 9a12b0a60084b2b92f728e1bddec884a47458459.
TestAddressMasks fails its first test on lldb-x86_64-debian,
lldb-arm-ubuntu, lldb-aarch64-ubuntu bots. Reverting while
investigating.
I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was adding
to SBProcess for all of the address mask types / memory regions. In this
update, I added enums to control type address mask type (code, data,
any) and address space specifiers (low, high, all) with defaulted
arguments for the most common case.
This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks. If lldb
were told that 64 bits are valid for addressing, this method would
overflow the calculation and set an invalid mask. Added tests to check
this specific bug while I was adding these APIs.
rdar://123530562
Layout information for a record gets stored in the `ClangASTImporter`
associated with the `DWARFASTParserClang` that originally parsed the
record. LLDB sometimes moves clang types from one AST to another (in the
reproducer the origin AST was a precompiled-header and the destination
was the AST backing the executable). When clang then asks LLDB to
`layoutRecordType`, it will do so with the help of the
`ClangASTImporter` the type is associated with. If the type's origin is
actually in a different LLDB module (and thus a different
`DWARFASTParserClang` was used to set its layout info), we won't find
the layout info in our local `ClangASTImporter`.
In the reproducer this meant we would drop the alignment info of the
origin type and misread a variable's contents with `frame var` and
`expr`.
There is logic in `ClangASTSource::layoutRecordType` to import an
origin's layout info. This patch re-uses that infrastructure to import
an origin's layout from one `ClangASTImporter` instance to another.
rdar://123274144
This patch moves the logic for copying the layout info of a
`RecordDecl`s origin into a target AST.
A follow-up patch re-uses the logic from within the `ClangASTImporter`,
so the natural choice was to move it there.
The timeout for this test was set to 1.0s which is very low, it should
be a default of 10s and be increased by a factor of 10 if ASAN is
enabled. This will help reduce the falkiness of the test, especially in
ASAN builds.
This executed Alex' idea [1] of adding pexpect to the list of "strict
test requirements" as we're planning to stop vendoring it. This will
ensure all the bots have the package before we toggle the default.
[1] https://github.com/llvm/llvm-project/pull/83191
If you run cmake without pexpect installed it errors as expected.
However, if you just `pip install pexpect` and cmake again it still
doesn't find it because it cached the result of the search.
Unset the result before looking for pexpect. So that this works
as expected:
cmake ...
pip3 install pexpect
cmake ...
We weren't checking to see if the partial_path was empty before adding
completions and this led to crashes when the class object and a variable
both start with the same substring.
Fixes [#81536](https://github.com/llvm/llvm-project/issues/81536)
---------
Co-authored-by: Michael Buch <michaelbuch12@gmail.com>
This executed Alex' idea [1] of adding pexpect to the list of "strict
test requirements" as we're planning to stop vendoring it. This will
ensure all the bots have the package before we toggle the default.
[1] https://github.com/llvm/llvm-project/pull/83191
There was a think-o in a previous commit that made us only able to
define 1 line commands when using command script add interactively.
There was also no test for this feature, so I fixed the think-o and
added a test.
This specifically addresses the warnings:
$LLVM/lldb/include/lldb/API/SBCommandReturnObject.h:119: Warning 509:
Overloaded method lldb::SBCommandReturnObject::PutCString(char const *)
effectively ignored,
$LLVM/lldb/include/lldb/API/SBCommandReturnObject.h:119: Warning 509: as
it is shadowed by lldb::SBCommandReturnObject::PutCString(char const
*,int).
There is exactly one declaration of SBCommandReturnObject::PutCString.
The second parameter (of type `int`) has default value `-1`. Without
investigating why SWIG believes there are 2 method declarations, I
believe it is safe to ignore this warning. It does not appear to
actually impact functionality in any way.
rdar://117744660
The help for the `-r` option to `image list` says:
-r[<width>] ( --ref-count=[<width>] )
Display the reference count if the module is still in the shared module
cache.
but that's not what it actually does. It unconditionally shows the
use_count for all Module shared pointers, regardless of whether they are
still in the shared module cache or whether they are just in the
ModuleCollection and other entities are keeping them alive. That seems
like a more useful behavior, but then it is also useful to know what's
in the shared cache, so I changed this to:
-r[<width>] ( --ref-count=[<width>] )
Display whether the module is still in the the shared module cache
(Y/N), and its shared pointer use_count.
So instead of just `{5}` you will see `{Y 5}` if it is in the shared
cache and `{N 5}` if not.
I didn't add tests for this because I'm not sure how much we want to fix
shared cache behavior in the testsuite.
The -d(ebug) option broke 5 years ago when I migrated the driver to
libOption. Since then, we were never check if the option is set. We were
incorrectly toggling the internal variable (m_debug_mode) based on
OPT_no_use_colors instead.
Given that the functionality doesn't seem particularly useful and nobody
noticed it has been broken for 5 years, I'm just removing the flag.
The goal here is to remove the third_party/Python/module tree, which
LLDB tests only use to `import pexpect`. This package is available on
`pip`, and I believe should not be hard to obtain.
However, in case it isn't easily available, deleting the tree right now
could cause disruption. This introduces a
`LLDB_TEST_USE_VENDOR_PACKAGES` cmake param that can be enabled, and the
tests will continue loading that tree. By default, it is enabled,
meaning there's really no change here. A followup change will disable it
by default once all known build bots are updated to include this
package. When disabled, an eager cmake check runs that makes sure
`pexpect` is available before waiting for the test to fail in an obscure
way.
Later, this option will go away, and when it does, we can delete the
tree too. Ideally this is not disruptive, and we can remove it in a week
or two.
If a SetDataBreakpointsRequest contains a list data breakpoints which
have duplicate starting addresses, the current behaviour is returning
`{verified: true}` to both watchpoints with duplicated starting
addresses. This confuses the client and what actually happens in lldb is
the second one overwrite the first one.
This fixes it by letting the last watchpoint at given address have
`{verified: true}` and all previous watchpoints at the same address
should have `{verfied: false}` at response.
https://github.com/modularml/mojo/issues/1796 discovered that if you try
to complete a space-only line in the REPL on Linux, LLDB crashes. I
suspect that editline doesn't behave the same way on linux and on
darwin, because I can't replicate this on darwin.
Adding a boundary check in the completion code prevents the crash from
happening.
The `EventThreadFunction` can end up calling `HandleCommand`
concurrently with the main request processing thread. The underlying API
does not appear to be thread safe, so add a narrowly scoped mutex lock
to prevent calling it in this place from more than one thread.
Fixes#81686. Prior to this, TestDAP_launch.py is 4% flaky. After, it
passes in 1000 runs.
Partly, there's just a lot of unnecessary boiler plate. It's also
possible to define combinations of arguments that make no sense (e.g.
eArgRepeatPlus followed by eArgRepeatPlain...) but these are never
checked since we just push_back directly into the argument definitions.
This commit is step 1 of this cleanup - do the obvious stuff. In it, all
the simple homogenous argument lists and the breakpoint/watchpoint
ID/Range types, are set with common functions. This is an NFC change, it
just centralizes boiler plate. There's no checking yet because you can't
get a single argument wrong.
The end goal is that all argument definition goes through functions and
m_arguments is hidden so that you can't define inconsistent argument
sets.
This updates the remaining SetOptionValue methods in
CommandObjectBreakpoint to use CreateOptionParsingError.
I found a few minor bugs that were fixed during this refactor (e.g.
using the wrong flag in an error message). That is one of the benefits
of centralizing error message creation.
I also found some option parsing code that is written incorrectly. I do
not make an attempt to update those here because this PR is primarily
about changing existing error handling code, not adding new error
handling code.
On a case insensitive file sytem, the build dir for `test_multiple_c` and
`test_multiple_C` are the same and therefore the log files are in the same
place. This means one tries to clear the log file for the other.
To fix this, make the names unique by adding the meaning of each
protocol packet.
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html#Packets
Looking ast the definition of both functions this is *almost* an NFC
change, except that Triple also looks at the SubArch (important) and
ObjectFormat (less so).
This fixes a bug that only manifests with how Xcode uses the SBAPI to
attach to a process by name: it guesses the architecture based on the
system. If the system is arm64 and the Process is arm64e Target fails
to update the triple because it deemed the two to be equivalent.
rdar://123338218
The `unittest2` package is unused since
5b386158aacac4b41126983a5379d36ed413d0ea.
The `progress` package was only used internally by `unittest2`, so it
can be deleted as well.
