Make result variables obey their dynamic values in subsequent expressions

lldb/include/lldb/Expression/ExpressionVariable.h

@@ -33,11 +33,19 @@ class ExpressionVariable
 
   virtual ~ExpressionVariable() = default;
 
-  llvm::Expected<uint64_t> GetByteSize() { return m_frozen_sp->GetByteSize(); }
+  llvm::Expected<uint64_t> GetByteSize() {
+    return GetValueObject()->GetByteSize();
+  }
 
   ConstString GetName() { return m_frozen_sp->GetName(); }
 
-  lldb::ValueObjectSP GetValueObject() { return m_frozen_sp; }
+  lldb::ValueObjectSP GetValueObject() {
+    lldb::ValueObjectSP dyn_sp =
+        m_frozen_sp->GetDynamicValue(lldb::eDynamicDontRunTarget);
+    if (dyn_sp && dyn_sp->UpdateValueIfNeeded())
+      return dyn_sp;
+    return m_frozen_sp;
+  }
 
   uint8_t *GetValueBytes();
 
@@ -52,31 +60,39 @@ class ExpressionVariable
         Value::ContextType::RegisterInfo, const_cast<RegisterInfo *>(reg_info));
   }
 
-  CompilerType GetCompilerType() { return m_frozen_sp->GetCompilerType(); }
+  CompilerType GetCompilerType() { return GetValueObject()->GetCompilerType(); }
 
   void SetCompilerType(const CompilerType &compiler_type) {
     m_frozen_sp->GetValue().SetCompilerType(compiler_type);
   }
 
   void SetName(ConstString name) { m_frozen_sp->SetName(name); }
 
-  // this function is used to copy the address-of m_live_sp into m_frozen_sp
-  // this is necessary because the results of certain cast and pointer-
+  // This function is used to copy the address-of m_live_sp into m_frozen_sp.
+  // It is necessary because the results of certain cast and pointer-
   // arithmetic operations (such as those described in bugzilla issues 11588
   // and 11618) generate frozen objects that do not have a valid address-of,
   // which can be troublesome when using synthetic children providers.
   // Transferring the address-of the live object solves these issues and
-  // provides the expected user-level behavior
-  void TransferAddress(bool force = false) {
-    if (m_live_sp.get() == nullptr)
-      return;
-
-    if (m_frozen_sp.get() == nullptr)
-      return;
-
-    if (force || (m_frozen_sp->GetLiveAddress() == LLDB_INVALID_ADDRESS))
-      m_frozen_sp->SetLiveAddress(m_live_sp->GetLiveAddress());
+  // provides the expected user-level behavior.
+  // The other job we do in TransferAddress is adjust the value in the live
+  // address slot in the target for the "offset to top" in multiply inherited
+  // class hierarchies.
+  void TransferAddress(bool force = false);
+
+  // When we build an expression variable we know whether we're going to use the
+  // static or dynamic result.  If we present the dynamic value once, we should
+  // use the dynamic value in future references to the variable, so we record
+  // that fact here.
+  void PreserveDynamicOption(lldb::DynamicValueType dyn_type) {
+    m_dyn_option = dyn_type;
   }
+  // We don't try to get the dynamic value of the live object when we fetch
+  // it here.  The live object describes the container of the value in the
+  // target, but it's type is of the object for convenience.  So it can't
+  // produce the dynamic value.  Instead, we use TransferAddress to adjust the
+  // value held by the LiveObject.
+  lldb::ValueObjectSP GetLiveObject() { return m_live_sp; }
 
   enum Flags {
     EVNone = 0,
@@ -110,6 +126,14 @@ class ExpressionVariable
   /// These members should be private.
   /// @{
   /// A value object whose value's data lives in host (lldb's) memory.
+  /// The m_frozen_sp holds the data & type of the expression variable or result
+  /// in the host.  The m_frozen_sp also can present a dynamic value if one is
+  /// available.
+  /// The m_frozen_sp manages the copy of this value in m_frozen_sp that we
+  /// insert in the target so that it can be referred to in future expressions.
+  /// We don't actually use the contents of the live_sp to create the value in
+  /// the target, that comes from the frozen sp.  The live_sp is mostly to track
+  /// the target-side of the value.
   lldb::ValueObjectSP m_frozen_sp;
   /// The ValueObject counterpart to m_frozen_sp that tracks the value in
   /// inferior memory. This object may not always exist; its presence depends on
@@ -119,6 +143,8 @@ class ExpressionVariable
   /// track.
   lldb::ValueObjectSP m_live_sp;
   /// @}
+
+  lldb::DynamicValueType m_dyn_option = lldb::eNoDynamicValues;
 };
 
 /// \class ExpressionVariableList ExpressionVariable.h

lldb/packages/Python/lldbsuite/test/lldbtest.py

@@ -2540,6 +2540,7 @@ def expect_expr(
         result_value=None,
         result_type=None,
         result_children=None,
+        options=None,
     ):
         """
         Evaluates the given expression and verifies the result.
@@ -2556,7 +2557,8 @@ def expect_expr(
         )
 
         frame = self.frame()
-        options = lldb.SBExpressionOptions()
+        if not options:
+            options = lldb.SBExpressionOptions()
 
         # Disable fix-its that tests don't pass by accident.
         options.SetAutoApplyFixIts(False)

lldb/source/Expression/ExpressionVariable.cpp

@@ -20,15 +20,15 @@ char ExpressionVariable::ID;
 ExpressionVariable::ExpressionVariable() : m_flags(0) {}
 
 uint8_t *ExpressionVariable::GetValueBytes() {
+  lldb::ValueObjectSP valobj_sp = GetValueObject();
   std::optional<uint64_t> byte_size =
-      llvm::expectedToOptional(m_frozen_sp->GetByteSize());
+      llvm::expectedToOptional(valobj_sp->GetByteSize());
   if (byte_size && *byte_size) {
-    if (m_frozen_sp->GetDataExtractor().GetByteSize() < *byte_size) {
-      m_frozen_sp->GetValue().ResizeData(*byte_size);
-      m_frozen_sp->GetValue().GetData(m_frozen_sp->GetDataExtractor());
+    if (valobj_sp->GetDataExtractor().GetByteSize() < *byte_size) {
+      valobj_sp->GetValue().ResizeData(*byte_size);
+      valobj_sp->GetValue().GetData(valobj_sp->GetDataExtractor());
     }
-    return const_cast<uint8_t *>(
-        m_frozen_sp->GetDataExtractor().GetDataStart());
+    return const_cast<uint8_t *>(valobj_sp->GetDataExtractor().GetDataStart());
   }
   return nullptr;
 }
@@ -37,6 +37,72 @@ char PersistentExpressionState::ID;
 
 PersistentExpressionState::PersistentExpressionState() = default;
 
+void ExpressionVariable::TransferAddress(bool force) {
+  if (!m_live_sp)
+    return;
+
+  if (!m_frozen_sp)
+    return;
+
+  if (force || (m_frozen_sp->GetLiveAddress() == LLDB_INVALID_ADDRESS)) {
+    lldb::addr_t live_addr = m_live_sp->GetLiveAddress();
+    m_frozen_sp->SetLiveAddress(live_addr);
+    // One more detail, if there's an offset_to_top in the frozen_sp, then we
+    // need to appy that offset by hand.  The live_sp can't compute this
+    // itself as its type is the type of the contained object which confuses
+    // the dynamic type calculation.  So we have to update the contents of the
+    // m_live_sp with the dynamic value.
+    // Note: We could get this right when we originally write the address, but
+    // that happens in different ways for the various flavors of
+    // Entity*::Materialize, but everything comes through here, and it's just
+    // one extra memory write.
+
+    // You can only have an "offset_to_top" with pointers or references:
+    if (!m_frozen_sp->GetCompilerType().IsPointerOrReferenceType())
+      return;
+
+    lldb::ProcessSP process_sp = m_frozen_sp->GetProcessSP();
+    // If there's no dynamic value, then there can't be an offset_to_top:
+    if (!process_sp ||
+        !process_sp->IsPossibleDynamicValue(*(m_frozen_sp.get())))
+      return;
+
+    lldb::ValueObjectSP dyn_sp = m_frozen_sp->GetDynamicValue(m_dyn_option);
+    if (!dyn_sp)
+      return;
+    ValueObject::AddrAndType static_addr = m_frozen_sp->GetPointerValue();
+    if (static_addr.type != eAddressTypeLoad)
+      return;
+
+    ValueObject::AddrAndType dynamic_addr = dyn_sp->GetPointerValue();
+    if (dynamic_addr.type != eAddressTypeLoad ||
+        static_addr.address == dynamic_addr.address)
+      return;
+
+    Status error;
+    Log *log = GetLog(LLDBLog::Expressions);
+    lldb::addr_t cur_value =
+        process_sp->ReadPointerFromMemory(live_addr, error);
+    if (error.Fail())
+      return;
+
+    if (cur_value != static_addr.address) {
+      LLDB_LOG(log,
+               "Stored value: {0} read from {1} doesn't "
+               "match static addr: {2}",
+               cur_value, live_addr, static_addr.address);
+      return;
+    }
+
+    if (!process_sp->WritePointerToMemory(live_addr, dynamic_addr.address,
+                                          error)) {
+      LLDB_LOG(log, "Got error: {0} writing dynamic value: {1} to {2}", error,
+               dynamic_addr.address, live_addr);
+      return;
+    }
+  }
+}
+
 PersistentExpressionState::~PersistentExpressionState() = default;
 
 lldb::addr_t PersistentExpressionState::LookupSymbol(ConstString name) {

lldb/source/Expression/LLVMUserExpression.cpp

@@ -65,7 +65,7 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
                               ExecutionContext &exe_ctx,
                               const EvaluateExpressionOptions &options,
                               lldb::UserExpressionSP &shared_ptr_to_me,
-                              lldb::ExpressionVariableSP &result) {
+                              lldb::ExpressionVariableSP &result_sp) {
   // The expression log is quite verbose, and if you're just tracking the
   // execution of the expression, it's quite convenient to have these logs come
   // out with the STEP log as well.
@@ -250,8 +250,14 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
     }
   }
 
-  if (FinalizeJITExecution(diagnostic_manager, exe_ctx, result,
+  if (FinalizeJITExecution(diagnostic_manager, exe_ctx, result_sp,
                            function_stack_bottom, function_stack_top)) {
+    //    if (result_sp) {
+    //      // This is a bit of a hack, replace with a real API.  Trying to
+    //      force
+    //      // fetching all the dynamic info at this point.
+    //      result_sp->GetValueObject();
+    //    }
     return lldb::eExpressionCompleted;
   }
 
@@ -289,8 +295,14 @@ bool LLVMUserExpression::FinalizeJITExecution(
   result =
       GetResultAfterDematerialization(exe_ctx.GetBestExecutionContextScope());
 
-  if (result)
+  if (result) {
+    EvaluateExpressionOptions *options = GetOptions();
+    // TransferAddress also does the offset_to_top calculation, so record the
+    // dynamic option before we do that.
+    if (options)
+      result->PreserveDynamicOption(options->GetUseDynamic());
     result->TransferAddress();
+  }
 
   m_dematerializer_sp.reset();
 

lldb/source/Expression/Materializer.cpp

@@ -76,10 +76,11 @@ class EntityPersistentVariable : public Materializer::Entity {
 
     const bool zero_memory = false;
     IRMemoryMap::AllocationPolicy used_policy;
-    auto address_or_error = map.Malloc(
+    uint64_t malloc_size =
         llvm::expectedToOptional(m_persistent_variable_sp->GetByteSize())
-            .value_or(0),
-        8, lldb::ePermissionsReadable | lldb::ePermissionsWritable,
+            .value_or(0);
+    auto address_or_error = map.Malloc(
+        malloc_size, 8, lldb::ePermissionsReadable | lldb::ePermissionsWritable,
         IRMemoryMap::eAllocationPolicyMirror, zero_memory, &used_policy);
     if (!address_or_error) {
       err = Status::FromErrorStringWithFormat(
@@ -90,8 +91,9 @@ class EntityPersistentVariable : public Materializer::Entity {
     }
     lldb::addr_t mem = *address_or_error;
 
-    LLDB_LOGF(log, "Allocated %s (0x%" PRIx64 ") successfully",
-              m_persistent_variable_sp->GetName().GetCString(), mem);
+    LLDB_LOGF(
+        log, "Allocated 0x%" PRIx64 "bytes for %s (0x%" PRIx64 ") successfully",
+        malloc_size, m_persistent_variable_sp->GetName().GetCString(), mem);
 
     // Put the location of the spare memory into the live data of the
     // ValueObject.
@@ -143,12 +145,12 @@ class EntityPersistentVariable : public Materializer::Entity {
   void DestroyAllocation(IRMemoryMap &map, Status &err) {
     Status deallocate_error;
 
-    map.Free((lldb::addr_t)m_persistent_variable_sp->m_live_sp->GetValue()
-                 .GetScalar()
-                 .ULongLong(),
+    lldb::ValueObjectSP live_valobj_sp =
+        m_persistent_variable_sp->GetLiveObject();
+    map.Free((lldb::addr_t)live_valobj_sp->GetValue().GetScalar().ULongLong(),
              deallocate_error);
 
-    m_persistent_variable_sp->m_live_sp.reset();
+    live_valobj_sp.reset();
 
     if (!deallocate_error.Success()) {
       err = Status::FromErrorStringWithFormat(
@@ -183,17 +185,17 @@ class EntityPersistentVariable : public Materializer::Entity {
         return;
     }
 
+    lldb::ValueObjectSP live_valobj_sp =
+        m_persistent_variable_sp->GetLiveObject();
     if ((m_persistent_variable_sp->m_flags &
              ExpressionVariable::EVIsProgramReference &&
-         m_persistent_variable_sp->m_live_sp) ||
+         live_valobj_sp) ||
         m_persistent_variable_sp->m_flags &
             ExpressionVariable::EVIsLLDBAllocated) {
       Status write_error;
 
-      map.WriteScalarToMemory(
-          load_addr,
-          m_persistent_variable_sp->m_live_sp->GetValue().GetScalar(),
-          map.GetAddressByteSize(), write_error);
+      map.WriteScalarToMemory(load_addr, live_valobj_sp->GetValue().GetScalar(),
+                              map.GetAddressByteSize(), write_error);
 
       if (!write_error.Success()) {
         err = Status::FromErrorStringWithFormat(
@@ -229,13 +231,15 @@ class EntityPersistentVariable : public Materializer::Entity {
       m_delegate->DidDematerialize(m_persistent_variable_sp);
     }
 
+    lldb::ValueObjectSP live_valobj_sp =
+        m_persistent_variable_sp->GetLiveObject();
     if ((m_persistent_variable_sp->m_flags &
          ExpressionVariable::EVIsLLDBAllocated) ||
         (m_persistent_variable_sp->m_flags &
          ExpressionVariable::EVIsProgramReference)) {
       if (m_persistent_variable_sp->m_flags &
               ExpressionVariable::EVIsProgramReference &&
-          !m_persistent_variable_sp->m_live_sp) {
+          !live_valobj_sp) {
         // If the reference comes from the program, then the
         // ClangExpressionVariable's live variable data hasn't been set up yet.
         // Do this now.
@@ -255,7 +259,7 @@ class EntityPersistentVariable : public Materializer::Entity {
 
         m_persistent_variable_sp->m_live_sp = ValueObjectConstResult::Create(
             map.GetBestExecutionContextScope(),
-            m_persistent_variable_sp.get()->GetCompilerType(),
+            m_persistent_variable_sp->GetCompilerType(),
             m_persistent_variable_sp->GetName(), location, eAddressTypeLoad,
             llvm::expectedToOptional(m_persistent_variable_sp->GetByteSize())
                 .value_or(0));
@@ -277,19 +281,17 @@ class EntityPersistentVariable : public Materializer::Entity {
         }
       }
 
-      lldb::addr_t mem = m_persistent_variable_sp->m_live_sp->GetValue()
-                             .GetScalar()
-                             .ULongLong();
-
-      if (!m_persistent_variable_sp->m_live_sp) {
+      if (!live_valobj_sp) {
         err = Status::FromErrorStringWithFormat(
             "couldn't find the memory area used to store %s",
             m_persistent_variable_sp->GetName().GetCString());
         return;
       }
 
-      if (m_persistent_variable_sp->m_live_sp->GetValue()
-              .GetValueAddressType() != eAddressTypeLoad) {
+      lldb::addr_t mem = live_valobj_sp->GetValue().GetScalar().ULongLong();
+
+      if (live_valobj_sp->GetValue().GetValueAddressType() !=
+          eAddressTypeLoad) {
         err = Status::FromErrorStringWithFormat(
             "the address of the memory area for %s is in an incorrect format",
             m_persistent_variable_sp->GetName().GetCString());
@@ -326,7 +328,6 @@ class EntityPersistentVariable : public Materializer::Entity {
               read_error.AsCString());
           return;
         }
-
         m_persistent_variable_sp->m_flags &=
             ~ExpressionVariable::EVNeedsFreezeDry;
       }

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp

@@ -59,6 +59,6 @@ ClangExpressionVariable::ClangExpressionVariable(
 }
 
 TypeFromUser ClangExpressionVariable::GetTypeFromUser() {
-  TypeFromUser tfu(m_frozen_sp->GetCompilerType());
+  TypeFromUser tfu(GetValueObject()->GetCompilerType());
   return tfu;
 }

lldb/source/Target/ABI.cpp

@@ -136,7 +136,7 @@ ValueObjectSP ABI::GetReturnValueObject(Thread &thread, CompilerType &ast_type,
           ExpressionVariable::EVNeedsAllocation;
       break;
     case Value::ValueType::LoadAddress:
-      expr_variable_sp->m_live_sp = live_valobj_sp;
+      expr_variable_sp->GetLiveObject() = live_valobj_sp;
       expr_variable_sp->m_flags |=
           ExpressionVariable::EVIsProgramReference;
       break;

lldb/test/API/functionalities/expr-result-var/Makefile

@@ -0,0 +1,3 @@
+CXX_SOURCES := two-bases.cpp
+
+include Makefile.rules