From 317c8bf84d185c6b4a51a415c0deb7f8af661cb6 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Thu, 7 Jul 2022 15:58:42 +0100 Subject: [PATCH] [LLDB][Expression] Allow instantiation of IR Entity from ValueObject This is required in preparation for the follow-up patch which adds support for evaluating expressions from within C++ lambdas. In such cases we need to materialize variables which are not part of the current frame but instead are ivars on a 'this' pointer of the current frame. --- lldb/include/lldb/Expression/Materializer.h | 22 +++ lldb/include/lldb/lldb-private-types.h | 6 + lldb/source/Expression/Materializer.cpp | 209 +++++++++++++++++---- .../breakpoint_on_lambda_capture/Makefile | 4 + .../TestBreakOnLambdaCapture.py | 54 ++++++ .../breakpoint_on_lambda_capture/main.cpp | 32 ++++ 6 files changed, 290 insertions(+), 37 deletions(-) create mode 100644 lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/Makefile create mode 100644 lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py create mode 100644 lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp diff --git a/lldb/include/lldb/Expression/Materializer.h b/lldb/include/lldb/Expression/Materializer.h index 25cf22a..aae94f8 100644 --- a/lldb/include/lldb/Expression/Materializer.h +++ b/lldb/include/lldb/Expression/Materializer.h @@ -78,6 +78,28 @@ public: AddPersistentVariable(lldb::ExpressionVariableSP &persistent_variable_sp, PersistentVariableDelegate *delegate, Status &err); uint32_t AddVariable(lldb::VariableSP &variable_sp, Status &err); + + /// Create entity from supplied ValueObject and count it as a member + /// of the materialized struct. + /// + /// Behaviour is undefined if 'valobj_provider' is empty. + /// + /// \param[in] name Name of variable to materialize + /// + /// \param[in] valobj_provider When materializing values multiple + /// times, this callback gets used to fetch a fresh + /// ValueObject corresponding to the supplied frame. + /// This is mainly used for conditional breakpoints + /// that re-apply an expression whatever the frame + /// happens to be when the breakpoint got hit. + /// + /// \param[out] err Error status that gets set on error. + /// + /// \returns Offset in bytes of the member we just added to the + /// materialized struct. + uint32_t AddValueObject(ConstString name, + ValueObjectProviderTy valobj_provider, Status &err); + uint32_t AddResultVariable(const CompilerType &type, bool is_lvalue, bool keep_in_memory, PersistentVariableDelegate *delegate, Status &err); diff --git a/lldb/include/lldb/lldb-private-types.h b/lldb/include/lldb/lldb-private-types.h index 3be7003..1b0d263 100644 --- a/lldb/include/lldb/lldb-private-types.h +++ b/lldb/include/lldb/lldb-private-types.h @@ -106,6 +106,12 @@ struct OptionValidator { typedef struct type128 { uint64_t x[2]; } type128; typedef struct type256 { uint64_t x[4]; } type256; +/// Functor that returns a ValueObjectSP for a variable given its name +/// and the StackFrame of interest. Used primarily in the Materializer +/// to refetch a ValueObject when the ExecutionContextScope changes. +using ValueObjectProviderTy = + std::function; + } // namespace lldb_private #endif // #if defined(__cplusplus) diff --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp index 8c7e74b..946ae10 100644 --- a/lldb/source/Expression/Materializer.cpp +++ b/lldb/source/Expression/Materializer.cpp @@ -22,6 +22,7 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/RegisterValue.h" +#include "lldb/lldb-forward.h" #include @@ -420,16 +421,19 @@ uint32_t Materializer::AddPersistentVariable( return ret; } -class EntityVariable : public Materializer::Entity { +/// Base class for materialization of Variables and ValueObjects. +/// +/// Subclasses specify how to obtain the Value which is to be +/// materialized. +class EntityVariableBase : public Materializer::Entity { public: - EntityVariable(lldb::VariableSP &variable_sp) - : Entity(), m_variable_sp(variable_sp) { + virtual ~EntityVariableBase() = default; + + EntityVariableBase() { // Hard-coding to maximum size of a pointer since all variables are // materialized by reference m_size = g_default_var_byte_size; m_alignment = g_default_var_alignment; - m_is_reference = - m_variable_sp->GetType()->GetForwardCompilerType().IsReferenceType(); } void Materialize(lldb::StackFrameSP &frame_sp, IRMemoryMap &map, @@ -441,7 +445,7 @@ public: LLDB_LOGF(log, "EntityVariable::Materialize [address = 0x%" PRIx64 ", m_variable_sp = %s]", - (uint64_t)load_addr, m_variable_sp->GetName().AsCString()); + (uint64_t)load_addr, GetName().GetCString()); } ExecutionContextScope *scope = frame_sp.get(); @@ -449,13 +453,11 @@ public: if (!scope) scope = map.GetBestExecutionContextScope(); - lldb::ValueObjectSP valobj_sp = - ValueObjectVariable::Create(scope, m_variable_sp); + lldb::ValueObjectSP valobj_sp = SetupValueObject(scope); if (!valobj_sp) { err.SetErrorStringWithFormat( - "couldn't get a value object for variable %s", - m_variable_sp->GetName().AsCString()); + "couldn't get a value object for variable %s", GetName().AsCString()); return; } @@ -463,7 +465,7 @@ public: if (valobj_error.Fail()) { err.SetErrorStringWithFormat("couldn't get the value of variable %s: %s", - m_variable_sp->GetName().AsCString(), + GetName().AsCString(), valobj_error.AsCString()); return; } @@ -476,7 +478,7 @@ public: if (!extract_error.Success()) { err.SetErrorStringWithFormat( "couldn't read contents of reference variable %s: %s", - m_variable_sp->GetName().AsCString(), extract_error.AsCString()); + GetName().AsCString(), extract_error.AsCString()); return; } @@ -489,7 +491,7 @@ public: if (!write_error.Success()) { err.SetErrorStringWithFormat("couldn't write the contents of reference " "variable %s to memory: %s", - m_variable_sp->GetName().AsCString(), + GetName().AsCString(), write_error.AsCString()); return; } @@ -505,7 +507,7 @@ public: if (!write_error.Success()) { err.SetErrorStringWithFormat( "couldn't write the address of variable %s to memory: %s", - m_variable_sp->GetName().AsCString(), write_error.AsCString()); + GetName().AsCString(), write_error.AsCString()); return; } } else { @@ -514,7 +516,7 @@ public: valobj_sp->GetData(data, extract_error); if (!extract_error.Success()) { err.SetErrorStringWithFormat("couldn't get the value of %s: %s", - m_variable_sp->GetName().AsCString(), + GetName().AsCString(), extract_error.AsCString()); return; } @@ -522,32 +524,29 @@ public: if (m_temporary_allocation != LLDB_INVALID_ADDRESS) { err.SetErrorStringWithFormat( "trying to create a temporary region for %s but one exists", - m_variable_sp->GetName().AsCString()); + GetName().AsCString()); return; } - if (data.GetByteSize() < m_variable_sp->GetType()->GetByteSize(scope)) { - if (data.GetByteSize() == 0 && - !m_variable_sp->LocationExpressionList().IsValid()) { + if (data.GetByteSize() < GetByteSize(scope)) { + if (data.GetByteSize() == 0 && !LocationExpressionIsValid()) { err.SetErrorStringWithFormat("the variable '%s' has no location, " "it may have been optimized out", - m_variable_sp->GetName().AsCString()); + GetName().AsCString()); } else { err.SetErrorStringWithFormat( "size of variable %s (%" PRIu64 ") is larger than the ValueObject's size (%" PRIu64 ")", - m_variable_sp->GetName().AsCString(), - m_variable_sp->GetType()->GetByteSize(scope).value_or(0), + GetName().AsCString(), GetByteSize(scope).value_or(0), data.GetByteSize()); } return; } - llvm::Optional opt_bit_align = - m_variable_sp->GetType()->GetLayoutCompilerType().GetTypeBitAlign(scope); + llvm::Optional opt_bit_align = GetTypeBitAlign(scope); if (!opt_bit_align) { err.SetErrorStringWithFormat("can't get the type alignment for %s", - m_variable_sp->GetName().AsCString()); + GetName().AsCString()); return; } @@ -569,7 +568,7 @@ public: if (!alloc_error.Success()) { err.SetErrorStringWithFormat( "couldn't allocate a temporary region for %s: %s", - m_variable_sp->GetName().AsCString(), alloc_error.AsCString()); + GetName().AsCString(), alloc_error.AsCString()); return; } @@ -581,7 +580,7 @@ public: if (!write_error.Success()) { err.SetErrorStringWithFormat( "couldn't write to the temporary region for %s: %s", - m_variable_sp->GetName().AsCString(), write_error.AsCString()); + GetName().AsCString(), write_error.AsCString()); return; } @@ -593,8 +592,7 @@ public: if (!pointer_write_error.Success()) { err.SetErrorStringWithFormat( "couldn't write the address of the temporary region for %s: %s", - m_variable_sp->GetName().AsCString(), - pointer_write_error.AsCString()); + GetName().AsCString(), pointer_write_error.AsCString()); } } } @@ -610,7 +608,7 @@ public: LLDB_LOGF(log, "EntityVariable::Dematerialize [address = 0x%" PRIx64 ", m_variable_sp = %s]", - (uint64_t)load_addr, m_variable_sp->GetName().AsCString()); + (uint64_t)load_addr, GetName().AsCString()); } if (m_temporary_allocation != LLDB_INVALID_ADDRESS) { @@ -619,13 +617,12 @@ public: if (!scope) scope = map.GetBestExecutionContextScope(); - lldb::ValueObjectSP valobj_sp = - ValueObjectVariable::Create(scope, m_variable_sp); + lldb::ValueObjectSP valobj_sp = SetupValueObject(scope); if (!valobj_sp) { err.SetErrorStringWithFormat( "couldn't get a value object for variable %s", - m_variable_sp->GetName().AsCString()); + GetName().AsCString()); return; } @@ -638,7 +635,7 @@ public: if (!extract_error.Success()) { err.SetErrorStringWithFormat("couldn't get the data for variable %s", - m_variable_sp->GetName().AsCString()); + GetName().AsCString()); return; } @@ -660,7 +657,7 @@ public: if (!set_error.Success()) { err.SetErrorStringWithFormat( "couldn't write the new contents of %s back into the variable", - m_variable_sp->GetName().AsCString()); + GetName().AsCString()); return; } } @@ -672,7 +669,7 @@ public: if (!free_error.Success()) { err.SetErrorStringWithFormat( "couldn't free the temporary region for %s: %s", - m_variable_sp->GetName().AsCString(), free_error.AsCString()); + GetName().AsCString(), free_error.AsCString()); return; } @@ -756,13 +753,140 @@ public: } private: - lldb::VariableSP m_variable_sp; + virtual ConstString GetName() const = 0; + + /// Creates and returns ValueObject tied to this variable + /// and prepares Entity for materialization. + /// + /// Called each time the Materializer (de)materializes a + /// variable. We re-create the ValueObject based on the + /// current ExecutionContextScope since clients such as + /// conditional breakpoints may materialize the same + /// EntityVariable multiple times with different frames. + /// + /// Each subsequent use of the EntityVariableBase interface + /// will query the newly created ValueObject until this + /// function is called again. + virtual lldb::ValueObjectSP + SetupValueObject(ExecutionContextScope *scope) = 0; + + /// Returns size in bytes of the type associated with this variable + /// + /// \returns On success, returns byte size of the type associated + /// with this variable. Returns NoneType otherwise. + virtual llvm::Optional + GetByteSize(ExecutionContextScope *scope) const = 0; + + /// Returns 'true' if the location expression associated with this variable + /// is valid. + virtual bool LocationExpressionIsValid() const = 0; + + /// Returns alignment of the type associated with this variable in bits. + /// + /// \returns On success, returns alignment in bits for the type associated + /// with this variable. Returns NoneType otherwise. + virtual llvm::Optional + GetTypeBitAlign(ExecutionContextScope *scope) const = 0; + +protected: bool m_is_reference = false; lldb::addr_t m_temporary_allocation = LLDB_INVALID_ADDRESS; size_t m_temporary_allocation_size = 0; lldb::DataBufferSP m_original_data; }; +/// Represents an Entity constructed from a VariableSP. +/// +/// This class is used for materialization of variables for which +/// the user has a VariableSP on hand. The ValueObject is then +/// derived from the associated DWARF location expression when needed +/// by the Materializer. +class EntityVariable : public EntityVariableBase { +public: + EntityVariable(lldb::VariableSP &variable_sp) : m_variable_sp(variable_sp) { + m_is_reference = + m_variable_sp->GetType()->GetForwardCompilerType().IsReferenceType(); + } + + ConstString GetName() const override { return m_variable_sp->GetName(); } + + lldb::ValueObjectSP SetupValueObject(ExecutionContextScope *scope) override { + assert(m_variable_sp != nullptr); + return ValueObjectVariable::Create(scope, m_variable_sp); + } + + llvm::Optional + GetByteSize(ExecutionContextScope *scope) const override { + return m_variable_sp->GetType()->GetByteSize(scope); + } + + bool LocationExpressionIsValid() const override { + return m_variable_sp->LocationExpressionList().IsValid(); + } + + llvm::Optional + GetTypeBitAlign(ExecutionContextScope *scope) const override { + return m_variable_sp->GetType()->GetLayoutCompilerType().GetTypeBitAlign( + scope); + } + +private: + lldb::VariableSP m_variable_sp; ///< Variable that this entity is based on. +}; + +/// Represents an Entity constructed from a VariableSP. +/// +/// This class is used for materialization of variables for +/// which the user does not have a VariableSP available (e.g., +/// when materializing ivars). +class EntityValueObject : public EntityVariableBase { +public: + EntityValueObject(ConstString name, ValueObjectProviderTy provider) + : m_name(name), m_valobj_provider(std::move(provider)) { + assert(m_valobj_provider); + } + + ConstString GetName() const override { return m_name; } + + lldb::ValueObjectSP SetupValueObject(ExecutionContextScope *scope) override { + m_valobj_sp = + m_valobj_provider(GetName(), scope->CalculateStackFrame().get()); + + if (m_valobj_sp) + m_is_reference = m_valobj_sp->GetCompilerType().IsReferenceType(); + + return m_valobj_sp; + } + + llvm::Optional + GetByteSize(ExecutionContextScope *scope) const override { + if (m_valobj_sp) + return m_valobj_sp->GetCompilerType().GetByteSize(scope); + + return {}; + } + + bool LocationExpressionIsValid() const override { + if (m_valobj_sp) + return m_valobj_sp->GetError().Success(); + + return false; + } + + llvm::Optional + GetTypeBitAlign(ExecutionContextScope *scope) const override { + if (m_valobj_sp) + return m_valobj_sp->GetCompilerType().GetTypeBitAlign(scope); + + return {}; + } + +private: + ConstString m_name; + lldb::ValueObjectSP m_valobj_sp; + ValueObjectProviderTy m_valobj_provider; +}; + uint32_t Materializer::AddVariable(lldb::VariableSP &variable_sp, Status &err) { EntityVector::iterator iter = m_entities.insert(m_entities.end(), EntityUP()); *iter = std::make_unique(variable_sp); @@ -771,6 +895,17 @@ uint32_t Materializer::AddVariable(lldb::VariableSP &variable_sp, Status &err) { return ret; } +uint32_t Materializer::AddValueObject(ConstString name, + ValueObjectProviderTy valobj_provider, + Status &err) { + assert(valobj_provider); + EntityVector::iterator iter = m_entities.insert(m_entities.end(), EntityUP()); + *iter = std::make_unique(name, std::move(valobj_provider)); + uint32_t ret = AddStructMember(**iter); + (*iter)->SetOffset(ret); + return ret; +} + class EntityResultVariable : public Materializer::Entity { public: EntityResultVariable(const CompilerType &type, bool is_program_reference, diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/Makefile b/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/Makefile new file mode 100644 index 0000000..c46619c --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/Makefile @@ -0,0 +1,4 @@ +CXX_SOURCES := main.cpp +ENABLE_THREADS := YES + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py b/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py new file mode 100644 index 0000000..da6af1e --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py @@ -0,0 +1,54 @@ +""" +Test that if we hit a breakpoint on a lambda capture +on two threads at the same time we stop only for +the correct one. +""" + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + + +class TestBreakOnLambdaCapture(TestBase): + + NO_DEBUG_INFO_TESTCASE = True + + def test_break_on_lambda_capture(self): + self.build() + self.main_source_file = lldb.SBFileSpec("main.cpp") + + (target, process, main_thread, _) = lldbutil.run_to_source_breakpoint(self, + "First break", self.main_source_file) + + # FIXME: This is working around a separate bug. If you hit a breakpoint and + # run an expression and it is the first expression you've ever run, on + # Darwin that will involve running the ObjC runtime parsing code, and we'll + # be in the middle of that when we do PerformAction on the other thread, + # which will cause the condition expression to fail. Calling another + # expression first works around this. + val_obj = main_thread.frame[0].EvaluateExpression("true") + self.assertSuccess(val_obj.GetError(), "Ran our expression successfully") + self.assertEqual(val_obj.value, "true", "Value was true.") + + bkpt = target.BreakpointCreateBySourceRegex("Break here in the helper", + self.main_source_file); + + bkpt.SetCondition("enable && usec == 1") + process.Continue() + + # This is hard to test definitively, becuase it requires hitting + # a breakpoint on multiple threads at the same time. On Darwin, this + # will happen pretty much ever time we continue. What we are really + # asserting is that we only ever stop on one thread, so we approximate that + # by continuing 20 times and assert we only ever hit the first thread. Either + # this is a platform that only reports one hit at a time, in which case all + # this code is unused, or we actually didn't hit the other thread. + + for idx in range(0, 20): + process.Continue() + for thread in process.threads: + if thread.id == main_thread.id: + self.assertEqual(thread.stop_reason, lldb.eStopReasonBreakpoint) + else: + self.assertEqual(thread.stop_reason, lldb.eStopReasonNone) diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp b/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp new file mode 100644 index 0000000..d86e9da --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp @@ -0,0 +1,32 @@ +#include +#include +#include + +struct Foo { + bool enable = true; + uint32_t offset = 0; + + void usleep_helper(uint32_t usec) { + [this, &usec] { + puts("Break here in the helper"); + std::this_thread::sleep_for( + std::chrono::duration(offset + usec)); + }(); + } +}; + +void *background_thread(void *) { + Foo f; + for (;;) { + f.usleep_helper(2); + } +} + +int main() { + std::puts("First break"); + std::thread main_thread(background_thread, nullptr); + Foo f; + for (;;) { + f.usleep_helper(1); + } +} -- 2.7.4