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

12 files changed, 75 insertions, 6 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;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 898cccb41ec..763c89e6f04 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1080,7 +1080,8 @@ void Debugger::AdoptTopIOHandlerFilesIfInvalid(StreamFileSP &in,
   }
 }
 
-void Debugger::PushIOHandler(const IOHandlerSP &reader_sp) {
+void Debugger::PushIOHandler(const IOHandlerSP &reader_sp,
+                             bool cancel_top_handler) {
   if (!reader_sp)
     return;
 
@@ -1101,7 +1102,8 @@ void Debugger::PushIOHandler(const IOHandlerSP &reader_sp) {
   // this new input reader take over
   if (top_reader_sp) {
     top_reader_sp->Deactivate();
-    top_reader_sp->Cancel();
+    if (cancel_top_handler)
+      top_reader_sp->Cancel();
   }
 }
 
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index af0e90d72c6..c3ed8a82db9 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -167,6 +167,7 @@ bool AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
   options.SetStopOthers(true);
   options.SetIgnoreBreakpoints(true);
   options.SetTimeout(g_po_function_timeout);
+  options.SetIsForUtilityExpr(true);
 
   ExpressionResults results = m_print_object_caller_up->ExecuteFunction(
       exe_ctx, &wrapper_struct_addr, options, diagnostics, ret);
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index a808512fb0c..122ad85cdda 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1409,6 +1409,7 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic(
     options.SetStopOthers(true);
     options.SetIgnoreBreakpoints(true);
     options.SetTimeout(g_utility_function_timeout);
+    options.SetIsForUtilityExpr(true);
 
     Value return_value;
     return_value.SetValueType(Value::eValueTypeScalar);
@@ -1659,6 +1660,7 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache() {
     options.SetStopOthers(true);
     options.SetIgnoreBreakpoints(true);
     options.SetTimeout(g_utility_function_timeout);
+    options.SetIsForUtilityExpr(true);
 
     Value return_value;
     return_value.SetValueType(Value::eValueTypeScalar);
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 5e7ffe71918..1bdef5e8d9a 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -1244,7 +1244,8 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process,
   options.SetTrapExceptions(false); // dlopen can't throw exceptions, so
                                     // don't do the work to trap them.
   options.SetTimeout(std::chrono::seconds(2));
-  
+  options.SetIsForUtilityExpr(true);
+
   Value return_value;
   // Fetch the clang types we will need:
   ClangASTContext *ast = process->GetTarget().GetScratchClangASTContext();
diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
index 007a59378fc..91187ae7f42 100644
--- a/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
+++ b/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
@@ -340,6 +340,7 @@ AppleGetItemInfoHandler::GetItemInfo(Thread &thread, uint64_t item,
   options.SetStopOthers(true);
   options.SetTimeout(std::chrono::milliseconds(500));
   options.SetTryAllThreads(false);
+  options.SetIsForUtilityExpr(true);
   thread.CalculateExecutionContext(exe_ctx);
 
   if (!m_get_item_info_impl_code) {
diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp
index 0de32bf1141..0084d44671f 100644
--- a/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp
+++ b/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp
@@ -349,6 +349,7 @@ AppleGetPendingItemsHandler::GetPendingItems(Thread &thread, addr_t queue,
   options.SetStopOthers(true);
   options.SetTimeout(std::chrono::milliseconds(500));
   options.SetTryAllThreads(false);
+  options.SetIsForUtilityExpr(true);
   thread.CalculateExecutionContext(exe_ctx);
 
   if (get_pending_items_caller == NULL) {
diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp
index 7855b3603a3..32fca164118 100644
--- a/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp
+++ b/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp
@@ -354,6 +354,7 @@ AppleGetQueuesHandler::GetCurrentQueues(Thread &thread, addr_t page_to_free,
   options.SetStopOthers(true);
   options.SetTimeout(std::chrono::milliseconds(500));
   options.SetTryAllThreads(false);
+  options.SetIsForUtilityExpr(true);
   thread.CalculateExecutionContext(exe_ctx);
 
   ExpressionResults func_call_ret;
diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp
index 09ab6600a9f..5c449dbe3ab 100644
--- a/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp
+++ b/lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp
@@ -351,6 +351,7 @@ AppleGetThreadItemInfoHandler::GetThreadItemInfo(Thread &thread,
   options.SetStopOthers(true);
   options.SetTimeout(std::chrono::milliseconds(500));
   options.SetTryAllThreads(false);
+  options.SetIsForUtilityExpr(true);
   thread.CalculateExecutionContext(exe_ctx);
 
   if (!m_get_thread_item_info_impl_code) {
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index f3c07832654..deb5cdfd62a 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1729,6 +1729,10 @@ void Process::SetRunningUserExpression(bool on) {
   m_mod_id.SetRunningUserExpression(on);
 }
 
+void Process::SetRunningUtilityFunction(bool on) {
+  m_mod_id.SetRunningUtilityFunction(on);
+}
+
 addr_t Process::GetImageInfoAddress() { return LLDB_INVALID_ADDRESS; }
 
 const lldb::ABISP &Process::GetABI() {
@@ -4685,7 +4689,12 @@ bool Process::PushProcessIOHandler() {
       log->Printf("Process::%s pushing IO handler", __FUNCTION__);
 
     io_handler_sp->SetIsDone(false);
-    GetTarget().GetDebugger().PushIOHandler(io_handler_sp);
+    // If we evaluate an utility function, then we don't cancel the current
+    // IOHandler. Our IOHandler is non-interactive and shouldn't disturb the
+    // existing IOHandler that potentially provides the user interface (e.g.
+    // the IOHandler for Editline).
+    bool cancel_top_handler = !m_mod_id.IsRunningUtilityFunction();
+    GetTarget().GetDebugger().PushIOHandler(io_handler_sp, cancel_top_handler);
     return true;
   }
   return false;
@@ -4875,6 +4884,11 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx,
   thread_plan_sp->SetIsMasterPlan(true);
   thread_plan_sp->SetOkayToDiscard(false);
 
+  // If we are running some utility expression for LLDB, we now have to mark
+  // this in the ProcesModID of this process. This RAII takes care of marking
+  // and reverting the mark it once we are done running the expression.
+  UtilityFunctionScope util_scope(options.IsForUtilityExpr() ? this : nullptr);
+
   if (m_private_state.GetValue() != eStateStopped) {
     diagnostic_manager.PutString(
         eDiagnosticSeverityError,