[Lldb-commits] [lldb] 7265f79 - Fix a bug with cancelling "attach -w" after you have run a process previously (#65822)

via lldb-commits lldb-commits at lists.llvm.org
Tue Sep 19 11:25:58 PDT 2023


Author: jimingham
Date: 2023-09-19T11:25:53-07:00
New Revision: 7265f792dc8e1157a3874aee5f8aed6d4d8236e7

URL: https://github.com/llvm/llvm-project/commit/7265f792dc8e1157a3874aee5f8aed6d4d8236e7
DIFF: https://github.com/llvm/llvm-project/commit/7265f792dc8e1157a3874aee5f8aed6d4d8236e7.diff

LOG: 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.

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 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 dcff53ff843f328..9f1f520abb198f0 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() const {
+bool CommandInterpreter::DidProcessStopAbnormally() {
   auto exe_ctx = GetExecutionContext();
   TargetSP target_sp = exe_ctx.GetTargetSP();
   if (!target_sp)
@@ -2976,10 +2976,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.  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();
 }
 
 void CommandInterpreter::OverrideExecutionContext(
@@ -3172,12 +3184,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 f82ab05362fbee9..d30557312f7a8f9 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3153,23 +3153,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..67bb134ec0148a0 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,65 @@ 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 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