A lot of comments in LLDB are surrounded by an ASCII line to delimit the
begging and end of the comment.
Its use is not really consistent across the code base, sometimes the
lines are longer, sometimes they are shorter and sometimes they are
omitted. Furthermore, it looks kind of weird with the 80 column limit,
where the comment actually extends past the line, but not by much.
Furthermore, when /// is used for Doxygen comments, it looks
particularly odd. And when // is used, it incorrectly gives the
impression that it's actually a Doxygen comment.
I assume these lines were added to improve distinguishing between
comments and code. However, given that todays editors and IDEs do a
great job at highlighting comments, I think it's worth to drop this for
the sake of consistency. The alternative is fixing all the
inconsistencies, which would create a lot more churn.
Differential revision: https://reviews.llvm.org/D60508
llvm-svn: 358135
Use '127.0.0.1' instead of 'localhost' in ConnectLocally() function
as this is the specific address the server is bound to. Using
'localhost' may involve trying IPv6 first which may accidentally be used
by another service.
While technically it might be interesting to support IPv6 here, it would
need to be supported properly, with the connection copying family
and address from the listening socket, and possibly without relying
on existence of 'localhost' at all.
Differential Revision: https://reviews.llvm.org/D58883
llvm-svn: 355285
Summary:
The field `m_decompression_scratch_type` is only used when `HAVE_LIBCOMPRESSION` is
defined, which caused a warning which I fixed in rLLDB350675 by just marking the variable as always used.
This patch fixes this in a better way by only defining the variable (and the related `m_decompression_scratch`
variable) when `HAVE_LIBCOMPRESSION` is defined. This also required changing the way we handle
`HAVE_LIBCOMPRESSION` works, as this was previously always defined on macOS within the source file
but not in the header. Now it's always defined from within our config header when CMake defines it or when
we are on macOS.
The field initialization was moved to the header to prevent that we have `#ifdef` within our initializer list.
Reviewers: #lldb, jasonmolenda, sgraenitz, labath
Reviewed By: labath
Subscribers: labath, beanz, mgorny, lldb-commits, dblaikie
Differential Revision: https://reviews.llvm.org/D57011
llvm-svn: 352175
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:
This adds unnamed pipe support in PipeWindows to support communication between a debug server and child process.
Modify PipeWindows::CreateNew to support the creation of an unnamed pipe.
Rename the previous method that created a named pipe to PipeWindows::CreateNewNamed.
Reviewers: zturner, llvm-commits
Reviewed By: zturner
Subscribers: Hui, labath, lldb-commits
Differential Revision: https://reviews.llvm.org/D56234
llvm-svn: 350784
Summary: The member is private and unused if HAVE_LIBCOMPRESSION is undefined, which triggers Clang's -Wunused-private-field warning.
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D56458
llvm-svn: 350675
systems. It has been available in the OS over over three years
now. If lldb doesn't link against -lcompression, it should be an
error.
Allocate a scratch buffer for libcompression to use when decoding
packets, instead of it having to allocate & free one on every call.
Fix a typeo with the size of the buffer that compression_decode_buffer()
is expanding into.
<rdar://problem/41601084>
llvm-svn: 349563
This patch simplifies boolean expressions acorss LLDB. It was generated
using clang-tidy with the following command:
run-clang-tidy.py -checks='-*,readability-simplify-boolean-expr' -format -fix $PWD
Differential revision: https://reviews.llvm.org/D55584
llvm-svn: 349215
Summary: Instead use a more reasonable value to start and rely on the fact that SmallString will resize if necessary.
Reviewers: labath, asmith
Reviewed By: labath
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D55457
llvm-svn: 348775
This patch removes the comments grouping header includes. They were
added after running IWYU over the LLDB codebase. However they add little
value, are often outdates and burdensome to maintain.
llvm-svn: 346626
This patch removes the logic for resolving paths out of FileSpec and
updates call sites to rely on the FileSystem class instead.
Differential revision: https://reviews.llvm.org/D53915
llvm-svn: 345890
This patch removes the Exists method from FileSpec and updates its uses
with calls to the FileSystem.
Differential revision: https://reviews.llvm.org/D53845
llvm-svn: 345854
Summary:
Instead of a function taking an enum value determining which path to
return, we now have a suite of functions, each returning a single path
kind. This makes it easy to move the python-path function into a
specific plugin in a follow-up commit.
All the users of GetLLDBPath were converted to call specific functions
instead. Most of them were hard-coding the enum value anyway, so this
conversion was simple. The only exception was SBHostOS, which I've
changed to use a switch on the incoming enum value.
Reviewers: clayborg, zturner
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D48272
llvm-svn: 335052
SetFile has an optional style argument which defaulted to the native
style. This patch makes that argument mandatory so clients of the
FileSpec class are forced to think about the correct syntax.
At the same time this introduces a (protected) convenience method to
update the file from within the FileSpec class that keeps the current
style.
These two changes together prevent a potential pitfall where the style
might be forgotten, leading to the path being updated and the style
unintentionally being changed to the host style.
llvm-svn: 334663
Summary:
The comments on this class were out of date with the implementation, and
the implementation itself was inconsistent with our usage of the Timeout
class (I started converting everything to use this class back in D27136,
but I missed this one). I avoid duplicating the waiting logic by
introducing a templated WaitFor function, and make other functions
delegate to that. This function can be also used as a replacement for
the unused WaitForBitToBeSet functions I removed, if it turns out to be
necessary.
As this changes the meaning of a "zero" timeout, I tracked down all the
callers of these functions and updated them accordingly. Propagating the
changes to all the callers of RunShellCommand was a bit too much for
this patch, so I stopped there and will continue that in a follow-up
patch.
I also add some basic unittests for the functions I modified.
Reviewers: jingham, clayborg
Subscribers: mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D46580
llvm-svn: 331880
This is intended as a clean up after the big clang-format commit
(r280751), which unfortunately resulted in many of the comment
paragraphs in LLDB being very hard to read.
FYI, the script I used was:
import textwrap
import commands
import os
import sys
import re
tmp = "%s.tmp"%sys.argv[1]
out = open(tmp, "w+")
with open(sys.argv[1], "r") as f:
header = ""
text = ""
comment = re.compile(r'^( *//) ([^ ].*)$')
special = re.compile(r'^((([A-Z]+[: ])|([0-9]+ )).*)|(.*;)$')
for line in f:
match = comment.match(line)
if match and not special.match(match.group(2)):
# skip intentionally short comments.
if not text and len(match.group(2)) < 40:
out.write(line)
continue
if text:
text += " " + match.group(2)
else:
header = match.group(1)
text = match.group(2)
continue
if text:
filled = textwrap.wrap(text, width=(78-len(header)),
break_long_words=False)
for l in filled:
out.write(header+" "+l+'\n')
text = ""
out.write(line)
os.rename(tmp, sys.argv[1])
Differential Revision: https://reviews.llvm.org/D46144
llvm-svn: 331197
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
Summary:
Gdb servers like openocd may send many $O reply packets for the client to output during a qRcmd command sequence. Currently, lldb interprets the first O packet as an unexpected response. Besides generating no output, this causes lldb to get out of sync with future commands because it continues reading O packets from the first command as response to subsequent commands.
This patch handles any O packets during an qRcmd, treating the first non-O packet as the true response.
Preliminary discussion at http://lists.llvm.org/pipermail/lldb-dev/2018-January/013078.html
Reviewers: clayborg
Reviewed By: clayborg
Subscribers: labath, lldb-commits
Differential Revision: https://reviews.llvm.org/D41745
Patch by Owen Shaw <llvm@owenpshaw.net>
llvm-svn: 322190
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
Null-checking functions which aren't marked weak_import is a no-op
(the compiler rewrites the check to 'true'), regardless of whether a
library providing its definition is weak-linked. If the deployment
target is greater than the minimum requirement, the availability markup
on APIs does not lower to weak_import.
Remove no-op null checks to clean up the code and silence warnings.
Differential Revision: https://reviews.llvm.org/D40812
llvm-svn: 319936
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
"Prevent negative chars from being sign-extended into isprint and isspace which take and int and crash if the int is negative"
https://reviews.llvm.org/D36620
llvm-svn: 311207
This renames the LLDB error class to Status, as discussed
on the lldb-dev mailing list.
A change of this magnitude cannot easily be done without
find and replace, but that has potential to catch unwanted
occurrences of common strings such as "Error". Every effort
was made to find all the obvious things such as the word "Error"
appearing in a string, etc, but it's possible there are still
some lingering occurences left around. Hopefully nothing too
serious.
llvm-svn: 302872
All references to Host and Core have been removed, so this
class can now safely be lowered into Utility.
Differential Revision: https://reviews.llvm.org/D30559
llvm-svn: 296909
Summary:
When lldb-server is started with the -P <port> or -m/-M <min/max port> options to specify which ports are available for remote connections the child debug server is told what port it should listen on. In those cases lldb-server needs to wait for the child to report it’s port number as otherwise it can tell the lldb client that the child is up and listening before it is actually listening on that port. lldb-server already waits in the cases where a port wasn’t specified by waiting until the child reports the port it is using. It was skipping this synchronisation step when passed a port numbers as it knew what the port would be however it does need to ensure the child process has had time to open that port and waiting until the child reports the port number makes sure this has happened.
This patch just removes the one case where a child was spawned and lldb-server did not wait for it to report it’s port number before telling the client lldb process the child is ready to connect to.
This issue was discussed on lldb-dev in a thread here:
http://lists.llvm.org/pipermail/lldb-dev/2017-February/012002.html
Reviewers: clayborg
Reviewed By: clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D30255
llvm-svn: 295947
and use it in the appropriate log statements.
Formatting of chrono types in log messages was very clunky. This should
make it much nicer to use and give better output. For details of the
formatting options see the chrono formatter in llvm.
llvm-svn: 294738
This moves the following classes from Core -> Utility.
ConstString
Error
RegularExpression
Stream
StreamString
The goal here is to get lldbUtility into a state where it has
no dependendencies except on itself and LLVM, so it can be the
starting point at which to start untangling LLDB's dependencies.
These are all low level and very widely used classes, and
previously lldbUtility had dependencies up to lldbCore in order
to use these classes. So moving then down to lldbUtility makes
sense from both the short term and long term perspective in
solving this problem.
Differential Revision: https://reviews.llvm.org/D29427
llvm-svn: 293941
This replaces the raw integer timeout parameters in the class with their
chrono-based equivalents. To achieve this, I have moved the Timeout class to a
more generic place and added a quick unit test for it.
llvm-svn: 287920
Summary:
This replaces the usage of raw integers with duration classes in the gdb-remote
packet management functions. The values are still converted back to integers once
they go into the generic Communication class -- that I am leaving to a separate
change.
The changes are mostly straight-forward (*), the only tricky part was
representation of infinite timeouts.
Currently, we use UINT32_MAX to denote infinite timeout. This is not well suited
for duration classes, as they tend to do arithmetic on the values, and the
identity of the MAX value can easily get lost (e.g.
microseconds(seconds(UINT32_MAX)).count() != UINT32_MAX). We cannot use zero to
represent infinity (as Listener classes do) because we already use it to do
non-blocking polling reads. For this reason, I chose to have an explicit value
for infinity.
The way I achieved that is via llvm::Optional, and I think it reads quite
natural. Passing llvm::None as "timeout" means "no timeout", while passing zero
means "poll". The only tricky part is this breaks implicit conversions (seconds
are implicitly convertible to microseconds, but Optional<seconds> cannot be
easily converted into Optional<microseconds>). For this reason I added a special
class Timeout, inheriting from Optional, and enabling the necessary conversions
one would normally expect.
(*) The other tricky part was GDBRemoteCommunication::PopPacketFromQueue, which
was needlessly complicated. I've simplified it, but that one is only used in
non-stop mode, and so is untested.
Reviewers: clayborg, zturner, jingham
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D26971
llvm-svn: 287864
This is a large API change that removes the two functions from
StreamString that return a std::string& and a const std::string&,
and instead provide one function which returns a StringRef.
Direct access to the underlying buffer violates the concept of
a "stream" which is intended to provide forward only access,
and makes porting to llvm::raw_ostream more difficult in the
future.
Differential Revision: https://reviews.llvm.org/D26698
llvm-svn: 287152
Summary:
Most of the changes are very straight-forward, the only tricky part was the
"packet speed-test" function, which is very time-heavy. As the function was
completely untested, I added a quick unit smoke test for it.
Reviewers: clayborg, zturner
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D25391
llvm-svn: 285602
This updates getters and setters to use StringRef instead of
const char *. I tested the build on Linux, Windows, and OSX
and saw no build or test failures. I cannot test any BSD
or Android variants, however I expect the required changes
to be minimal or non-existant.
llvm-svn: 282079
This patch also marks the const char* versions as =delete to prevent
their use. This has the potential to cause build breakages on some
platforms which I can't compile. I have tested on Windows, Linux,
and OSX. Best practices for fixing broken callsites are outlined in
Args.h in a comment above the deleted function declarations.
Eventually we can remove these =delete declarations, but for now they
are important to make sure that all implicit conversions from
const char * are manually audited to make sure that they do not invoke a
conversion from nullptr.
llvm-svn: 281919
*** to conform to clang-format’s LLVM style. This kind of mass change has
*** two obvious implications:
Firstly, merging this particular commit into a downstream fork may be a huge
effort. Alternatively, it may be worth merging all changes up to this commit,
performing the same reformatting operation locally, and then discarding the
merge for this particular commit. The commands used to accomplish this
reformatting were as follows (with current working directory as the root of
the repository):
find . \( -iname "*.c" -or -iname "*.cpp" -or -iname "*.h" -or -iname "*.mm" \) -exec clang-format -i {} +
find . -iname "*.py" -exec autopep8 --in-place --aggressive --aggressive {} + ;
The version of clang-format used was 3.9.0, and autopep8 was 1.2.4.
Secondly, “blame” style tools will generally point to this commit instead of
a meaningful prior commit. There are alternatives available that will attempt
to look through this change and find the appropriate prior commit. YMMV.
llvm-svn: 280751
This started as an effort to change StringExtractor to store a
StringRef internally instead of a std::string. I got that working
locally with just 1 test failure which I was unable to figure out the
cause of. But it was also a massive changelist due to a trickle
down effect of changes.
So I'm starting over, using what I learned from the first time to
tackle smaller, more isolated changes hopefully leading up to
a full conversion by the end.
At first the changes (such as in this CL) will seem mostly
a matter of preference and pointless otherwise. However, there
are some places in my larger CL where using StringRef turned 20+
lines of code into 2, drastically simplifying logic. Hopefully
once these go in they will illustrate some of the benefits of
thinking in terms of StringRef.
llvm-svn: 279917
The commit started passing a nullptr port into GDBRemoteCommunication::StartDebugserverProcess.
The function was mostly handling the null value correctly, but it one case it did not check it's
value before assigning to it. Fix that.
llvm-svn: 278662
This change opens a socket pair and passes the second socket pair file descriptor down to the debugserver binary using a new option: "--fd=N" where N is the file descriptor. This file descriptor gets passed via posix_spawn() so that there is no need to do any bind/listen or bind/accept calls and eliminates the hanshake unix socket that is used to pass the result of the actual port that ends up being used so it can save time on launch as well as being faster.
This is currently only enabled on __APPLE__ builds. Other OSs should try modifying the #define from ProcessGDBRemote.cpp but the first person will need to port the --fd option over to lldb-server. Any OSs that enable USE_SOCKETPAIR_FOR_LOCAL_CONNECTION in their native builds can use the socket pair stuff. The #define is Apple only right now, but looks like:
#if defined (__APPLE__)
#define USE_SOCKETPAIR_FOR_LOCAL_CONNECTION 1
#endif
<rdar://problem/27814880>
llvm-svn: 278524
It's always hard to remember when to include this file, and
when you do include it it's hard to remember what preprocessor
check it needs to be behind, and then you further have to remember
whether it's windows.h or win32.h which you need to include.
This patch changes the name to PosixApi.h, which is more appropriately
named, and makes it independent of any preprocessor setting.
There's still the issue of people not knowing when to include this,
because there's not a well-defined set of things it exposes other
than "whatever is missing on Windows", but at least this should
make it less painful to fix when problems arise.
This patch depends on LLVM revision r278170.
llvm-svn: 278177
Resumbitting the commit after fixing the following problems:
- broken unit tests on windows: incorrect gtest usage on my part (TEST vs. TEST_F)
- the new code did not correctly handle the case where we went to interrupt the process, but it
stopped due to a different reason - the interrupt request would remain queued and would
interfere with the following "continue". I also added a unit test for this case.
This reapplies r277156 and r277139.
llvm-svn: 278118
It only contained a reimplementation of std::to_string, which I have replaced with usages of
pre-existing llvm::to_string (also, injecting members into the std namespace is evil).
llvm-svn: 278000
This reverts commit r277139, because:
- broken unittest on windows (likely typo on my part)
- seems to break TestCallThatRestart (needs investigation)
llvm-svn: 277154
SendContinuePacketAndWaitForResponse was huge function with very complex interactions with
several other functions (SendAsyncSignal, SendInterrupt, SendPacket). This meant that making any
changes to how packet sending functions and threads interact was very difficult and error-prone.
This change does not add any functionality yet, it merely paves the way for future changes. In a
follow-up, I plan to add the ability to have multiple query packets in flight (i.e.,
request,request,response,response instead of the usual request,response sequences) and use that
to speed up qModuleInfo packet processing.
Here, I introduce two special kinds of locks: ContinueLock, which is used by the continue thread,
and Lock, which is used by everyone else. ContinueLock (atomically) sends a continue packet, and
blocks any other async threads from accessing the connection. Other threads create an instance of
the Lock object when they want to access the connection. This object, while in scope prevents the
continue from being send. Optionally, it can also interrupt the process to gain access to the
connection for async processing.
Most of the syncrhonization logic is encapsulated within these two classes. Some of it still
had to bleed over into the SendContinuePacketAndWaitForResponse, but the function is still much
more manageable than before -- partly because of most of the work is done in the ContinueLock
class, and partly because I have factored out a lot of the packet processing code separate
functions (this also makes the functionality more easily testable). Most importantly, there is
none of syncrhonization code in the async thread users -- as far as they are concerned, they just
need to declare a Lock object, and they are good to go (SendPacketAndWaitForResponse is now a
very thin wrapper around the NoLock version of the function, whereas previously it had over 100
lines of synchronization code). This will make my follow up changes there easy.
I have written a number of unit tests for the new code and I have ran the test suite on linux and
osx with no regressions.
Subscribers: tberghammer
Differential Revision: https://reviews.llvm.org/D22629
llvm-svn: 277139
This finally removes the use of the Mutex and Condition classes. This is an
intricate patch as the Mutex and Condition classes were tied together.
Furthermore, many places had slightly differing uses of time values. Convert
timeout values to relative everywhere to permit the use of
std::chrono::duration, which is required for the use of
std::condition_variable's timeout. Adjust all Condition and related Mutex
classes over to std::{,recursive_}mutex and std::condition_variable.
This change primarily comes at the cost of breaking the TracingMutex which was
based around the Mutex class. It would be possible to write a wrapper to
provide similar functionality, but that is beyond the scope of this change.
llvm-svn: 277011
This change implements dumping the executable, triple,
args and environment when using ProcessInfo::Dump().
It also tweaks the way Args::Dump() works so that it prints
a configurable label rather than argv[{index}]={value}. By
default it behaves the same, but if the Dump() method with
the additional arg is provided, it can be overridden. The
environment variables dumped as part of ProcessInfo::Dump()
make use of that.
lldb-server has been modified to dump the gdb-remote stub's
ProcessInfo before launching if the "gdb-remote process" channel
is logged.
llvm-svn: 271312
This is a pretty straightforward first pass over removing a number of uses of
Mutex in favor of std::mutex or std::recursive_mutex. The problem is that there
are interfaces which take Mutex::Locker & to lock internal locks. This patch
cleans up most of the easy cases. The only non-trivial change is in
CommandObjectTarget.cpp where a Mutex::Locker was split into two.
llvm-svn: 269877
the xcode project file to catch switch statements that have a
case that falls through unintentionally.
Define LLVM_FALLTHROUGH to indicate instances where a case has code
and intends to fall through. This should be in llvm/Support/Compiler.h;
Peter Collingbourne originally checked in there (r237766), then
reverted (r237941) because he didn't have time to mark up all the
'case' statements that were intended to fall through. I put together
a patch to get this back in llvm http://reviews.llvm.org/D17063 but
it hasn't been approved in the past week. I added a new
lldb-private-defines.h to hold the definition for now.
Every place in lldb where there is a comment that the fall-through
is intentional, I added LLVM_FALLTHROUGH to silence the warning.
I haven't tried to identify whether the fallthrough is a bug or
not in the other places.
I haven't tried to add this to the cmake option build flags.
This warning will only work for clang.
This build cleanly (with some new warnings) on macosx with clang
under xcodebuild, but if this causes problems for people on other
configurations, I'll back it out.
llvm-svn: 260930
The standard remote debugging workflow with gdb is to start the
application on the remote host under gdbserver (e.g.: gdbserver :5039
a.out) and then connect to it with gdb.
The same workflow is supported by debugserver/lldb-gdbserver with a very
similar syntax but to access all features of lldb we need to be
connected also to an lldb-platform instance running on the target.
Before this change this had to be done manually with starting a separate
lldb-platform on the target machine and then connecting to it with lldb
before connecting to the process.
This change modifies the behavior of "platform connect" with
automatically connecting to the process instance if it was started by
the remote platform. With this command replacing gdbserver in a gdb
based worflow is usually as simple as replacing the command to execute
gdbserver with executing lldb-platform.
Differential revision: http://reviews.llvm.org/D14952
llvm-svn: 255016
This allows open source MacOSX clients to not have to build debugserver and the current LLDB can find debugserver inside the selected Xcode.app on your system.
<rdar://problem/23167253>
llvm-svn: 250735
on iOS devices; fallout from Vince's cleanups made
in r237218 back in May. iOS native lldbs will call
StartDebugserverProcess() with a random port #
(see ProcessGDBRemote::LaunchAndConnectToDebugserver)
and neither side of this conditional expression should
be followed in that case.
I added an "if (in_port == 0) { ..." check around the
entire if/then/else and indented the block of code so
the diff looks larger than it really is.
<rdar://problem/21712643>
llvm-svn: 248343
When lldb receives a gdb-remote protocol packet that has
nonprintable characters, it will print the packet in
gdb-remote logging with binary-hex encoding so we don't
dump random 8-bit characters into the packet log.
I'm changing this check to allow whitespace characters
(newlines, linefeeds, tabs) to be printed if those are
the only non-printable characters in the packet.
This is primarily to get the response to the
qXfer:features:read:target.xml packet to show up in the
packet logs in human readable form. Right now we just
get a dozen kilobytes of hex-ascii and it's hard to
figure out what register number scheme is being used.
llvm-svn: 247120
working with (the Communication m_bytes ivar) contained a single packet.
Instead, it may contain multitudes. Find the boundaries of the first packet
in the buffer and replace that with the decompressed version leaving the
rest of the buffer unmodified.
<rdar://problem/21841377>
llvm-svn: 243846
Summary:
If we used unnamed pipes instead of named pipes, we can avoid having the
file system littered with debugserver-named-pipes if lldb-server happens to
crash for whatever reason. Also, on some buggy systems, it's possible to be
able to create but not to delete a fifo. Ideally, support for unnamed pipes
should be added to debugserver as well, so we can avoid the `#ifdef` here.
Reviewers: clayborg, vharron, chying
Subscribers: lldb-commits
Differential Revision: http://reviews.llvm.org/D11609
llvm-svn: 243667
For some communication channels, sending large packets can be very
slow. In those cases, it may be faster to compress the contents of
the packet on the target device and decompress it on the debug host
system. For instance, communicating with a device using something
like Bluetooth may be an environment where this tradeoff is a good one.
This patch adds a new field to the response to the "qSupported" packet
(which returns a "qXfer:features:" response) -- SupportedCompressions
and DefaultCompressionMinSize. These tell you what the remote
stub can support.
lldb, if it wants to enable compression and can handle one of those
algorithms, it can send a QEnableCompression packet specifying the
algorithm and optionally the minimum packet size to use compression
on. lldb may have better knowledge about the best tradeoff for
a given communication channel.
I added support to debugserver an lldb to use the zlib APIs
(if -DHAVE_LIBZ=1 is in CFLAGS and -lz is in LDFLAGS) and the
libcompression APIs on Mac OS X 10.11 and later
(if -DHAVE_LIBCOMPRESSION=1). libz "zlib-deflate" compression.
libcompression can support deflate, lz4, lzma, and a proprietary
lzfse algorithm. libcompression has been hand-tuned for Apple
hardware so it should be preferred if available.
debugserver currently only adds the SupportedCompressions when
it is being run on an Apple watch (TARGET_OS_WATCH). Comment
that #if out from RNBRemote.cpp if you want to enable it to
see how it works. I haven't tested this on a native system
configuration but surely it will be slower to compress & decompress
the packets in a same-system debug session.
I haven't had a chance to add support for this to
GDBRemoteCommunciationServer.cpp yet.
<rdar://problem/21090180>
llvm-svn: 240066
In order to support asynchronous notifications for non-stop mode this patch adds a packet read thread. This is done by implementing AppendBytesToCache() from the communications class, which continually reads packets into a packet queue. To initialize this thread StartReadThread() must be called by the client, so since llgs and platform tools use the GBDRemoteCommunicatos code they must also call this function as well as ProcessGDBRemote.
When the read thread detects an async notify packet it broadcasts this event, where the matching listener will be added in the next non-stop patch.
Packets are now accessed by calling ReadPacket() which pops a packet from the queue, instead of using WaitForPacketWithTimeoutMicroSecondsNoLock()
Reviewers: vharron, clayborg
Subscribers: lldb-commits, labath, ted, domipheus, deepak2427
Differential Revision: http://reviews.llvm.org/D10085
llvm-svn: 239824
qEcho:%s
where '%s' is any valid string. The response to this packet is the exact packet itself with no changes, just reply with what you received!
This will help us to recover from packets timing out much more gracefully. Currently if a packet times out, LLDB quickly will hose up the debug session. For example, if we send a "abc" packet and we expect "ABC" back in response, but the "abc" command takes longer than the current timeout value this will happen:
--> "abc"
<-- <<<error: timeout>>>
Now we want to send "def" and get "DEF" back:
--> "def"
<-- "ABC"
We got the wrong response for the "def" packet because we didn't sync up with the server to clear any current responses from previously issues commands.
The fix is to modify GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock() so that when it gets a timeout, it syncs itself up with the client by sending a "qEcho:%u" where %u is an increasing integer, one for each time we timeout. We then wait for 3 timeout periods to sync back up. So the above "abc" session would look like:
--> "abc"
<-- <<<error: timeout>>> 1 second
--> "qEcho:1"
<-- <<<error: timeout>>> 1 second
<-- <<<error: timeout>>> 1 second
<-- "abc"
<-- "qEcho:1"
The first timeout is from trying to get the response, then we know we timed out and we send the "qEcho:1" packet and wait for 3 timeout periods to get back in sync knowing that we might actually get the response for the "abc" packet in the mean time...
In this case we would actually succeed in getting the response for "abc". But lets say the remote GDB server is deadlocked and will never response, it would look like:
--> "abc"
<-- <<<error: timeout>>> 1 second
--> "qEcho:1"
<-- <<<error: timeout>>> 1 second
<-- <<<error: timeout>>> 1 second
<-- <<<error: timeout>>> 1 second
We then disconnect and say we lost connection.
We might also have a bad GDB server that just dropped the "abc" packet on the floor. We can still recover in this case and it would look like:
--> "abc"
<-- <<<error: timeout>>> 1 second
--> "qEcho:1"
<-- "qEcho:1"
Then we know our remote GDB server is still alive and well, and it just dropped the "abc" response on the floor and we can continue to debug.
<rdar://problem/21082939>
llvm-svn: 238530
In ProcessGDBRemote we currently have a single packet, m_last_stop_packet, used to set the thread stop info.
However in non-stop mode we can receive several stop reply packets in a sequence for different threads. As a result we need to use a container to hold them before they are processed.
This patch also changes the return type of CheckPacket() so we can detect async notification packets.
Reviewers: clayborg
Subscribers: labath, ted, deepak2427, lldb-commits
Differential Revision: http://reviews.llvm.org/D9853
llvm-svn: 238323
Summary:
This patch is the beginnings of support for Non-stop mode in the remote protocol. Letting a user examine stopped threads, while other threads execute freely.
Non-stop mode is enabled using the setting target.non-stop-mode, which sends a QNonStop packet when establishing the remote connection.
Changes are also made to treat the '?' stop reply packet differently in non-stop mode, according to spec https://sourceware.org/gdb/current/onlinedocs/gdb/Remote-Non_002dStop.html#Remote-Non_002dStop.
A setting for querying the remote for default thread on setup is also included.
Handling of '%' async notification packets will be added next.
Reviewers: clayborg
Subscribers: lldb-commits, ADodds, ted, deepak2427
Differential Revision: http://reviews.llvm.org/D9656
llvm-svn: 237239
Removed some unused variables, added some consts, changed some casts
to const_cast. I don't think any of these changes are very
controversial.
Differential Revision: http://reviews.llvm.org/D9674
llvm-svn: 237218
Summary:
New dotest options that allow arbitrary log channels and
categories to be enabled. Also enables logging for locally run
debug servers.
Log messages are separated into separate files per test case.
(this makes it possible to log in dosep runs)
These new log files are stored side-by-side with trace files in the
session directory.
These files are deleted by default if the test run is successful.
If --log-success is specified, even successful logs are retained.
--log-success is useful for creating reference log files.
Test Plan:
add '--channel "lldb all" --channel "gdb-remote packets" --log-success'
to your dotest options
Tested on OSX and Linux
Differential Revision: http://reviews.llvm.org/D9594
llvm-svn: 236956
Summary:
Currently, launching lldb-gdbserver from platform on Android requires root for
mkfifo() and an explicit TMPDIR variable. This should remove both requirements.
Test Plan: Successfully launched lldb-gdbserver on a non-rooted Android device.
Reviewers: tberghammer, vharron, clayborg
Reviewed By: clayborg
Subscribers: tberghammer, lldb-commits
Differential Revision: http://reviews.llvm.org/D9307
llvm-svn: 235940
This new class makes it easier to change the timeout of a
GDBRemoteCommunication instance for a short time and then restore it to
its original value.
Differential revision: http://reviews.llvm.org/D7826
llvm-svn: 230319
This commit merges lldb-platform and lldb-gdbserver into a single binary
of the same size as each of the previous individual binaries. Execution
mode is controlled by the first argument being either platform or
gdbserver.
Patch from: flackr <flackr@google.com>
Differential revision: http://reviews.llvm.org/D7545
llvm-svn: 229683
The refactor was motivated by some comments that Greg made
http://reviews.llvm.org/D6918
and also to break a dependency cascade that caused functions linking
in string->int conversion functions to pull in most of lldb
llvm-svn: 226199
This reverts commit 4a5ad2c077166cc3d6e7ab4cc6e3dcbbe922af86.
Windows doesn't support select() for pipe objects, and this also fails
to compile on Windows. Reverting this until we can get it sorted out
to keep the windows build working.
llvm-svn: 223392
The details are: large packets (like large memory reads (m packets) or large binary memory reads (x packet)) can get responses that come in across multiple read() calls. The while loop that was added meant that if only a partial packet came in (like only "$abc" coming for a response) GDBRemoteCommunication::CheckForPacket() was called, it would deadlock in the while loop because no more data is going to come in as this function needs to be called again with more data from another read. So the original fix will need to be corrected and resubmitted.
<rdar://problem/18853744>
llvm-svn: 221181
As part of getting ConnectionFileDescriptor working on Windows,
there is going to be alot of platform specific work to be done.
As a result, the implementation is moving into Host. This patch
performs the code move and fixes up call-sites appropriately.
Reviewed by: Greg Clayton
Differential Revision: http://reviews.llvm.org/D5548
llvm-svn: 219143
This patch moves creates a thread abstraction that represents a
thread running inside the LLDB process. This is a replacement for
otherwise using lldb::thread_t, and provides a platform agnostic
interface to managing these threads.
Differential Revision: http://reviews.llvm.org/D5198
Reviewed by: Jim Ingham
llvm-svn: 217460
This patch accepts environment variables of the form:
LLDB_DEBUGSERVER_EXTRA_ARG_n
where n starts with 1, and may continue nearly indefinitely (up through std::numeric_limits<uint32_t>::max()).
The code loops around, starting with 1, until it doesn't find one of the environment variables. For each one it does find defined, it appends the environment variable's contents to the end of the debugserver/llgs startup command line issued when the stub is started for local debugging.
I am using this to add arbitrary startup commands to the llgs command line for turning on additional logging. For example:
export LLDB_DEBUGSERVER_EXTRA_ARG_1="-c"
export LLDB_DEBUGSERVER_EXTRA_ARG_2="log enable -f /tmp/llgs_packets.log gdb-remote packets"
export LLDB_DEBUGSERVER_EXTRA_ARG_3="-c"
export LLDB_DEBUGSERVER_EXTRA_ARG_4="log enable -f /tmp/llgs_process.log lldb process"
llvm-svn: 216745
Saleem Abdulrasool