Author: David Spickett Date: 2023-09-20T08:19:53Z New Revision: 1446e3cf7605f0988b914fac0a34d63045394ff3
URL: https://github.com/llvm/llvm-project/commit/1446e3cf7605f0988b914fac0a34d63045394ff3 DIFF: https://github.com/llvm/llvm-project/commit/1446e3cf7605f0988b914fac0a34d63045394ff3.diff LOG: Revert "Fix a bug with cancelling "attach -w" after you have run a process previously (#65822)" This reverts commit 7265f792dc8e1157a3874aee5f8aed6d4d8236e7. The new test case is flaky on Linux AArch64 (https://lab.llvm.org/buildbot/#/builders/96) and more flaky on Windows on Arm (https://lab.llvm.org/buildbot/#/builders/219/builds/5735). Added: Modified: lldb/include/lldb/Interpreter/CommandInterpreter.h lldb/source/Interpreter/CommandInterpreter.cpp lldb/source/Target/Process.cpp lldb/test/API/commands/process/attach/TestProcessAttach.py lldb/test/API/commands/process/attach/main.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 960a1c18f0130ab..747188a15312fa3 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -447,7 +447,7 @@ class CommandInterpreter : public Broadcaster, Debugger &GetDebugger() { return m_debugger; } - ExecutionContext GetExecutionContext(); + ExecutionContext GetExecutionContext() const; lldb::PlatformSP GetPlatform(bool prefer_target_platform); @@ -661,7 +661,7 @@ class CommandInterpreter : public Broadcaster, void GetProcessOutput(); - bool DidProcessStopAbnormally(); + bool DidProcessStopAbnormally() const; void SetSynchronous(bool value); diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 9f1f520abb198f0..dcff53ff843f328 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -2471,7 +2471,7 @@ PlatformSP CommandInterpreter::GetPlatform(bool prefer_target_platform) { return platform_sp; } -bool CommandInterpreter::DidProcessStopAbnormally() { +bool CommandInterpreter::DidProcessStopAbnormally() const { auto exe_ctx = GetExecutionContext(); TargetSP target_sp = exe_ctx.GetTargetSP(); if (!target_sp) @@ -2976,22 +2976,10 @@ void CommandInterpreter::FindCommandsForApropos(llvm::StringRef search_word, m_alias_dict); } -ExecutionContext CommandInterpreter::GetExecutionContext() { - ExecutionContext exe_ctx; - if (!m_overriden_exe_contexts.empty()) { - // During the course of a command, the target may have replaced the process - // coming in with another. I fix that here: - exe_ctx = m_overriden_exe_contexts.top(); - // Don't use HasProcessScope, that returns false if there is a process but - // it's no longer valid, which is one of the cases we want to catch here. - if (exe_ctx.HasTargetScope() && exe_ctx.GetProcessPtr()) { - ProcessSP actual_proc_sp = exe_ctx.GetTargetSP()->GetProcessSP(); - if (actual_proc_sp != exe_ctx.GetProcessSP()) - m_overriden_exe_contexts.top().SetContext(actual_proc_sp); - } - return m_overriden_exe_contexts.top(); - } - return m_debugger.GetSelectedExecutionContext(); +ExecutionContext CommandInterpreter::GetExecutionContext() const { + return !m_overriden_exe_contexts.empty() + ? m_overriden_exe_contexts.top() + : m_debugger.GetSelectedExecutionContext(); } void CommandInterpreter::OverrideExecutionContext( @@ -3184,17 +3172,12 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler, } bool CommandInterpreter::IOHandlerInterrupt(IOHandler &io_handler) { - // InterruptCommand returns true if this is the first time - // we initiate an interrupt for this command. So we give the - // command a chance to handle the interrupt on the first - // interrupt, but if that didn't do anything, a second - // interrupt will do more work to halt the process/interpreter. - if (InterruptCommand()) - return true; - ExecutionContext exe_ctx(GetExecutionContext()); Process *process = exe_ctx.GetProcessPtr(); + if (InterruptCommand()) + return true; + if (process) { StateType state = process->GetState(); if (StateIsRunningState(state)) { diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index d30557312f7a8f9..f82ab05362fbee9 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -3153,14 +3153,6 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) { // case it was already set and some thread plan logic calls halt on its own. m_clear_thread_plans_on_stop |= clear_thread_plans; - if (m_public_state.GetValue() == eStateAttaching) { - // Don't hijack and eat the eStateExited as the code that was doing the - // attach will be waiting for this event... - SetExitStatus(SIGKILL, "Cancelled async attach."); - Destroy(false); - return Status(); - } - ListenerSP halt_listener_sp( Listener::MakeListener("lldb.process.halt_listener")); HijackProcessEvents(halt_listener_sp); @@ -3169,6 +3161,15 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) { SendAsyncInterrupt(); + if (m_public_state.GetValue() == eStateAttaching) { + // Don't hijack and eat the eStateExited as the code that was doing the + // attach will be waiting for this event... + RestoreProcessEvents(); + SetExitStatus(SIGKILL, "Cancelled async attach."); + Destroy(false); + return Status(); + } + // Wait for the process halt timeout seconds for the process to stop. // If we are going to use the run lock, that means we're stopping out to the // user, so we should also select the most relevant frame. diff --git a/lldb/test/API/commands/process/attach/TestProcessAttach.py b/lldb/test/API/commands/process/attach/TestProcessAttach.py index 99ff3a4ec4db816..0e916d2e8e4cbe1 100644 --- a/lldb/test/API/commands/process/attach/TestProcessAttach.py +++ b/lldb/test/API/commands/process/attach/TestProcessAttach.py @@ -4,7 +4,6 @@ import os -import threading import lldb import shutil from lldbsuite.test.decorators import * @@ -129,65 +128,3 @@ def tearDown(self): # Call super's tearDown(). TestBase.tearDown(self) - - def test_run_then_attach_wait_interrupt(self): - # Test that having run one process doesn't cause us to be unable - # to interrupt a subsequent attach attempt. - self.build() - exe = self.getBuildArtifact(exe_name) - - target = lldbutil.run_to_breakpoint_make_target(self, exe_name, True) - launch_info = target.GetLaunchInfo() - launch_info.SetArguments(["q"], True) - error = lldb.SBError() - target.Launch(launch_info, error) - self.assertSuccess(error, "Launched a process") - self.assertState(target.process.state, lldb.eStateExited, "and it exited.") - - # Okay now we've run a process, try to attach/wait to something - # and make sure that we can interrupt that. - - options = lldb.SBCommandInterpreterRunOptions() - options.SetPrintResults(True) - options.SetEchoCommands(False) - - self.stdin_path = self.getBuildArtifact("stdin.txt") - - with open(self.stdin_path, "w") as input_handle: - input_handle.write("process attach -w -n noone_would_use_this_name\nquit") - - # Python will close the file descriptor if all references - # to the filehandle object lapse, so we need to keep one - # around. - self.filehandle = open(self.stdin_path, "r") - self.dbg.SetInputFileHandle(self.filehandle, False) - - # No need to track the output - self.stdout_path = self.getBuildArtifact("stdout.txt") - self.out_filehandle = open(self.stdout_path, "w") - self.dbg.SetOutputFileHandle(self.out_filehandle, False) - self.dbg.SetErrorFileHandle(self.out_filehandle, False) - - n_errors, quit_req, crashed = self.dbg.RunCommandInterpreter( - True, True, options, 0, False, False) - - while 1: - time.sleep(1) - if target.process.state == lldb.eStateAttaching: - break - - self.dbg.DispatchInputInterrupt() - self.dbg.DispatchInputInterrupt() - - self.out_filehandle.flush() - reader = open(self.stdout_path, "r") - results = reader.readlines() - found_result = False - for line in results: - if "Cancelled async attach" in line: - found_result = True - break - self.assertTrue(found_result, "Found async error in results") - # We shouldn't still have a process in the "attaching" state: - state = self.dbg.GetSelectedTarget().process.state - self.assertState(state, lldb.eStateExited, "Process not exited after attach cancellation") diff --git a/lldb/test/API/commands/process/attach/main.cpp b/lldb/test/API/commands/process/attach/main.cpp index 45ca78f494f3710..b4ed48fade30603 100644 --- a/lldb/test/API/commands/process/attach/main.cpp +++ b/lldb/test/API/commands/process/attach/main.cpp @@ -12,13 +12,10 @@ int main(int argc, char const *argv[]) { // Waiting to be attached by the debugger. temp = 0; - if (argc > 1 && argv[1][0] == 'q') - return 0; - while (temp < 30) { - std::this_thread::sleep_for(std::chrono::seconds(2)); // Waiting to be attached... - temp++; - } + std::this_thread::sleep_for(std::chrono::seconds(2)); // Waiting to be attached... + temp++; + } printf("Exiting now\n"); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
