Skip to content

Commit 7265f79

Browse files
authored
Fix a bug with cancelling "attach -w" after you have run a process previously (#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.
1 parent 1a8c691 commit 7265f79

File tree

5 files changed

+104
-22
lines changed

5 files changed

+104
-22
lines changed

lldb/include/lldb/Interpreter/CommandInterpreter.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ class CommandInterpreter : public Broadcaster,
447447

448448
Debugger &GetDebugger() { return m_debugger; }
449449

450-
ExecutionContext GetExecutionContext() const;
450+
ExecutionContext GetExecutionContext();
451451

452452
lldb::PlatformSP GetPlatform(bool prefer_target_platform);
453453

@@ -661,7 +661,7 @@ class CommandInterpreter : public Broadcaster,
661661

662662
void GetProcessOutput();
663663

664-
bool DidProcessStopAbnormally() const;
664+
bool DidProcessStopAbnormally();
665665

666666
void SetSynchronous(bool value);
667667

lldb/source/Interpreter/CommandInterpreter.cpp

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2471,7 +2471,7 @@ PlatformSP CommandInterpreter::GetPlatform(bool prefer_target_platform) {
24712471
return platform_sp;
24722472
}
24732473

2474-
bool CommandInterpreter::DidProcessStopAbnormally() const {
2474+
bool CommandInterpreter::DidProcessStopAbnormally() {
24752475
auto exe_ctx = GetExecutionContext();
24762476
TargetSP target_sp = exe_ctx.GetTargetSP();
24772477
if (!target_sp)
@@ -2976,10 +2976,22 @@ void CommandInterpreter::FindCommandsForApropos(llvm::StringRef search_word,
29762976
m_alias_dict);
29772977
}
29782978

2979-
ExecutionContext CommandInterpreter::GetExecutionContext() const {
2980-
return !m_overriden_exe_contexts.empty()
2981-
? m_overriden_exe_contexts.top()
2982-
: m_debugger.GetSelectedExecutionContext();
2979+
ExecutionContext CommandInterpreter::GetExecutionContext() {
2980+
ExecutionContext exe_ctx;
2981+
if (!m_overriden_exe_contexts.empty()) {
2982+
// During the course of a command, the target may have replaced the process
2983+
// coming in with another. I fix that here:
2984+
exe_ctx = m_overriden_exe_contexts.top();
2985+
// Don't use HasProcessScope, that returns false if there is a process but
2986+
// it's no longer valid, which is one of the cases we want to catch here.
2987+
if (exe_ctx.HasTargetScope() && exe_ctx.GetProcessPtr()) {
2988+
ProcessSP actual_proc_sp = exe_ctx.GetTargetSP()->GetProcessSP();
2989+
if (actual_proc_sp != exe_ctx.GetProcessSP())
2990+
m_overriden_exe_contexts.top().SetContext(actual_proc_sp);
2991+
}
2992+
return m_overriden_exe_contexts.top();
2993+
}
2994+
return m_debugger.GetSelectedExecutionContext();
29832995
}
29842996

29852997
void CommandInterpreter::OverrideExecutionContext(
@@ -3172,12 +3184,17 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
31723184
}
31733185

31743186
bool CommandInterpreter::IOHandlerInterrupt(IOHandler &io_handler) {
3187+
// InterruptCommand returns true if this is the first time
3188+
// we initiate an interrupt for this command. So we give the
3189+
// command a chance to handle the interrupt on the first
3190+
// interrupt, but if that didn't do anything, a second
3191+
// interrupt will do more work to halt the process/interpreter.
3192+
if (InterruptCommand())
3193+
return true;
3194+
31753195
ExecutionContext exe_ctx(GetExecutionContext());
31763196
Process *process = exe_ctx.GetProcessPtr();
31773197

3178-
if (InterruptCommand())
3179-
return true;
3180-
31813198
if (process) {
31823199
StateType state = process->GetState();
31833200
if (StateIsRunningState(state)) {

lldb/source/Target/Process.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3153,23 +3153,22 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) {
31533153
// case it was already set and some thread plan logic calls halt on its own.
31543154
m_clear_thread_plans_on_stop |= clear_thread_plans;
31553155

3156-
ListenerSP halt_listener_sp(
3157-
Listener::MakeListener("lldb.process.halt_listener"));
3158-
HijackProcessEvents(halt_listener_sp);
3159-
3160-
EventSP event_sp;
3161-
3162-
SendAsyncInterrupt();
3163-
31643156
if (m_public_state.GetValue() == eStateAttaching) {
31653157
// Don't hijack and eat the eStateExited as the code that was doing the
31663158
// attach will be waiting for this event...
3167-
RestoreProcessEvents();
31683159
SetExitStatus(SIGKILL, "Cancelled async attach.");
31693160
Destroy(false);
31703161
return Status();
31713162
}
31723163

3164+
ListenerSP halt_listener_sp(
3165+
Listener::MakeListener("lldb.process.halt_listener"));
3166+
HijackProcessEvents(halt_listener_sp);
3167+
3168+
EventSP event_sp;
3169+
3170+
SendAsyncInterrupt();
3171+
31733172
// Wait for the process halt timeout seconds for the process to stop.
31743173
// If we are going to use the run lock, that means we're stopping out to the
31753174
// user, so we should also select the most relevant frame.

lldb/test/API/commands/process/attach/TestProcessAttach.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55

66
import os
7+
import threading
78
import lldb
89
import shutil
910
from lldbsuite.test.decorators import *
@@ -127,3 +128,65 @@ def tearDown(self):
127128

128129
# Call super's tearDown().
129130
TestBase.tearDown(self)
131+
132+
def test_run_then_attach_wait_interrupt(self):
133+
# Test that having run one process doesn't cause us to be unable
134+
# to interrupt a subsequent attach attempt.
135+
self.build()
136+
exe = self.getBuildArtifact(exe_name)
137+
138+
target = lldbutil.run_to_breakpoint_make_target(self, exe_name, True)
139+
launch_info = target.GetLaunchInfo()
140+
launch_info.SetArguments(["q"], True)
141+
error = lldb.SBError()
142+
target.Launch(launch_info, error)
143+
self.assertSuccess(error, "Launched a process")
144+
self.assertState(target.process.state, lldb.eStateExited, "and it exited.")
145+
146+
# Okay now we've run a process, try to attach/wait to something
147+
# and make sure that we can interrupt that.
148+
149+
options = lldb.SBCommandInterpreterRunOptions()
150+
options.SetPrintResults(True)
151+
options.SetEchoCommands(False)
152+
153+
self.stdin_path = self.getBuildArtifact("stdin.txt")
154+
155+
with open(self.stdin_path, "w") as input_handle:
156+
input_handle.write("process attach -w -n noone_would_use_this_name\nquit")
157+
158+
# Python will close the file descriptor if all references
159+
# to the filehandle object lapse, so we need to keep one
160+
# around.
161+
self.filehandle = open(self.stdin_path, "r")
162+
self.dbg.SetInputFileHandle(self.filehandle, False)
163+
164+
# No need to track the output
165+
self.stdout_path = self.getBuildArtifact("stdout.txt")
166+
self.out_filehandle = open(self.stdout_path, "w")
167+
self.dbg.SetOutputFileHandle(self.out_filehandle, False)
168+
self.dbg.SetErrorFileHandle(self.out_filehandle, False)
169+
170+
n_errors, quit_req, crashed = self.dbg.RunCommandInterpreter(
171+
True, True, options, 0, False, False)
172+
173+
while 1:
174+
time.sleep(1)
175+
if target.process.state == lldb.eStateAttaching:
176+
break
177+
178+
self.dbg.DispatchInputInterrupt()
179+
self.dbg.DispatchInputInterrupt()
180+
181+
self.out_filehandle.flush()
182+
reader = open(self.stdout_path, "r")
183+
results = reader.readlines()
184+
found_result = False
185+
for line in results:
186+
if "Cancelled async attach" in line:
187+
found_result = True
188+
break
189+
self.assertTrue(found_result, "Found async error in results")
190+
# We shouldn't still have a process in the "attaching" state:
191+
state = self.dbg.GetSelectedTarget().process.state
192+
self.assertState(state, lldb.eStateExited, "Process not exited after attach cancellation")

lldb/test/API/commands/process/attach/main.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,13 @@ int main(int argc, char const *argv[]) {
1212
// Waiting to be attached by the debugger.
1313
temp = 0;
1414

15+
if (argc > 1 && argv[1][0] == 'q')
16+
return 0;
17+
1518
while (temp < 30) {
16-
std::this_thread::sleep_for(std::chrono::seconds(2)); // Waiting to be attached...
17-
temp++;
18-
}
19+
std::this_thread::sleep_for(std::chrono::seconds(2)); // Waiting to be attached...
20+
temp++;
21+
}
1922

2023
printf("Exiting now\n");
2124
}

0 commit comments

Comments
 (0)