llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: None (llvmbot) <details> <summary>Changes</summary> Backport 58f623c Requested by: @<!-- -->medismailben --- Full diff: https://github.com/llvm/llvm-project/pull/179500.diff 4 Files Affected: - (modified) lldb/include/lldb/API/SBValue.h (+3-4) - (modified) lldb/include/lldb/ValueObject/ValueObject.h (+78) - (modified) lldb/source/API/SBValue.cpp (-166) - (modified) lldb/source/ValueObject/ValueObject.cpp (+91) ``````````diff diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h index dead11fba19fe..0583f2a4983f6 100644 --- a/lldb/include/lldb/API/SBValue.h +++ b/lldb/include/lldb/API/SBValue.h @@ -13,10 +13,9 @@ #include "lldb/API/SBDefines.h" #include "lldb/API/SBType.h" +namespace lldb_private { class ValueImpl; class ValueLocker; - -namespace lldb_private { namespace python { class SWIGBridge; } @@ -490,7 +489,7 @@ class LLDB_API SBValue { /// \return /// A ValueObjectSP of the best kind (static, dynamic or synthetic) we /// can cons up, in accordance with the SBValue's settings. - lldb::ValueObjectSP GetSP(ValueLocker &value_locker) const; + lldb::ValueObjectSP GetSP(lldb_private::ValueLocker &value_locker) const; // these calls do the right thing WRT adjusting their settings according to // the target's preferences @@ -507,7 +506,7 @@ class LLDB_API SBValue { bool use_synthetic, const char *name); private: - typedef std::shared_ptr<ValueImpl> ValueImplSP; + typedef std::shared_ptr<lldb_private::ValueImpl> ValueImplSP; ValueImplSP m_opaque_sp; void SetSP(ValueImplSP impl_sp); diff --git a/lldb/include/lldb/ValueObject/ValueObject.h b/lldb/include/lldb/ValueObject/ValueObject.h index 3f9f2b5de8dbe..8a528bad15f94 100644 --- a/lldb/include/lldb/ValueObject/ValueObject.h +++ b/lldb/include/lldb/ValueObject/ValueObject.h @@ -1121,6 +1121,84 @@ class ValueObject { const ValueObject &operator=(const ValueObject &) = delete; }; +// The two classes below are used by the public SBValue API implementation. This +// is useful here because we need them in order to access the underlying +// ValueObject from SBValue without introducing a back-dependency from the API +// library to the more core libs. + +class ValueImpl { +public: + ValueImpl() = default; + + ValueImpl(lldb::ValueObjectSP in_valobj_sp, + lldb::DynamicValueType use_dynamic, bool use_synthetic, + const char *name = nullptr); + + ValueImpl(const ValueImpl &rhs) = default; + + ValueImpl &operator=(const ValueImpl &rhs); + + bool IsValid(); + + lldb::ValueObjectSP GetRootSP() { return m_valobj_sp; } + + lldb::ValueObjectSP GetSP(Process::StopLocker &stop_locker, + std::unique_lock<std::recursive_mutex> &lock, + Status &error); + + void SetUseDynamic(lldb::DynamicValueType use_dynamic) { + m_use_dynamic = use_dynamic; + } + + void SetUseSynthetic(bool use_synthetic) { m_use_synthetic = use_synthetic; } + + lldb::DynamicValueType GetUseDynamic() { return m_use_dynamic; } + + bool GetUseSynthetic() { return m_use_synthetic; } + + // All the derived values that we would make from the m_valobj_sp will share + // the ExecutionContext with m_valobj_sp, so we don't need to do the + // calculations in GetSP to return the Target, Process, Thread or Frame. It + // is convenient to provide simple accessors for these, which I do here. + lldb::TargetSP GetTargetSP() { + return m_valobj_sp ? m_valobj_sp->GetTargetSP() : lldb::TargetSP{}; + } + + lldb::ProcessSP GetProcessSP() { + return m_valobj_sp ? m_valobj_sp->GetProcessSP() : lldb::ProcessSP{}; + } + + lldb::ThreadSP GetThreadSP() { + return m_valobj_sp ? m_valobj_sp->GetThreadSP() : lldb::ThreadSP{}; + } + + lldb::StackFrameSP GetFrameSP() { + return m_valobj_sp ? m_valobj_sp->GetFrameSP() : lldb::StackFrameSP{}; + } + +private: + lldb::ValueObjectSP m_valobj_sp; + lldb::DynamicValueType m_use_dynamic; + bool m_use_synthetic; + ConstString m_name; +}; + +class ValueLocker { +public: + ValueLocker() = default; + + lldb::ValueObjectSP GetLockedSP(ValueImpl &in_value) { + return in_value.GetSP(m_stop_locker, m_lock, m_lock_error); + } + + Status &GetError() { return m_lock_error; } + +private: + Process::StopLocker m_stop_locker; + std::unique_lock<std::recursive_mutex> m_lock; + Status m_lock_error; +}; + } // namespace lldb_private #endif // LLDB_VALUEOBJECT_VALUEOBJECT_H diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp index 5b67270859da2..adc03785602e1 100644 --- a/lldb/source/API/SBValue.cpp +++ b/lldb/source/API/SBValue.cpp @@ -52,172 +52,6 @@ using namespace lldb; using namespace lldb_private; -class ValueImpl { -public: - ValueImpl() = default; - - ValueImpl(lldb::ValueObjectSP in_valobj_sp, - lldb::DynamicValueType use_dynamic, bool use_synthetic, - const char *name = nullptr) - : m_use_dynamic(use_dynamic), m_use_synthetic(use_synthetic), - m_name(name) { - if (in_valobj_sp) { - if ((m_valobj_sp = in_valobj_sp->GetQualifiedRepresentationIfAvailable( - lldb::eNoDynamicValues, false))) { - if (!m_name.IsEmpty()) - m_valobj_sp->SetName(m_name); - } - } - } - - ValueImpl(const ValueImpl &rhs) = default; - - ValueImpl &operator=(const ValueImpl &rhs) { - if (this != &rhs) { - m_valobj_sp = rhs.m_valobj_sp; - m_use_dynamic = rhs.m_use_dynamic; - m_use_synthetic = rhs.m_use_synthetic; - m_name = rhs.m_name; - } - return *this; - } - - bool IsValid() { - if (m_valobj_sp.get() == nullptr) - return false; - else { - // FIXME: This check is necessary but not sufficient. We for sure don't - // want to touch SBValues whose owning - // targets have gone away. This check is a little weak in that it - // enforces that restriction when you call IsValid, but since IsValid - // doesn't lock the target, you have no guarantee that the SBValue won't - // go invalid after you call this... Also, an SBValue could depend on - // data from one of the modules in the target, and those could go away - // independently of the target, for instance if a module is unloaded. - // But right now, neither SBValues nor ValueObjects know which modules - // they depend on. So I have no good way to make that check without - // tracking that in all the ValueObject subclasses. - TargetSP target_sp = m_valobj_sp->GetTargetSP(); - return target_sp && target_sp->IsValid(); - } - } - - lldb::ValueObjectSP GetRootSP() { return m_valobj_sp; } - - lldb::ValueObjectSP GetSP(Process::StopLocker &stop_locker, - std::unique_lock<std::recursive_mutex> &lock, - Status &error) { - if (!m_valobj_sp) { - error = Status::FromErrorString("invalid value object"); - return m_valobj_sp; - } - - lldb::ValueObjectSP value_sp = m_valobj_sp; - - Target *target = value_sp->GetTargetSP().get(); - // If this ValueObject holds an error, then it is valuable for that. - if (value_sp->GetError().Fail()) - return value_sp; - - if (!target) - return ValueObjectSP(); - - lock = std::unique_lock<std::recursive_mutex>(target->GetAPIMutex()); - - ProcessSP process_sp(value_sp->GetProcessSP()); - if (process_sp && !stop_locker.TryLock(&process_sp->GetRunLock())) { - // We don't allow people to play around with ValueObject if the process - // is running. If you want to look at values, pause the process, then - // look. - error = Status::FromErrorString("process must be stopped."); - return ValueObjectSP(); - } - - if (m_use_dynamic != eNoDynamicValues) { - ValueObjectSP dynamic_sp = value_sp->GetDynamicValue(m_use_dynamic); - if (dynamic_sp) - value_sp = dynamic_sp; - } - - if (m_use_synthetic) { - ValueObjectSP synthetic_sp = value_sp->GetSyntheticValue(); - if (synthetic_sp) - value_sp = synthetic_sp; - } - - if (!value_sp) - error = Status::FromErrorString("invalid value object"); - if (!m_name.IsEmpty()) - value_sp->SetName(m_name); - - return value_sp; - } - - void SetUseDynamic(lldb::DynamicValueType use_dynamic) { - m_use_dynamic = use_dynamic; - } - - void SetUseSynthetic(bool use_synthetic) { m_use_synthetic = use_synthetic; } - - lldb::DynamicValueType GetUseDynamic() { return m_use_dynamic; } - - bool GetUseSynthetic() { return m_use_synthetic; } - - // All the derived values that we would make from the m_valobj_sp will share - // the ExecutionContext with m_valobj_sp, so we don't need to do the - // calculations in GetSP to return the Target, Process, Thread or Frame. It - // is convenient to provide simple accessors for these, which I do here. - TargetSP GetTargetSP() { - if (m_valobj_sp) - return m_valobj_sp->GetTargetSP(); - else - return TargetSP(); - } - - ProcessSP GetProcessSP() { - if (m_valobj_sp) - return m_valobj_sp->GetProcessSP(); - else - return ProcessSP(); - } - - ThreadSP GetThreadSP() { - if (m_valobj_sp) - return m_valobj_sp->GetThreadSP(); - else - return ThreadSP(); - } - - StackFrameSP GetFrameSP() { - if (m_valobj_sp) - return m_valobj_sp->GetFrameSP(); - else - return StackFrameSP(); - } - -private: - lldb::ValueObjectSP m_valobj_sp; - lldb::DynamicValueType m_use_dynamic; - bool m_use_synthetic; - ConstString m_name; -}; - -class ValueLocker { -public: - ValueLocker() = default; - - ValueObjectSP GetLockedSP(ValueImpl &in_value) { - return in_value.GetSP(m_stop_locker, m_lock, m_lock_error); - } - - Status &GetError() { return m_lock_error; } - -private: - Process::StopLocker m_stop_locker; - std::unique_lock<std::recursive_mutex> m_lock; - Status m_lock_error; -}; - SBValue::SBValue() { LLDB_INSTRUMENT_VA(this); } SBValue::SBValue(const lldb::ValueObjectSP &value_sp) { diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp index 121054e3e92ed..5b7c65a102e5d 100644 --- a/lldb/source/ValueObject/ValueObject.cpp +++ b/lldb/source/ValueObject/ValueObject.cpp @@ -3768,3 +3768,94 @@ ValueObjectSP ValueObject::Persist() { lldb::ValueObjectSP ValueObject::GetVTable() { return ValueObjectVTable::Create(*this); } + +ValueImpl::ValueImpl(lldb::ValueObjectSP in_valobj_sp, + lldb::DynamicValueType use_dynamic, bool use_synthetic, + const char *name) + : m_use_dynamic(use_dynamic), m_use_synthetic(use_synthetic), m_name(name) { + if (in_valobj_sp) { + if ((m_valobj_sp = in_valobj_sp->GetQualifiedRepresentationIfAvailable( + lldb::eNoDynamicValues, false))) { + if (!m_name.IsEmpty()) + m_valobj_sp->SetName(m_name); + } + } +} + +ValueImpl &ValueImpl::operator=(const ValueImpl &rhs) { + if (this != &rhs) { + m_valobj_sp = rhs.m_valobj_sp; + m_use_dynamic = rhs.m_use_dynamic; + m_use_synthetic = rhs.m_use_synthetic; + m_name = rhs.m_name; + } + return *this; +} + +bool ValueImpl::IsValid() { + if (m_valobj_sp.get() == nullptr) + return false; + + // FIXME: This check is necessary but not sufficient. We for sure don't + // want to touch SBValues whose owning + // targets have gone away. This check is a little weak in that it + // enforces that restriction when you call IsValid, but since IsValid + // doesn't lock the target, you have no guarantee that the SBValue won't + // go invalid after you call this... Also, an SBValue could depend on + // data from one of the modules in the target, and those could go away + // independently of the target, for instance if a module is unloaded. + // But right now, neither SBValues nor ValueObjects know which modules + // they depend on. So I have no good way to make that check without + // tracking that in all the ValueObject subclasses. + TargetSP target_sp = m_valobj_sp->GetTargetSP(); + return target_sp && target_sp->IsValid(); +} + +lldb::ValueObjectSP +ValueImpl::GetSP(Process::StopLocker &stop_locker, + std::unique_lock<std::recursive_mutex> &lock, Status &error) { + if (!m_valobj_sp) { + error = Status::FromErrorString("invalid value object"); + return m_valobj_sp; + } + + lldb::ValueObjectSP value_sp = m_valobj_sp; + + Target *target = value_sp->GetTargetSP().get(); + // If this ValueObject holds an error, then it is valuable for that. + if (value_sp->GetError().Fail()) + return value_sp; + + if (!target) + return ValueObjectSP(); + + lock = std::unique_lock<std::recursive_mutex>(target->GetAPIMutex()); + + ProcessSP process_sp(value_sp->GetProcessSP()); + if (process_sp && !stop_locker.TryLock(&process_sp->GetRunLock())) { + // We don't allow people to play around with ValueObject if the process + // is running. If you want to look at values, pause the process, then + // look. + error = Status::FromErrorString("process must be stopped."); + return ValueObjectSP(); + } + + if (m_use_dynamic != eNoDynamicValues) { + ValueObjectSP dynamic_sp = value_sp->GetDynamicValue(m_use_dynamic); + if (dynamic_sp) + value_sp = dynamic_sp; + } + + if (m_use_synthetic) { + ValueObjectSP synthetic_sp = value_sp->GetSyntheticValue(); + if (synthetic_sp) + value_sp = synthetic_sp; + } + + if (!value_sp) + error = Status::FromErrorString("invalid value object"); + if (!m_name.IsEmpty()) + value_sp->SetName(m_name); + + return value_sp; +} `````````` </details> https://github.com/llvm/llvm-project/pull/179500 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
