[Lldb-commits] [PATCH] D139674: Fix ValueObject::GetAddressOf for subclasses using ValueObjectConstResultImpl backends

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: Michael137, JDevlieghere, labath.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The original code always set the m_live_address of children of the ValueObjects that
use ValueObjectConstResultImpl backends to the parent m_live_address + child_byte_offset.
That is correct for structure types, but wrong for pointer types, since m_live_address
for a pointer type is the address of the storage for the pointer, not of the pointee.

      

Also added a test which was failing before this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139674

Files:
  lldb/source/Core/ValueObjectConstResultImpl.cpp
  lldb/test/API/lang/c/parray_vrs_char_array/Makefile
  lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
  lldb/test/API/lang/c/parray_vrs_char_array/main.c


Index: lldb/test/API/lang/c/parray_vrs_char_array/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/lang/c/parray_vrs_char_array/main.c
@@ -0,0 +1,15 @@
+struct MyStruct {
+  int before;
+  char var[5];
+  int after;
+};
+
+int
+main()
+{
+  struct MyStruct struct_arr[3] = {{112, "abcd", 221},
+                                   {313, "efgh", 414},
+                                   {515, "ijkl", 616}};
+  struct MyStruct *struct_ptr = struct_arr;
+  return struct_ptr->before;  // Set a breakpoint here
+}
Index: lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
===================================================================
--- /dev/null
+++ lldb/test/API/lang/c/parray_vrs_char_array/TestParrayVrsCharArrayChild.py
@@ -0,0 +1,37 @@
+"""
+Test that parray of a struct with an embedded char array works.
+This was failing because the "live address" of the child elements
+was calculated incorrectly - as a offset from the pointer live 
+address.  It only happened for char[] children because they used
+GetAddressOf which relies on the live address.
+"""
+
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestParrayVrsCharArrayChild(TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_parray_struct_with_char_array_child(self):
+        """This is the basic test for does parray get the char values right."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.do_array_test()
+
+    def do_array_test(self):
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   "Set a breakpoint here", self.main_source_file)
+
+        frame = thread.GetFrameAtIndex(0)
+ 
+        self.expect("expr -Z 3 -- struct_ptr",
+                    substrs = ['before = 112, var = "abcd", after = 221',
+                               'before = 313, var = "efgh", after = 414',
+                               'before = 515, var = "ijkl", after = 616'])
+
+        
Index: lldb/test/API/lang/c/parray_vrs_char_array/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/lang/c/parray_vrs_char_array/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
Index: lldb/source/Core/ValueObjectConstResultImpl.cpp
===================================================================
--- lldb/source/Core/ValueObjectConstResultImpl.cpp
+++ lldb/source/Core/ValueObjectConstResultImpl.cpp
@@ -89,13 +89,20 @@
     if (!child_name_str.empty())
       child_name.SetCString(child_name_str.c_str());
 
+    lldb::addr_t child_live_addr = LLDB_INVALID_ADDRESS;
+    // Transfer the live address (with offset) to the child.  But if
+    // the parent is a pointer, the live address is where that pointer
+    // value lives in memory, so the children live addresses aren't
+    // offsets from that value, they are just other load addresses that
+    // are recorded in the Value of the child ValueObjects.
+    if (m_live_address != LLDB_INVALID_ADDRESS) {
+      if (!compiler_type.IsPointerType())
+        child_live_addr = m_live_address + child_byte_offset;
+    }
     valobj = new ValueObjectConstResultChild(
         *m_impl_backend, child_compiler_type, child_name, child_byte_size,
         child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
-        child_is_base_class, child_is_deref_of_parent,
-        m_live_address == LLDB_INVALID_ADDRESS
-            ? m_live_address
-            : m_live_address + child_byte_offset,
+        child_is_base_class, child_is_deref_of_parent, child_live_addr,
         language_flags);
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139674.481449.patch
Type: text/x-patch
Size: 3806 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20221208/7783b7e0/attachment.bin>