[Lldb-commits] [lldb] 97ca9ca - [lldb] Fix bitfield "frame var" for pointers (pr47743)
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 26 04:01:29 PDT 2020
Author: Pavel Labath
Date: 2020-10-26T12:01:20+01:00
New Revision: 97ca9ca180f0810adcc1637d1a6dd32a04f63cfe
URL: https://github.com/llvm/llvm-project/commit/97ca9ca180f0810adcc1637d1a6dd32a04f63cfe
DIFF: https://github.com/llvm/llvm-project/commit/97ca9ca180f0810adcc1637d1a6dd32a04f63cfe.diff
LOG: [lldb] Fix bitfield "frame var" for pointers (pr47743)
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)
Differential Revision: https://reviews.llvm.org/D89236
Added:
Modified:
lldb/source/Core/ValueObjectChild.cpp
lldb/test/API/lang/c/bitfields/TestBitfields.py
lldb/test/API/lang/c/bitfields/main.c
Removed:
################################################################################
diff --git a/lldb/source/Core/ValueObjectChild.cpp b/lldb/source/Core/ValueObjectChild.cpp
index 63661a482313..97974d7b98fb 100644
--- a/lldb/source/Core/ValueObjectChild.cpp
+++ b/lldb/source/Core/ValueObjectChild.cpp
@@ -111,8 +111,7 @@ bool ValueObjectChild::UpdateValue() {
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 @@ bool ValueObjectChild::UpdateValue() {
(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()) {
diff --git a/lldb/test/API/lang/c/bitfields/TestBitfields.py b/lldb/test/API/lang/c/bitfields/TestBitfields.py
index d838a886bf2d..5e4d22ff0b17 100644
--- a/lldb/test/API/lang/c/bitfields/TestBitfields.py
+++ b/lldb/test/API/lang/c/bitfields/TestBitfields.py
@@ -147,6 +147,12 @@ def test_and_run_command(self):
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
diff --git a/lldb/test/API/lang/c/bitfields/main.c b/lldb/test/API/lang/c/bitfields/main.c
index 15e68dd1d6ef..f3f4d6a2c6f7 100644
--- a/lldb/test/API/lang/c/bitfields/main.c
+++ b/lldb/test/API/lang/c/bitfields/main.c
@@ -90,6 +90,7 @@ int main (int argc, char const *argv[])
struct LargePackedBits large_packed =
(struct LargePackedBits){ 0xcbbbbaaaa, 0xdffffeeee };
+ struct LargePackedBits *large_packed_ptr = &large_packed;
return 0; //// Set break point at this line.
More information about the lldb-commits
mailing list