Restore process events after a launch that stopped at the entry point

[jingham]

Summary

We found a bug introduced by changes for the ScriptedProcess. It only manifests itself is you run a process with stop-at-entry set to true and when you are using an async debugger and when you are using a remote process. The solution is trivial. The majority of the patch is a test for stop-at-entry in the four cases of "sync & async" debugger crossed with "remote or local" process. The test both tests that stop-at-entry really does stop at entry, and then tests that we can resume afterwards.

I wrote the test for darwin because I don't know how to assert that we successfully stopped at entry on Linux or Windows. If someone more familiar with those platforms wants to extend the test to either of those hosts, it should be straightforward.

[mib]

LGTM besides some minor things. Thanks for fixing this @jingham !

lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py

86

Did you forget to remove these comments or did you leave them on purpose ?

90

This comment doesn't match the following condition

135

As you mentioned in the FIXME above, this should be a cleanup (similarly to what TestPlatformSDK.py does)

[jingham]

Address Ismail's comments.

[jingham]

Thanks for pointing these out.

[clayborg]

lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py

22

This hard coded port worries me for buildbot flakiness. Do other tests do this?

[jingham]

lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py

22

The test I copied it from does...

[jingham]

lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py

22

That is "TestPlatformSDK.py". I didn't centralize that logic because the two uses were different, and the implementation is not very many lines. If we see flakiness in either of these tests we can centralize this logic and handle the port fetching better. But I'd rather not do that as part of this patch.

[clayborg]

ok, lgtm

lldb/source/Target/Process.cpp

if (error.Success()) {
           // right time to set up signal filters before resuming.
           UpdateAutomaticSignalFiltering();
 
           // Note, the stop event was consumed above, but not handled. This
           // was done to give DidLaunch a chance to run. The target is either
           // stopped or crashed. Directly set the state.  This is done to
           // prevent a stop message with a bunch of spurious output on thread
           // status, as well as not pop a ProcessIOHandler.
+          // We are done with the launch hijack listener, and this stop should
+          // go to the public state listener:
+          RestoreProcessEvents();
           SetPublicState(state, false);
 
           if (PrivateStateThreadIsValid())
             ResumePrivateStateThread();
           else
             StartPrivateStateThread();
 
           // Target was stopped at entry as was intended. Need to notify the

lldb/test/API/functionalities/launch_stop_at_entry/Makefile

+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/functionalities/launch_stop_at_entry/TestStopAtEntry.py

+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbgdbserverutils import get_debugserver_exe
+
+import os
+import platform
+import shutil
+import time
+import socket
+
+
+class TestStopAtEntry(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    # The port used by debugserver.
+    PORT = 54637
+
+    # The number of attempts.
+    ATTEMPTS = 10
+
+    # Time given to the binary to launch and to debugserver to attach to it for
+    # every attempt. We'll wait a maximum of 10 times 2 seconds while the
+    # inferior will wait 10 times 10 seconds.
+    TIMEOUT = 2
+
+    def no_debugserver(self):
+        if get_debugserver_exe() is None:
+            return 'no debugserver'
+        return None
+
+    def port_not_available(self):
+        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        if s.connect_ex(('127.0.0.1', self.PORT)) == 0:
+            return '{} not available'.format(self.PORT)
+        return None
+
+    @skipUnlessDarwin
+    def test_stop_default_platform_sync(self):
+        self.do_test_stop_at_entry(True, False)
+
+    @skipUnlessDarwin
+    def test_stop_default_platform_async(self):
+        self.do_test_stop_at_entry(False, False)
+
+    @skipUnlessDarwin
+    @expectedFailureIfFn(no_debugserver)
+    @expectedFailureIfFn(port_not_available)
+    def test_stop_remote_platform_sync(self):
+        self.do_test_stop_at_entry(True, True)
+
+    @skipUnlessDarwin
+    @expectedFailureIfFn(no_debugserver)
+    @expectedFailureIfFn(port_not_available)
+    def test_stop_remote_platform_async(self):
+        self.do_test_stop_at_entry(False, True)
+
+    def do_test_stop_at_entry(self, synchronous, remote):        
+        """Test the normal launch path in either sync or async mode"""
+        self.build()
+
+        target = lldbutil.run_to_breakpoint_make_target(self)
+        launch_info = target.GetLaunchInfo()
+        launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
+        old_async = self.dbg.GetAsync()
+        def cleanup ():
+            self.dbg.SetAsync(old_async)
+        self.addTearDownHook(cleanup)
+
+        if not synchronous:
+            self.dbg.SetAsync(True)
+            listener = lldb.SBListener("test-process-listener")
+            mask = listener.StartListeningForEventClass(self.dbg, lldb.SBProcess.GetBroadcasterClassName(), lldb.SBProcess.eBroadcastBitStateChanged)
+            self.assertEqual(mask, lldb.SBProcess.eBroadcastBitStateChanged, "Got right mask for listener")
+            launch_info.SetListener(listener)
+        else:
+            self.dbg.SetAsync(False)
+
+        if remote:
+            self.setup_remote_platform()
+
+        error = lldb.SBError()
+
+        process = target.Launch(launch_info, error)
+        self.assertTrue(error.Success(), "Launch failed: {0}".format(error.description))
+        # If we are asynchronous, we have to wait for the events:
+        if not synchronous:
+            listener = launch_info.GetListener()
+            event = lldb.SBEvent()
+            result = listener.WaitForEvent(30, event)
+            self.assertTrue(result, "Timed out waiting for event from process")
+            state = lldb.SBProcess.GetStateFromEvent(event)
+            self.assertEqual(state, lldb.eStateStopped, "Didn't get a stopped state after launch")
+
+        # Okay, we should be stopped.  Make sure we are indeed at the
+        # entry point.  I only know how to do this on darwin:
+        self.assertEqual(len(process.threads), 1, "Should only have one thread at entry")
+        thread = process.threads[0]
+        frame = thread.GetFrameAtIndex(0)
+        stop_func = frame.name
+        self.assertEqual(stop_func, "_dyld_start")
+
+        # Now make sure that we can resume the process and have it exit.
+        error = process.Continue()
+        self.assertTrue(error.Success(), "Error continuing: {0}".format(error.description))
+        # Fetch events till we get eStateExited:
+        if not synchronous:
+            # Get events till exited.
+            listener = launch_info.GetListener()
+            event = lldb.SBEvent()
+            # We get two running events in a row here???  That's a bug
+            # but not the one I'm testing for, so for now just fetch as
+            # many as were sent.
+            num_running = 0
+            state = lldb.eStateRunning
+            while state == lldb.eStateRunning:
+                num_running += 1
+                result = listener.WaitForEvent(30, event)
+                self.assertTrue(result, "Timed out waiting for running")
+                state = lldb.SBProcess.GetStateFromEvent(event)
+                if num_running == 1:
+                    self.assertEqual(state, lldb.eStateRunning, "Got running event")
+            # The last event we should get is the exited event
+            self.assertEqual(state, lldb.eStateExited, "Got running event")
+        else:
+            # Make sure that the process has indeed exited
+            state = process.GetState()
+            self.assertEqual(state, lldb.eStateExited);
+
+    def setup_remote_platform(self):
+        return
+        self.build()
+
+        exe = self.getBuildArtifact('a.out')
+        # Launch our test binary.
+
+        # Attach to it with debugserver.
+        debugserver = get_debugserver_exe()
+        debugserver_args = [
+            'localhost:{}'.format(self.PORT)
+        ]
+        self.spawnSubprocess(debugserver, debugserver_args)
+
+        # Select the platform.
+        self.expect('platform select remote-macosx', substrs=[sdk_dir])
+
+        # Connect to debugserver
+        interpreter = self.dbg.GetCommandInterpreter()
+        connected = False
+        for i in range(self.ATTEMPTS):
+            result = lldb.SBCommandReturnObject()
+            interpreter.HandleCommand('gdb-remote {}'.format(self.PORT),
+                                      result)
+            connected = result.Succeeded()
+            if connected:
+                break
+            time.sleep(self.TIMEOUT)
+
+        self.assertTrue(connected, "could not connect to debugserver")

lldb/test/API/functionalities/launch_stop_at_entry/main.c

+int main(int argc, char **argv) {
+  /* We just want to make sure this ran, so
+     it doesn't actually need to do anything. */
+  return 0;
+}