[Lldb-commits] [lldb] Fix a bug with cancelling "attach -w" after you have run a process previously (PR #65822)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 8 16:52:39 PDT 2023
https://github.com/jimingham created https://github.com/llvm/llvm-project/pull/65822:
The problem is that the when the "attach" command is initiated, the ExecutionContext for the command has a process - it's the exited one from the previour run. But the `attach wait` creates a new process for the attach, and then errors out instead of interrupting when it finds that its process and the one in the command's ExecutionContext don't match.
This change checks that if we're returning a target from GetExecutionContext, we fill the context with it's current process, not some historical one.
>From e98672b9f4b02c2baac5ed5dcd94afa9a78e35b6 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Fri, 8 Sep 2023 16:18:48 -0700
Subject: [PATCH] Fix a bug with cancelling "attach -w" after you have run a
process previously.
The problem is that the when the "attach" command is initiated, the ExecutionContext
for the command has a process - it's the exited one from the previour run. But the
`attach wait` creates a new process for the attach, and then errors out instead of
interrupting when it finds that its process and the one in the command's ExecutionContext
don't match.
This change checks that if we're returning a target from GetExecutionContext, we fill
the context with it's current process, not some historical one.
---
.../lldb/Interpreter/CommandInterpreter.h | 4 +-
.../source/Interpreter/CommandInterpreter.cpp | 33 ++++++++---
lldb/source/Target/Process.cpp | 17 +++---
.../process/attach/TestProcessAttach.py | 59 +++++++++++++++++++
.../test/API/commands/process/attach/main.cpp | 9 ++-
5 files changed, 100 insertions(+), 22 deletions(-)
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 747188a15312fa3..960a1c18f0130ab 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() const;
+ ExecutionContext GetExecutionContext();
lldb::PlatformSP GetPlatform(bool prefer_target_platform);
@@ -661,7 +661,7 @@ class CommandInterpreter : public Broadcaster,
void GetProcessOutput();
- bool DidProcessStopAbnormally() const;
+ bool DidProcessStopAbnormally();
void SetSynchronous(bool value);
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 1a8b691090ba2dc..3bd91c02620d1d9 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2491,7 +2491,7 @@ PlatformSP CommandInterpreter::GetPlatform(bool prefer_target_platform) {
return platform_sp;
}
-bool CommandInterpreter::DidProcessStopAbnormally() const {
+bool CommandInterpreter::DidProcessStopAbnormally() {
auto exe_ctx = GetExecutionContext();
TargetSP target_sp = exe_ctx.GetTargetSP();
if (!target_sp)
@@ -2996,10 +2996,22 @@ void CommandInterpreter::FindCommandsForApropos(llvm::StringRef search_word,
m_alias_dict);
}
-ExecutionContext CommandInterpreter::GetExecutionContext() const {
- return !m_overriden_exe_contexts.empty()
- ? m_overriden_exe_contexts.top()
- : m_debugger.GetSelectedExecutionContext();
+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. If 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();
}
void CommandInterpreter::OverrideExecutionContext(
@@ -3192,12 +3204,17 @@ 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 85ca0f6ab8d153a..0cb1c2e0b8e21e9 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3156,23 +3156,22 @@ 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;
- ListenerSP halt_listener_sp(
- Listener::MakeListener("lldb.process.halt_listener"));
- HijackProcessEvents(halt_listener_sp);
-
- EventSP event_sp;
-
- 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();
}
+ ListenerSP halt_listener_sp(
+ Listener::MakeListener("lldb.process.halt_listener"));
+ HijackProcessEvents(halt_listener_sp);
+
+ EventSP event_sp;
+
+ SendAsyncInterrupt();
+
// 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 7da9c62851889ae..8f887fe8791c642 100644
--- a/lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ b/lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -4,6 +4,7 @@
import os
+import threading
import lldb
import shutil
from lldbsuite.test.decorators import *
@@ -127,3 +128,61 @@ 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)
+
+ time.sleep(5)
+ 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 b4ed48fade30603..45ca78f494f3710 100644
--- a/lldb/test/API/commands/process/attach/main.cpp
+++ b/lldb/test/API/commands/process/attach/main.cpp
@@ -12,10 +12,13 @@ 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");
}
More information about the lldb-commits
mailing list