Don't cancel the current IOHandler when we push a handler for an utility function run.

Summary:
D48465 is currently blocked by the fact that tab-completing the first expression is deadlocking LLDB.

The reason for this deadlock is that when we push the ProcessIO handler for reading the Objective-C runtime
information from the executable (which is triggered when we parse the an expression for the first time),
the IOHandler can't be pushed as the Editline::Cancel method is deadlocking.

The deadlock in Editline is coming from the m_output_mutex, which is locked before we go into tab completion.
Even without this lock, calling Cancel on Editline will mean that Editline cleans up behind itself and deletes the
current user-input, which is screws up the console when we are tab-completing at the same time.

I think for now the most reasonable way of fixing this is to just not call Cancel on the current IOHandler when we push
the IOHandler for running an internal utility function.

As we can't really write unit tests for IOHandler itself (due to the hard dependency on an initialized Debugger including
all its global state) and Editline completion is currently also not really testable in an automatic fashion, the test for this has
to be that the expression command completion in D48465 doesn't fail when requesting completion the first time.

A more precise test plan for this is:

1. Apply D48465.
2. Start lldb and break in some function.
3. Type `expr foo` and press tab to request completion.
4. Without this patch, we deadlock and LLDB stops responding.

I'll provide an actual unit test for this once I got around and made the IOHandler code testable,
but for now unblocking D48465 is more critical.

Thanks to Jim for helping me debugging this.

Reviewers: jingham

Reviewed By: jingham

Subscribers: emaste, clayborg, abidh, lldb-commits

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

llvm-svn: 340988

3 files changed, 47 insertions, 2 deletions

diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index fc84b13efbb..69b4d8f072b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -192,7 +192,8 @@ public:
                                        lldb::StreamFileSP &out,
                                        lldb::StreamFileSP &err);
 
-  void PushIOHandler(const lldb::IOHandlerSP &reader_sp);
+  void PushIOHandler(const lldb::IOHandlerSP &reader_sp,
+                     bool cancel_top_handler = true);
 
   bool PopIOHandler(const lldb::IOHandlerSP &reader_sp);
 
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 66ac5a69252..d669f56287c 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -402,7 +402,8 @@ class ProcessModID {
 public:
   ProcessModID()
       : m_stop_id(0), m_last_natural_stop_id(0), m_resume_id(0), m_memory_id(0),
-        m_last_user_expression_resume(0), m_running_user_expression(false) {}
+        m_last_user_expression_resume(0), m_running_user_expression(false),
+        m_running_utility_function(0) {}
 
   ProcessModID(const ProcessModID &rhs)
       : m_stop_id(rhs.m_stop_id), m_memory_id(rhs.m_memory_id) {}
@@ -431,6 +432,10 @@ public:
       m_last_user_expression_resume = m_resume_id;
   }
 
+  bool IsRunningUtilityFunction() const {
+    return m_running_utility_function > 0;
+  }
+
   uint32_t GetStopID() const { return m_stop_id; }
   uint32_t GetLastNaturalStopID() const { return m_last_natural_stop_id; }
   uint32_t GetMemoryID() const { return m_memory_id; }
@@ -467,6 +472,17 @@ public:
       m_running_user_expression--;
   }
 
+  void SetRunningUtilityFunction(bool on) {
+    if (on)
+      m_running_utility_function++;
+    else {
+      assert(m_running_utility_function > 0 &&
+             "Called SetRunningUtilityFunction(false) without calling "
+             "SetRunningUtilityFunction(true) before?");
+      m_running_utility_function--;
+    }
+  }
+
   void SetStopEventForLastNaturalStopID(lldb::EventSP event_sp) {
     m_last_natural_stop_event = event_sp;
   }
@@ -484,6 +500,7 @@ private:
   uint32_t m_memory_id;
   uint32_t m_last_user_expression_resume;
   uint32_t m_running_user_expression;
+  uint32_t m_running_utility_function;
   lldb::EventSP m_last_natural_stop_event;
 };
 
@@ -2554,6 +2571,7 @@ public:
   virtual bool StopNoticingNewThreads() { return true; }
 
   void SetRunningUserExpression(bool on);
+  void SetRunningUtilityFunction(bool on);
 
   //------------------------------------------------------------------
   // lldb::ExecutionContextScope pure virtual functions
@@ -3225,6 +3243,24 @@ private:
   DISALLOW_COPY_AND_ASSIGN(Process);
 };
 
+//------------------------------------------------------------------
+/// RAII guard that should be aquired when an utility function is called within
+/// a given process.
+//------------------------------------------------------------------
+class UtilityFunctionScope {
+  Process *m_process;
+
+public:
+  UtilityFunctionScope(Process *p) : m_process(p) {
+    if (m_process)
+      m_process->SetRunningUtilityFunction(true);
+  }
+  ~UtilityFunctionScope() {
+    if (m_process)
+      m_process->SetRunningUtilityFunction(false);
+  }
+};
+
 } // namespace lldb_private
 
 #endif // liblldb_Process_h_
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index a4a00eb2d37..5b0bb0e09ab 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -375,6 +375,10 @@ public:
 
   bool GetAutoApplyFixIts() const { return m_auto_apply_fixits; }
 
+  bool IsForUtilityExpr() const { return m_running_utility_expression; }
+
+  void SetIsForUtilityExpr(bool b) { m_running_utility_expression = b; }
+
 private:
   ExecutionPolicy m_execution_policy = default_execution_policy;
   lldb::LanguageType m_language = lldb::eLanguageTypeUnknown;
@@ -392,6 +396,10 @@ private:
   bool m_ansi_color_errors = false;
   bool m_result_is_internal = false;
   bool m_auto_apply_fixits = true;
+  /// True if the executed code should be treated as utility code that is only
+  /// used by LLDB internally.
+  bool m_running_utility_expression = false;
+
   lldb::DynamicValueType m_use_dynamic = lldb::eNoDynamicValues;
   Timeout<std::micro> m_timeout = default_timeout;
   Timeout<std::micro> m_one_thread_timeout = llvm::None;