llvm-project/lldb
Greg Clayton 830c81d511 Fixed an issue that could cause debugserver to return two stop reply packets ($T packets) for one \x03 interrupt. The problem was that when a \x03 byte is sent to debugserver while the process is running, and up calling:
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
2016-04-01 00:41:29 +00:00
..
cmake Follow-up for r264162 to fix the CMake build (update LLDBDependencies.cmake). 2016-03-23 16:54:23 +00:00
docs Include -c, -core in the lldb(1) man page 2015-12-16 15:37:21 +00:00
examples Python 3 - modernize exception catching syntax. 2015-11-03 19:49:05 +00:00
include/lldb Figure out what the fixed expression is, and print it. Added another target setting to 2016-03-29 22:00:08 +00:00
lit Rename MSVC top-level folder to avoid name collision. 2016-01-13 22:00:44 +00:00
lldb.xcodeproj Add ClangUtil.cpp to the xcode project 2016-03-29 12:06:37 +00:00
lldb.xcworkspace Working on getting the OSX build green 2015-05-12 02:20:27 +00:00
packages/Python/lldbsuite Don't vary debug info for lldb-server tests 2016-03-31 14:22:52 +00:00
resources Bump the lldb version # in the xcode project files from 2016-03-15 04:36:11 +00:00
scripts Add missing swig wrappers for r264662 2016-03-29 10:41:40 +00:00
source Fixed an issue that could cause debugserver to return two stop reply packets ($T packets) for one \x03 interrupt. The problem was that when a \x03 byte is sent to debugserver while the process is running, and up calling: 2016-04-01 00:41:29 +00:00
test Fix ResourceWarning about unclosed file in use_lldb_suite_root.py. 2016-01-15 22:22:35 +00:00
third_party/Python/module Put progress.py back, apparently this can't be deleted. 2015-12-09 21:32:28 +00:00
tools Fixed an issue that could cause debugserver to return two stop reply packets ($T packets) for one \x03 interrupt. The problem was that when a \x03 byte is sent to debugserver while the process is running, and up calling: 2016-04-01 00:41:29 +00:00
unittests Fix SocketAddressTest (again) 2016-03-30 09:43:04 +00:00
utils Python 3: Modernize exception raising syntax. 2015-11-03 21:01:45 +00:00
www Update the website with lots of new info about building / testing. 2016-02-24 22:19:23 +00:00
.arcconfig
.clang-format Update .clang-format file to support break after return type. 2015-12-28 22:09:29 +00:00
.gitignore modify Xcode build to use cmake/ninja for internal llvm/clang 2016-01-28 07:36:44 +00:00
CMakeLists.txt Fix CMake dependency on lldb.py 2015-11-18 21:09:55 +00:00
CODE_OWNERS.txt Added myself to the CODE_OWNERS.txt list for a few subsystems. 2015-11-09 01:24:36 +00:00
INSTALL.txt Revert the patch to Test Commit Access 2015-07-06 11:26:51 +00:00
LICENSE.TXT
use_lldb_suite_root.py Preparation for turning lldbsuite into a Python package. 2015-10-27 22:33:47 +00:00