[Lldb-commits] [lldb] Revert "Make result variables obey their dynamic values in subsequent expressions (#168611)" (PR #172780)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Dec 17 17:56:42 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Dave Lee (kastiglione)
<details>
<summary>Changes</summary>
This reverts commit 6344e3aa8106dfdfb30cac36c8ca02bc4c52ce24.
---
Patch is 27.82 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/172780.diff
10 Files Affected:
- (modified) lldb/include/lldb/Expression/ExpressionVariable.h (+19-45)
- (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+1-3)
- (modified) lldb/source/Expression/ExpressionVariable.cpp (+6-72)
- (modified) lldb/source/Expression/LLVMUserExpression.cpp (+5-9)
- (modified) lldb/source/Expression/Materializer.cpp (+24-25)
- (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp (+1-1)
- (modified) lldb/source/Target/ABI.cpp (+1-1)
- (removed) lldb/test/API/functionalities/expr-result-var/Makefile (-3)
- (removed) lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py (-173)
- (removed) lldb/test/API/functionalities/expr-result-var/two-bases.cpp (-55)
``````````diff
diff --git a/lldb/include/lldb/Expression/ExpressionVariable.h b/lldb/include/lldb/Expression/ExpressionVariable.h
index 10078f8539dbe..68fa1c878a0e3 100644
--- a/lldb/include/lldb/Expression/ExpressionVariable.h
+++ b/lldb/include/lldb/Expression/ExpressionVariable.h
@@ -33,19 +33,11 @@ class ExpressionVariable
virtual ~ExpressionVariable() = default;
- llvm::Expected<uint64_t> GetByteSize() {
- return GetValueObject()->GetByteSize();
- }
+ llvm::Expected<uint64_t> GetByteSize() { return m_frozen_sp->GetByteSize(); }
ConstString GetName() { return m_frozen_sp->GetName(); }
- 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;
- }
+ lldb::ValueObjectSP GetValueObject() { return m_frozen_sp; }
uint8_t *GetValueBytes();
@@ -60,7 +52,7 @@ class ExpressionVariable
Value::ContextType::RegisterInfo, const_cast<RegisterInfo *>(reg_info));
}
- CompilerType GetCompilerType() { return GetValueObject()->GetCompilerType(); }
+ CompilerType GetCompilerType() { return m_frozen_sp->GetCompilerType(); }
void SetCompilerType(const CompilerType &compiler_type) {
m_frozen_sp->GetValue().SetCompilerType(compiler_type);
@@ -68,31 +60,23 @@ class ExpressionVariable
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.
- /// 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.
- /// 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;
+ // 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-
+ // 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());
}
- /// 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,
@@ -126,14 +110,6 @@ 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
@@ -143,8 +119,6 @@ class ExpressionVariable
/// track.
lldb::ValueObjectSP m_live_sp;
/// @}
-
- lldb::DynamicValueType m_dyn_option = lldb::eNoDynamicValues;
};
/// \class ExpressionVariableList ExpressionVariable.h
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index f326909308589..8c1eea97620e2 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2540,7 +2540,6 @@ def expect_expr(
result_value=None,
result_type=None,
result_children=None,
- options=None,
):
"""
Evaluates the given expression and verifies the result.
@@ -2557,8 +2556,7 @@ def expect_expr(
)
frame = self.frame()
- if not options:
- options = lldb.SBExpressionOptions()
+ options = lldb.SBExpressionOptions()
# Disable fix-its that tests don't pass by accident.
options.SetAutoApplyFixIts(False)
diff --git a/lldb/source/Expression/ExpressionVariable.cpp b/lldb/source/Expression/ExpressionVariable.cpp
index 4c9568106b346..9e8ea60f8e052 100644
--- a/lldb/source/Expression/ExpressionVariable.cpp
+++ b/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(valobj_sp->GetByteSize());
+ llvm::expectedToOptional(m_frozen_sp->GetByteSize());
if (byte_size && *byte_size) {
- if (valobj_sp->GetDataExtractor().GetByteSize() < *byte_size) {
- valobj_sp->GetValue().ResizeData(*byte_size);
- valobj_sp->GetValue().GetData(valobj_sp->GetDataExtractor());
+ if (m_frozen_sp->GetDataExtractor().GetByteSize() < *byte_size) {
+ m_frozen_sp->GetValue().ResizeData(*byte_size);
+ m_frozen_sp->GetValue().GetData(m_frozen_sp->GetDataExtractor());
}
- return const_cast<uint8_t *>(valobj_sp->GetDataExtractor().GetDataStart());
+ return const_cast<uint8_t *>(
+ m_frozen_sp->GetDataExtractor().GetDataStart());
}
return nullptr;
}
@@ -37,72 +37,6 @@ 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) {
diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp
index da14d36192463..2d59194027b57 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/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_sp) {
+ lldb::ExpressionVariableSP &result) {
// 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,9 +250,10 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
}
}
- if (FinalizeJITExecution(diagnostic_manager, exe_ctx, result_sp,
- function_stack_bottom, function_stack_top))
+ if (FinalizeJITExecution(diagnostic_manager, exe_ctx, result,
+ function_stack_bottom, function_stack_top)) {
return lldb::eExpressionCompleted;
+ }
return lldb::eExpressionResultUnavailable;
}
@@ -288,13 +289,8 @@ bool LLVMUserExpression::FinalizeJITExecution(
result =
GetResultAfterDematerialization(exe_ctx.GetBestExecutionContextScope());
- if (result) {
- // TransferAddress also does the offset_to_top calculation, so record the
- // dynamic option before we do that.
- if (EvaluateExpressionOptions *options = GetOptions())
- result->PreserveDynamicOption(options->GetUseDynamic());
+ if (result)
result->TransferAddress();
- }
m_dematerializer_sp.reset();
diff --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp
index dba763c9b6905..771a9ab84a20c 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -76,11 +76,10 @@ class EntityPersistentVariable : public Materializer::Entity {
const bool zero_memory = false;
IRMemoryMap::AllocationPolicy used_policy;
- const uint64_t malloc_size =
- llvm::expectedToOptional(m_persistent_variable_sp->GetByteSize())
- .value_or(0);
auto address_or_error = map.Malloc(
- malloc_size, 8, lldb::ePermissionsReadable | lldb::ePermissionsWritable,
+ llvm::expectedToOptional(m_persistent_variable_sp->GetByteSize())
+ .value_or(0),
+ 8, lldb::ePermissionsReadable | lldb::ePermissionsWritable,
IRMemoryMap::eAllocationPolicyMirror, zero_memory, &used_policy);
if (!address_or_error) {
err = Status::FromErrorStringWithFormat(
@@ -91,9 +90,8 @@ class EntityPersistentVariable : public Materializer::Entity {
}
lldb::addr_t mem = *address_or_error;
- LLDB_LOGF(
- log, "Allocated 0x%" PRIx64 "bytes for %s (0x%" PRIx64 ") successfully",
- malloc_size, m_persistent_variable_sp->GetName().GetCString(), mem);
+ LLDB_LOGF(log, "Allocated %s (0x%" PRIx64 ") successfully",
+ m_persistent_variable_sp->GetName().GetCString(), mem);
// Put the location of the spare memory into the live data of the
// ValueObject.
@@ -145,12 +143,12 @@ class EntityPersistentVariable : public Materializer::Entity {
void DestroyAllocation(IRMemoryMap &map, Status &err) {
Status deallocate_error;
- lldb::ValueObjectSP live_valobj_sp =
- m_persistent_variable_sp->GetLiveObject();
- map.Free((lldb::addr_t)live_valobj_sp->GetValue().GetScalar().ULongLong(),
+ map.Free((lldb::addr_t)m_persistent_variable_sp->m_live_sp->GetValue()
+ .GetScalar()
+ .ULongLong(),
deallocate_error);
- live_valobj_sp.reset();
+ m_persistent_variable_sp->m_live_sp.reset();
if (!deallocate_error.Success()) {
err = Status::FromErrorStringWithFormat(
@@ -185,17 +183,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 &&
- live_valobj_sp) ||
+ m_persistent_variable_sp->m_live_sp) ||
m_persistent_variable_sp->m_flags &
ExpressionVariable::EVIsLLDBAllocated) {
Status write_error;
- map.WriteScalarToMemory(load_addr, live_valobj_sp->GetValue().GetScalar(),
- map.GetAddressByteSize(), write_error);
+ map.WriteScalarToMemory(
+ load_addr,
+ m_persistent_variable_sp->m_live_sp->GetValue().GetScalar(),
+ map.GetAddressByteSize(), write_error);
if (!write_error.Success()) {
err = Status::FromErrorStringWithFormat(
@@ -231,15 +229,13 @@ 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 &&
- !live_valobj_sp) {
+ !m_persistent_variable_sp->m_live_sp) {
// If the reference comes from the program, then the
// ClangExpressionVariable's live variable data hasn't been set up yet.
// Do this now.
@@ -259,7 +255,7 @@ class EntityPersistentVariable : public Materializer::Entity {
m_persistent_variable_sp->m_live_sp = ValueObjectConstResult::Create(
map.GetBestExecutionContextScope(),
- m_persistent_variable_sp->GetCompilerType(),
+ m_persistent_variable_sp.get()->GetCompilerType(),
m_persistent_variable_sp->GetName(), location, eAddressTypeLoad,
llvm::expectedToOptional(m_persistent_variable_sp->GetByteSize())
.value_or(0));
@@ -281,17 +277,19 @@ class EntityPersistentVariable : public Materializer::Entity {
}
}
- if (!live_valobj_sp) {
+ lldb::addr_t mem = m_persistent_variable_sp->m_live_sp->GetValue()
+ .GetScalar()
+ .ULongLong();
+
+ if (!m_persistent_variable_sp->m_live_sp) {
err = Status::FromErrorStringWithFormat(
"couldn't find the memory area used to store %s",
m_persistent_variable_sp->GetName().GetCString());
return;
}
- lldb::addr_t mem = live_valobj_sp->GetValue().GetScalar().ULongLong();
-
- if (live_valobj_sp->GetValue().GetValueAddressType() !=
- eAddressTypeLoad) {
+ if (m_persistent_variable_sp->m_live_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());
@@ -328,6 +326,7 @@ class EntityPersistentVariable : public Materializer::Entity {
read_error.AsCString());
return;
}
+
m_persistent_variable_sp->m_flags &=
~ExpressionVariable::EVNeedsFreezeDry;
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
index d7b3fe0167299..e2fb4a054daf3 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
@@ -59,6 +59,6 @@ ClangExpressionVariable::ClangExpressionVariable(
}
TypeFromUser ClangExpressionVariable::GetTypeFromUser() {
- TypeFromUser tfu(GetValueObject()->GetCompilerType());
+ TypeFromUser tfu(m_frozen_sp->GetCompilerType());
return tfu;
}
diff --git a/lldb/source/Target/ABI.cpp b/lldb/source/Target/ABI.cpp
index 33bfa6b66fd06..3c51074340149 100644
--- a/lldb/source/Target/ABI.cpp
+++ b/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->GetLiveObject() = live_valobj_sp;
+ expr_variable_sp->m_live_sp = live_valobj_sp;
expr_variable_sp->m_flags |=
ExpressionVariable::EVIsProgramReference;
break;
diff --git a/lldb/test/API/functionalities/expr-result-var/Makefile b/lldb/test/API/functionalities/expr-result-var/Makefile
deleted file mode 100644
index 6c259307ef229..0000000000000
--- a/lldb/test/API/functionalities/expr-result-var/Makefile
+++ /dev/null
@@ -1,3 +0,0 @@
-CXX_SOURCES := two-bases.cpp
-
-include Makefile.rules
diff --git a/lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py b/lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py
deleted file mode 100644
index cd6f1252de7f5..0000000000000
--- a/lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py
+++ /dev/null
@@ -1,173 +0,0 @@
-"""
-Test the reuse of C++ result variables, particularly making sure
-that the dynamic typing is preserved.
-"""
-
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class TestCPPResultVariables(TestBase):
- NO_DEBUG_INFO_TESTCASE = True
-
- def setUp(self):
- TestBase.setUp(self)
- self.main_source_file = lldb.SBFileSpec("two-bases.cpp")
-
- def check_dereference(self, result_varname, frame, expr_options):
- deref_expr = "*{0}".format(result_varname)
- base_children = ValueCheck(
- name="Base", value="", children=[ValueCheck(name="base_int", value="100")]
- )
- base_1_arr_children = [
- ValueCheck(name="[0]", value="100"),
- ValueCheck(name="[1]", value="101"),
- ValueCheck(name="[2]", value="102"),
- ValueCheck(name="[3]", value="103"),
- ValueCheck(name="[4]", value="104"),
- ValueCheck(name="[5]", value="105"),
- ValueCheck(name="[6]", value="106"),
- ValueCheck(name="[7]", value="107"),
- ValueCheck(name="[8]", v...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/172780
More information about the lldb-commits
mailing list