https://github.com/jimingham updated
https://github.com/llvm/llvm-project/pull/65822:
>From e98672b9f4b02c2baac5ed5dcd94afa9a78e35b6 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Fri, 8 Sep 2023 16:18:48 -0700
Subject: [PATCH 1/2] 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");
}
>From cd68bd47aaa57326270e3e6b3640ca45b0622604 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Fri, 8 Sep 2023 17:20:31 -0700
Subject: [PATCH 2/2] Address review comments
---
lldb/source/Interpreter/CommandInterpreter.cpp | 2 +-
lldb/test/API/commands/process/attach/TestProcessAttach.py | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 3bd91c02620d1d9..4646e71cc3dd4a6 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3000,7 +3000,7 @@ 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:
+ // 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.
diff --git a/lldb/test/API/commands/process/attach/TestProcessAttach.py
b/lldb/test/API/commands/process/attach/TestProcessAttach.py
index 8f887fe8791c642..67bb134ec0148a0 100644
--- a/lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ b/lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -170,7 +170,11 @@ def test_run_then_attach_wait_interrupt(self):
n_errors, quit_req, crashed = self.dbg.RunCommandInterpreter(
True, True, options, 0, False, False)
- time.sleep(5)
+ while 1:
+ time.sleep(1)
+ if target.process.state == lldb.eStateAttaching:
+ break
+
self.dbg.DispatchInputInterrupt()
self.dbg.DispatchInputInterrupt()
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
