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 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
