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
The mock server was listening for only one packet (I forgot to put a loop around
it), which caused the client to stall in debug builds, as the timeout there is
1000 seconds. In case of a release builds the test would just silently succeed as
the tested function does not check or report errors (which should be fixed).
This fixes the test by adding the server loop. Since the test was taking quite a
long time now (8s), I have added a parameter to control the amount of data sent
(default 4MB), and call it with a smaller value in the test, to make the test run
faster.
llvm-svn: 285992
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
Where possible, remove the const char* version. To keep the
risk and impact here minimal, I've only done the simplest
functions.
In the process, I found a few opportunities for adding some
unit tests, so I added those as well.
Tested on Windows, Linux, and OSX.
llvm-svn: 281799
Summary:
It fixes the following compile warnings:
1. '0' flag ignored with precision and ‘%d’ gnu_printf format
2. enumeral and non-enumeral type in conditional expression
3. format ‘%d’ expects argument of type ‘int’, but argument 4 has type ...
4. enumeration value ‘...’ not handled in switch
5. cast from type ‘const uint64_t* {aka ...}’ to type ‘int64_t* {aka ...}’ casts away qualifiers
6. extra ‘;’
7. comparison between signed and unsigned integer expressions
8. variable ‘register_operand’ set but not used
9. control reaches end of non-void function
Reviewers: jingham, emaste, zturner, clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D24331
llvm-svn: 281191
Summary:
This adds the jModulesInfo packet, which is the equivalent of qModulesInfo, but it enables us to
query multiple modules at once. This makes a significant speed improvement in case the
application has many (over a hundred) modules, and the communication link has a non-negligible
latency. This functionality is accessed by ProcessGdbRemote::PrefetchModuleSpecs(), which does
the caching. GetModuleSpecs() is modified to first consult the cache before asking the remote
stub. PrefetchModuleSpecs is currently only called from POSIX-DYLD dynamic loader plugin, after
it reads the list of modules from the inferior memory, but other uses are possible.
This decreases the attach time to an android application by about 40%.
Reviewers: clayborg
Subscribers: tberghammer, lldb-commits, danalbert
Differential Revision: https://reviews.llvm.org/D24236
llvm-svn: 280919
*** 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 rewrite of StringExtractor::GetHexMaxU32 changes functionality in a way which makes
lldb-server crash. The crash (assert) happens when parsing the "qRegisterInfo0" packet, because
the function tries to drop_front more bytes than the packet contains. It's not clear to me
whether we should consider this a bug in the caller or the callee, but it any case, it worked
before, so I am reverting this until we can figure out what the proper interface should be.
llvm-svn: 280207
Makes Peek() return a StringRef instead of a const char*.
This leads to a few callers of Peek() being able to be made a
little nicer (for example using StringRef member functions instead
of c-style strncmp and related functions) and generally safer
usage.
llvm-svn: 280139
MutableArrayRef<T> is essentially a safer version of passing around
(T*, length) pairs and provides some convenient functions for working
with the data without having to manually manipulate indices.
This is a minor NFC.
llvm-svn: 280123
StringExtractor::GetNameColonValue() looks for a substring of the
form "<name>:<value>" and returns <name> and <value> to the caller.
This results in two unnecessary string copies, since the name and
value are not translated in any way and simply returned as-is.
By converting this to return StringRefs we can get rid of hundreds
of string copies.
llvm-svn: 280000
Summary:
This is a preparatory commit for D22914, where I'd like to replace this mutex by an R/W lock
(which is also not recursive). This required a couple of changes:
- The only caller of Read/WriteRegister, GDBRemoteRegisterContext class, was already acquiring
the mutex, so these functions do not need to. All functions which now do not take a lock, take
an lock argument instead, to remind the caller of this fact.
- GetThreadSuffixSupported() was being called from locked and unlocked contexts (including
contexts where the process was running, and the call would fail if it did not have the result
cached). I have split this into two functions, one which computes the thread suffix support and
caches it (this one always takes the lock), and another, which returns the cached value (and
never needs to take the lock). This feels quite natural as ProcessGdbRemote was already
pre-caching this value at the start.
Reviewers: clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D23802
llvm-svn: 279725
Summary:
The tricky part here was that the exisiting implementation of WriteAllRegisters was expecting
hex-encoded data (as that was what the first implementation I replaced was using, but here we had
binary data to begin with. I thought the read/write register functions would be more useful if
they handled the hex-encoding themselves (all the other client functions provide the responses in
a more-or-less digested form). The read functions return a DataBuffer, so they can allocate as
much memory as they need to, while the write functions functions take an llvm::ArrayRef, as that
can be constructed from pretty much anything.
Reviewers: clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D23659
llvm-svn: 279232
Take 2, with missing cmake line fixed. Build tested on
Ubuntu 14.04 with clang-3.6.
See docs/structured_data/StructuredDataPlugins.md for details.
differential review: https://reviews.llvm.org/D22976
reviewers: clayborg, jingham
llvm-svn: 279202
Summary:
Before this, each function had a copy of the code which handled appending of the thread suffix to
the packet (or using $Hg instead). I have moved that code into a single function and made
everyone else use that. The function takes the partial packet as a StreamString rvalue reference,
to avoid a copy and to remind the users that the packet will have undeterminate contents after
the call.
This also fixes the incorrect formatting of the QRestoreRegisterState packet in case thread
suffix is not supported.
Reviewers: clayborg
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D23604
llvm-svn: 279040
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
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
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
"Incorrect" file name seen on Android whene the main executable is
called "app_process32" (or 64) but the linker specifies the package
name (e.g. com.android.calculator2). Additionally it can be present
in case of some linker bugs.
This CL adds logic to try to fetch the correct file name from the proc
file system based on the base address sepcified by the linker in case
we are failed to load the module by name.
Differential revision: http://reviews.llvm.org/D22219
llvm-svn: 276411
debugserver jGetSharedCacheInfo packet instead of reading
the dyld internal data structures directly. This code is
(currently) only used for ios native lldb's - I should really
move this ObjectFileMachO::GetProcessSharedCacheUUID method
somewhere else, it makes less and less sense being in the
file reader.
<rdar://problem/25251243>
llvm-svn: 276369
Summary:
This patch fills in the implementation of GetMemoryRegions() on the Linux and Mac OS core file implementations of lldb_private::Process (ProcessElfCore::GetMemoryRegions and ProcessMachCore::GetMemoryRegions.) The GetMemoryRegions API was added under: http://reviews.llvm.org/D20565
The patch re-uses the m_core_range_infos list that was recently added to implement GetMemoryRegionInfo in both ProcessElfCore and ProcessMachCore to ensure the returned regions match the regions returned by Process::GetMemoryRegionInfo(addr_t load_addr, MemoryRegionInfo ®ion_info).
Reviewers: clayborg
Subscribers: labath, lldb-commits
Differential Revision: http://reviews.llvm.org/D21751
llvm-svn: 274741
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
os to "ios" or "macosx" if it is unspecified. For environments
where there genuinely is no os, we don't want to errantly
convert that to ios/macosx, e.g. bare board debugging.
Change PlatformRemoteiOS, PlatformRemoteAppleWatch, and
PlatformRemoteAppleTV to not create themselves if we have
an unspecified OS. Same problem - these are not appropriate
platforms for bare board debugging environments.
Have Process::Attach's logging take place if either
process or target logging is enabled.
<rdar://problem/25592378>
llvm-svn: 265732
rnb_err_t
RNBRemote::HandlePacket_stop_process (const char *p)
{
if (!DNBProcessInterrupt(m_ctx.ProcessID()))
HandlePacket_last_signal (NULL);
return rnb_success;
}
In the call to DNBProcessInterrupt we did:
nub_bool_t
DNBProcessInterrupt(nub_process_t pid)
{
MachProcessSP procSP;
if (GetProcessSP (pid, procSP))
return procSP->Interrupt();
return false;
}
This would always return false. It would cause HandlePacket_stop_process to always call "HandlePacket_last_signal (NULL);" which would send an extra stop reply packet _if_ the process is stopped. On a machine with enough cores, it would call DNBProcessInterrupt(...) and then HandlePacket_last_signal(NULL) so quickly that it will never send out an extra stop reply packet. But if the machine is slow enough or doesn't have enough cores, it could cause the call to HandlePacket_last_signal() to actually succeed and send an extra stop reply packet. This would cause problems up in GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse() where it would get the first stop reply packet and then possibly return or execute an async packet. If it returned, then the next packet that was sent will get the second stop reply as its response. If it executes an async packet, the async packet will get the wrong response.
To fix this I did the following:
1 - in debugserver, I fixed "bool MachProcess::Interrupt()" to return true if it sends the signal so we avoid sending the stop reply twice on slower machines
2 - Added a log line to RNBRemote::HandlePacket_stop_process() to say if we ever send an extra stop reply so we will see this in the darwin console output if this does happen
3 - Added response validators to StringExtractorGDBRemote so that we can verify some responses to some packets.
4 - Added validators to packets that often follow stop reply packets like the "m" packet for memory reads, JSON packets since "jThreadsInfo" is often sent immediately following a stop reply.
5 - Modified GDBRemoteCommunicationClient::SendPacketAndWaitForResponseNoLock() to validate responses. Any "StringExtractorGDBRemote &response" that contains a valid response verifier will verify the response and keep looking for correct responses up to 3 times. This will help us get back on track if we do get extra stop replies. If a StringExtractorGDBRemote does not have a response validator, it will accept any packet in response.
6 - In GDBRemoteCommunicationClient::SendPacketAndWaitForResponse we copy the response validator from the "response" argument over into m_async_response so that if we send the packet by interrupting the running process, we can validate the response we actually get in GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse()
7 - Modified GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse() to always check for an extra stop reply packet for 100ms when the process is interrupted. We were already doing this because we might interrupt a process with a \x03 packet, yet the process was in the process of stopping due to another reason. This race condition could cause an extra stop reply packet because the GDB remote protocol says if a \x03 packet is sent while the process is stopped, we should send a stop reply packet back. Now we always check for an extra stop reply packet when we manually interrupt a process.
The issue was showing up when our IDE would attempt to set a breakpoint while the process is running and this would happen:
--> \x03
<-- $T<stop reply 1>
--> z0,AAAAA,BB (set breakpoint)
<-- $T<stop reply 1> (incorrect extra stop reply packet)
--> c
<-- OK (response from z0 packet)
Now all packet traffic was off by one response. Since we now have a validator on the response for "z" packets, we do this:
--> \x03
<-- $T<stop reply 1>
--> z0,AAAAA,BB (set breakpoint)
<-- $T<stop reply 1> (Ignore this because this can't be the response to z0 packets)
<-- OK -- (we are back on track as this is a valid response to z0)
...
As time goes on we should add more packet validators.
<rdar://problem/22859505>
llvm-svn: 265086
to each other. This should remove some infrequent teardown crashes when the
listener is not the debugger's listener.
Processes now need to take a ListenerSP, not a Listener&.
This required changing over the Process plugin class constructors to take a ListenerSP, instead
of a Listener&. Other than that there should be no functional change.
<rdar://problem/24580184> CrashTracer: [USER] Xcode at …ework: lldb_private::Listener::BroadcasterWillDestruct + 39
llvm-svn: 262863
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
This is a packet which allows the remote gdb stub to ask for the address
of a symbol in the process. lldb sends the packet (offering to provide
addresses for symbol names) after every solib loaded. I changed lldb so
that once the stub has indicated that it doesn't need any more symbol
addresses, lldb will stop sending the qSymbol:: packet on new solib loads.
This can yield a performance benefit over slower communication links when
there are many solibs involved.
<rdar://problem/23310049>
llvm-svn: 257569
Summary:
Some debug servers don't support it so there's no point in spamming
this.
Reviewers: clayborg
Subscribers: fjricci, lldb-commits
Differential Revision: http://reviews.llvm.org/D15972
llvm-svn: 257116
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
They get treated as special RLE encoding symbols and packets get
corrupted. Most other packet types already know about this apparently,
but QEnvironment missed these two.
Should fix PR25300.
llvm-svn: 252521
Summary:
Gdb-remote's async thread sent out the eBroadcastBitRunPacketSent message *before* actually
sending out the continue packet. Since it's this message the actually triggers the public state
transition, it could happen (and it did happen in TestAttachResume, which does an "process
interrupt" right after a continue) that we attempt to stop the inferior before it was actually
started (which obviously did not end well). This fixes the problem by moving the broadcast after
the packet was actually sent.
Reviewers: clayborg
Subscribers: lldb-commits
Differential Revision: http://reviews.llvm.org/D14083
llvm-svn: 251399
disabled the use of the jThreadGetExtendedInfo packet which is used
to retrieve additional information about a thread, such as the QoS
setting for that thread on darwin systems.
Re-enable the use of the jThreadGetExtendedInfo packet, and add
some quick tests to the TestQueues mac test case which will verify
that we can retrieve the QoS names for these test threads.
<rdar://problem/22925096>
llvm-svn: 250364
Mehdi Amini