Revert "[lldb] Remove redundant .c_str() and .get() calls"

This reverts commit fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd.

This has broken all LLDB buildbots:
https://lab.llvm.org/buildbot/#/builders

Revert "[lldb] Remove redundant .c_str() and .get() calls"

This reverts commit fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd.

This has broken all LLDB buildbots:
https://lab.llvm.org/buildbot/#/builders/68/builds/44990
https://lab.llvm.org/buildbot/#/builders/96/builds/33160

show more ...

[lldb] Remove redundant .c_str() and .get() calls

Removing .c_str() has a semantics difference, but the use scenarios
likely do not matter as we don't have NUL in the strings.

[lldb] Use std::nullopt instead of None (NFC)

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 ma

[lldb] Use std::nullopt instead of None (NFC)

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

show more ...

[LLDB] Format lldb-server's target XML

So that the XML isn't one giant line. Which wasn't
a problem for lldb but was for me trying to troubleshoot
it using the logs.

It now looks like:
```
<?xml ve

[LLDB] Format lldb-server's target XML

So that the XML isn't one giant line. Which wasn't
a problem for lldb but was for me trying to troubleshoot
it using the logs.

It now looks like:
```
<?xml version="1.0"?>
<target version="1.0">
<architecture>aarch64</architecture>
<feature>
<...>
<reg name="fpcr" .../>
</feature>
</target>
```

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D134035

show more ...

[gdb-remote] Move broadcasting logic down to GDBRemoteClientBase

Move the broadcasting support from GDBRemoteCommunication
to GDBRemoteClientBase since this is where it is actually used. Remove
GDB

[gdb-remote] Move broadcasting logic down to GDBRemoteClientBase

Move the broadcasting support from GDBRemoteCommunication
to GDBRemoteClientBase since this is where it is actually used. Remove
GDBRemoteCommunication and subclass constructor arguments left over
after Communication cleanup.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D133427

show more ...

[lldb-server] Report launch error in vRun packets

Uses our existing "error string" extension to provide a better
indication of why the launch failed (the client does not make use of the
error yet).

[lldb-server] Report launch error in vRun packets

Uses our existing "error string" extension to provide a better
indication of why the launch failed (the client does not make use of the
error yet).

Also, fix the way we obtain the launch error message (make sure we read
the whole message, and skip trailing garbage), and reduce the size of
TestLldbGdbServer by splitting some tests into a separate file.

Differential Revision: https://reviews.llvm.org/D133352

show more ...

[lldb] [Core] Split read thread support into ThreadedCommunication

Split the read thread support from Communication into a dedicated
ThreadedCommunication subclass. The read thread support is used

[lldb] [Core] Split read thread support into ThreadedCommunication

Split the read thread support from Communication into a dedicated
ThreadedCommunication subclass. The read thread support is used only
by a subset of Communication consumers, and it adds a lot of complexity
to the base class. Furthermore, having a dedicated subclass makes it
clear whether a particular consumer needs to account for the possibility
of read thread being running or not.

The modules currently calling `StartReadThread()` are updated to use
`ThreadedCommunication`. The remaining modules use the simplified
`Communication` class.

`SBCommunication` is changed to use `ThreadedCommunication` in order
to avoid changing the public API.

`CommunicationKDP` is updated in order to (hopefully) compile with
the new code. However, I do not have a Darwin box to test it, so I've
limited the changes to the bare minimum.

`GDBRemoteCommunication` is updated to become a `Broadcaster` directly.
Since it does not inherit from `ThreadedCommunication`, its event
support no longer collides with the one used for read thread and can
be implemented cleanly. The support for
`eBroadcastBitReadThreadDidExit` is removed from the code -- since
the read thread was not used, this event was never reported.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D133251

show more ...

[lldb] [gdb-remote] Use Communication::WriteAll() over Write()

Replace the uses of Communication::Write() with WriteAll() to avoid
partial writes. None of the call sites actually accounted for that

[lldb] [gdb-remote] Use Communication::WriteAll() over Write()

Replace the uses of Communication::Write() with WriteAll() to avoid
partial writes. None of the call sites actually accounted for that
possibility and even if it is unlikely to actually happen, there doesn't
seem to be any real harm from using WriteAll() instead.

Ideally, we'd remove Write() from the public API. However, that would
change the API of SBCommunication. The alternative would be to alias it
to WriteAll().

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D132395

show more ...

[lldb] LLVM_FALLTHROUGH => [[fallthrough]]. NFC

[lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

Support using the vCont packet to resume multiple processes
simultaneously when in non-stop mode. The new logic now assumes th

[lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

Support using the vCont packet to resume multiple processes
simultaneously when in non-stop mode. The new logic now assumes that:

- actions without a thread-id or with process id of "p-1" apply to all
debugged processes

- actions with a thread-id without process id apply to the current
process (m_continue_process)

As with the other continue packets, it is only possible to resume
processes that are currently stopped (or stop these that are running).
It is unsupported to resume or stop individual threads of a running
process.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128989

show more ...

[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

Resubmission of https://reviews.llvm.org/D130309 with the 2 patches that fixed the linux buildb

[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

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

show more ...

Revert "[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute." and follow-ups

This reverts commit 9429b67b8e300e638d7828bbcb95585f85c4df4d.

It broke

Revert "[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute." and follow-ups

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.

show more ...

[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

The FileSpect APIs allow users to modify instance variables directly by getting a non const ref

[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

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

show more ...

[lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

Enable stdio forwarding when nonstop mode is enabled, and disable it
once it is disabled. This makes it possible to cleanly handle std

[lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

Enable stdio forwarding when nonstop mode is enabled, and disable it
once it is disabled. This makes it possible to cleanly handle stdio
forwarding while running multiple processes in non-stop mode.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128932

show more ...

[lldb] [llgs] Fix disabling non-stop mode

Stop all processes and clear notification queues when disabling non-stop
mode. Ensure that no stop notifications are sent for processes stopped
due to the

[lldb] [llgs] Fix disabling non-stop mode

Stop all processes and clear notification queues when disabling non-stop
mode. Ensure that no stop notifications are sent for processes stopped
due to the mode switch.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128893

show more ...

[lldb] [llgs] Fix `?` packet response for running threads

Fix the response to `?` packet for threads that are running at the time
(in non-stop mode). The previous code would wrongly send or queue
a

[lldb] [llgs] Fix `?` packet response for running threads

Fix the response to `?` packet for threads that are running at the time
(in non-stop mode). The previous code would wrongly send or queue
an empty response for them.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128879

show more ...

[lldb] [llgs] Send process output asynchronously in non-stop mode

Introduce a new %Stdio notification category and use it to send process
output asynchronously when running in non-stop mode. This i

[lldb] [llgs] Send process output asynchronously in non-stop mode

Introduce a new %Stdio notification category and use it to send process
output asynchronously when running in non-stop mode. This is an LLDB
extension since GDB does not use the 'O' packet for process output,
just for replies to 'qRcmd' packets.

Using the async notification mechanism implies that only the first
output packet is sent immediately to the client. The client needs
to request subsequent notifications (if any) using the new vStdio packet
(that works pretty much like vStopped for the Stop notification queue).

The packet handler in lldb-server tests is updated to handle the async
stdio packets in addition to the regular O packets. However, due
to the implications noted above, it can only handle the first output
packet sent by the server. Subsequent packets need to be explicitly
requested via vStdio.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128849

show more ...

Reland "[lldb] [llgs] Fix multi-resume bugs with nonstop mode"

Improve handling of multiple successive continue packets in non-stop
mode. More specifically:

1. Explicitly send error response (inst

Reland "[lldb] [llgs] Fix multi-resume bugs with nonstop mode"

Improve handling of multiple successive continue packets in non-stop
mode. More specifically:

1. Explicitly send error response (instead of crashing on assertion)
if the user attempts to resume the same process twice. Since we
do not support thread-level non-stop mode, one needs to always stop
the process explicitly before resuming another thread set.

2. Actually stop the process if "vCont;t" is delivered to a running
process. Similarly, we only support stopping all the running threads
simultaneously (via -1) and return an error in any other case.

With this patch, running multiple processes simultaneously is still
unsupported. The patch also employs a hack to avoid enabling stdio
forwarding on "vCont;t" packet. Both of these issues are addressed
by followup patches.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128710

show more ...

Revert "[lldb] [llgs] Fix multi-resume bugs with nonstop mode"

This reverts commit f8605da8758fbae16410e4ed5493a39429fd73ec.
This is causing buildbot failures and now I see that I have not updated
t

Revert "[lldb] [llgs] Fix multi-resume bugs with nonstop mode"

This reverts commit f8605da8758fbae16410e4ed5493a39429fd73ec.
This is causing buildbot failures and now I see that I have not updated
the tests to use "stop" instead of "trap".

show more ...

[lldb] [llgs] Fix multi-resume bugs with nonstop mode

Improve handling of multiple successive continue packets in non-stop
mode. More specifically:

1. Explicitly send error response (instead of cr

[lldb] [llgs] Fix multi-resume bugs with nonstop mode

Improve handling of multiple successive continue packets in non-stop
mode. More specifically:

1. Explicitly send error response (instead of crashing on assertion)
if the user attempts to resume the same process twice. Since we
do not support thread-level non-stop mode, one needs to always stop
the process explicitly before resuming another thread set.

2. Actually stop the process if "vCont;t" is delivered to a running
process. Similarly, we only support stopping all the running threads
simultaneously (via -1) and return an error in any other case.

With this patch, running multiple processes simultaneously is still
unsupported. The patch also employs a hack to avoid enabling stdio
forwarding on "vCont;t" packet. Both of these issues are addressed
by followup patches.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128710

show more ...

[lldb] [llgs] Remove not-really-used m_inferior_prev_state

Remove m_inferior_prev_state that's not suitable for multiprocess
debugging and that does not seem to be really used at all.

The only use

[lldb] [llgs] Remove not-really-used m_inferior_prev_state

Remove m_inferior_prev_state that's not suitable for multiprocess
debugging and that does not seem to be really used at all.

The only use of the variable right now is to "prevent" sending the stop
reason after attach/launch. However, this code is never actually run
since none of the process plugins actually use eStateLaunching or
eStateAttaching. Through adding an assert, I've confirmed that it's
never hit in any of the LLDB tests or while attaching/launching debugged
process via lldb-server and via lldb CLI.

Differential Revision: https://reviews.llvm.org/D128878
Sponsored by: The FreeBSD Foundation

show more ...

[lldb] [llgs] Convert m_debugged_processes into a map of structs

Convert the m_debugged_processes map from NativeProcessProtocol pointers
to structs, and combine the additional set(s) holding the ad

[lldb] [llgs] Convert m_debugged_processes into a map of structs

Convert the m_debugged_processes map from NativeProcessProtocol pointers
to structs, and combine the additional set(s) holding the additional
process properties into a flag field inside this struct. This is
desirable since there are more properties to come and having a single
structure with all information should be cleaner and more efficient than
using multiple sets for that.

Suggested by Pavel Labath in D128893.

Differential Revision: https://reviews.llvm.org/D129652

show more ...

[lldb] [llgs] Fix premature server exit if multiprocess+nonstop

Fix lldb-server in the non-stop + multiprocess mode to exit on vStopped
only if all processes have exited, rather than when the first

[lldb] [llgs] Fix premature server exit if multiprocess+nonstop

Fix lldb-server in the non-stop + multiprocess mode to exit on vStopped
only if all processes have exited, rather than when the first one exits.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128639

show more ...

[lldb] Add a NativeProcessProtocol::Threads() iterable

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128698

[lldb] [llgs] Support "t" vCont action

Implement support for the "t" action that is used to stop a thread.
Normally this action is used only in non-stop mode. However, there's
no technical reason w

[lldb] [llgs] Support "t" vCont action

Implement support for the "t" action that is used to stop a thread.
Normally this action is used only in non-stop mode. However, there's
no technical reason why it couldn't be also used in all-stop mode,
e.g. to express "resume all threads except ..." (`t:...;c`).

While at it, add a more complete test for vCont correctly resuming
a subset of program's threads.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D126983

show more ...