[Lldb-commits] [lldb] 78b00c1 - Revert "[lldb] Improve maintainability and readability for ValueObject methods (#75865)"
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 23 23:13:18 PST 2024
Author: Pavel Labath
Date: 2024-01-24T07:12:52Z
New Revision: 78b00c116be8b3b53ff13552e31eb305b11cb169
URL: https://github.com/llvm/llvm-project/commit/78b00c116be8b3b53ff13552e31eb305b11cb169
DIFF: https://github.com/llvm/llvm-project/commit/78b00c116be8b3b53ff13552e31eb305b11cb169.diff
LOG: Revert "[lldb] Improve maintainability and readability for ValueObject methods (#75865)"
This reverts commit d657519838e4b2310e13ec5ff52599e041860825 as it
breaks two dozen tests. The breakages are related to variable path
expression parsing and summary string parsing (possibly the same code).
Added:
Modified:
lldb/source/Core/ValueObject.cpp
Removed:
################################################################################
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index d58bf2ca763d9e2..9208047be36668d 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1582,64 +1582,62 @@ bool ValueObject::IsUninitializedReference() {
ValueObjectSP ValueObject::GetSyntheticArrayMember(size_t index,
bool can_create) {
- if (!IsPointerType() && !IsArrayType())
- return ValueObjectSP();
-
- std::string index_str = llvm::formatv("[{0}]", index);
- ConstString index_const_str(index_str);
- // Check if we have already created a synthetic array member in this valid
- // object. If we have we will re-use it.
- if (auto existing_synthetic_child = GetSyntheticChild(index_const_str))
- return existing_synthetic_child;
-
- // We haven't made a synthetic array member for INDEX yet, so lets make
- // one and cache it for any future reference.
- ValueObject *synthetic_child = CreateChildAtIndex(0, true, index);
-
- if (!synthetic_child)
- return ValueObjectSP();
-
- // Cache the synthetic child's value because it's valid.
- AddSyntheticChild(index_const_str, synthetic_child);
- auto synthetic_child_sp = synthetic_child->GetSP();
- synthetic_child_sp->SetName(ConstString(index_str));
- synthetic_child_sp->m_flags.m_is_array_item_for_pointer = true;
+ ValueObjectSP synthetic_child_sp;
+ if (IsPointerType() || IsArrayType()) {
+ std::string index_str = llvm::formatv("[{0}]", index);
+ ConstString index_const_str(index_str);
+ // Check if we have already created a synthetic array member in this valid
+ // object. If we have we will re-use it.
+ synthetic_child_sp = GetSyntheticChild(index_const_str);
+ if (!synthetic_child_sp) {
+ ValueObject *synthetic_child;
+ // We haven't made a synthetic array member for INDEX yet, so lets make
+ // one and cache it for any future reference.
+ synthetic_child = CreateChildAtIndex(0, true, index);
+
+ // Cache the value if we got one back...
+ if (synthetic_child) {
+ AddSyntheticChild(index_const_str, synthetic_child);
+ synthetic_child_sp = synthetic_child->GetSP();
+ synthetic_child_sp->SetName(ConstString(index_str));
+ synthetic_child_sp->m_flags.m_is_array_item_for_pointer = true;
+ }
+ }
+ }
return synthetic_child_sp;
}
ValueObjectSP ValueObject::GetSyntheticBitFieldChild(uint32_t from, uint32_t to,
bool can_create) {
- if (!IsScalarType())
- return ValueObjectSP();
-
- std::string index_str = llvm::formatv("[{0}-{1}]", from, to);
- ConstString index_const_str(index_str);
-
- // Check if we have already created a synthetic array member in this valid
- // object. If we have we will re-use it.
- if (auto existing_synthetic_child = GetSyntheticChild(index_const_str))
- return existing_synthetic_child;
-
- uint32_t bit_field_size = to - from + 1;
- uint32_t bit_field_offset = from;
- if (GetDataExtractor().GetByteOrder() == eByteOrderBig)
- bit_field_offset =
- GetByteSize().value_or(0) * 8 - bit_field_size - bit_field_offset;
-
- // We haven't made a synthetic array member for INDEX yet, so lets make
- // one and cache it for any future reference.
- ValueObjectChild *synthetic_child = new ValueObjectChild(
- *this, GetCompilerType(), index_const_str, GetByteSize().value_or(0), 0,
- bit_field_size, bit_field_offset, false, false, eAddressTypeInvalid, 0);
-
- if (!synthetic_child)
- return ValueObjectSP();
-
- // Cache the synthetic child's value because it's valid.
- AddSyntheticChild(index_const_str, synthetic_child);
- auto synthetic_child_sp = synthetic_child->GetSP();
- synthetic_child_sp->SetName(ConstString(index_str));
- synthetic_child_sp->m_flags.m_is_bitfield_for_scalar = true;
+ ValueObjectSP synthetic_child_sp;
+ if (IsScalarType()) {
+ std::string index_str = llvm::formatv("[{0}-{1}]", from, to);
+ ConstString index_const_str(index_str);
+ // Check if we have already created a synthetic array member in this valid
+ // object. If we have we will re-use it.
+ synthetic_child_sp = GetSyntheticChild(index_const_str);
+ if (!synthetic_child_sp) {
+ uint32_t bit_field_size = to - from + 1;
+ uint32_t bit_field_offset = from;
+ if (GetDataExtractor().GetByteOrder() == eByteOrderBig)
+ bit_field_offset =
+ GetByteSize().value_or(0) * 8 - bit_field_size - bit_field_offset;
+ // We haven't made a synthetic array member for INDEX yet, so lets make
+ // one and cache it for any future reference.
+ ValueObjectChild *synthetic_child = new ValueObjectChild(
+ *this, GetCompilerType(), index_const_str, GetByteSize().value_or(0),
+ 0, bit_field_size, bit_field_offset, false, false,
+ eAddressTypeInvalid, 0);
+
+ // Cache the value if we got one back...
+ if (synthetic_child) {
+ AddSyntheticChild(index_const_str, synthetic_child);
+ synthetic_child_sp = synthetic_child->GetSP();
+ synthetic_child_sp->SetName(ConstString(index_str));
+ synthetic_child_sp->m_flags.m_is_bitfield_for_scalar = true;
+ }
+ }
+ }
return synthetic_child_sp;
}
@@ -1649,8 +1647,9 @@ ValueObjectSP ValueObject::GetSyntheticChildAtOffset(
ValueObjectSP synthetic_child_sp;
- if (name_const_str.IsEmpty())
+ if (name_const_str.IsEmpty()) {
name_const_str.SetString("@" + std::to_string(offset));
+ }
// Check if we have already created a synthetic array member in this valid
// object. If we have we will re-use it.
@@ -1660,13 +1659,13 @@ ValueObjectSP ValueObject::GetSyntheticChildAtOffset(
return synthetic_child_sp;
if (!can_create)
- return ValueObjectSP();
+ return {};
ExecutionContext exe_ctx(GetExecutionContextRef());
std::optional<uint64_t> size =
type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
if (!size)
- return ValueObjectSP();
+ return {};
ValueObjectChild *synthetic_child =
new ValueObjectChild(*this, type, name_const_str, *size, offset, 0, 0,
false, false, eAddressTypeInvalid, 0);
@@ -1700,7 +1699,7 @@ ValueObjectSP ValueObject::GetSyntheticBase(uint32_t offset,
return synthetic_child_sp;
if (!can_create)
- return ValueObjectSP();
+ return {};
const bool is_base_class = true;
@@ -1708,7 +1707,7 @@ ValueObjectSP ValueObject::GetSyntheticBase(uint32_t offset,
std::optional<uint64_t> size =
type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
if (!size)
- return ValueObjectSP();
+ return {};
ValueObjectChild *synthetic_child =
new ValueObjectChild(*this, type, name_const_str, *size, offset, 0, 0,
is_base_class, false, eAddressTypeInvalid, 0);
@@ -1737,30 +1736,30 @@ static const char *SkipLeadingExpressionPathSeparators(const char *expression) {
ValueObjectSP
ValueObject::GetSyntheticExpressionPathChild(const char *expression,
bool can_create) {
+ ValueObjectSP synthetic_child_sp;
ConstString name_const_string(expression);
// Check if we have already created a synthetic array member in this valid
// object. If we have we will re-use it.
- if (auto existing_synthetic_child = GetSyntheticChild(name_const_string))
- return existing_synthetic_child;
-
- // We haven't made a synthetic array member for expression yet, so lets
- // make one and cache it for any future reference.
- auto path_options = GetValueForExpressionPathOptions();
- path_options.SetSyntheticChildrenTraversal(
- GetValueForExpressionPathOptions::SyntheticChildrenTraversal::None);
- auto synthetic_child =
- GetValueForExpressionPath(expression, nullptr, nullptr, path_options);
-
- if (!synthetic_child)
- return ValueObjectSP();
-
- // Cache the synthetic child's value because it's valid.
- // FIXME: this causes a "real" child to end up with its name changed to
- // the contents of expression
- AddSyntheticChild(name_const_string, synthetic_child.get());
- synthetic_child->SetName(
- ConstString(SkipLeadingExpressionPathSeparators(expression)));
- return synthetic_child;
+ synthetic_child_sp = GetSyntheticChild(name_const_string);
+ if (!synthetic_child_sp) {
+ // We haven't made a synthetic array member for expression yet, so lets
+ // make one and cache it for any future reference.
+ synthetic_child_sp = GetValueForExpressionPath(
+ expression, nullptr, nullptr,
+ GetValueForExpressionPathOptions().SetSyntheticChildrenTraversal(
+ GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
+ None));
+
+ // Cache the value if we got one back...
+ if (synthetic_child_sp.get()) {
+ // FIXME: this causes a "real" child to end up with its name changed to
+ // the contents of expression
+ AddSyntheticChild(name_const_string, synthetic_child_sp.get());
+ synthetic_child_sp->SetName(
+ ConstString(SkipLeadingExpressionPathSeparators(expression)));
+ }
+ }
+ return synthetic_child_sp;
}
void ValueObject::CalculateSyntheticValue() {
@@ -1957,55 +1956,66 @@ ValueObjectSP ValueObject::GetValueForExpressionPath(
const GetValueForExpressionPathOptions &options,
ExpressionPathAftermath *final_task_on_target) {
- auto dummy_stop_reason = eExpressionPathScanEndReasonUnknown;
- auto dummy_value_type = eExpressionPathEndResultTypeInvalid;
- auto dummy_final_task = eExpressionPathAftermathNothing;
-
- auto proxy_stop_reason = reason_to_stop ? reason_to_stop : &dummy_stop_reason;
- auto proxy_value_type =
- final_value_type ? final_value_type : &dummy_value_type;
- auto proxy_final_task =
- final_task_on_target ? final_task_on_target : &dummy_final_task;
-
- auto ret_value = GetValueForExpressionPath_Impl(expression, proxy_stop_reason,
- proxy_value_type, options,
- proxy_final_task);
-
- // The caller knows nothing happened if `final_task_on_target` doesn't change.
- if (!ret_value || (*proxy_value_type) != eExpressionPathEndResultTypePlain ||
- !final_task_on_target)
- return ValueObjectSP();
-
- ExpressionPathAftermath &final_task_on_target_ref = (*final_task_on_target);
- ExpressionPathScanEndReason stop_reason_for_error;
- Status error;
- // The method can only dereference and take the address of plain objects.
- switch (final_task_on_target_ref) {
- case eExpressionPathAftermathNothing:
- return ret_value;
-
- case eExpressionPathAftermathDereference:
- ret_value = ret_value->Dereference(error);
- stop_reason_for_error = eExpressionPathScanEndReasonDereferencingFailed;
- break;
-
- case eExpressionPathAftermathTakeAddress:
- ret_value = ret_value->AddressOf(error);
- stop_reason_for_error = eExpressionPathScanEndReasonTakingAddressFailed;
- break;
- }
-
- if (ret_value && error.Success()) {
- final_task_on_target_ref = eExpressionPathAftermathNothing;
- return ret_value;
+ ExpressionPathScanEndReason dummy_reason_to_stop =
+ ValueObject::eExpressionPathScanEndReasonUnknown;
+ ExpressionPathEndResultType dummy_final_value_type =
+ ValueObject::eExpressionPathEndResultTypeInvalid;
+ ExpressionPathAftermath dummy_final_task_on_target =
+ ValueObject::eExpressionPathAftermathNothing;
+
+ ValueObjectSP ret_val = GetValueForExpressionPath_Impl(
+ expression, reason_to_stop ? reason_to_stop : &dummy_reason_to_stop,
+ final_value_type ? final_value_type : &dummy_final_value_type, options,
+ final_task_on_target ? final_task_on_target
+ : &dummy_final_task_on_target);
+
+ if (!final_task_on_target ||
+ *final_task_on_target == ValueObject::eExpressionPathAftermathNothing)
+ return ret_val;
+
+ if (ret_val.get() &&
+ ((final_value_type ? *final_value_type : dummy_final_value_type) ==
+ eExpressionPathEndResultTypePlain)) // I can only deref and takeaddress
+ // of plain objects
+ {
+ if ((final_task_on_target ? *final_task_on_target
+ : dummy_final_task_on_target) ==
+ ValueObject::eExpressionPathAftermathDereference) {
+ Status error;
+ ValueObjectSP final_value = ret_val->Dereference(error);
+ if (error.Fail() || !final_value.get()) {
+ if (reason_to_stop)
+ *reason_to_stop =
+ ValueObject::eExpressionPathScanEndReasonDereferencingFailed;
+ if (final_value_type)
+ *final_value_type = ValueObject::eExpressionPathEndResultTypeInvalid;
+ return ValueObjectSP();
+ } else {
+ if (final_task_on_target)
+ *final_task_on_target = ValueObject::eExpressionPathAftermathNothing;
+ return final_value;
+ }
+ }
+ if (*final_task_on_target ==
+ ValueObject::eExpressionPathAftermathTakeAddress) {
+ Status error;
+ ValueObjectSP final_value = ret_val->AddressOf(error);
+ if (error.Fail() || !final_value.get()) {
+ if (reason_to_stop)
+ *reason_to_stop =
+ ValueObject::eExpressionPathScanEndReasonTakingAddressFailed;
+ if (final_value_type)
+ *final_value_type = ValueObject::eExpressionPathEndResultTypeInvalid;
+ return ValueObjectSP();
+ } else {
+ if (final_task_on_target)
+ *final_task_on_target = ValueObject::eExpressionPathAftermathNothing;
+ return final_value;
+ }
+ }
}
-
- if (reason_to_stop)
- *reason_to_stop = stop_reason_for_error;
-
- if (final_value_type)
- *final_value_type = eExpressionPathEndResultTypeInvalid;
- return ValueObjectSP();
+ return ret_val; // final_task_on_target will still have its original value, so
+ // you know I did not do it
}
ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
@@ -2676,47 +2686,39 @@ ValueObjectSP ValueObject::AddressOf(Status &error) {
const bool scalar_is_load_address = false;
addr_t addr = GetAddressOf(scalar_is_load_address, &address_type);
error.Clear();
+ if (addr != LLDB_INVALID_ADDRESS && address_type != eAddressTypeHost) {
+ switch (address_type) {
+ case eAddressTypeInvalid: {
+ StreamString expr_path_strm;
+ GetExpressionPath(expr_path_strm);
+ error.SetErrorStringWithFormat("'%s' is not in memory",
+ expr_path_strm.GetData());
+ } break;
- StreamString expr_path_strm;
- GetExpressionPath(expr_path_strm);
- const char *expr_path_str = expr_path_strm.GetData();
-
- ExecutionContext exe_ctx(GetExecutionContextRef());
- auto scope = exe_ctx.GetBestExecutionContextScope();
-
- if (addr == LLDB_INVALID_ADDRESS) {
+ case eAddressTypeFile:
+ case eAddressTypeLoad: {
+ CompilerType compiler_type = GetCompilerType();
+ if (compiler_type) {
+ std::string name(1, '&');
+ name.append(m_name.AsCString(""));
+ ExecutionContext exe_ctx(GetExecutionContextRef());
+ m_addr_of_valobj_sp = ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(),
+ compiler_type.GetPointerType(), ConstString(name.c_str()), addr,
+ eAddressTypeInvalid, m_data.GetAddressByteSize());
+ }
+ } break;
+ default:
+ break;
+ }
+ } else {
+ StreamString expr_path_strm;
+ GetExpressionPath(expr_path_strm);
error.SetErrorStringWithFormat("'%s' doesn't have a valid address",
- expr_path_str);
- return ValueObjectSP();
+ expr_path_strm.GetData());
}
- switch (address_type) {
- case eAddressTypeInvalid:
- error.SetErrorStringWithFormat("'%s' is not in memory", expr_path_str);
- return ValueObjectSP();
-
- case eAddressTypeHost:
- error.SetErrorStringWithFormat("'%s' is in host process (LLDB) memory",
- expr_path_str);
- return ValueObjectSP();
-
- case eAddressTypeFile:
- case eAddressTypeLoad: {
- CompilerType compiler_type = GetCompilerType();
- if (!compiler_type) {
- error.SetErrorStringWithFormat("'%s' doesn't have a compiler type",
- expr_path_str);
- return ValueObjectSP();
- }
-
- std::string name(1, '&');
- name.append(m_name.AsCString(""));
- m_addr_of_valobj_sp = ValueObjectConstResult::Create(
- scope, compiler_type.GetPointerType(), ConstString(name.c_str()), addr,
- eAddressTypeInvalid, m_data.GetAddressByteSize());
- return m_addr_of_valobj_sp;
- }
- }
+ return m_addr_of_valobj_sp;
}
ValueObjectSP ValueObject::DoCast(const CompilerType &compiler_type) {
More information about the lldb-commits
mailing list