jingham created this revision.
jingham added a reviewer: labath.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

This is a refinement on 96601ec28b7efe5abf3479a1aa91bcedb235bbbd.  The intent 
of that change was to do the same work for the computation of the locations of 
the children of ValueObjectVariable as was done for the root 
ValueObjectVariable.  This original patch did that by moving the computation 
from ValueObjectVariable to ValueObject.  That fixed the problem but caused a 
handful of swift-lldb testsuite failures and a crash or two.

The problem is that synthetic value objects can sometimes represent objects in 
target memory, and other times they might be made up wholly in lldb memory, 
with pointers from one synthetic object to another, and so the 
ValueObjectVariable computation was not appropriate.

This patch delegates the computation to the root of the ValueObject in 
question.  That solves the problem for ValueObjectVariable while not messing up 
the computation for ValueObjectConstResult or ValueObjectSynthetic.

This patch resolves all the swift-lldb failures and also still passes the 
non-swift testsuite.

The representation through ValueObjects of Swift Types is quite byzantine, and 
the ValueObject system is frustratingly self-healing, and I haven't yet been 
able to cons up a non-swift test case that will trigger the same failures we 
were seeing with swift data types.  I'm still working on that, but I thought 
I'd get the code changes out for review while working on that, since we 
actually DO have tests for the change, just not in the non-swift tree...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83450

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectVariable.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectVariable.cpp

Index: lldb/source/Core/ValueObjectVariable.cpp
===================================================================
--- lldb/source/Core/ValueObjectVariable.cpp
+++ lldb/source/Core/ValueObjectVariable.cpp
@@ -242,9 +242,64 @@
       m_resolved_value.SetContext(Value::eContextTypeInvalid, nullptr);
     }
   }
+  
   return m_error.Success();
 }
 
+void ValueObjectVariable::DoUpdateChildrenAddressType(ValueObject &valobj) {
+  Value::ValueType value_type = valobj.GetValue().GetValueType();
+  ExecutionContext exe_ctx(GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+  const bool process_is_alive = process && process->IsAlive();
+  const uint32_t type_info = valobj.GetCompilerType().GetTypeInfo();
+  const bool is_pointer_or_ref =
+      (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
+
+  switch (value_type) {
+  case Value::eValueTypeFileAddress:
+    // If this type is a pointer, then its children will be considered load
+    // addresses if the pointer or reference is dereferenced, but only if
+    // the process is alive.
+    //
+    // There could be global variables like in the following code:
+    // struct LinkedListNode { Foo* foo; LinkedListNode* next; };
+    // Foo g_foo1;
+    // Foo g_foo2;
+    // LinkedListNode g_second_node = { &g_foo2, NULL };
+    // LinkedListNode g_first_node = { &g_foo1, &g_second_node };
+    //
+    // When we aren't running, we should be able to look at these variables
+    // using the "target variable" command. Children of the "g_first_node"
+    // always will be of the same address type as the parent. But children
+    // of the "next" member of LinkedListNode will become load addresses if
+    // we have a live process, or remain a file address if it was a file
+    // address.
+    if (process_is_alive && is_pointer_or_ref)
+      valobj.SetAddressTypeOfChildren(eAddressTypeLoad);
+    else
+      valobj.SetAddressTypeOfChildren(eAddressTypeFile);
+    break;
+  case Value::eValueTypeHostAddress:
+    // Same as above for load addresses, except children of pointer or refs
+    // are always load addresses. Host addresses are used to store freeze
+    // dried variables. If this type is a struct, the entire struct
+    // contents will be copied into the heap of the
+    // LLDB process, but we do not currently follow any pointers.
+    if (is_pointer_or_ref)
+      valobj.SetAddressTypeOfChildren(eAddressTypeLoad);
+    else
+      valobj.SetAddressTypeOfChildren(eAddressTypeHost);
+    break;
+  case Value::eValueTypeLoadAddress:
+  case Value::eValueTypeScalar:
+  case Value::eValueTypeVector:
+    valobj.SetAddressTypeOfChildren(eAddressTypeLoad);
+    break;
+  }
+}
+
+
+
 bool ValueObjectVariable::IsInScope() {
   const ExecutionContextRef &exe_ctx_ref = GetExecutionContextRef();
   if (exe_ctx_ref.HasFrameRef()) {
Index: lldb/source/Core/ValueObject.cpp
===================================================================
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -140,58 +140,6 @@
 // Destructor
 ValueObject::~ValueObject() {}
 
-void ValueObject::UpdateChildrenAddressType() {
-  Value::ValueType value_type = m_value.GetValueType();
-  ExecutionContext exe_ctx(GetExecutionContextRef());
-  Process *process = exe_ctx.GetProcessPtr();
-  const bool process_is_alive = process && process->IsAlive();
-  const uint32_t type_info = GetCompilerType().GetTypeInfo();
-  const bool is_pointer_or_ref =
-      (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
-
-  switch (value_type) {
-  case Value::eValueTypeFileAddress:
-    // If this type is a pointer, then its children will be considered load
-    // addresses if the pointer or reference is dereferenced, but only if
-    // the process is alive.
-    //
-    // There could be global variables like in the following code:
-    // struct LinkedListNode { Foo* foo; LinkedListNode* next; };
-    // Foo g_foo1;
-    // Foo g_foo2;
-    // LinkedListNode g_second_node = { &g_foo2, NULL };
-    // LinkedListNode g_first_node = { &g_foo1, &g_second_node };
-    //
-    // When we aren't running, we should be able to look at these variables
-    // using the "target variable" command. Children of the "g_first_node"
-    // always will be of the same address type as the parent. But children
-    // of the "next" member of LinkedListNode will become load addresses if
-    // we have a live process, or remain a file address if it was a file
-    // address.
-    if (process_is_alive && is_pointer_or_ref)
-      SetAddressTypeOfChildren(eAddressTypeLoad);
-    else
-      SetAddressTypeOfChildren(eAddressTypeFile);
-    break;
-  case Value::eValueTypeHostAddress:
-    // Same as above for load addresses, except children of pointer or refs
-    // are always load addresses. Host addresses are used to store freeze
-    // dried variables. If this type is a struct, the entire struct
-    // contents will be copied into the heap of the
-    // LLDB process, but we do not currently follow any pointers.
-    if (is_pointer_or_ref)
-      SetAddressTypeOfChildren(eAddressTypeLoad);
-    else
-      SetAddressTypeOfChildren(eAddressTypeHost);
-    break;
-  case Value::eValueTypeLoadAddress:
-  case Value::eValueTypeScalar:
-  case Value::eValueTypeVector:
-    SetAddressTypeOfChildren(eAddressTypeLoad);
-    break;
-  }
-}
-
 bool ValueObject::UpdateValueIfNeeded(bool update_format) {
 
   bool did_change_formats = false;
Index: lldb/include/lldb/Core/ValueObjectVariable.h
===================================================================
--- lldb/include/lldb/Core/ValueObjectVariable.h
+++ lldb/include/lldb/Core/ValueObjectVariable.h
@@ -67,6 +67,8 @@
 
 protected:
   bool UpdateValue() override;
+  
+  void DoUpdateChildrenAddressType(ValueObject &valobj) override;
 
   CompilerType GetCompilerTypeImpl() override;
 
Index: lldb/include/lldb/Core/ValueObject.h
===================================================================
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -963,9 +963,14 @@
 
   void SetPreferredDisplayLanguageIfNeeded(lldb::LanguageType);
 
+protected:
+  virtual void DoUpdateChildrenAddressType(ValueObject &valobj) { return; };
+
 private:
   virtual CompilerType MaybeCalculateCompleteType();
-  void UpdateChildrenAddressType();
+  void UpdateChildrenAddressType() {
+    GetRoot()->DoUpdateChildrenAddressType(*this);
+  }
 
   lldb::ValueObjectSP GetValueForExpressionPath_Impl(
       llvm::StringRef expression_cstr,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to