Author: Jim Ingham Date: 2022-07-26T10:13:19-07:00 New Revision: 8b7775a472e3665dc0a9e5953f84c43da295eddd
URL: https://github.com/llvm/llvm-project/commit/8b7775a472e3665dc0a9e5953f84c43da295eddd DIFF: https://github.com/llvm/llvm-project/commit/8b7775a472e3665dc0a9e5953f84c43da295eddd.diff LOG: StackFrame::GetValueObjectForFrameVariable holds the StackFrame lock too long. This can cause a deadlock if other threads use the common pattern of "lock the StackFrameList, get a frame, lock the StackFrame." Differential Revision: https://reviews.llvm.org/D130524 Added: Modified: lldb/source/Target/StackFrame.cpp Removed: ################################################################################ diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index e87cf5af3e399..4fb5ba0b735e6 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -1145,26 +1145,34 @@ bool StackFrame::HasDebugInformation() { ValueObjectSP StackFrame::GetValueObjectForFrameVariable(const VariableSP &variable_sp, DynamicValueType use_dynamic) { - std::lock_guard<std::recursive_mutex> guard(m_mutex); ValueObjectSP valobj_sp; - if (IsHistorical()) { - return valobj_sp; - } - VariableList *var_list = GetVariableList(true); - if (var_list) { - // Make sure the variable is a frame variable - const uint32_t var_idx = var_list->FindIndexForVariable(variable_sp.get()); - const uint32_t num_variables = var_list->GetSize(); - if (var_idx < num_variables) { - valobj_sp = m_variable_list_value_objects.GetValueObjectAtIndex(var_idx); - if (!valobj_sp) { - if (m_variable_list_value_objects.GetSize() < num_variables) - m_variable_list_value_objects.Resize(num_variables); - valobj_sp = ValueObjectVariable::Create(this, variable_sp); - m_variable_list_value_objects.SetValueObjectAtIndex(var_idx, valobj_sp); + { // Scope for stack frame mutex. We need to drop this mutex before we figure + // out the dynamic value. That will require converting the StackID in the + // VO back to a StackFrame, which will in turn require locking the + // StackFrameList. If we still hold the StackFrame mutex, we could suffer + // lock inversion against the pattern of getting the StackFrameList and + // then the stack frame, which is fairly common. + std::lock_guard<std::recursive_mutex> guard(m_mutex); + if (IsHistorical()) { + return valobj_sp; + } + VariableList *var_list = GetVariableList(true); + if (var_list) { + // Make sure the variable is a frame variable + const uint32_t var_idx = var_list->FindIndexForVariable(variable_sp.get()); + const uint32_t num_variables = var_list->GetSize(); + if (var_idx < num_variables) { + valobj_sp = m_variable_list_value_objects.GetValueObjectAtIndex(var_idx); + if (!valobj_sp) { + if (m_variable_list_value_objects.GetSize() < num_variables) + m_variable_list_value_objects.Resize(num_variables); + valobj_sp = ValueObjectVariable::Create(this, variable_sp); + m_variable_list_value_objects.SetValueObjectAtIndex(var_idx, + valobj_sp); + } } } - } + } // End of StackFrame mutex scope. if (use_dynamic != eNoDynamicValues && valobj_sp) { ValueObjectSP dynamic_sp = valobj_sp->GetDynamicValue(use_dynamic); if (dynamic_sp) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
