Summary:
This test base class is missing the teardown making the second set of tests extending it to fail in an assertion in the FileSystem::Initialize() (as it's being initialized twice).
Not sure why this isn't failing the build bots.. (unless they're running without asserts?).
With this fix `ninja LLDBServerTests && ./tools/lldb/unittests/tools/lldb-server/tests/LLDBServerTests` successfully runs and passes all tests.
Reviewers: clayborg, xiaobai, labath
Reviewed By: xiaobai, labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D62788
llvm-svn: 362406
Summary:
NFC = [[ https://llvm.org/docs/Lexicon.html#nfc | Non functional change ]]
This commit is the result of modernizing the LLDB codebase by using
`nullptr` instread of `0` or `NULL`. See
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-nullptr.html
for more information.
This is the command I ran and I to fix and format the code base:
```
run-clang-tidy.py \
-header-filter='.*' \
-checks='-*,modernize-use-nullptr' \
-fix ~/dev/llvm-project/lldb/.* \
-format \
-style LLVM \
-p ~/llvm-builds/debug-ninja-gcc
```
NOTE: There were also changes to `llvm/utils/unittest` but I did not
include them because I felt that maybe this library shall be updated in
isolation somehow.
NOTE: I know this is a rather large commit but it is a nobrainer in most
parts.
Reviewers: martong, espindola, shafik, #lldb, JDevlieghere
Reviewed By: JDevlieghere
Subscribers: arsenm, jvesely, nhaehnle, hiraditya, JDevlieghere, teemperor, rnkovacs, emaste, kubamracek, nemanjai, ki.stfu, javed.absar, arichardson, kbarton, jrtc27, MaskRay, atanasyan, dexonsmith, arphaman, jfb, jsji, jdoerfert, lldb-commits, llvm-commits
Tags: #lldb, #llvm
Differential Revision: https://reviews.llvm.org/D61847
llvm-svn: 361484
This reverts commit b5a8abd57f23e2f621d5ceb0f64f1bb8f9579c3f. This
should not be needed as the lldb-server tool will add
`LLDB_CAN_USE_LLDB_SERVER` which will never be set to true on Windows.
llvm-svn: 360745
We can piggyback off the existing add_lldb_tool_subdirectory to decide
whether or not lldb-server should be built.
Differential revision: https://reviews.llvm.org/D61872
llvm-svn: 360621
Fix the tests not to use '127.0.0.1' and 'localhost' interchangeably.
More specifically, since tests bind specifically to 127.0.0.1, connect
to that address as well; using 'localhost' can resolve to IPv6 address
which can cause issues -- for example, if the matching port happens to
be used by some other process, the tests hang forever waiting for
the client to connect.
While technically the case of randomly selected IPv4 port being taken
on IPv6 loopback is not very likely, NetBSD happens to be suffering from
some weird kernel issue where connection to that port succeeds
nevertheless. Until we can really figure out what goes wrong there,
this saves us from the tests hanging randomly.
Differential Revision: https://reviews.llvm.org/D58131
llvm-svn: 353868
Summary:
These classes describe the details of the process we are about to
launch, and so they are naturally used by the launching code in the Host
module. Previously they were present in Target because that is the most
important (but by far not the only) user of the launching code.
Since the launching code has other customers, must of which do not care
about Targets, it makes sense to move these classes to the Host layer,
next to the launching code.
This move reduces the number of times that Target is included from host
to 8 (it used to be 14).
Reviewers: zturner, clayborg, jingham, davide, teemperor
Subscribers: emaste, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D56602
llvm-svn: 353047
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
Summary:
Major fixes after D54476 (use Diff1 as base for comparison to see only recent changes):
* In standalone builds target directory for debugserver must be LLDB's bin, not LLVM's bin
* Default identity for code signing must not force-override LLVM_CODESIGNING_IDENTITY globally
We have a lot of cases, make them explicit:
* ID used for code signing (debugserver and in tests):
** `LLDB_CODESIGN_IDENTITY` if set explicitly, or otherwise
** `LLVM_CODESIGNING_IDENTITY` if set explicitly, or otherwise
** `lldb_codesign` as the default
* On Darwin we have a debugserver target that:
* On other systems, the debugserver target is not defined, which is equivalent to **[3A]**
Common configurations on Darwin:
* **[1A]** `cmake -GNinja ../llvm` builds debugserver from source and signs with `lldb_codesign`, no code signing for other binaries (prints status: //lldb debugserver: /path/to/bin/debugserver//)
* **[1A]** `cmake -GNinja -DLLVM_CODESIGNING_IDENTITY=- -DLLDB_CODESIGN_IDENTITY=lldb_codesign ../llvm` builds debugserver from source and signs with `lldb_codesign`, ad-hoc code signing for other binaries (prints status: //lldb debugserver: /path/to/bin/debugserver//)
* **[2A]** `cmake -GNinja -DLLVM_CODESIGNING_IDENTITY=- -DLLDB_USE_SYSTEM_DEBUGSERVER=ON ../llvm` copies debugserver from system, ad-hoc code signing for other binaries (prints status: //Copy system debugserver from: /path/to/system/debugserver//)
* **[2B]** `cmake -GNinja -DLLVM_CODESIGNING_IDENTITY=- ../llvm` same, but prints additional warning: //Cannot code sign debugserver with identity '-'. Will fall back to system's debugserver. Pass -DLLDB_CODESIGN_IDENTITY=lldb_codesign to override the LLVM value for debugserver.//
* **[3A]** `cmake -GNinja -DLLVM_CODESIGNING_IDENTITY=- -DLLDB_NO_DEBUGSERVER=ON ../llvm` debugserver not available (prints status: //lldb debugserver will not be available)//
Reviewers: JDevlieghere, beanz, davide, vsk, aprantl, labath
Reviewed By: JDevlieghere, labath
Subscribers: mgorny, #lldb, lldb-commits
Differential Revision: https://reviews.llvm.org/D55013
llvm-svn: 350388
Summary:
Use llvm_codesign to sign debugserver with entitlements.
Set global LLVM_CODESIGNING_IDENTITY from LLDB_CODESIGN_IDENTITY (if given).
Pass through ENTITLEMENTS from add_lldb_executable to add_llvm_executable.
Handle reconfigurations correctly.
We have a lot of cases, make them explicit:
(1) build and sign debugserver, if all conditions apply:
* LLDB_NO_DEBUGSERVER=OFF (default)
* On Darwin: LLDB_USE_SYSTEM_DEBUGSERVER=OFF (default)
* On Darwin: LLVM_CODESIGNING_IDENTITY == lldb_codesign
(2) use system debugserver, if on Darwin and any of:
* LLDB_USE_SYSTEM_DEBUGSERVER=ON and found on system (explicit case)
* LLVM_CODESIGNING_IDENTITY != lldb_codesign and found on system (fallback case)
(3) debugserver will not be available, in case of:
* LLDB_NO_DEBUGSERVER=ON
* On Darwin: LLVM_CODESIGNING_IDENTITY != lldb_codesign and not found on system
(4) error state, in case of:
* LLDB_USE_SYSTEM_DEBUGSERVER=ON and not found on system
* LLDB_USE_SYSTEM_DEBUGSERVER=ON and LLDB_NO_DEBUGSERVER=ON
Reviewers: xiaobai, beanz, vsk, JDevlieghere
Subscribers: mgorny, lldb-commits, llvm-commits
Differential Revision: https://reviews.llvm.org/D54476
llvm-svn: 347305
Summary:
There is currently a way to skip the debugserver build. See how the CMake
variables SKIP_DEBUGSERVER and LLDB_CODESIGN_IDENTITY are used if you're
interested in that.
This allows us to skip building lldb-server as well. There is another
debug server called ds2 that can be used with LLDB. If you choose to use
ds2, this flag is very useful because it can cut down the build time of LLDB.
Differential Revision: https://reviews.llvm.org/D49282
llvm-svn: 340560
These three classes have no external dependencies, but they are used
from various low-level APIs. Moving them down to Utility improves
overall code layering (although it still does not break any particular
dependency completely).
The XCode project will need to be updated after this change.
Differential Revision: https://reviews.llvm.org/D49740
llvm-svn: 339127
The first fix wasn't enough, there is still a missing
ProcessInstanceInfo include in Host.mm. I won't be able to test a fix
before leaving work, so I am reverting both commits.
This reverts commit r332250 and the subsequent fix attempt.
llvm-svn: 332261
The Process class was only being referenced because of the last-ditch
effort in the process launchers to set a process death callback in case
one isn't set already.
Although launching a process for debugging is the most important kind of
"launch" we are doing, it is by far not the only one, so assuming this
particular callback is the one to be used is not a good idea (besides
breaking layering). Instead of assuming a particular exit callback, I
change the launcher code to require the callback to be set by the user (and fix
up the two call sites which did not set the callback already).
Reviewers: jingham, davide
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D46395
llvm-svn: 332250
The default packet timeout of 1 second is a bit too small for these
tests, particularly as they are working in ack-mode, which means they
need to fit twice as many packets into the timeslot.
This does not seem to be a problem on the bots, but for some people
these tests are timing out regularly. I can't be sure increasing this
will solve their problem, but this does seem like a likely culprit.
llvm-svn: 330578
Summary:
The Args class is used in plenty of places besides the command
interpreter (e.g., anything requiring an argc+argv combo, such as when
launching a process), so it needs to be in a lower layer. Now that the
class has no external dependencies, it can be moved down to the Utility
module.
This removes the last (direct) dependency from the Host module to
Interpreter, so I remove the Interpreter module from Host's dependency
list.
Reviewers: zturner, jingham, davide
Subscribers: mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D45480
llvm-svn: 330200
There are plenty of ways attaching can go wrong. Having the server
report the exact error means we can give better feedback to the user.
(This patch does not do the second part, it only makes sure the
information is sent from the server.)
Triggering all possible error conditions in a test would prove
challenging, but there is one error that is very easy to reproduce
(attempting to attach while debugging), so I write a test based on that.
The test immediately exposed a bug where the m_send_error_strings field
was being used uninitialized (so it was sometimes true from the get-go),
so I fix that as well.
llvm-svn: 329803
Summary:
We've had a mismatch in the checksum computation between the sender and
receiver. The sender computed the payload checksum using the wire
encoding of the packet, while the receiver did this after expanding
un-escaping and expanding run-length-encoded sequences. This resulted in
communication breakdown if packets using these feature were sent in the
ack mode.
Normally, this did not cause any issues since the only packet we send in
the ack-mode is the QStartNoAckMode packet, but I ran into this when
debugging the lldb-server tests which (for better or worse) don't use
this mode.
According to the gdb-remote documentation "The two-digit checksum is computed as
the modulo 256 sum of all characters between the leading ‘$’ and the
trailing ‘#’", it seems that our sender is doing the right thing here.
Therefore, I fix the receiver the match the sender behavior and add a
test.
With this bug fixed, we can see that lldb-server is sending a stop-reply
after receiving the "k" in the same way as debugserver does (but we
weren't detecting this because at that point the connection was dead
already). I fix that expectation as well.
Reviewers: clayborg, jasonmolenda
Subscribers: mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D44922
llvm-svn: 328693
The lldb-server unit tests don't test the right thing when the debug
server in use is copied from somewhere else. This can lead to spurious
test failures.
Disable these unit tests when an external debug server is in use.
Fixes llvm.org/PR36494.
llvm-svn: 326001
Summary:
The issue was that we were parsing the registers into 64-bit integers
and the calling swapByteOrder without regard for the actual size of the
register. This switches the test to use the RegisterValue class which
tracks the register size, and knows how to initialize itself from a
piece of memory (so we don't need to swap byte order ourselves).
Reviewers: eugene, davide
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D43376
llvm-svn: 325511
Summary:
Right now the test client is not parsing register values correctly,
which is manifesting itself in one test failing on 32-bit architectures
(pr36013). This parses the information from the qRegisterInfo packets
and stores it in the client, which will enable fixing the parsing in a
follow up commit.
I am also adding a new templated SendMessage overload, which enables one
to send a message get a parsed response in a single call.
Reviewers: eugene, davide
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D43076
llvm-svn: 324722
Summary:
There was some confusion in the code about how to represent process
environment. Most of the code (ab)used the Args class for this purpose,
but some of it used a more basic StringList class instead. In either
case, the fact that the underlying abstraction did not provide primitive
operations for the typical environment operations meant that even a
simple operation like checking for an environment variable value was
several lines of code.
This patch adds a separate Environment class, which is essentialy a
llvm::StringMap<std::string> in disguise. To standard StringMap
functionality, it adds a couple of new functions, which are specific to
the environment use case:
- (most important) envp conversion for passing into execve() and likes.
Instead of trying to maintain a constantly up-to-date envp view, it
provides a function which creates a envp view on demand, with the
expectation that this will be called as the very last thing before
handing the value to the system function.
- insert(StringRef KeyEqValue) - splits KeyEqValue into (key, value)
pair and inserts it into the environment map.
- compose(value_type KeyValue) - takes a map entry and converts in back
into "KEY=VALUE" representation.
With this interface most of the environment-manipulating code becomes
one-liners. The only tricky part was maintaining compatibility in
SBLaunchInfo, which expects that the environment entries are accessible
by index and that the returned const char* is backed by the launch info
object (random access into maps is hard and the map stores the entry in
a deconstructed form, so we cannot just return a .c_str() value). To
solve this, I have the SBLaunchInfo convert the environment into the
"envp" form, and use it to answer the environment queries. Extra code is
added to make sure the envp version is always in sync.
(This also improves the layering situation as Args was in the Interpreter module
whereas Environment is in Utility.)
Reviewers: zturner, davide, jingham, clayborg
Subscribers: emaste, lldb-commits, mgorny
Differential Revision: https://reviews.llvm.org/D41359
llvm-svn: 322174
Summary:
Make sure we propagate environment when starting debugserver with a pre-loaded
inferior. AFAIK, RNBRunLoopLaunchInferior is only called in pre-loaded inferior
scenario, so we can just pick up the debugserver environment instead of trying
to construct an envp from the (empty) context.
This makes debugserver pass an test added for an equivalent lldb-server fix.
Reviewers: jasonmolenda, clayborg
Subscribers: JDevlieghere, lldb-commits
Differential Revision: https://reviews.llvm.org/D41352
llvm-svn: 321355
Summary:
We were failing to propagate the environment when lldb-server was
started with a pre-loaded process
(e.g.: lldb-server gdbserver -- inferior --inferior_args)
This patch makes sure the environment is propagated. Instead of adding a
new GDBRemoteCommunicationServerLLGS::SetLaunchEnvironment function to
complement SetLaunchArgs and SetLaunchFlags, I replace these with a
more generic SetLaunchInfo, which can be used to set any launch-related
property.
The accompanying test also verifies that the server correctly terminates
the connection after sending the exit packet (specifically, that it does
not send the exit packet twice).
Reviewers: clayborg, eugene
Subscribers: lldb-commits, mgorny
Differential Revision: https://reviews.llvm.org/D41070
llvm-svn: 320984
Summary:
lldb-server was sending the "exit" packet (W??) twice. This happened
because it was handling both the pre-exit (PTRACE_EVENT_EXIT) and
post-exit (WIFEXITED) as exit events. We had some code which was trying
to detect when we've already sent the exit packet, but this stopped
working quite a while ago.
This never really caused any problems in practice because the client
automatically closes the connection after receiving the first packet, so
the only effect of this was some warning messages about extra packets
from the lldb-server test suite, which were ignored because they didn't
fail the test.
The new test suite will be stricter about this, so I fix this issue
ignoring the first event. I think this is the correct behavior, as the
inferior is not really dead at that point, so it's premature to send the
exit packet.
There isn't an actual test yet which would verify the exit behavior, but
in my next patch I will add a test which will also test this
functionality.
Reviewers: eugene
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D41069
llvm-svn: 320961
Summary:
This makes StopReply class abstract, so that we can represent different
types of stop replies such as StopReplyStop and StopReplyExit (there
should also be a StopReplySignal, but I don't need that right now so I
haven't implemented it yet).
This prepares the ground for a new test I'm writing.
Reviewers: eugene, zturner
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D41067
llvm-svn: 320820
Summary:
Adding a new test would require one to duplicate a significant part of
the existing test that we have. This attempts to reduce that by moving
some part of that code to the test fixture. The StandardStartupTest
fixture automatically starts up the server and connects it to the
client. I also add a more low-level TestBase fixture, which allows one
to start up the client and server in a custom way (I am going to need
this for the test I am writing).
Reviewers: eugene, zturner
Subscribers: lldb-commits, mgorny
Differential Revision: https://reviews.llvm.org/D41066
llvm-svn: 320809
The rationale here is that ArchSpec is used throughout the codebase,
including in places which should not depend on the rest of the code in
the Core module.
This commit touches many files, but most of it is just renaming of
#include lines. In a couple of cases, I removed the #include ArchSpec
line altogether, as the file was not using it. In one or two places,
this necessitated adding other #includes like lldb-private-defines.h.
llvm-svn: 318048
Summary:
These tests used to log the error message and return plain bool mainly
because at the time they we written, we did not have a nice way to
assert on llvm::Error values. That is no longer true, so replace this
pattern with a more idiomatic approach.
As a part of this patch, I also move the formatting of
GDBRemoteCommunication::PacketResult values out of the test code, as
that can be useful elsewhere.
Reviewers: zturner, eugene
Subscribers: mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D39790
llvm-svn: 317795
Summary:
Without this, the launching of the test inferior may fail if it depends
on some component of the environment (most likely LD_LIBRARY_PATH). This
makes sure we propagate the environment variable to the inferior
process.
Reviewers: eugene
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D39010
llvm-svn: 316244
Summary:
This adds support for running the lldb-server test suite (currently consisting
of only one test) against the debugserver. Currently, the choice which binary
to test is based on the host system. This will need to be replaced by something
more elaborate if/when lldb-server starts supporting debugging on darwin.
I need to make a couple of tweaks to the test client to work with debugserver:
- debugserver has different command-line arguments - launching code adjusted to
handle that
- debugserver sends duplicate "medata" fields in the stop reply packet -
adjusted stop-reply parsing code to handle that
- debugserver replies to the k packet instead of just dropping the connection -
stopping code adjusted, although we should probably consider aligning the
behavior of the two stubs in this case
Reviewers: jmajors, beanz
Subscribers: srhines, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D35311
llvm-svn: 316010
Summary:
It had a dependency on StringConvert and file reading code, which is not
in Utility. I've replaced that code by equivalent llvm operations.
I've added a unit test to demonstrate that parsing a file still works.
Reviewers: zturner, jingham
Subscribers: kubamracek, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D34625
llvm-svn: 306394
Summary:
ProcessLauncherPosix was using posix_spawn for launching the process,
but this function is not available on all platforms we support, and even
where it was avaialable, it did not support the full range of options we
require for launching (most importantly, launching in stop-on-entry
mode). For these reasons, the set of ifdefs around these functions has
grown untractably large, and we were forced to implement our own
launcher from more basic primitives anyway (ProcessLauncherPosixFork --
used on Linux, Android, and NetBSD).
Therefore, I remove this class, and move the relevant parts of the code
to the darwin-specific Host.mm file. This is the platform that code was
originally written for anyway, and it's the only platform where this
implementation makes sense (e.g. the lack of the "thread-specific
working directory" concept makes these functions racy on all other
platforms). This allows us to remove a lot of ifdefs and simplify the
code.
Effectively, the only change this introduces is that FreeBSD will now
use the fork-based launcher instead of posix_spawnp. That sholdn't be a
problem as this approach works at least on one other BSD-based system
already.
Reviewers: krytarowski, emaste, jingham
Subscribers: srhines, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D34236
llvm-svn: 305686
Summary:
This is a new C++ test framework based on Google Test, and one working
example test.
The intention is to replace the existing tests in
packages/Python/lldbsuite/test/tools/lldb-server/ with this suite
and use this framework for all new client server tests.
Reviewers: labath, beanz
Reviewed By: labath, beanz
Subscribers: beanz, emaste, zturner, tberghammer, takuto.ikuta, krytarowski, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D32930
Patch by Jason Majors <jmajors@google.com>
llvm-svn: 304793
Antonio Afonso