labath created this revision.
labath added reviewers: shafik, teemperor.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLDB.
labath requested review of this revision.
Displaying large packed bitfields did not work if one was accessing them
through a pointer, and he used the "->" notation ("[0]." notation is
fine). The reason for that is that implicit dereference in -> is plumbed
all the way down to ValueObjectChild::UpdateValue, where the process of
fetching the child value was forked for this flag. The bitfield
"sliding" code was implemented only for the branch which did not require
dereferencing.
This patch restructures the function to avoid this mistake. Processing
now happens in two stages.
- first the parent is dereferenced (if needed)
- then the child value is computed (this step includes sliding and is common
for both branches)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89236
Files:
lldb/source/Core/ValueObjectChild.cpp
lldb/test/API/lang/c/bitfields/TestBitfields.py
lldb/test/API/lang/c/bitfields/main.c
Index: lldb/test/API/lang/c/bitfields/main.c
===================================================================
--- lldb/test/API/lang/c/bitfields/main.c
+++ lldb/test/API/lang/c/bitfields/main.c
@@ -90,6 +90,7 @@
struct LargePackedBits large_packed =
(struct LargePackedBits){ 0xcbbbbaaaa, 0xdffffeeee };
+ struct LargePackedBits *large_packed_ptr = &large_packed;
return 0; //// Set break point at this line.
Index: lldb/test/API/lang/c/bitfields/TestBitfields.py
===================================================================
--- lldb/test/API/lang/c/bitfields/TestBitfields.py
+++ lldb/test/API/lang/c/bitfields/TestBitfields.py
@@ -147,6 +147,12 @@
self.expect("v/x large_packed", VARIABLES_DISPLAYED_CORRECTLY,
substrs=["a = 0x0000000cbbbbaaaa", "b = 0x0000000dffffeee"])
+ # Check reading a bitfield through a pointer in various ways (PR47743)
+ self.expect("v/x large_packed_ptr->b",
+ substrs=["large_packed_ptr->b = 0x0000000dffffeeee"])
+ self.expect("v/x large_packed_ptr[0].b",
+ substrs=["large_packed_ptr[0].b = 0x0000000dffffeeee"])
+
# BitFields exhibit crashes in record layout on Windows
# (http://llvm.org/pr21800)
@skipIfWindows
Index: lldb/source/Core/ValueObjectChild.cpp
===================================================================
--- lldb/source/Core/ValueObjectChild.cpp
+++ lldb/source/Core/ValueObjectChild.cpp
@@ -111,8 +111,7 @@
CompilerType parent_type(parent->GetCompilerType());
// Copy the parent scalar value and the scalar value type
m_value.GetScalar() = parent->GetValue().GetScalar();
- Value::ValueType value_type = parent->GetValue().GetValueType();
- m_value.SetValueType(value_type);
+ m_value.SetValueType(parent->GetValue().GetValueType());
Flags parent_type_flags(parent_type.GetTypeInfo());
const bool is_instance_ptr_base =
@@ -120,93 +119,80 @@
(parent_type_flags.AnySet(lldb::eTypeInstanceIsPointer)));
if (parent->GetCompilerType().ShouldTreatScalarValueAsAddress()) {
- lldb::addr_t addr = parent->GetPointerValue();
- m_value.GetScalar() = addr;
-
+ m_value.GetScalar() = parent->GetPointerValue();
+
+ switch (parent->GetAddressTypeOfChildren()) {
+ case eAddressTypeFile: {
+ lldb::ProcessSP process_sp(GetProcessSP());
+ if (process_sp && process_sp->IsAlive())
+ m_value.SetValueType(Value::eValueTypeLoadAddress);
+ else
+ m_value.SetValueType(Value::eValueTypeFileAddress);
+ } break;
+ case eAddressTypeLoad:
+ m_value.SetValueType(is_instance_ptr_base
+ ? Value::eValueTypeScalar
+ : Value::eValueTypeLoadAddress);
+ break;
+ case eAddressTypeHost:
+ m_value.SetValueType(Value::eValueTypeHostAddress);
+ break;
+ case eAddressTypeInvalid:
+ // TODO: does this make sense?
+ m_value.SetValueType(Value::eValueTypeScalar);
+ break;
+ }
+ }
+ switch (m_value.GetValueType()) {
+ case Value::eValueTypeLoadAddress:
+ case Value::eValueTypeFileAddress:
+ case Value::eValueTypeHostAddress: {
+ lldb::addr_t addr = m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
if (addr == LLDB_INVALID_ADDRESS) {
m_error.SetErrorString("parent address is invalid.");
} else if (addr == 0) {
m_error.SetErrorString("parent is NULL");
} else {
- m_value.GetScalar() += m_byte_offset;
- AddressType addr_type = parent->GetAddressTypeOfChildren();
-
- switch (addr_type) {
- case eAddressTypeFile: {
- lldb::ProcessSP process_sp(GetProcessSP());
- if (process_sp && process_sp->IsAlive())
- m_value.SetValueType(Value::eValueTypeLoadAddress);
- else
- m_value.SetValueType(Value::eValueTypeFileAddress);
- } break;
- case eAddressTypeLoad:
- m_value.SetValueType(is_instance_ptr_base
- ? Value::eValueTypeScalar
- : Value::eValueTypeLoadAddress);
- break;
- case eAddressTypeHost:
- m_value.SetValueType(Value::eValueTypeHostAddress);
- break;
- case eAddressTypeInvalid:
- // TODO: does this make sense?
- m_value.SetValueType(Value::eValueTypeScalar);
- break;
- }
- }
- } else {
- switch (value_type) {
- case Value::eValueTypeLoadAddress:
- case Value::eValueTypeFileAddress:
- case Value::eValueTypeHostAddress: {
- lldb::addr_t addr =
- m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
- if (addr == LLDB_INVALID_ADDRESS) {
- m_error.SetErrorString("parent address is invalid.");
- } else if (addr == 0) {
- m_error.SetErrorString("parent is NULL");
- } else {
- // If a bitfield doesn't fit into the child_byte_size'd
- // window at child_byte_offset, move the window forward
- // until it fits. The problem here is that Value has no
- // notion of bitfields and thus the Value's DataExtractor
- // is sized like the bitfields CompilerType; a sequence of
- // bitfields, however, can be larger than their underlying
- // type.
- if (m_bitfield_bit_offset) {
- const bool thread_and_frame_only_if_stopped = true;
- ExecutionContext exe_ctx(GetExecutionContextRef().Lock(
- thread_and_frame_only_if_stopped));
- if (auto type_bit_size = GetCompilerType().GetBitSize(
- exe_ctx.GetBestExecutionContextScope())) {
- uint64_t bitfield_end =
- m_bitfield_bit_size + m_bitfield_bit_offset;
- if (bitfield_end > *type_bit_size) {
- uint64_t overhang_bytes =
- (bitfield_end - *type_bit_size + 7) / 8;
- m_byte_offset += overhang_bytes;
- m_bitfield_bit_offset -= overhang_bytes * 8;
- }
+ // If a bitfield doesn't fit into the child_byte_size'd window at
+ // child_byte_offset, move the window forward until it fits. The
+ // problem here is that Value has no notion of bitfields and thus the
+ // Value's DataExtractor is sized like the bitfields CompilerType; a
+ // sequence of bitfields, however, can be larger than their underlying
+ // type.
+ if (m_bitfield_bit_offset) {
+ const bool thread_and_frame_only_if_stopped = true;
+ ExecutionContext exe_ctx(GetExecutionContextRef().Lock(
+ thread_and_frame_only_if_stopped));
+ if (auto type_bit_size = GetCompilerType().GetBitSize(
+ exe_ctx.GetBestExecutionContextScope())) {
+ uint64_t bitfield_end =
+ m_bitfield_bit_size + m_bitfield_bit_offset;
+ if (bitfield_end > *type_bit_size) {
+ uint64_t overhang_bytes =
+ (bitfield_end - *type_bit_size + 7) / 8;
+ m_byte_offset += overhang_bytes;
+ m_bitfield_bit_offset -= overhang_bytes * 8;
}
}
-
- // Set this object's scalar value to the address of its value by
- // adding its byte offset to the parent address
- m_value.GetScalar() += m_byte_offset;
}
- } break;
- case Value::eValueTypeScalar:
- // try to extract the child value from the parent's scalar value
- {
- Scalar scalar(m_value.GetScalar());
- scalar.ExtractBitfield(8 * m_byte_size, 8 * m_byte_offset);
- m_value.GetScalar() = scalar;
- }
- break;
- default:
- m_error.SetErrorString("parent has invalid value.");
- break;
+ // Set this object's scalar value to the address of its value by
+ // adding its byte offset to the parent address
+ m_value.GetScalar() += m_byte_offset;
+ }
+ } break;
+
+ case Value::eValueTypeScalar:
+ // try to extract the child value from the parent's scalar value
+ {
+ Scalar scalar(m_value.GetScalar());
+ scalar.ExtractBitfield(8 * m_byte_size, 8 * m_byte_offset);
+ m_value.GetScalar() = scalar;
}
+ break;
+ default:
+ m_error.SetErrorString("parent has invalid value.");
+ break;
}
if (m_error.Success()) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
