[Lldb-commits] [lldb] Revert "[lldb] Implement basic support for reverse-continue" (PR #123906)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 22 00:43:24 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
<details>
<summary>Changes</summary>
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
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]
``````````
</details>
https://github.com/llvm/llvm-project/pull/123906
More information about the lldb-commits
mailing list