[Lldb-commits] [lldb] r340988 - Don't cancel the current IOHandler when we push a handler for an utility function run.
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Wed Aug 29 15:50:54 PDT 2018
Author: teemperor
Date: Wed Aug 29 15:50:54 2018
New Revision: 340988
URL: http://llvm.org/viewvc/llvm-project?rev=340988&view=rev
Log:
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
Modified:
lldb/trunk/include/lldb/Core/Debugger.h
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp
lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp
lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp
lldb/trunk/source/Target/Process.cpp
Modified: lldb/trunk/include/lldb/Core/Debugger.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Debugger.h (original)
+++ lldb/trunk/include/lldb/Core/Debugger.h Wed Aug 29 15:50:54 2018
@@ -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);
Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Wed Aug 29 15:50:54 2018
@@ -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_
Modified: lldb/trunk/include/lldb/Target/Target.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Wed Aug 29 15:50:54 2018
@@ -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;
Modified: lldb/trunk/source/Core/Debugger.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Wed Aug 29 15:50:54 2018
@@ -1080,7 +1080,8 @@ void Debugger::AdoptTopIOHandlerFilesIfI
}
}
-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 IOHan
// 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();
}
}
Modified: lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp (original)
+++ lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp Wed Aug 29 15:50:54 2018
@@ -167,6 +167,7 @@ bool AppleObjCRuntime::GetObjectDescript
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);
Modified: lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp (original)
+++ lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp Wed Aug 29 15:50:54 2018
@@ -1409,6 +1409,7 @@ AppleObjCRuntimeV2::UpdateISAToDescripto
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::UpdateISAToDescripto
options.SetStopOthers(true);
options.SetIgnoreBreakpoints(true);
options.SetTimeout(g_utility_function_timeout);
+ options.SetIsForUtilityExpr(true);
Value return_value;
return_value.SetValueType(Value::eValueTypeScalar);
Modified: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp Wed Aug 29 15:50:54 2018
@@ -1244,7 +1244,8 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb
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();
Modified: lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp (original)
+++ lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp Wed Aug 29 15:50:54 2018
@@ -340,6 +340,7 @@ AppleGetItemInfoHandler::GetItemInfo(Thr
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) {
Modified: lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp (original)
+++ lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.cpp Wed Aug 29 15:50:54 2018
@@ -349,6 +349,7 @@ AppleGetPendingItemsHandler::GetPendingI
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) {
Modified: lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp (original)
+++ lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.cpp Wed Aug 29 15:50:54 2018
@@ -354,6 +354,7 @@ AppleGetQueuesHandler::GetCurrentQueues(
options.SetStopOthers(true);
options.SetTimeout(std::chrono::milliseconds(500));
options.SetTryAllThreads(false);
+ options.SetIsForUtilityExpr(true);
thread.CalculateExecutionContext(exe_ctx);
ExpressionResults func_call_ret;
Modified: lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp (original)
+++ lldb/trunk/source/Plugins/SystemRuntime/MacOSX/AppleGetThreadItemInfoHandler.cpp Wed Aug 29 15:50:54 2018
@@ -351,6 +351,7 @@ AppleGetThreadItemInfoHandler::GetThread
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) {
Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=340988&r1=340987&r2=340988&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Wed Aug 29 15:50:54 2018
@@ -1729,6 +1729,10 @@ void Process::SetRunningUserExpression(b
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
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,
More information about the lldb-commits
mailing list