Don't stop execution in batch mode when process stops with SIGINT or SIGSTOP

Summary: Usually, SIGINT and SIGSTOP don't imply a crash, e.g. SIGSTOP is sent on process launch and attach on some platforms.

Differential Revision: https://reviews.llvm.org/D67776


git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@372961 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 91 insertions, 70 deletions

diff --git a/include/lldb/Interpreter/CommandInterpreter.h b/include/lldb/Interpreter/CommandInterpreter.h
index edf92d7ca..3b98b2a52 100644
--- a/include/lldb/Interpreter/CommandInterpreter.h
+++ b/include/lldb/Interpreter/CommandInterpreter.h
@@ -502,6 +502,8 @@ protected:
 
   void GetProcessOutput();
 
+  bool DidProcessStopAbnormally() const;
+
   void SetSynchronous(bool value);
 
   lldb::CommandObjectSP GetCommandSP(llvm::StringRef cmd,
diff --git a/include/lldb/Interpreter/CommandReturnObject.h b/include/lldb/Interpreter/CommandReturnObject.h
index a5f612614..670002dad 100644
--- a/include/lldb/Interpreter/CommandReturnObject.h
+++ b/include/lldb/Interpreter/CommandReturnObject.h
@@ -144,14 +144,6 @@ public:
 
   void SetInteractive(bool b);
 
-  bool GetAbnormalStopWasExpected() const {
-    return m_abnormal_stop_was_expected;
-  }
-
-  void SetAbnormalStopWasExpected(bool signal_was_expected) {
-    m_abnormal_stop_was_expected = signal_was_expected;
-  }
-
 private:
   enum { eStreamStringIndex = 0, eImmediateStreamIndex = 1 };
 
@@ -162,14 +154,6 @@ private:
   bool m_did_change_process_state;
   bool m_interactive; // If true, then the input handle from the debugger will
                       // be hooked up
-  bool m_abnormal_stop_was_expected; // This is to support
-                                     // eHandleCommandFlagStopOnCrash vrs.
-                                     // attach.
-  // The attach command often ends up with the process stopped due to a signal.
-  // Normally that would mean stop on crash should halt batch execution, but we
-  // obviously don't want that for attach.  Using this flag, the attach command
-  // (and anything else for which this is relevant) can say that the signal is
-  // expected, and batch command execution can continue.
 };
 
 } // namespace lldb_private
diff --git a/packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py b/packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py
index 01e11fc09..41c6019ad 100644
--- a/packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py
+++ b/packages/Python/lldbsuite/test/driver/batch_mode/TestBatchMode.py
@@ -78,6 +78,35 @@ class DriverBatchModeTest(PExpectTest):
         import pexpect
         child.expect(pexpect.EOF)
 
+    @expectedFlakeyFreeBSD("llvm.org/pr25172 fails rarely on the buildbot")
+    def test_batch_mode_launch_stop_at_entry(self):
+        """Test that the lldb driver's batch mode works correctly for process launch."""
+        self.build()
+
+        exe = self.getBuildArtifact("a.out")
+
+        # Launch with the option '--stop-at-entry' stops with a signal (usually SIGSTOP)
+        # that should be suppressed since it doesn't imply a crash and
+        # this is not a reason to exit batch mode.
+        extra_args = ['-b',
+            '-o', 'process launch --stop-at-entry',
+            '-o', 'continue',
+        ]
+        self.launch(executable=exe, extra_args=extra_args)
+        child = self.child
+
+        # Check that the process has been launched:
+        child.expect("Process ([0-9]+) launched:")
+        # We should have continued:
+        child.expect_exact("continue")
+        # The App should have not have crashed:
+        child.expect_exact("Got there on time and it did not crash.")
+        
+        # Then lldb should exit.
+        child.expect_exact("exited")
+        import pexpect
+        child.expect(pexpect.EOF)
+
     def closeVictim(self):
         if self.victim is not None:
             self.victim.close()
diff --git a/source/Commands/CommandObjectProcess.cpp b/source/Commands/CommandObjectProcess.cpp
index b494740c6..e5aa78afa 100644
--- a/source/Commands/CommandObjectProcess.cpp
+++ b/source/Commands/CommandObjectProcess.cpp
@@ -429,7 +429,6 @@ protected:
         result.AppendMessage(stream.GetString());
         result.SetStatus(eReturnStatusSuccessFinishNoResult);
         result.SetDidChangeProcessState(true);
-        result.SetAbnormalStopWasExpected(true);
       } else {
         result.AppendError(
             "no error returned from Target::Attach, and target has no process");
diff --git a/source/Interpreter/CommandInterpreter.cpp b/source/Interpreter/CommandInterpreter.cpp
index 71f766ed7..a5b2d24ae 100644
--- a/source/Interpreter/CommandInterpreter.cpp
+++ b/source/Interpreter/CommandInterpreter.cpp
@@ -63,8 +63,10 @@
 #include "lldb/Utility/Args.h"
 
 #include "lldb/Target/Process.h"
+#include "lldb/Target/StopInfo.h"
 #include "lldb/Target/TargetList.h"
 #include "lldb/Target/Thread.h"
+#include "lldb/Target/UnixSignals.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -2138,6 +2140,45 @@ PlatformSP CommandInterpreter::GetPlatform(bool prefer_target_platform) {
   return platform_sp;
 }
 
+bool CommandInterpreter::DidProcessStopAbnormally() const {
+  TargetSP target_sp = m_debugger.GetTargetList().GetSelectedTarget();
+  if (!target_sp)
+    return false;
+
+  ProcessSP process_sp(target_sp->GetProcessSP());
+  if (!process_sp)
+    return false;
+
+  if (eStateStopped != process_sp->GetState())
+    return false;
+
+  for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
+    StopInfoSP stop_info = thread_sp->GetStopInfo();
+    if (!stop_info)
+      return false;
+
+    const StopReason reason = stop_info->GetStopReason();
+    if (reason == eStopReasonException || reason == eStopReasonInstrumentation)
+      return true;
+
+    if (reason == eStopReasonSignal) {
+      const auto stop_signal = static_cast<int32_t>(stop_info->GetValue());
+      UnixSignalsSP signals_sp = process_sp->GetUnixSignals();
+      if (!signals_sp || !signals_sp->SignalIsValid(stop_signal))
+        // The signal is unknown, treat it as abnormal.
+        return true;
+
+      const auto sigint_num = signals_sp->GetSignalNumberFromName("SIGINT");
+      const auto sigstop_num = signals_sp->GetSignalNumberFromName("SIGSTOP");
+      if ((stop_signal != sigint_num) && (stop_signal != sigstop_num))
+        // The signal very likely implies a crash.
+        return true;
+    }
+  }
+
+  return false;
+}
+
 void CommandInterpreter::HandleCommands(const StringList &commands,
                                         ExecutionContext *override_context,
                                         CommandInterpreterRunOptions &options,
@@ -2248,38 +2289,22 @@ void CommandInterpreter::HandleCommands(const StringList &commands,
     }
 
     // Also check for "stop on crash here:
-    bool should_stop = false;
-    if (tmp_result.GetDidChangeProcessState() && options.GetStopOnCrash()) {
-      TargetSP target_sp(m_debugger.GetTargetList().GetSelectedTarget());
-      if (target_sp) {
-        ProcessSP process_sp(target_sp->GetProcessSP());
-        if (process_sp) {
-          for (ThreadSP thread_sp : process_sp->GetThreadList().Threads()) {
-            StopReason reason = thread_sp->GetStopReason();
-            if (reason == eStopReasonSignal || reason == eStopReasonException ||
-                reason == eStopReasonInstrumentation) {
-              should_stop = true;
-              break;
-            }
-          }
-        }
-      }
-      if (should_stop) {
-        if (idx != num_lines - 1)
-          result.AppendErrorWithFormat(
-              "Aborting reading of commands after command #%" PRIu64
-              ": '%s' stopped with a signal or exception.\n",
-              (uint64_t)idx + 1, cmd);
-        else
-          result.AppendMessageWithFormat(
-              "Command #%" PRIu64 " '%s' stopped with a signal or exception.\n",
-              (uint64_t)idx + 1, cmd);
+    if (tmp_result.GetDidChangeProcessState() && options.GetStopOnCrash() &&
+        DidProcessStopAbnormally()) {
+      if (idx != num_lines - 1)
+        result.AppendErrorWithFormat(
+            "Aborting reading of commands after command #%" PRIu64
+            ": '%s' stopped with a signal or exception.\n",
+            (uint64_t)idx + 1, cmd);
+      else
+        result.AppendMessageWithFormat(
+            "Command #%" PRIu64 " '%s' stopped with a signal or exception.\n",
+            (uint64_t)idx + 1, cmd);
 
-        result.SetStatus(tmp_result.GetStatus());
-        m_debugger.SetAsyncExecution(old_async_execution);
+      result.SetStatus(tmp_result.GetStatus());
+      m_debugger.SetAsyncExecution(old_async_execution);
 
-        return;
-      }
+      return;
     }
   }
 
@@ -2767,27 +2792,10 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
 
   // Finally, if we're going to stop on crash, check that here:
   if (!m_quit_requested && result.GetDidChangeProcessState() &&
-      io_handler.GetFlags().Test(eHandleCommandFlagStopOnCrash)) {
-    bool should_stop = false;
-    TargetSP target_sp(m_debugger.GetTargetList().GetSelectedTarget());
-    if (target_sp) {
-      ProcessSP process_sp(target_sp->GetProcessSP());
-      if (process_sp) {
-        for (ThreadSP thread_sp : process_sp->GetThreadList().Threads()) {
-          StopReason reason = thread_sp->GetStopReason();
-          if ((reason == eStopReasonSignal || reason == eStopReasonException ||
-               reason == eStopReasonInstrumentation) &&
-              !result.GetAbnormalStopWasExpected()) {
-            should_stop = true;
-            break;
-          }
-        }
-      }
-    }
-    if (should_stop) {
-      io_handler.SetIsDone(true);
-      m_stopped_for_crash = true;
-    }
+      io_handler.GetFlags().Test(eHandleCommandFlagStopOnCrash) &&
+      DidProcessStopAbnormally()) {
+    io_handler.SetIsDone(true);
+    m_stopped_for_crash = true;
   }
 }
 
diff --git a/source/Interpreter/CommandReturnObject.cpp b/source/Interpreter/CommandReturnObject.cpp
index 3a7a8755d..c17390be7 100644
--- a/source/Interpreter/CommandReturnObject.cpp
+++ b/source/Interpreter/CommandReturnObject.cpp
@@ -33,8 +33,7 @@ static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s,
 
 CommandReturnObject::CommandReturnObject()
     : m_out_stream(), m_err_stream(), m_status(eReturnStatusStarted),
-      m_did_change_process_state(false), m_interactive(true),
-      m_abnormal_stop_was_expected(false) {}
+      m_did_change_process_state(false), m_interactive(true) {}
 
 CommandReturnObject::~CommandReturnObject() {}