This patch mechanically replaces None with std::nullopt where the
compiler would warn if None were deprecated. The intent is to reduce
the amount of manual work required in migrating from Optional to
std::optional.
This is part of an effort to migrate from llvm::Optional to
std::optional:
https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
Resubmission of https://reviews.llvm.org/D130309 with the 2 patches that fixed the linux buildbot, and new windows fixes.
The FileSpec APIs allow users to modify instance variables directly by getting a non const reference to the directory and filename instance variables. This makes it impossible to control all of the times the FileSpec object is modified so we can clear cached member variables like m_resolved and with an upcoming patch caching if the file is relative or absolute. This patch modifies the APIs of FileSpec so no one can modify the directory or filename instance variables directly by adding set accessors and by removing the get accessors that are non const.
Many clients were using FileSpec::GetCString(...) which returned a unique C string from a ConstString'ified version of the result of GetPath() which returned a std::string. This caused many locations to use this convenient function incorrectly and could cause many strings to be added to the constant string pool that didn't need to. Most clients were converted to using FileSpec::GetPath().c_str() when possible. Other clients were modified to use the newly renamed version of this function which returns an actualy ConstString:
ConstString FileSpec::GetPathAsConstString(bool denormalize = true) const;
This avoids the issue where people were getting an already uniqued "const char *" that came from a ConstString only to put the "const char *" back into a "ConstString" object. By returning the ConstString instead of a "const char *" clients can be more efficient with the result.
The patch:
- Removes the non const GetDirectory() and GetFilename() get accessors
- Adds set accessors to replace the above functions: SetDirectory() and SetFilename().
- Adds ClearDirectory() and ClearFilename() to replace usage of the FileSpec::GetDirectory().Clear()/FileSpec::GetFilename().Clear() call sites
- Fixed all incorrect usage of FileSpec::GetCString() to use FileSpec::GetPath().c_str() where appropriate, and updated other call sites that wanted a ConstString to use the newly returned ConstString appropriately and efficiently.
Differential Revision: https://reviews.llvm.org/D130549
This reverts commit 9429b67b8e300e638d7828bbcb95585f85c4df4d.
It broke the build on Windows, see comments on https://reviews.llvm.org/D130309
It also reverts these follow-ups:
Revert "Fix buildbot breakage after https://reviews.llvm.org/D130309."
This reverts commit f959d815f4637890ebbacca379f1c38ab47e4e14.
Revert "Fix buildbot breakage after https://reviews.llvm.org/D130309."
This reverts commit 0bbce7a4c2d2bff622bdadd4323f93f5d90e6d24.
Revert "Cache the value for absolute path in FileSpec."
This reverts commit dabe877248b85b34878e75d5510339325ee087d0.
The FileSpect APIs allow users to modify instance variables directly by getting a non const reference to the directory and filename instance variables. This makes it impossibly to control all of the times the FileSpec object is modified so we can clear the cache. This patch modifies the APIs of FileSpec so no one can modify the directory or filename directly by adding set accessors and by removing the get accessors that are non const.
Many clients were using FileSpec::GetCString(...) which returned a unique C string from a ConstString'ified version of the result of GetPath() which returned a std::string. This caused many locations to use this convenient function incorrectly and could cause many strings to be added to the constant string pool that didn't need to. Most clients were converted to using FileSpec::GetPath().c_str() when possible. Other clients were modified to use the newly renamed version of this function which returns an actualy ConstString:
ConstString FileSpec::GetPathAsConstString(bool denormalize = true) const;
This avoids the issue where people were getting an already uniqued "const char *" that came from a ConstString only to put the "const char *" back into a "ConstString" object. By returning the ConstString instead of a "const char *" clients can be more efficient with the result.
The patch:
- Removes the non const GetDirectory() and GetFilename() get accessors
- Adds set accessors to replace the above functions: SetDirectory() and SetFilename().
- Adds ClearDirectory() and ClearFilename() to replace usage of the FileSpec::GetDirectory().Clear()/FileSpec::GetFilename().Clear() call sites
- Fixed all incorrect usage of FileSpec::GetCString() to use FileSpec::GetPath().c_str() where appropriate, and updated other call sites that wanted a ConstString to use the newly returned ConstString appropriately and efficiently.
Differential Revision: https://reviews.llvm.org/D130309
As it exists today, Host::SystemLog is used exclusively for error
reporting. With the introduction of diagnostic events, we have a better
way of reporting those. Instead of printing directly to stderr, these
messages now get printed to the debugger's error stream (when using the
default event handler). Alternatively, if someone is listening for these
events, they can decide how to display them, for example in the context
of an IDE such as Xcode.
This change also means we no longer write these messages to the system
log on Darwin. As far as I know, nobody is relying on this, but I think
this is something we could add to the diagnostic event mechanism.
Differential revision: https://reviews.llvm.org/D128480
Although I cannot find any mention of this in the specification, both
gdb and lldb agree on sending an initial + packet after establishing the
connection.
OTOH, gdbserver and lldb-server behavior is subtly different. While
lldb-server *expects* the initial ack, and drops the connection if it is
not received, gdbserver will just ignore a spurious ack at _any_ point
in the connection.
This patch changes lldb's behavior to match that of gdb. An ACK packet
is ignored at any point in the connection (except when expecting an ACK
packet, of course). This is inline with the "be strict in what you
generate, and lenient in what you accept" philosophy, and also enables
us to remove some special cases from the server code. I've extended the
same handling to NAK (-) packets, mainly because I don't see a reason to
treat them differently here.
(The background here is that we had a stub which was sending spurious
+ packets. This bug has since been fixed, but I think this change makes
sense nonetheless.)
Differential Revision: https://reviews.llvm.org/D114520
This diff modifies the LLDB server return codes to more accurately reflect usage
error paths. Specifically we always propagate the return codes from the main
entrypoints into GDB remote LLDB server, and platform LLDB server. This way, the
top-level caller of LLDB server will be able to correctly check whether the
executable exited with or without an error.
We additionally modify and extend the associated shell unit tests to expect
nonzero return codes on error conditions.
Test Plan:
LLDB tests pass:
```
ninja check-lldb
```
Reviewed By: teemperor
Differential Revision: https://reviews.llvm.org/D108351
The C headers are deprecated so as requested in D102845, this is replacing them
all with their (not deprecated) C++ equivalent.
Reviewed By: shafik
Differential Revision: https://reviews.llvm.org/D103084
Previously we used UINT16_MAX to mean no port/no specifc
port. This leads to confusion because 65535 is a valid
port number.
Instead use an optional. If you want a specific port call
LaunchGDBServer as normal, otherwise pass an empty optional
and it will be set to the port that gets chosen.
(or left empty in the case where we fail to find a port)
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D92035
Previously if you did:
$ lldb-server platform --server <...> --min-gdbserver-port 12346
--max-gdbserver-port 12347
(meaning only use port 12346 for gdbservers)
Then tried to launch two gdbservers on the same connection,
the second one would return port 65535. Which is a real port
number but it actually means lldb-server didn't find one it was
allowed to use.
send packet: $qLaunchGDBServer;<...>
read packet: $pid:1919;port:12346;#c0
<...>
send packet: $qLaunchGDBServer;<...>
read packet: $pid:1927;port:65535;#c7
This situation should be an error even if port 65535 does happen
to be available on the current machine.
To fix this make PortMap it's own class within
GDBRemoteCommunicationServerPlatform.
This almost the same as the old typedef but for
GetNextAvailablePort() returning an llvm::Expected.
This means we have to handle not finding a port,
by returning an error packet.
Also add unit tests for this new PortMap class.
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D91634
`lldb-server platform --socket-file /any/path` currently always fails to create
the socket file. This stopped working after D67424 which changed the
input variables of `writeFileAtomically` slightly. We're expected to
pass in a temporary path template (`/tmp/foo-%%%%%`) and the final
path we want to write. Instead we currently pass in the never set
`temp_file_path` as the temporary path (which will make this function always
fail) and pass in the temp_file_spec's path as the final path (which is actually
the template path such as `/tmp/foo-%%%%%`) instead of the actual path
we want to write (e.g. `/tmp/foo`).
Reviewed By: labath
Differential Revision: https://reviews.llvm.org/D85890
StringRef will call strlen on the C string which is inefficient (as ConstString already
knows the string lenght and so does StringRef). This patch replaces all those calls
with GetStringRef() which doesn't recompute the length.
Summary:
This commit contains three small changes to enable lldb-server on Windows.
- Add lldb-server for Windows to the build
- Disable pty redirection on Windows for the initial lldb-server bring up
- Add a support to get the parent pid for a process on Windows
- Ifdef some signals which aren't supported on Windows
Thanks to Hui Huang for the help with this patch!
Reviewers: labath
Reviewed By: labath
Subscribers: JDevlieghere, compnerd, Hui, amccarth, xiaobai, srhines, mgorny, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D61686
llvm-svn: 368774
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
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
In mail
[lldb-dev] Remote debugging a docker process
https://lists.llvm.org/pipermail/lldb-dev/2019-March/014795.html
user was confused by --min-gdbserver-port and --max-gdbserver-port options
being ignored. I think there is even a bug that --max-gdbserver-port is upper
exclusive limit (and not upper inclusive limit appropriate for max).
At least this patch should catch such mistake by an error message. The question
is whether --max-gdbserver-port should not be changed to really be max and not
max+1 but that would break backward compatibility.
Now the mail example does produce:
error: --min-gdbserver-port (5001) is not lower than --max-gdbserver-port (5001)
Differential Revision: https://reviews.llvm.org/D58962
llvm-svn: 355554
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
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
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
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
Summary:
ofstream does not handle paths with non-ascii characters correctly on
windows, so I am switching these to llvm streams to fix that.
Reviewers: zturner, eugene
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D31079
llvm-svn: 298375
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
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 forcing to use Error::success(), which is in a wide majority
of cases a lot more readable.
Differential Revision: https://reviews.llvm.org/D26481
llvm-svn: 286561
*** 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
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
Summary:
This is the first part of our effort to make llgs single threaded. Currently, llgs consists of
about three threads and the synchronisation between them is a major source of latency when
debugging linux and android applications.
In order to be able to go single threaded, we must have the ability to listen for events from
multiple sources (primarily, client commands coming over the network and debug events from the
inferior) and perform necessary actions. For this reason I introduce the concept of a MainLoop.
A main loop has the ability to register callback's which will be invoked upon receipt of certain
events. MainLoopPosix has the ability to listen for file descriptors and signals.
For the moment, I have merely made the GDBRemoteCommunicationServerLLGS class use MainLoop
instead of waiting on the network socket directly, but the other threads still remain. In the
followup patches I indend to migrate NativeProcessLinux to this class and remove the remaining
threads.
Reviewers: ovyalov, clayborg, amccarth, zturner, emaste
Subscribers: tberghammer, lldb-commits
Differential Revision: http://reviews.llvm.org/D11066
llvm-svn: 242018
Since interaction with the python interpreter is moving towards
being more isolated, we won't be able to include this header from
normal files anymore, all includes of it should be localized to
the python library which will live under source/bindings/API/Python
after a future patch.
None of the files that were including this header actually depended
on it anyway, so it was just a dead include in every single instance.
llvm-svn: 238581
This change also get rid of an unused Debugger instance in
GDBRemoteCommunicationServerLLGS and the command interpreter from
lldb-platform what was used only for enabling logging.
Differential revision: http://reviews.llvm.org/D9876
llvm-svn: 238319
If binding to port 0 is selected, the actual port is printed.
This improves the reliability of platform startup by ensuring that
a free port can be found.
TEST PLAN
./lldb-server platform --listen *:0
Listening for a connection from <port-number>...
Will appear on stdout (with other stuff potentially)
llvm-svn: 238173
