[Lldb-commits] [lldb] 317c8bf - [LLDB][Expression] Allow instantiation of IR Entity from ValueObject

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Fri Jul 22 00:02:51 PDT 2022


Author: Michael Buch
Date: 2022-07-22T08:02:08+01:00
New Revision: 317c8bf84d185c6b4a51a415c0deb7f8af661cb6

URL: https://github.com/llvm/llvm-project/commit/317c8bf84d185c6b4a51a415c0deb7f8af661cb6
DIFF: https://github.com/llvm/llvm-project/commit/317c8bf84d185c6b4a51a415c0deb7f8af661cb6.diff

LOG: [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.

Added: 
    lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/Makefile
    lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
    lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp

Modified: 
    lldb/include/lldb/Expression/Materializer.h
    lldb/include/lldb/lldb-private-types.h
    lldb/source/Expression/Materializer.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Expression/Materializer.h b/lldb/include/lldb/Expression/Materializer.h
index 25cf22a8b5b0b..aae94f86a71e6 100644
--- a/lldb/include/lldb/Expression/Materializer.h
+++ b/lldb/include/lldb/Expression/Materializer.h
@@ -78,6 +78,28 @@ class Materializer {
   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 3be7003cd0fb2..1b0d263e2073b 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<lldb::ValueObjectSP(ConstString, StackFrame *)>;
+
 } // namespace lldb_private
 
 #endif // #if defined(__cplusplus)

diff  --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp
index 8c7e74b6e51e5..946ae10d69c28 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 <memory>
 
@@ -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 @@ class EntityVariable : public Materializer::Entity {
       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 @@ class EntityVariable : public Materializer::Entity {
     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 @@ class EntityVariable : public Materializer::Entity {
 
     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 @@ class EntityVariable : public Materializer::Entity {
       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 @@ class EntityVariable : public Materializer::Entity {
       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 @@ class EntityVariable : public Materializer::Entity {
         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 @@ class EntityVariable : public Materializer::Entity {
         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 @@ class EntityVariable : public Materializer::Entity {
         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<size_t> opt_bit_align =
-            m_variable_sp->GetType()->GetLayoutCompilerType().GetTypeBitAlign(scope);
+        llvm::Optional<size_t> 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 @@ class EntityVariable : public Materializer::Entity {
         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 @@ class EntityVariable : public Materializer::Entity {
         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 @@ class EntityVariable : public Materializer::Entity {
         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 @@ class EntityVariable : public Materializer::Entity {
       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 @@ class EntityVariable : public Materializer::Entity {
       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 @@ class EntityVariable : public Materializer::Entity {
 
       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 @@ class EntityVariable : public Materializer::Entity {
         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 @@ class EntityVariable : public Materializer::Entity {
       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 @@ class EntityVariable : public Materializer::Entity {
   }
 
 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 
diff erent 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<uint64_t>
+  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<size_t>
+  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<uint64_t>
+  GetByteSize(ExecutionContextScope *scope) const override {
+    return m_variable_sp->GetType()->GetByteSize(scope);
+  }
+
+  bool LocationExpressionIsValid() const override {
+    return m_variable_sp->LocationExpressionList().IsValid();
+  }
+
+  llvm::Optional<size_t>
+  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<uint64_t>
+  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<size_t>
+  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<EntityVariable>(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<EntityValueObject>(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 0000000000000..c46619c662348
--- /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 0000000000000..da6af1ea5f60d
--- /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 0000000000000..d86e9daacf670
--- /dev/null
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp
@@ -0,0 +1,32 @@
+#include <chrono>
+#include <cstdio>
+#include <thread>
+
+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<unsigned int, std::milli>(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);
+  }
+}


        


More information about the lldb-commits mailing list