Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (#123945)

This reverts commit 22561cf and fixes
b7b9ccf (#112079).

The problem is that x86_64 and Arm 32-bit have memory regions above the
stack that are readable but not writeable. First Arm:
```
(lldb) memory region --all
<...>
[0x00000000fffcf000-0x00000000ffff0000) rw- [stack]
[0x00000000ffff0000-0x00000000ffff1000) r-x [vectors]
[0x00000000ffff1000-0xffffffffffffffff) ---
```
Then x86_64:
```
$ cat /proc/self/maps
<...>
7ffdcd148000-7ffdcd16a000 rw-p 00000000 00:00 0                          [stack]
7ffdcd193000-7ffdcd196000 r--p 00000000 00:00 0                          [vvar]
7ffdcd196000-7ffdcd197000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0                  [vsyscall]
```
Compare this to AArch64 where the test did pass:
```
$ cat /proc/self/maps
<...>
ffffb87dc000-ffffb87dd000 r--p 00000000 00:00 0                          [vvar]
ffffb87dd000-ffffb87de000 r-xp 00000000 00:00 0                          [vdso]
ffffb87de000-ffffb87e0000 r--p 0002a000 00:3c 76927217                   /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
ffffb87e0000-ffffb87e2000 rw-p 0002c000 00:3c 76927217                   /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
fffff4216000-fffff4237000 rw-p 00000000 00:00 0                          [stack]
```
To solve this, look up the memory region of the stack pointer (using
https://lldb.llvm.org/resources/lldbgdbremote.html#qmemoryregioninfo-addr)
and constrain the read to within that region. Since we know the stack is
all readable and writeable.

I have also added skipIfRemote to the tests, since getting them working
in that context is too complex to be worth it.

Memory write failures now display the range they tried to write, and
register write errors will show the name of the register where possible.

The patch also includes a workaround for a an issue where the test code
could mistake an `x` response that happens to begin with an `O` for an
output packet (stdout). This workaround will not be necessary one we
start using the [new
implementation](https://discourse.llvm.org/t/rfc-fixing-incompatibilties-of-the-x-packet-w-r-t-gdb/84288)
of the `x` packet.

---------

Co-authored-by: Pavel Labath <[email protected]>

Author: DavidSpickett

lldb/include/lldb/API/SBProcess.h

@@ -159,6 +159,7 @@ class LLDB_API SBProcess {
   lldb::SBError Destroy();
 
   lldb::SBError Continue();
+  lldb::SBError ContinueInDirection(lldb::RunDirection direction);
 
   lldb::SBError Stop();
 

lldb/include/lldb/Target/Process.h

@@ -1089,6 +1089,13 @@ class Process : public std::enable_shared_from_this<Process>,
   ///     Returns an error object.
   virtual Status WillResume() { return Status(); }
 
+  /// Reports whether this process supports reverse execution.
+  ///
+  /// \return
+  ///     Returns true if the process supports reverse execution (at least
+  /// under some circumstances).
+  virtual bool SupportsReverseDirection() { return false; }
+
   /// Resumes all of a process's threads as configured using the Thread run
   /// control functions.
   ///
@@ -1104,9 +1111,13 @@ class Process : public std::enable_shared_from_this<Process>,
   /// \see Thread:Resume()
   /// \see Thread:Step()
   /// \see Thread:Suspend()
-  virtual Status DoResume() {
+  virtual Status DoResume(lldb::RunDirection direction) {
+    if (direction == lldb::RunDirection::eRunForward)
+      return Status::FromErrorStringWithFormatv(
+          "error: {0} does not support resuming processes", GetPluginName());
     return Status::FromErrorStringWithFormatv(
-        "error: {0} does not support resuming processes", GetPluginName());
+        "error: {0} does not support reverse execution of processes",
+        GetPluginName());
   }
 
   /// Called after resuming a process.
@@ -2676,6 +2687,18 @@ void PruneThreadPlans();
                             const AddressRange &range, size_t alignment,
                             Status &error);
 
+  /// Get the base run direction for the process.
+  /// The base direction is the direction the process will execute in
+  /// (forward or backward) if no thread plan overrides the direction.
+  lldb::RunDirection GetBaseDirection() const { return m_base_direction; }
+  /// Set the base run direction for the process.
+  /// As a side-effect, if this changes the base direction, then we
+  /// discard all non-base thread plans to ensure that when execution resumes
+  /// we definitely execute in the requested direction.
+  /// FIXME: this is overkill. In some situations ensuring the latter
+  /// would not require discarding all non-base thread plans.
+  void SetBaseDirection(lldb::RunDirection direction);
+
 protected:
   friend class Trace;
 
@@ -3075,6 +3098,7 @@ void PruneThreadPlans();
   ThreadList
       m_extended_thread_list; ///< Constituent for extended threads that may be
                               /// generated, cleared on natural stops
+  lldb::RunDirection m_base_direction; ///< ThreadPlanBase run direction
   uint32_t m_extended_thread_stop_id; ///< The natural stop id when
                                       ///extended_thread_list was last updated
   QueueList

lldb/include/lldb/Target/StopInfo.h

@@ -20,6 +20,7 @@ namespace lldb_private {
 class StopInfo : public std::enable_shared_from_this<StopInfo> {
   friend class Process::ProcessEventData;
   friend class ThreadPlanBase;
+  friend class ThreadPlanReverseContinue;
 
 public:
   // Constructors and Destructors
@@ -154,6 +155,12 @@ class StopInfo : public std::enable_shared_from_this<StopInfo> {
   static lldb::StopInfoSP
   CreateStopReasonProcessorTrace(Thread &thread, const char *description);
 
+  // This creates a StopInfo indicating that execution stopped because
+  // it was replaying some recorded execution history, and execution reached
+  // the end of that recorded history.
+  static lldb::StopInfoSP
+  CreateStopReasonHistoryBoundary(Thread &thread, const char *description);
+
   static lldb::StopInfoSP CreateStopReasonFork(Thread &thread,
                                                lldb::pid_t child_pid,
                                                lldb::tid_t child_tid);

lldb/include/lldb/Target/Thread.h

@@ -200,14 +200,13 @@ class Thread : public std::enable_shared_from_this<Thread>,
   ///    The User resume state for this thread.
   lldb::StateType GetResumeState() const { return m_resume_state; }
 
-  /// This function is called on all the threads before "ShouldResume" and
-  /// "WillResume" in case a thread needs to change its state before the
-  /// ThreadList polls all the threads to figure out which ones actually will
-  /// get to run and how.
+  // This function is called to determine whether the thread needs to
+  // step over a breakpoint and if so, push a step-over-breakpoint thread
+  // plan.
   ///
   /// \return
   ///    True if we pushed a ThreadPlanStepOverBreakpoint
-  bool SetupForResume();
+  bool SetupToStepOverBreakpointIfNeeded(lldb::RunDirection direction);
 
   // Do not override this function, it is for thread plan logic only
   bool ShouldResume(lldb::StateType resume_state);

lldb/include/lldb/Target/ThreadList.h

@@ -115,14 +115,18 @@ class ThreadList : public ThreadCollection {
   /// If a thread can "resume" without having to resume the target, it
   /// will return false for WillResume, and then the process will not be
   /// restarted.
+  /// Sets *direction to the run direction of the thread(s) that will
+  /// be resumed. If threads that we want to run disagree about the
+  /// direction, we execute forwards and pop any of the thread plans
+  /// that requested reverse execution.
   ///
   /// \return
   ///    \b true instructs the process to resume normally,
   ///    \b false means start & stopped events will be generated, but
   ///    the process will not actually run.  The thread must then return
   ///    the correct StopInfo when asked.
   ///
-  bool WillResume();
+  bool WillResume(lldb::RunDirection &direction);
 
   void DidResume();
 

lldb/include/lldb/Target/ThreadPlan.h

@@ -283,6 +283,15 @@ namespace lldb_private {
 //  report_run_vote argument to the constructor works like report_stop_vote, and
 //  is a way for a plan to instruct a sub-plan on how to respond to
 //  ShouldReportStop.
+//
+//  Reverse execution:
+//
+//  Every thread plan has an associated RunDirection (forward or backward).
+//  For ThreadPlanBase, this direction is the Process's base direction.
+//  Whenever we resume the target, we need to ensure that the topmost thread
+//  plans for each runnable thread all agree on their direction. This is
+//  ensured in ThreadList::WillResume(), which chooses a direction and then
+//  discards thread plans incompatible with that direction.
 
 class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
                    public UserID {
@@ -497,6 +506,10 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
 
   virtual lldb::StateType GetPlanRunState() = 0;
 
+  virtual lldb::RunDirection GetDirection() const {
+    return lldb::RunDirection::eRunForward;
+  }
+
 protected:
   // Constructors and Destructors
   ThreadPlan(ThreadPlanKind kind, const char *name, Thread &thread,

lldb/include/lldb/Target/ThreadPlanBase.h

@@ -38,6 +38,8 @@ class ThreadPlanBase : public ThreadPlan {
 
   bool IsBasePlan() override { return true; }
 
+  lldb::RunDirection GetDirection() const override;
+
 protected:
   bool DoWillResume(lldb::StateType resume_state, bool current_plan) override;
   bool DoPlanExplainsStop(Event *event_ptr) override;

lldb/include/lldb/lldb-enumerations.h

@@ -135,6 +135,9 @@ FLAGS_ENUM(LaunchFlags){
 /// Thread Run Modes.
 enum RunMode { eOnlyThisThread, eAllThreads, eOnlyDuringStepping };
 
+/// Execution directions
+enum RunDirection { eRunForward, eRunReverse };
+
 /// Byte ordering definitions.
 enum ByteOrder {
   eByteOrderInvalid = 0,
@@ -254,6 +257,9 @@ enum StopReason {
   eStopReasonVFork,
   eStopReasonVForkDone,
   eStopReasonInterrupt, ///< Thread requested interrupt
+  // Indicates that execution stopped because the debugger backend relies
+  // on recorded data and we reached the end of that data.
+  eStopReasonHistoryBoundary,
 };
 
 /// Command Return Status Types.

lldb/packages/Python/lldbsuite/test/gdbclientutils.py

@@ -510,8 +510,9 @@ def start(self):
         self._thread.start()
 
     def stop(self):
-        self._thread.join()
-        self._thread = None
+        if self._thread is not None:
+            self._thread.join()
+            self._thread = None
 
     def get_connect_address(self):
         return self._socket.get_connect_address()

lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py

@@ -0,0 +1,175 @@
+import logging
+import os
+import os.path
+import random
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.gdbclientutils import *
+import lldbgdbserverutils
+from lldbsuite.support import seven
+
+
+class GDBProxyTestBase(TestBase):
+    """
+    Base class for gdbserver proxy tests.
+
+    This class will setup and start a mock GDB server for the test to use.
+    It pases through requests to a regular lldb-server/debugserver and
+    forwards replies back to the LLDB under test.
+    """
+
+    """The gdbserver that we implement."""
+    server = None
+    """The inner lldb-server/debugserver process that we proxy requests into."""
+    monitor_server = None
+    monitor_sock = None
+
+    server_socket_class = TCPServerSocket
+
+    DEFAULT_TIMEOUT = 20 * (10 if ("ASAN_OPTIONS" in os.environ) else 1)
+
+    _verbose_log_handler = None
+    _log_formatter = logging.Formatter(fmt="%(asctime)-15s %(levelname)-8s %(message)s")
+
+    def setUpBaseLogging(self):
+        self.logger = logging.getLogger(__name__)
+
+        self.logger.propagate = False
+        self.logger.setLevel(logging.DEBUG)
+
+        # log all warnings to stderr
+        handler = logging.StreamHandler()
+        handler.setLevel(logging.WARNING)
+        handler.setFormatter(self._log_formatter)
+        self.logger.addHandler(handler)
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+        self.setUpBaseLogging()
+
+        if self.isVerboseLoggingRequested():
+            # If requested, full logs go to a log file
+            log_file_name = self.getLogBasenameForCurrentTest() + "-proxy.log"
+            self._verbose_log_handler = logging.FileHandler(log_file_name)
+            self._verbose_log_handler.setFormatter(self._log_formatter)
+            self._verbose_log_handler.setLevel(logging.DEBUG)
+            self.logger.addHandler(self._verbose_log_handler)
+
+        if lldbplatformutil.getPlatform() == "macosx":
+            self.debug_monitor_exe = lldbgdbserverutils.get_debugserver_exe()
+            self.debug_monitor_extra_args = []
+        else:
+            self.debug_monitor_exe = lldbgdbserverutils.get_lldb_server_exe()
+            self.debug_monitor_extra_args = ["gdbserver"]
+        self.assertIsNotNone(self.debug_monitor_exe)
+
+        self.server = MockGDBServer(self.server_socket_class())
+        self.server.responder = self
+
+    def tearDown(self):
+        # TestBase.tearDown will kill the process, but we need to kill it early
+        # so its client connection closes and we can stop the server before
+        # finally calling the base tearDown.
+        if self.process() is not None:
+            self.process().Kill()
+        self.server.stop()
+
+        self.logger.removeHandler(self._verbose_log_handler)
+        self._verbose_log_handler = None
+
+        TestBase.tearDown(self)
+
+    def isVerboseLoggingRequested(self):
+        # We will report our detailed logs if the user requested that the "gdb-remote" channel is
+        # logged.
+        return any(("gdb-remote" in channel) for channel in lldbtest_config.channels)
+
+    def connect(self, target):
+        """
+        Create a process by connecting to the mock GDB server.
+        """
+        self.prep_debug_monitor_and_inferior()
+        self.server.start()
+
+        listener = self.dbg.GetListener()
+        error = lldb.SBError()
+        process = target.ConnectRemote(
+            listener, self.server.get_connect_url(), "gdb-remote", error
+        )
+        self.assertTrue(error.Success(), error.description)
+        self.assertTrue(process, PROCESS_IS_VALID)
+        return process
+
+    def prep_debug_monitor_and_inferior(self):
+        inferior_exe_path = self.getBuildArtifact("a.out")
+        self.connect_to_debug_monitor([inferior_exe_path])
+        self.assertIsNotNone(self.monitor_server)
+        self.initial_handshake()
+
+    def initial_handshake(self):
+        self.monitor_server.send_packet(seven.bitcast_to_bytes("+"))
+        reply = seven.bitcast_to_string(self.monitor_server.get_normal_packet())
+        self.assertEqual(reply, "+")
+        self.monitor_server.send_packet(seven.bitcast_to_bytes("QStartNoAckMode"))
+        reply = seven.bitcast_to_string(self.monitor_server.get_normal_packet())
+        self.assertEqual(reply, "+")
+        reply = seven.bitcast_to_string(self.monitor_server.get_normal_packet())
+        self.assertEqual(reply, "OK")
+        self.monitor_server.set_validate_checksums(False)
+        self.monitor_server.send_packet(seven.bitcast_to_bytes("+"))
+        reply = seven.bitcast_to_string(self.monitor_server.get_normal_packet())
+        self.assertEqual(reply, "+")
+
+    def get_debug_monitor_command_line_args(self, connect_address, launch_args):
+        return (
+            self.debug_monitor_extra_args
+            + ["--reverse-connect", connect_address]
+            + launch_args
+        )
+
+    def launch_debug_monitor(self, launch_args):
+        family, type, proto, _, addr = socket.getaddrinfo(
+            "localhost", 0, proto=socket.IPPROTO_TCP
+        )[0]
+        sock = socket.socket(family, type, proto)
+        sock.settimeout(self.DEFAULT_TIMEOUT)
+        sock.bind(addr)
+        sock.listen(1)
+        addr = sock.getsockname()
+        connect_address = "[{}]:{}".format(*addr)
+
+        commandline_args = self.get_debug_monitor_command_line_args(
+            connect_address, launch_args
+        )
+
+        # Start the server.
+        self.logger.info(f"Spawning monitor {commandline_args}")
+        monitor_process = self.spawnSubprocess(
+            self.debug_monitor_exe, commandline_args, install_remote=False
+        )
+        self.assertIsNotNone(monitor_process)
+
+        self.monitor_sock = sock.accept()[0]
+        self.monitor_sock.settimeout(self.DEFAULT_TIMEOUT)
+        return monitor_process
+
+    def connect_to_debug_monitor(self, launch_args):
+        monitor_process = self.launch_debug_monitor(launch_args)
+        # Turn off checksum validation because debugserver does not produce
+        # correct checksums.
+        self.monitor_server = lldbgdbserverutils.Server(
+            self.monitor_sock, monitor_process
+        )
+
+    def respond(self, packet):
+        """Subclasses can override this to change how packets are handled."""
+        return self.pass_through(packet)
+
+    def pass_through(self, packet):
+        self.logger.info(f"Sending packet {packet}")
+        self.monitor_server.send_packet(seven.bitcast_to_bytes(packet))
+        reply = seven.bitcast_to_string(self.monitor_server.get_normal_packet())
+        self.logger.info(f"Received reply {reply}")
+        return reply