Commit Graph

9 Commits

Author SHA1 Message Date
Todd Fiala fcdb1af655 async structured data packet handling improvements
This change does the following:
* Changes the signature for the continuation delegate method that handles
  async structured data from accepting an already-parsed structured data
  element to taking just the packet contents.
* Moves the conversion of the JSON-async: packet contents from
  GDBRemoteClientBase to the continuation delegate method.
* Adds a new unit test for verifying that the $JSON-asyc: packets get
  decoded and that the decoded packets get forwarded on to the delegate
  for further processing. Thanks to Pavel for making that whole section of
  code easily unit testable!
* Tightens up the packet verification on reception of a $JSON-async:
  packet contents. The code prior to this change is susceptible to a
  segfault if a packet is carefully crafted that starts with $J but
  has a total length shorter than the length of "$JSON-async:".

Reviewers: labath, clayborg, zturner

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

llvm-svn: 281121
2016-09-10 00:06:29 +00:00
Pavel Labath 46031e6fec Fix unittest compilation on windows
After the reformat, the unittests do not compile due to missing due to redefinition errors
between PosixApi.h and ucrt/direct.h. This is a bit of a shot in the dark, as I have not tested
it on windows, but I am restoring the original include order, so it should hopefully fix it.

llvm-svn: 280793
2016-09-07 08:46:50 +00:00
Kate Stone b9c1b51e45 *** This commit represents a complete reformatting of the LLDB source code
*** 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
2016-09-06 20:57:50 +00:00
Todd Fiala 49178e5efe fix broken gdb-remote gtest
This change adds the Process/gdb-remote gtests to the Xcode
build.  It also adds a virtual method impl to the continuation
delegate that I added with the StructuredDataPlugin change.

llvm-svn: 279203
2016-08-19 04:38:44 +00:00
Pavel Labath 56d7262b69 Move packet construction from GDBRemoteRegisterContext go the communication class
Summary:
When saving/restoring registers the GDBRemoteRegisterContext class was manually constructing
the register save/restore packets. This creates appropriate helper functions in
GDBRemoteCommunicationClient, and switches the class to use those. It also removes what a
duplicate packet send in some of those functions, a thing that I can only attribute to a bad
merge artefact.

I also add a test framework for testing gdb-remote client functionality and add tests for the new
functions I introduced. I'd like to be able to test the register context changes in isolation as
well, but currently there doesn't seem to be a way to reasonably construct a standalone register
context object, so we'll have to rely on the end-to-end tests to verify that.

Reviewers: clayborg

Subscribers: lldb-commits

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

llvm-svn: 278915
2016-08-17 08:53:31 +00:00
Pavel Labath 5a123c4e37 Remove GetThreadSuffixSupported from GDBRemoteCommunication **base** class
Despite its comment, the function is only used in the Client class, and its presence was merely
complicating mock implementation in unit tests.

llvm-svn: 278785
2016-08-16 09:36:29 +00:00
Pavel Labath 8c1b6bd7d2 Reapply "Rewrite gdb-remote's SendContinuePacketAndWaitForResponse"
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
2016-08-09 12:04:46 +00:00
Pavel Labath 4cb699260c Revert "Rewrite gdb-remote's SendContinuePacketAndWaitForResponse"
This reverts commit r277139, because:
- broken unittest on windows (likely typo on my part)
- seems to break TestCallThatRestart (needs investigation)

llvm-svn: 277154
2016-07-29 15:41:52 +00:00
Pavel Labath e768c4b858 Rewrite gdb-remote's SendContinuePacketAndWaitForResponse
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
2016-07-29 13:10:02 +00:00