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

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Wed Sep 20 01:21:09 PDT 2023


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");
 }


        


More information about the lldb-commits mailing list