From e32025162f6865c1cb886ddb592a63050fafa174 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 15 Nov 2022 13:03:55 -0800 Subject: [PATCH 1/3] Make result variables obey their dynamic values in subsequent expressions. At this stage, result variables work, but persistent expression variables do not. I have a test that which should work but does not. --- .../lldb/Expression/ExpressionVariable.h | 61 +++++++--- .../Python/lldbsuite/test/lldbtest.py | 4 +- lldb/source/Expression/ExpressionVariable.cpp | 74 +++++++++++- lldb/source/Expression/LLVMUserExpression.cpp | 17 ++- lldb/source/Expression/Materializer.cpp | 45 +++---- .../Clang/ClangExpressionVariable.cpp | 2 +- lldb/source/Target/ABI.cpp | 2 +- .../functionalities/expr-result-var/Makefile | 3 + .../expr-result-var/TestCPPExprResult.py | 113 ++++++++++++++++++ .../expr-result-var/two-bases.cpp | 64 ++++++++++ 10 files changed, 339 insertions(+), 46 deletions(-) create mode 100644 lldb/test/API/functionalities/expr-result-var/Makefile create mode 100644 lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py create mode 100644 lldb/test/API/functionalities/expr-result-var/two-bases.cpp diff --git a/lldb/include/lldb/Expression/ExpressionVariable.h b/lldb/include/lldb/Expression/ExpressionVariable.h index 68fa1c878a0e3..8b7eee63f0f92 100644 --- a/lldb/include/lldb/Expression/ExpressionVariable.h +++ b/lldb/include/lldb/Expression/ExpressionVariable.h @@ -33,11 +33,21 @@ class ExpressionVariable virtual ~ExpressionVariable() = default; - llvm::Expected GetByteSize() { return m_frozen_sp->GetByteSize(); } + llvm::Expected 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; + else + return m_frozen_sp; + } uint8_t *GetValueBytes(); @@ -52,7 +62,7 @@ class ExpressionVariable Value::ContextType::RegisterInfo, const_cast(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); @@ -60,22 +70,32 @@ 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 - // 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. + // 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()); + // 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 { @@ -110,6 +130,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 +147,9 @@ class ExpressionVariable /// track. lldb::ValueObjectSP m_live_sp; /// @} + + //LLVMCastKind m_kind; + 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 8c1eea97620e2..927fa7679bbb5 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/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) diff --git a/lldb/source/Expression/ExpressionVariable.cpp b/lldb/source/Expression/ExpressionVariable.cpp index 9e8ea60f8e052..9d361bc853620 100644 --- a/lldb/source/Expression/ExpressionVariable.cpp +++ b/lldb/source/Expression/ExpressionVariable.cpp @@ -20,15 +20,16 @@ char ExpressionVariable::ID; ExpressionVariable::ExpressionVariable() : m_flags(0) {} uint8_t *ExpressionVariable::GetValueBytes() { + lldb::ValueObjectSP valobj_sp = GetValueObject(); std::optional 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( - m_frozen_sp->GetDataExtractor().GetDataStart()); + valobj_sp->GetDataExtractor().GetDataStart()); } return nullptr; } @@ -37,6 +38,69 @@ char PersistentExpressionState::ID; PersistentExpressionState::PersistentExpressionState() = default; +void ExpressionVariable::TransferAddress(bool force) { + if (m_live_sp.get() == nullptr) + return; + + if (m_frozen_sp.get() == nullptr) + 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 2d59194027b57..dac572e815fac 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) { + 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,13 @@ 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 +294,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(); diff --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp index 771a9ab84a20c..6561aff39217a 100644 --- a/lldb/source/Expression/Materializer.cpp +++ b/lldb/source/Expression/Materializer.cpp @@ -76,9 +76,9 @@ class EntityPersistentVariable : public Materializer::Entity { const bool zero_memory = false; IRMemoryMap::AllocationPolicy used_policy; - auto address_or_error = map.Malloc( - llvm::expectedToOptional(m_persistent_variable_sp->GetByteSize()) - .value_or(0), + 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, IRMemoryMap::eAllocationPolicyMirror, zero_memory, &used_policy); if (!address_or_error) { @@ -90,7 +90,8 @@ class EntityPersistentVariable : public Materializer::Entity { } lldb::addr_t mem = *address_or_error; - LLDB_LOGF(log, "Allocated %s (0x%" PRIx64 ") successfully", + 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 @@ -143,12 +144,14 @@ 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() + 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,16 +186,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) || + ExpressionVariable::EVIsProgramReference && 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(), + live_valobj_sp->GetValue().GetScalar(), map.GetAddressByteSize(), write_error); if (!write_error.Success()) { @@ -229,18 +233,20 @@ 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. - lldb::addr_t location; + lldb::addr_t location; Status read_error; map.ReadPointerFromMemory(&location, load_addr, read_error); @@ -255,7 +261,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 +283,19 @@ 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 +332,6 @@ 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 e2fb4a054daf3..d7b3fe0167299 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(m_frozen_sp->GetCompilerType()); + TypeFromUser tfu(GetValueObject()->GetCompilerType()); return tfu; } diff --git a/lldb/source/Target/ABI.cpp b/lldb/source/Target/ABI.cpp index 3c51074340149..33bfa6b66fd06 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->m_live_sp = live_valobj_sp; + expr_variable_sp->GetLiveObject() = 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 new file mode 100644 index 0000000000000..6c259307ef229 --- /dev/null +++ b/lldb/test/API/functionalities/expr-result-var/Makefile @@ -0,0 +1,3 @@ +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 new file mode 100644 index 0000000000000..020fdd341faaa --- /dev/null +++ b/lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py @@ -0,0 +1,113 @@ +""" +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]", value="108"), ValueCheck(name="[9]", value="109")] + base_2_arr_children= [ValueCheck(name="[0]", value="200"), ValueCheck(name="[1]", value="201"), + ValueCheck(name="[2]", value="202"), ValueCheck(name="[3]", value="203"), + ValueCheck(name="[4]", value="204"), ValueCheck(name="[5]", value="205"), + ValueCheck(name="[6]", value="206"), ValueCheck(name="[7]", value="207"), + ValueCheck(name="[8]", value="208"), ValueCheck(name="[9]", value="209")] + deref_children=[ValueCheck(name="Base_1", value="", + children=[base_children, ValueCheck(name="base_1_arr", value="", children=base_1_arr_children)]), + ValueCheck(name="Base_2", value="", + children=[base_children, ValueCheck(name="base_2_arr", value="", children = base_2_arr_children)]), + ValueCheck(name="derived_int", value="1000")] + result_var_deref = self.expect_expr(deref_expr, result_type="Derived", + result_children = deref_children, options=expr_options) + + direct_access_expr = "{0}->derived_int".format(result_varname) + self.expect_expr(direct_access_expr, result_type="int", result_value="1000") + + # Also check this by directly accessing the result variable: + result_value = frame.FindValue(result_varname, lldb.eValueTypeConstResult, True) + self.assertTrue(result_value.error.success, "Found my result variable") + value_check = ValueCheck(children = deref_children) + value_check.check_value(self, result_value, f"{result_varname} children are correct") + + # Make sure we can also call a function through the derived type: + method_result = self.expect_expr( + f"{result_varname}->method_of_derived()", + result_type="int", + options=expr_options) + self.assertEqual(method_result.signed, 500, "Got the right result value") + + def test_virtual_dynamic_results(self): + self.do_test_dynamic_results(True) + + def test_non_virtual_dynamic_results(self): + self.do_test_dynamic_results(False) + + def do_test_dynamic_results(self, virtual): + """Test that when we uses a result variable in a subsequent expression it + uses the dynamic value - if that was requested when the result variable was made.""" + if virtual: + self.build(dictionary={"CFLAGS_EXTRAS": "-DVIRTUAL=''"}) + else: + self.build(dictionary={"CFLAGS_EXTRAS": "-DVIRTUAL='virtual'"}) + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, + "Set a breakpoint here", self.main_source_file) + + frame = thread.GetFrameAtIndex(0) + expr_options = lldb.SBExpressionOptions() + expr_options.SetFetchDynamicValue(lldb.eDynamicDontRunTarget) + base_1_ptr = self.expect_expr("base_1_ptr", result_type="Derived *", options=expr_options) + result_varname = base_1_ptr.GetName() + self.check_dereference(result_varname, frame, expr_options) + + # Now do the same thing, but use a persistent result variable: + empty_var = frame.EvaluateExpression("void *$base_1_ptr = base_1_ptr", expr_options) + self.assertIn( + empty_var.error.description, "unknown error", + "Expressions that don't have results return this error" + ) + persist_base_1_ptr = frame.FindValue("$base_1_ptr", lldb.eValueTypeConstResult, True) + self.assertTrue(persist_base_1_ptr.error.success, "Got the persistent variable") + self.check_dereference("$base_1_ptr", frame, expr_options) + + # Now check the second of the multiply inherited bases, this one will have an offset_to_top + # that we need to calculate: + base_2_ptr = self.expect_expr("base_2_ptr", result_type="Derived *", options=expr_options) + self.check_dereference(base_2_ptr.GetName(), frame, expr_options) + + # Again, do the same thing for a persistent expression variable: + empty_var = frame.EvaluateExpression("void *$base_2_ptr = base_2_ptr", expr_options) + self.check_dereference("$base_2_ptr", frame, expr_options) + + # Now try starting from a virtual base class of both our bases: + base_through_1 = self.expect_expr("base_through_1", result_type = "Derived *", options=expr_options) + self.check_dereference(base_through_1.GetName(), frame, expr_options) + + # Now try starting from a virtual base class of both our bases: + base_through_2 = self.expect_expr("base_through_2", result_type = "Derived *", options=expr_options) + self.check_dereference(base_through_2.GetName(), frame, expr_options) + + # Now check that we get the right results when we run an + # expression to get the base class object: + base_through_expr = self.expect_expr("MakeADerivedReportABase()", result_type = "Derived *", options=expr_options) + self.check_dereference(base_through_expr.GetName(), frame, expr_options) diff --git a/lldb/test/API/functionalities/expr-result-var/two-bases.cpp b/lldb/test/API/functionalities/expr-result-var/two-bases.cpp new file mode 100644 index 0000000000000..fa6fab0b47279 --- /dev/null +++ b/lldb/test/API/functionalities/expr-result-var/two-bases.cpp @@ -0,0 +1,64 @@ +#include +#include + +struct Base { + virtual ~Base() = default; + int base_int = 100; + Base *return_me() { return this; } +}; + +struct Base_1 : public VIRTUAL Base { + virtual ~Base_1() = default; + int base_1_arr[10] = { 100, 101, 102, 103, 104, 105, 106, 107, 108, 109 }; + Base *return_base_1() { return return_me(); } +}; + +struct Base_2 :public VIRTUAL Base { + virtual ~Base_2() = default; + int base_2_arr[10] = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 209 }; + Base *return_base_2() { return return_me(); } +}; + +struct Derived : public Base_1, Base_2 +{ + virtual ~Derived() = default; + int derived_int = 1000; + int method_of_derived() { + return 500; + } +}; + +Base *MakeADerivedReportABase() { + return (Base *) ((Base_1 *) new Derived()); +} + +int +main() +{ + Derived my_derived; + int call_it = my_derived.method_of_derived(); + + Base_1 *base_1_ptr = (Base_1 *) &my_derived; + + Base_2 *base_2_ptr = (Base_2 *) &my_derived; + + Base *base_through_1 = my_derived.return_base_1(); + Base *base_through_2 = my_derived.return_base_2();; + + // Call this to make sure the compiler makes it. + Base *fake_base = MakeADerivedReportABase(); + + uint64_t base_through_1_addr = (uint64_t) base_through_1; + uint64_t base_through_2_addr = (uint64_t) base_through_2; + int64_t base_offset = base_through_2_addr - base_through_1_addr; + printf("Base offset (should be 0): 0x%llx.\n", base_offset); + uint64_t base_1_addr = (uint64_t) base_1_ptr; + uint64_t base_2_addr = (uint64_t) base_2_ptr; + int64_t offset = base_2_addr - base_1_addr; + + // Set a breakpoint here + return my_derived.derived_int + base_1_ptr->base_1_arr[0] + + base_2_ptr->base_2_arr[0] + my_derived.return_base_1()->base_int; + +} + From de74f3ffca33d0b4879006800b8f792350f5131d Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 18 Nov 2025 13:56:28 -0800 Subject: [PATCH 2/3] Formatting --- .../lldb/Expression/ExpressionVariable.h | 19 +-- .../Python/lldbsuite/test/lldbtest.py | 4 +- lldb/source/Expression/ExpressionVariable.cpp | 40 ++--- lldb/source/Expression/LLVMUserExpression.cpp | 11 +- lldb/source/Expression/Materializer.cpp | 50 +++--- .../expr-result-var/TestCPPExprResult.py | 142 ++++++++++++------ .../expr-result-var/two-bases.cpp | 46 +++--- 7 files changed, 179 insertions(+), 133 deletions(-) diff --git a/lldb/include/lldb/Expression/ExpressionVariable.h b/lldb/include/lldb/Expression/ExpressionVariable.h index 8b7eee63f0f92..705263a3715e3 100644 --- a/lldb/include/lldb/Expression/ExpressionVariable.h +++ b/lldb/include/lldb/Expression/ExpressionVariable.h @@ -33,20 +33,19 @@ class ExpressionVariable virtual ~ExpressionVariable() = default; - llvm::Expected GetByteSize() { - return GetValueObject()->GetByteSize(); - + llvm::Expected GetByteSize() { + return GetValueObject()->GetByteSize(); } ConstString GetName() { return m_frozen_sp->GetName(); } lldb::ValueObjectSP GetValueObject() { - lldb::ValueObjectSP dyn_sp = + lldb::ValueObjectSP dyn_sp = m_frozen_sp->GetDynamicValue(lldb::eDynamicDontRunTarget); if (dyn_sp && dyn_sp->UpdateValueIfNeeded()) return dyn_sp; else - return m_frozen_sp; + return m_frozen_sp; } uint8_t *GetValueBytes(); @@ -91,12 +90,10 @@ class ExpressionVariable } // 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 + // 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; - } + lldb::ValueObjectSP GetLiveObject() { return m_live_sp; } enum Flags { EVNone = 0, @@ -133,7 +130,7 @@ class ExpressionVariable /// 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 + /// 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 @@ -148,7 +145,7 @@ class ExpressionVariable lldb::ValueObjectSP m_live_sp; /// @} - //LLVMCastKind m_kind; + // LLVMCastKind m_kind; lldb::DynamicValueType m_dyn_option = lldb::eNoDynamicValues; }; diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index 927fa7679bbb5..f326909308589 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -2540,7 +2540,7 @@ def expect_expr( result_value=None, result_type=None, result_children=None, - options=None + options=None, ): """ Evaluates the given expression and verifies the result. @@ -2558,7 +2558,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 9d361bc853620..7c9c99f0a5cd6 100644 --- a/lldb/source/Expression/ExpressionVariable.cpp +++ b/lldb/source/Expression/ExpressionVariable.cpp @@ -28,8 +28,7 @@ uint8_t *ExpressionVariable::GetValueBytes() { valobj_sp->GetValue().ResizeData(*byte_size); valobj_sp->GetValue().GetData(valobj_sp->GetDataExtractor()); } - return const_cast( - valobj_sp->GetDataExtractor().GetDataStart()); + return const_cast(valobj_sp->GetDataExtractor().GetDataStart()); } return nullptr; } @@ -49,23 +48,23 @@ void ExpressionVariable::TransferAddress(bool force) { 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 + // 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 + // 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()))) + if (!process_sp || + !process_sp->IsPossibleDynamicValue(*(m_frozen_sp.get()))) return; lldb::ValueObjectSP dyn_sp = m_frozen_sp->GetDynamicValue(m_dyn_option); @@ -75,27 +74,30 @@ void ExpressionVariable::TransferAddress(bool force) { 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; - + 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); + 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 " + 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); + + 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; } } diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp index dac572e815fac..e78543c4abdb2 100644 --- a/lldb/source/Expression/LLVMUserExpression.cpp +++ b/lldb/source/Expression/LLVMUserExpression.cpp @@ -252,11 +252,12 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager, 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(); -// } + // 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; } diff --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp index 6561aff39217a..cbc197e3d1392 100644 --- a/lldb/source/Expression/Materializer.cpp +++ b/lldb/source/Expression/Materializer.cpp @@ -76,10 +76,11 @@ class EntityPersistentVariable : public Materializer::Entity { const bool zero_memory = false; IRMemoryMap::AllocationPolicy used_policy; - 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, + 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, IRMemoryMap::eAllocationPolicyMirror, zero_memory, &used_policy); if (!address_or_error) { err = Status::FromErrorStringWithFormat( @@ -90,9 +91,9 @@ 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 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. @@ -144,11 +145,9 @@ 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(), + lldb::ValueObjectSP live_valobj_sp = + m_persistent_variable_sp->GetLiveObject(); + map.Free((lldb::addr_t)live_valobj_sp->GetValue().GetScalar().ULongLong(), deallocate_error); live_valobj_sp.reset(); @@ -186,18 +185,17 @@ class EntityPersistentVariable : public Materializer::Entity { return; } - lldb::ValueObjectSP live_valobj_sp - = m_persistent_variable_sp->GetLiveObject(); + lldb::ValueObjectSP live_valobj_sp = + m_persistent_variable_sp->GetLiveObject(); if ((m_persistent_variable_sp->m_flags & - ExpressionVariable::EVIsProgramReference && live_valobj_sp) || + ExpressionVariable::EVIsProgramReference && + live_valobj_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, live_valobj_sp->GetValue().GetScalar(), + map.GetAddressByteSize(), write_error); if (!write_error.Success()) { err = Status::FromErrorStringWithFormat( @@ -233,8 +231,8 @@ class EntityPersistentVariable : public Materializer::Entity { m_delegate->DidDematerialize(m_persistent_variable_sp); } - lldb::ValueObjectSP live_valobj_sp - = m_persistent_variable_sp->GetLiveObject(); + 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 & @@ -246,7 +244,7 @@ class EntityPersistentVariable : public Materializer::Entity { // ClangExpressionVariable's live variable data hasn't been set up yet. // Do this now. - lldb::addr_t location; + lldb::addr_t location; Status read_error; map.ReadPointerFromMemory(&location, load_addr, read_error); @@ -290,12 +288,10 @@ class EntityPersistentVariable : public Materializer::Entity { return; } - lldb::addr_t mem = live_valobj_sp->GetValue() - .GetScalar() - .ULongLong(); + lldb::addr_t mem = live_valobj_sp->GetValue().GetScalar().ULongLong(); - if (live_valobj_sp->GetValue().GetValueAddressType() - != eAddressTypeLoad) { + 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()); diff --git a/lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py b/lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py index 020fdd341faaa..d560ce9059d15 100644 --- a/lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py +++ b/lldb/test/API/functionalities/expr-result-var/TestCPPExprResult.py @@ -4,7 +4,6 @@ """ - import lldb from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * @@ -12,33 +11,70 @@ 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]", value="108"), ValueCheck(name="[9]", value="109")] - base_2_arr_children= [ValueCheck(name="[0]", value="200"), ValueCheck(name="[1]", value="201"), - ValueCheck(name="[2]", value="202"), ValueCheck(name="[3]", value="203"), - ValueCheck(name="[4]", value="204"), ValueCheck(name="[5]", value="205"), - ValueCheck(name="[6]", value="206"), ValueCheck(name="[7]", value="207"), - ValueCheck(name="[8]", value="208"), ValueCheck(name="[9]", value="209")] - deref_children=[ValueCheck(name="Base_1", value="", - children=[base_children, ValueCheck(name="base_1_arr", value="", children=base_1_arr_children)]), - ValueCheck(name="Base_2", value="", - children=[base_children, ValueCheck(name="base_2_arr", value="", children = base_2_arr_children)]), - ValueCheck(name="derived_int", value="1000")] - result_var_deref = self.expect_expr(deref_expr, result_type="Derived", - result_children = deref_children, options=expr_options) + 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]", value="108"), + ValueCheck(name="[9]", value="109"), + ] + base_2_arr_children = [ + ValueCheck(name="[0]", value="200"), + ValueCheck(name="[1]", value="201"), + ValueCheck(name="[2]", value="202"), + ValueCheck(name="[3]", value="203"), + ValueCheck(name="[4]", value="204"), + ValueCheck(name="[5]", value="205"), + ValueCheck(name="[6]", value="206"), + ValueCheck(name="[7]", value="207"), + ValueCheck(name="[8]", value="208"), + ValueCheck(name="[9]", value="209"), + ] + deref_children = [ + ValueCheck( + name="Base_1", + value="", + children=[ + base_children, + ValueCheck( + name="base_1_arr", value="", children=base_1_arr_children + ), + ], + ), + ValueCheck( + name="Base_2", + value="", + children=[ + base_children, + ValueCheck( + name="base_2_arr", value="", children=base_2_arr_children + ), + ], + ), + ValueCheck(name="derived_int", value="1000"), + ] + result_var_deref = self.expect_expr( + deref_expr, + result_type="Derived", + result_children=deref_children, + options=expr_options, + ) direct_access_expr = "{0}->derived_int".format(result_varname) self.expect_expr(direct_access_expr, result_type="int", result_value="1000") @@ -46,14 +82,17 @@ def check_dereference(self, result_varname, frame, expr_options): # Also check this by directly accessing the result variable: result_value = frame.FindValue(result_varname, lldb.eValueTypeConstResult, True) self.assertTrue(result_value.error.success, "Found my result variable") - value_check = ValueCheck(children = deref_children) - value_check.check_value(self, result_value, f"{result_varname} children are correct") - + value_check = ValueCheck(children=deref_children) + value_check.check_value( + self, result_value, f"{result_varname} children are correct" + ) + # Make sure we can also call a function through the derived type: method_result = self.expect_expr( f"{result_varname}->method_of_derived()", result_type="int", - options=expr_options) + options=expr_options, + ) self.assertEqual(method_result.signed, 500, "Got the right result value") def test_virtual_dynamic_results(self): @@ -61,53 +100,72 @@ def test_virtual_dynamic_results(self): def test_non_virtual_dynamic_results(self): self.do_test_dynamic_results(False) - + def do_test_dynamic_results(self, virtual): """Test that when we uses a result variable in a subsequent expression it - uses the dynamic value - if that was requested when the result variable was made.""" + uses the dynamic value - if that was requested when the result variable was made. + """ if virtual: self.build(dictionary={"CFLAGS_EXTRAS": "-DVIRTUAL=''"}) else: self.build(dictionary={"CFLAGS_EXTRAS": "-DVIRTUAL='virtual'"}) - (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, - "Set a breakpoint here", self.main_source_file) + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "Set a breakpoint here", self.main_source_file + ) frame = thread.GetFrameAtIndex(0) expr_options = lldb.SBExpressionOptions() expr_options.SetFetchDynamicValue(lldb.eDynamicDontRunTarget) - base_1_ptr = self.expect_expr("base_1_ptr", result_type="Derived *", options=expr_options) + base_1_ptr = self.expect_expr( + "base_1_ptr", result_type="Derived *", options=expr_options + ) result_varname = base_1_ptr.GetName() self.check_dereference(result_varname, frame, expr_options) # Now do the same thing, but use a persistent result variable: - empty_var = frame.EvaluateExpression("void *$base_1_ptr = base_1_ptr", expr_options) + empty_var = frame.EvaluateExpression( + "void *$base_1_ptr = base_1_ptr", expr_options + ) self.assertIn( - empty_var.error.description, "unknown error", - "Expressions that don't have results return this error" + empty_var.error.description, + "unknown error", + "Expressions that don't have results return this error", + ) + persist_base_1_ptr = frame.FindValue( + "$base_1_ptr", lldb.eValueTypeConstResult, True ) - persist_base_1_ptr = frame.FindValue("$base_1_ptr", lldb.eValueTypeConstResult, True) self.assertTrue(persist_base_1_ptr.error.success, "Got the persistent variable") self.check_dereference("$base_1_ptr", frame, expr_options) - + # Now check the second of the multiply inherited bases, this one will have an offset_to_top # that we need to calculate: - base_2_ptr = self.expect_expr("base_2_ptr", result_type="Derived *", options=expr_options) + base_2_ptr = self.expect_expr( + "base_2_ptr", result_type="Derived *", options=expr_options + ) self.check_dereference(base_2_ptr.GetName(), frame, expr_options) # Again, do the same thing for a persistent expression variable: - empty_var = frame.EvaluateExpression("void *$base_2_ptr = base_2_ptr", expr_options) + empty_var = frame.EvaluateExpression( + "void *$base_2_ptr = base_2_ptr", expr_options + ) self.check_dereference("$base_2_ptr", frame, expr_options) # Now try starting from a virtual base class of both our bases: - base_through_1 = self.expect_expr("base_through_1", result_type = "Derived *", options=expr_options) + base_through_1 = self.expect_expr( + "base_through_1", result_type="Derived *", options=expr_options + ) self.check_dereference(base_through_1.GetName(), frame, expr_options) # Now try starting from a virtual base class of both our bases: - base_through_2 = self.expect_expr("base_through_2", result_type = "Derived *", options=expr_options) + base_through_2 = self.expect_expr( + "base_through_2", result_type="Derived *", options=expr_options + ) self.check_dereference(base_through_2.GetName(), frame, expr_options) - + # Now check that we get the right results when we run an # expression to get the base class object: - base_through_expr = self.expect_expr("MakeADerivedReportABase()", result_type = "Derived *", options=expr_options) - self.check_dereference(base_through_expr.GetName(), frame, expr_options) + base_through_expr = self.expect_expr( + "MakeADerivedReportABase()", result_type="Derived *", options=expr_options + ) + self.check_dereference(base_through_expr.GetName(), frame, expr_options) diff --git a/lldb/test/API/functionalities/expr-result-var/two-bases.cpp b/lldb/test/API/functionalities/expr-result-var/two-bases.cpp index fa6fab0b47279..21e277cd8595c 100644 --- a/lldb/test/API/functionalities/expr-result-var/two-bases.cpp +++ b/lldb/test/API/functionalities/expr-result-var/two-bases.cpp @@ -1,5 +1,5 @@ -#include #include +#include struct Base { virtual ~Base() = default; @@ -9,56 +9,48 @@ struct Base { struct Base_1 : public VIRTUAL Base { virtual ~Base_1() = default; - int base_1_arr[10] = { 100, 101, 102, 103, 104, 105, 106, 107, 108, 109 }; + int base_1_arr[10] = {100, 101, 102, 103, 104, 105, 106, 107, 108, 109}; Base *return_base_1() { return return_me(); } }; -struct Base_2 :public VIRTUAL Base { +struct Base_2 : public VIRTUAL Base { virtual ~Base_2() = default; - int base_2_arr[10] = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 209 }; + int base_2_arr[10] = {200, 201, 202, 203, 204, 205, 206, 207, 208, 209}; Base *return_base_2() { return return_me(); } }; -struct Derived : public Base_1, Base_2 -{ +struct Derived : public Base_1, Base_2 { virtual ~Derived() = default; int derived_int = 1000; - int method_of_derived() { - return 500; - } + int method_of_derived() { return 500; } }; -Base *MakeADerivedReportABase() { - return (Base *) ((Base_1 *) new Derived()); -} +Base *MakeADerivedReportABase() { return (Base *)((Base_1 *)new Derived()); } -int -main() -{ +int main() { Derived my_derived; int call_it = my_derived.method_of_derived(); - Base_1 *base_1_ptr = (Base_1 *) &my_derived; + Base_1 *base_1_ptr = (Base_1 *)&my_derived; - Base_2 *base_2_ptr = (Base_2 *) &my_derived; + Base_2 *base_2_ptr = (Base_2 *)&my_derived; Base *base_through_1 = my_derived.return_base_1(); - Base *base_through_2 = my_derived.return_base_2();; + Base *base_through_2 = my_derived.return_base_2(); + ; - // Call this to make sure the compiler makes it. + // Call this to make sure the compiler makes it. Base *fake_base = MakeADerivedReportABase(); - uint64_t base_through_1_addr = (uint64_t) base_through_1; - uint64_t base_through_2_addr = (uint64_t) base_through_2; + uint64_t base_through_1_addr = (uint64_t)base_through_1; + uint64_t base_through_2_addr = (uint64_t)base_through_2; int64_t base_offset = base_through_2_addr - base_through_1_addr; printf("Base offset (should be 0): 0x%llx.\n", base_offset); - uint64_t base_1_addr = (uint64_t) base_1_ptr; - uint64_t base_2_addr = (uint64_t) base_2_ptr; + uint64_t base_1_addr = (uint64_t)base_1_ptr; + uint64_t base_2_addr = (uint64_t)base_2_ptr; int64_t offset = base_2_addr - base_1_addr; // Set a breakpoint here - return my_derived.derived_int + base_1_ptr->base_1_arr[0] - + base_2_ptr->base_2_arr[0] + my_derived.return_base_1()->base_int; - + return my_derived.derived_int + base_1_ptr->base_1_arr[0] + + base_2_ptr->base_2_arr[0] + my_derived.return_base_1()->base_int; } - From fa4aea65bb7f9dedbd02b6080aec9a2fe083cb15 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Tue, 18 Nov 2025 16:14:42 -0800 Subject: [PATCH 3/3] Review comments --- lldb/include/lldb/Expression/ExpressionVariable.h | 6 ++---- lldb/source/Expression/ExpressionVariable.cpp | 4 ++-- lldb/test/API/functionalities/expr-result-var/two-bases.cpp | 1 - 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lldb/include/lldb/Expression/ExpressionVariable.h b/lldb/include/lldb/Expression/ExpressionVariable.h index 705263a3715e3..7e0a4f3e6e020 100644 --- a/lldb/include/lldb/Expression/ExpressionVariable.h +++ b/lldb/include/lldb/Expression/ExpressionVariable.h @@ -44,8 +44,7 @@ class ExpressionVariable m_frozen_sp->GetDynamicValue(lldb::eDynamicDontRunTarget); if (dyn_sp && dyn_sp->UpdateValueIfNeeded()) return dyn_sp; - else - return m_frozen_sp; + return m_frozen_sp; } uint8_t *GetValueBytes(); @@ -70,7 +69,7 @@ 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. - // This is necessary because the results of certain cast and pointer- + // 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. @@ -145,7 +144,6 @@ class ExpressionVariable lldb::ValueObjectSP m_live_sp; /// @} - // LLVMCastKind m_kind; lldb::DynamicValueType m_dyn_option = lldb::eNoDynamicValues; }; diff --git a/lldb/source/Expression/ExpressionVariable.cpp b/lldb/source/Expression/ExpressionVariable.cpp index 7c9c99f0a5cd6..4c9568106b346 100644 --- a/lldb/source/Expression/ExpressionVariable.cpp +++ b/lldb/source/Expression/ExpressionVariable.cpp @@ -38,10 +38,10 @@ char PersistentExpressionState::ID; PersistentExpressionState::PersistentExpressionState() = default; void ExpressionVariable::TransferAddress(bool force) { - if (m_live_sp.get() == nullptr) + if (!m_live_sp) return; - if (m_frozen_sp.get() == nullptr) + if (!m_frozen_sp) return; if (force || (m_frozen_sp->GetLiveAddress() == LLDB_INVALID_ADDRESS)) { diff --git a/lldb/test/API/functionalities/expr-result-var/two-bases.cpp b/lldb/test/API/functionalities/expr-result-var/two-bases.cpp index 21e277cd8595c..55af757d02bd8 100644 --- a/lldb/test/API/functionalities/expr-result-var/two-bases.cpp +++ b/lldb/test/API/functionalities/expr-result-var/two-bases.cpp @@ -37,7 +37,6 @@ int main() { Base *base_through_1 = my_derived.return_base_1(); Base *base_through_2 = my_derived.return_base_2(); - ; // Call this to make sure the compiler makes it. Base *fake_base = MakeADerivedReportABase();