This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG97ca9ca180f0: [lldb] Fix bitfield "frame var" for
pointers (pr47743) (authored by labath).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89236/new/
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits