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
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] ... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-comm... Cameron via Phabricator via lldb-commits
    • [Lldb-comm... Marianne Mailhot-Sarrasin via Phabricator via lldb-commits
    • [Lldb-comm... Francois Pichet via Phabricator via lldb-commits
    • [Lldb-comm... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-comm... Raphael Isemann via Phabricator via lldb-commits
    • [Lldb-comm... Pavel Labath via Phabricator via lldb-commits

Reply via email to