Rewrite the flaky test_restart_bug test in a more deterministic way

[labath]

Summary

The test was trying to reproduce a bug in handling of two concurrent
events, which was impossible to do reliably in a black-box style test.
In practice, this meant the test was only ever failing on remote
targets, as these were slow enough to trigger this.

Fortunately, we now have the ability to mock the server side of the
connection, which means we can simulate the failure deterministically,
so I rewrite the test to use the new gdb-client framework.

I've needed to add a couple of new packets to the mock server to be able
to do this. Instead of trying to guess how a "typical" gdb-client test
will want to handle this, I throw an exception in the implementation to
force the user to override them (the packets are only sent if the test
explicitly performs some action which will trigger them, so a basic test
which e.g. does not need the "continue" functionality will not need to
implement them).

[owenpshaw]

Looks good to me

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestRestartBug.py

+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestRestartBug(GDBRemoteTestBase):
+
+    @expectedFailureAll(bugnumber="llvm.org/pr24530")
+    def test(self):
+        """
+        Test auto-continue behavior when a process is interrupted to deliver
+        an "asynchronous" packet. This simulates the situation when a process
+        stops on its own just as lldb client is about to interrupt it. The
+        client should not auto-continue in this case, unless the user has
+        explicitly requested that we ignore signals of this type.
+        """
+        class MyResponder(MockGDBServerResponder):
+            continueCount = 0
+
+            def setBreakpoint(self, packet):
+                return "OK"
+
+            def interrupt(self):
+                # Simulate process stopping due to a raise(SIGINT) just as lldb
+                # is about to interrupt it.
+                return "T02reason:signal"
+
+            def cont(self):
+                self.continueCount += 1
+                if self.continueCount == 1:
+                    # No response, wait for the client to interrupt us.
+                    return None
+                return "W00" # Exit
+
+        self.server.responder = MyResponder()
+        target = self.createTarget("a.yaml")
+        process = self.connect(target)
+        self.dbg.SetAsync(True)
+        process.Continue()
+
+        # resume the process and immediately try to set another breakpoint. When using the remote
+        # stub, this will trigger a request to stop the process.  Make sure we
+        # do not lose this signal.
+        bkpt = target.BreakpointCreateByAddress(0x1234)
+        self.assertTrue(bkpt.IsValid())
+        self.assertEqual(bkpt.GetNumLocations(), 1)
+
+        event = lldb.SBEvent()
+        while self.dbg.GetListener().WaitForEvent(2, event):
+            if self.TraceOn():
+                print("Process changing state to:",
+                    self.dbg.StateAsCString(process.GetStateFromEvent(event)))
+            if process.GetStateFromEvent(event) == lldb.eStateExited:
+                break
+
+        # We should get only one continue packet as the client should not
+        # auto-continue after setting the breakpoint.
+        self.assertEqual(self.server.responder.continueCount, 1)
+        # And the process should end up in the stopped state.
+        self.assertEqual(process.GetState(), lldb.eStateStopped)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py

def __init__(self):
         self.packetLog = []
 
     def respond(self, packet):
         """
         Return the unframed packet data that the server should issue in response
         to the given packet received from the client.
         """
         self.packetLog.append(packet)
+        if packet is MockGDBServer.PACKET_INTERRUPT:
+            return self.interrupt()
+        if packet == "c":
+            return self.cont()
         if packet == "g":
             return self.readRegisters()
         if packet[0] == "G":
             return self.writeRegisters(packet[1:])
         if packet[0] == "p":
             return self.readRegister(int(packet[1:], 16))
         if packet[0] == "P":
             register, value = packet[1:].split("=")
def respond(self, packet):
             return self.selectThread(packet[1], int(packet[2:], 16))
         if packet[0:6] == "qXfer:":
             obj, read, annex, location = packet[6:].split(":")
             offset, length = [int(x, 16) for x in location.split(',')]
             data, has_more = self.qXferRead(obj, annex, offset, length)
             if data is not None:
                 return self._qXferResponse(data, has_more)
             return ""
+        if packet[0] == "Z":
+            return self.setBreakpoint(packet)
         return self.other(packet)
 
+    def interrupt(self):
+        raise self.UnexpectedPacketException()
+
+    def cont(self):
+        raise self.UnexpectedPacketException()
+
     def readRegisters(self):
         return "00000000" * self.registerCount
 
     def readRegister(self, register):
         return "00000000"
 
     def writeRegisters(self, registers_hex):
         return "OK"
def qXferRead(self, obj, annex, offset, length):
         return None, False
 
     def _qXferResponse(self, data, has_more):
         return "%s%s" % ("m" if has_more else "l", escape_binary(data))
 
     def selectThread(self, op, thread_id):
         return "OK"
 
+    def setBreakpoint(self, packet):
+        raise self.UnexpectedPacketException()
+
     def other(self, packet):
         # empty string means unsupported
         return ""
 
+    """
+    Raised when we receive a packet for which there is no default action.
+    Override the responder class to implement behavior suitable for the test at
+    hand.
+    """
+    class UnexpectedPacketException(Exception):
+        pass
+
 
 class MockGDBServer:
     """
     A simple TCP-based GDB server that can test client behavior by receiving
     commands and issuing custom-tailored responses.
 
     Responses are generated via the .responder property, which should be an
     instance of a class based on MockGDBServerResponder.
def _parsePacket(self):
             return None
         if i == 0:
             # If we're looking at the start of the received data, that means
             # we're looking for the start of a new packet, denoted by a $.
             # It's also possible we'll see an ACK here, denoted by a +
             if data[0] == '+':
                 self._receivedData = data[1:]
                 return self.PACKET_ACK
+            if ord(data[0]) == 3:
+                self._receivedData = data[1:]
+                return self.PACKET_INTERRUPT
             if data[0] == '$':
                 i += 1
             else:
                 raise self.InvalidPacketException(
                         "Unexexpected leading byte: %s" % data[0])
 
         # If we're looking beyond the start of the received data, then we're
         # looking for the end of the packet content, denoted by a #.
def _handlePacket(self, packet):
             self._client.sendall('+')
         if packet == "QStartNoAckMode":
             self._shouldSendAck = False
             response = "OK"
         elif self.responder is not None:
             # Delegate everything else to our responder
             response = self.responder.respond(packet)
         # Handle packet framing since we don't want to bother tests with it.
+        if response is not None:
             framed = frame_packet(response)
             self._client.sendall(framed)
 
     PACKET_ACK = object()
+    PACKET_INTERRUPT = object()
 
     class InvalidPacketException(Exception):
         pass
 
 
 class GDBRemoteTestBase(TestBase):
     """
     Base class for GDB client tests.
def connect(self, target):
         Includes assertions that the process was successfully created.
         """
         listener = self.dbg.GetListener()
         error = lldb.SBError()
         url = "connect://localhost:%d" % self.server.port
         process = target.ConnectRemote(listener, url, "gdb-remote", error)
         self.assertTrue(error.Success(), error.description)
         self.assertTrue(process, PROCESS_IS_VALID)
+        return process
 
     def assertPacketLogContains(self, packets):
         """
         Assert that the mock server's packet log contains the given packets.
 
         The packet log includes all packets sent by the client and received
         by the server.  This fuction makes it easy to verify that the client
         sent the expected packets to the server.

lldb/trunk/packages/Python/lldbsuite/test/functionalities/signal/raise/TestRaise.py

def signal_test(self, signal, test_passing):
             "process continue",
             substrs=["stopped and restarted"],
             matching=False)
         self.assertEqual(process.GetState(), lldb.eStateExited)
         self.assertEqual(process.GetExitStatus(), signo)
 
         # reset signal handling to default
         self.set_handle(signal, default_pass, default_stop, default_notify)
-
-    @skipIfTargetAndroid()
-    def test_restart_bug(self):
-        """Test that we catch a signal in the edge case where the process receives it while we are
-        about to interrupt it"""
-        self.build()
-        exe = self.getBuildArtifact("a.out")
-
-        # Create a target by the debugger.
-        target = self.dbg.CreateTarget(exe)
-        self.assertTrue(target, VALID_TARGET)
-        bkpt = target.BreakpointCreateByName("main")
-        self.assertTrue(bkpt.IsValid(), VALID_BREAKPOINT)
-
-        # launch the inferior and don't wait for it to stop
-        self.dbg.SetAsync(True)
-        error = lldb.SBError()
-        listener = lldb.SBListener("my listener")
-        process = target.Launch(listener,
-                                ["SIGSTOP"],  # argv
-                                None,        # envp
-                                None,        # stdin_path
-                                None,        # stdout_path
-                                None,        # stderr_path
-                                None,        # working directory
-                                0,           # launch flags
-                                False,       # Stop at entry
-                                error)       # error
-
-        self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
-
-        event = lldb.SBEvent()
-
-        # Give the child enough time to reach the breakpoint,
-        # while clearing out all the pending events.
-        # The last WaitForEvent call will time out after 2 seconds.
-        while listener.WaitForEvent(2, event):
-            if self.TraceOn():
-                print(
-                    "Process changing state to:",
-                    self.dbg.StateAsCString(
-                        process.GetStateFromEvent(event)))
-
-        # now the process should be stopped
-        self.assertEqual(
-            process.GetState(),
-            lldb.eStateStopped,
-            PROCESS_STOPPED)
-        self.assertEqual(len(lldbutil.get_threads_stopped_at_breakpoint(
-            process, bkpt)), 1, "A thread should be stopped at breakpoint")
-
-        # Remove all breakpoints. This makes sure we don't have to single-step over them when we
-        # resume the process below
-        target.DeleteAllBreakpoints()
-
-        # resume the process and immediately try to set another breakpoint. When using the remote
-        # stub, this will trigger a request to stop the process just as it is about to stop
-        # naturally due to a SIGSTOP signal it raises. Make sure we do not lose
-        # this signal.
-        process.Continue()
-        self.assertTrue(target.BreakpointCreateByName(
-            "handler").IsValid(), VALID_BREAKPOINT)
-
-        # Clear the events again
-        while listener.WaitForEvent(2, event):
-            if self.TraceOn():
-                print(
-                    "Process changing state to:",
-                    self.dbg.StateAsCString(
-                        process.GetStateFromEvent(event)))
-
-        # The process should be stopped due to a signal
-        self.assertEqual(process.GetState(), lldb.eStateStopped)
-        thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonSignal)
-        self.assertTrue(
-            thread.IsValid(),
-            "Thread should be stopped due to a signal")
-        self.assertTrue(
-            thread.GetStopReasonDataCount() >= 1,
-            "There was data in the event.")
-        signo = process.GetUnixSignals().GetSignalNumberFromName("SIGSTOP")
-        self.assertEqual(thread.GetStopReasonDataAtIndex(0), signo,
-                         "The stop signal was %s" % signal)
-
-        # We are done
-        process.Kill()