Skip to content

Revert "[lldb] Implement basic support for reverse-continue" #123906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

labath
Copy link
Collaborator

@labath labath commented Jan 22, 2025

Reverts #112079 due to failures on the arm bot.

@labath labath requested a review from JDevlieghere as a code owner January 22, 2025 08:42
@llvmbot llvmbot added the lldb label Jan 22, 2025
@labath labath merged commit 22561cf into main Jan 22, 2025
7 of 8 checks passed
@llvmbot
Copy link
Member

llvmbot commented Jan 22, 2025

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

Reverts llvm/llvm-project#112079 due to failures on the arm bot.


Patch is 84.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/123906.diff

40 Files Affected:

  • (modified) lldb/include/lldb/API/SBProcess.h (-1)
  • (modified) lldb/include/lldb/Target/Process.h (+2-26)
  • (modified) lldb/include/lldb/Target/StopInfo.h (-7)
  • (modified) lldb/include/lldb/Target/Thread.h (+5-4)
  • (modified) lldb/include/lldb/Target/ThreadList.h (+1-5)
  • (modified) lldb/include/lldb/Target/ThreadPlan.h (-13)
  • (modified) lldb/include/lldb/Target/ThreadPlanBase.h (-2)
  • (modified) lldb/include/lldb/lldb-enumerations.h (-6)
  • (modified) lldb/packages/Python/lldbsuite/test/gdbclientutils.py (+2-3)
  • (removed) lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py (-175)
  • (removed) lldb/packages/Python/lldbsuite/test/lldbreverse.py (-492)
  • (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (-2)
  • (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py (+5-9)
  • (modified) lldb/source/API/SBProcess.cpp (-12)
  • (modified) lldb/source/API/SBThread.cpp (-2)
  • (modified) lldb/source/Interpreter/CommandInterpreter.cpp (+1-2)
  • (modified) lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp (-3)
  • (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (+1-7)
  • (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h (+1-1)
  • (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp (+1-8)
  • (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h (+1-1)
  • (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (-20)
  • (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (-6)
  • (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (-1)
  • (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+13-85)
  • (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+1-3)
  • (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp (+2-7)
  • (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.h (+1-1)
  • (modified) lldb/source/Target/Process.cpp (+4-20)
  • (modified) lldb/source/Target/StopInfo.cpp (-28)
  • (modified) lldb/source/Target/Thread.cpp (+3-6)
  • (modified) lldb/source/Target/ThreadList.cpp (+3-29)
  • (modified) lldb/source/Target/ThreadPlanBase.cpp (-4)
  • (removed) lldb/test/API/functionalities/reverse-execution/Makefile (-3)
  • (removed) lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py (-149)
  • (removed) lldb/test/API/functionalities/reverse-execution/TestReverseContinueNotSupported.py (-31)
  • (removed) lldb/test/API/functionalities/reverse-execution/TestReverseContinueWatchpoints.py (-130)
  • (removed) lldb/test/API/functionalities/reverse-execution/main.c (-25)
  • (modified) lldb/tools/lldb-dap/JSONUtils.cpp (-3)
  • (modified) lldb/tools/lldb-dap/LLDBUtils.cpp (-1)
diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h
index 882b8bd837131d..1624e02070b1b2 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -159,7 +159,6 @@ class LLDB_API SBProcess {
   lldb::SBError Destroy();
 
   lldb::SBError Continue();
-  lldb::SBError ContinueInDirection(lldb::RunDirection direction);
 
   lldb::SBError Stop();
 
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index b14eb3fbd91d00..a184e6dd891aff 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1089,13 +1089,6 @@ 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.
   ///
@@ -1111,13 +1104,9 @@ class Process : public std::enable_shared_from_this<Process>,
   /// \see Thread:Resume()
   /// \see Thread:Step()
   /// \see Thread:Suspend()
-  virtual Status DoResume(lldb::RunDirection direction) {
-    if (direction == lldb::RunDirection::eRunForward)
-      return Status::FromErrorStringWithFormatv(
-          "error: {0} does not support resuming processes", GetPluginName());
+  virtual Status DoResume() {
     return Status::FromErrorStringWithFormatv(
-        "error: {0} does not support reverse execution of processes",
-        GetPluginName());
+        "error: {0} does not support resuming processes", GetPluginName());
   }
 
   /// Called after resuming a process.
@@ -2687,18 +2676,6 @@ 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;
 
@@ -3098,7 +3075,6 @@ 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
diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h
index 9a13371708be52..45beac129e86f7 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -20,7 +20,6 @@ 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
@@ -155,12 +154,6 @@ 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);
diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index cd82ee7d756030..ef66fa11574db9 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -200,13 +200,14 @@ 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 to determine whether the thread needs to
-  // step over a breakpoint and if so, push a step-over-breakpoint thread
-  // plan.
+  /// 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.
   ///
   /// \return
   ///    True if we pushed a ThreadPlanStepOverBreakpoint
-  bool SetupToStepOverBreakpointIfNeeded(lldb::RunDirection direction);
+  bool SetupForResume();
 
   // Do not override this function, it is for thread plan logic only
   bool ShouldResume(lldb::StateType resume_state);
diff --git a/lldb/include/lldb/Target/ThreadList.h b/lldb/include/lldb/Target/ThreadList.h
index c796975de60153..f931bb83a8ceaf 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/lldb/include/lldb/Target/ThreadList.h
@@ -115,10 +115,6 @@ 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,
@@ -126,7 +122,7 @@ class ThreadList : public ThreadCollection {
   ///    the process will not actually run.  The thread must then return
   ///    the correct StopInfo when asked.
   ///
-  bool WillResume(lldb::RunDirection &direction);
+  bool WillResume();
 
   void DidResume();
 
diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h
index a7bac8cc5ecf6c..d6da484f4fc137 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -283,15 +283,6 @@ 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 {
@@ -506,10 +497,6 @@ 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,
diff --git a/lldb/include/lldb/Target/ThreadPlanBase.h b/lldb/include/lldb/Target/ThreadPlanBase.h
index f4418d779a4dab..5c44b9fb17b271 100644
--- a/lldb/include/lldb/Target/ThreadPlanBase.h
+++ b/lldb/include/lldb/Target/ThreadPlanBase.h
@@ -38,8 +38,6 @@ 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;
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 5f12e648684d7f..50d2233509de6f 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -135,9 +135,6 @@ FLAGS_ENUM(LaunchFlags){
 /// Thread Run Modes.
 enum RunMode { eOnlyThisThread, eAllThreads, eOnlyDuringStepping };
 
-/// Execution directions
-enum RunDirection { eRunForward, eRunReverse };
-
 /// Byte ordering definitions.
 enum ByteOrder {
   eByteOrderInvalid = 0,
@@ -257,9 +254,6 @@ 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.
diff --git a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
index 732d6171320680..1784487323ad6b 100644
--- a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
+++ b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py
@@ -510,9 +510,8 @@ def start(self):
         self._thread.start()
 
     def stop(self):
-        if self._thread is not None:
-            self._thread.join()
-            self._thread = None
+        self._thread.join()
+        self._thread = None
 
     def get_connect_address(self):
         return self._socket.get_connect_address()
diff --git a/lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py b/lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py
deleted file mode 100644
index a84c80f155a0a4..00000000000000
--- a/lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py
+++ /dev/null
@@ -1,175 +0,0 @@
-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
diff --git a/lldb/packages/Python/lldbsuite/test/lldbreverse.py b/lldb/packages/Python/lldbsuite/test/lldbreverse.py
deleted file mode 100644
index bf95667f0094d8..00000000000000
--- a/lldb/packages/Python/lldbsuite/test/lldbreverse.py
+++ /dev/null
@@ -1,492 +0,0 @@
-import os
-import os.path
-import lldb
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test.gdbclientutils import *
-from lldbsuite.test.lldbgdbproxy import *
-import lldbgdbserverutils
-import re
-
-
-class ThreadSnapshot:
-    def __init__(self, thread_id, registers):
-        self.thread_id = thread_id
-        self.registers = registers
-
-
-class MemoryBlockSnapshot:
-    def __init__(self, address, data):
-        self.address = address
-        self.data = data
-
-
-class StateSnapshot:
-    def __init__(self, thread_snapshots, memory):
-        self.thread_snapshots = thread_snapshots
-        self.memory = memory
-        self.thread_id = None
-
-
-class RegisterInfo:
-    def __init__(self, lldb_index, bitsize, little_endian):
-        self.lldb_index = lldb_index
-        self.bitsize = bitsize
-        self.little_endian = little_endian
-
-
-BELOW_STACK_POINTER = 16384
-ABOVE_STACK_POINTER = 4096
-
-BLOCK_SIZE = 1024
-
-SOFTWARE_BREAKPOINTS = 0
-HARDWARE_BREAKPOINTS = 1
-WRITE_WATCHPOINTS = 2
-
-
-class ReverseTestBase(GDBProxyTestBase):
-    """
-    Base class for tests that need reverse execution.
-
-    This class uses a gdbserver proxy to add very limited reverse-
-    execution capability to lldb-server/debugserver for testing
-    purposes only.
-
-    To use this class, run the inferior forward until some stopping point.
-    Then call `start_recording()` and execute forward again until reaching
-    a software breakpoint; this class records the state before each execution executes.
-    At that point, the server will accept "bc" and "bs" packets to step
-    backwards through the state.
-    When executing during recording, we only allow single-step and continue without
-    delivering a signal, and only software breakpoint stops are allowed.
-
-    We assume that while recording is enabled, the only effects of instructions
-    are on general-purpose registers (read/written by the 'g' and 'G' packets)
-    and on memory bytes between [SP - BELOW_STACK_POINTER, SP + ABOVE_STACK_POINTER).
-    """
-
-    NO_DEBUG_INFO_TESTCASE = True
-
-    """
-    A list of StateSnapshots in time order.
-
-    There is one snapshot per single-stepped instruction,
-    representing the state before that instruction was
-    executed. The last snapshot in the list is the
-    snapshot before the last instruction was executed.
-    This is an undo log; we snapshot a superset of the state that may have
-    been changed by the instruction's execution.
-    """
-    snapshots = None
-    recording_enabled = False
-
-    breakpoints = None
-
-    pc_register_info = None
-    sp_register_info = None
-    general_purpose_register_info = None
-
-    def __init__(self, *args, **kwargs):
-        GDBProxyTestBase.__init__(self, *args, **kwargs)
-        self.breakpoints = [set(), set(), set(), set(), set()]
-
-    def respond(self, packet):
-        if not packet:
-            raise ValueError("Invalid empty packet")
-        if packet == self.server.PACKET_INTERRUPT:
-            # Don't send a response. We'll just run to completion.
-            return []
-        if self.is_com...
[truncated]

@labath labath deleted the revert-112079-reverse-continue branch January 22, 2025 08:43
DavidSpickett added a commit to DavidSpickett/llvm-project that referenced this pull request Jan 22, 2025
…3906)"

This reverts commit 22561cf and fixes
b7b9ccf (llvm#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 and constrain
the read to within that region. Since we know the stack is all readable
and writeable.
DavidSpickett added a commit that referenced this pull request Jan 30, 2025
…" (#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]>
DavidSpickett added a commit that referenced this pull request Jan 30, 2025
…123906)"" (#125091)

Reverts #123945

Has failed on the Windows on Arm buildbot:
https://lab.llvm.org/buildbot/#/builders/141/builds/5865
```
********************
Unresolved Tests (2):
  lldb-api :: functionalities/reverse-execution/TestReverseContinueBreakpoints.py
  lldb-api :: functionalities/reverse-execution/TestReverseContinueWatchpoints.py
********************
Failed Tests (1):
  lldb-api :: functionalities/reverse-execution/TestReverseContinueNotSupported.py
```
Reverting while I reproduce locally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants