[lldb] Don't model std::atomic as a transparent data structure in the data formatter

[teemperor]

Summary

Currently the data formatter is treating std::atomic variables as transparent wrappers
around their underlying value type. This causes that when printing std::atomic<A *>, the data
formatter will forward all requests for the children of the atomic variable to the A * pointer type
which will then return the respective members of A. If A in turn has a member that contains
the original atomic variable, this causes LLDB to infinitely recurse when printing an object with
such a std::atomic pointer member.

We could implement a workaround similar to whatever we do for pointer values but this patch
just implements the std::atomic formatter in the same way as we already implement other
formatters (e.g. smart pointers or std::optional) that just model the contents of the as a child
"Value". This way LLDB knows when it actually prints a pointer and can just use its normal
workaround if "Value" is a recursive pointer.

Fixes rdar://59189235

[shafik]

LGTM otherwise.

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py

70 ↗

(On Diff #243508)

What about inspecting p and p.child.parent?

[teemperor]

• Expand tests as suggested by Shafik (thanks!)

lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp

 //===-- LibCxxAtomic.cpp --------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "LibCxxAtomic.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
 //
 // We are supporting two versions of libc++ std::atomic
 //
public:
   lldb::ValueObjectSP GetChildAtIndex(size_t idx) override;
 
   bool Update() override;
 
   bool MightHaveChildren() override;
 
   size_t GetIndexOfChildWithName(ConstString name) override;
 
-  lldb::ValueObjectSP GetSyntheticValue() override;
-
 private:
   ValueObject *m_real_child;
 };
 } // namespace formatters
 } // namespace lldb_private
 
 lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
     LibcxxStdAtomicSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)

 
 bool lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
     MightHaveChildren() {
   return true;
 }
 
 size_t lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
     CalculateNumChildren() {
-  return m_real_child ? m_real_child->GetNumChildren() : 0;
+  return m_real_child ? 1 : 0;
 }
 
 lldb::ValueObjectSP
 lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::GetChildAtIndex(
     size_t idx) {
-  return m_real_child ? m_real_child->GetChildAtIndex(idx, true) : nullptr;
+  if (idx == 0)
+    return m_real_child->GetSP()->Clone(ConstString("Value"));
+  return nullptr;
 }
 
 size_t lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
     GetIndexOfChildWithName(ConstString name) {
-  return m_real_child ? m_real_child->GetIndexOfChildWithName(name)
-                      : UINT32_MAX;
-}
-
-lldb::ValueObjectSP lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
-    GetSyntheticValue() {
-  if (m_real_child && m_real_child->CanProvideValue())
-    return m_real_child->GetSP();
-  return nullptr;
+  return formatters::ExtractIndexFromString(name.GetCString());
 }
 
 SyntheticChildrenFrontEnd *
 lldb_private::formatters::LibcxxAtomicSyntheticFrontEndCreator(
     CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
   if (valobj_sp)
     return new LibcxxStdAtomicSyntheticFrontEnd(valobj_sp);
   return nullptr;
 }

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py

def test(self):
         lldbutil.skip_if_library_missing(
             self, self.target(), lldbutil.PrintableRegex("libc\+\+"))
 
         # The stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
                     substrs=['stopped',
                              'stop reason = breakpoint'])
 
-        s = self.get_variable('s')
-        i = self.get_variable('i')
+        s_atomic = self.get_variable('s')
+        i_atomic = self.get_variable('i')
 
         if self.TraceOn():
-            print(s)
+            print(s_atomic)
         if self.TraceOn():
-            print(i)
+            print(i_atomic)
+
+        # Extract the content of the std::atomic wrappers.
+        self.assertEqual(s_atomic.GetNumChildren(), 1)
+        s = s_atomic.GetChildAtIndex(0)
+        self.assertEqual(i_atomic.GetNumChildren(), 1)
+        i = i_atomic.GetChildAtIndex(0)
 
         self.assertTrue(i.GetValueAsUnsigned(0) == 5, "i == 5")
         self.assertTrue(s.GetNumChildren() == 2, "s has two children")
         self.assertTrue(
             s.GetChildAtIndex(0).GetValueAsUnsigned(0) == 1,
             "s.x == 1")
         self.assertTrue(
             s.GetChildAtIndex(1).GetValueAsUnsigned(0) == 2,
             "s.y == 2")
+
+        # Try printing the child that points to its own parent object.
+        # This should just treat the atomic pointer as a normal pointer.
+        self.expect("frame var p.child", substrs=["Value = 0x"])
+        self.expect("frame var p", substrs=["parent = {", "Value = 0x", "}"])
+        self.expect("frame var p.child.parent", substrs=["p.child.parent = {\n  Value = 0x"])

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/main.cpp

 //===-- main.cpp --------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <atomic>
 
+// Define a Parent and Child struct that can point to each other.
+class Parent;
+struct Child {
+  // This should point to the parent which in turn owns this
+  // child instance. This cycle should not cause LLDB to infinite loop
+  // during printing.
+  std::atomic<Parent*> parent{nullptr};
+};
+struct Parent {
+  Child child;
+};
+
 struct S {
     int x = 1;
     int y = 2;
 };
 
 int main ()
 {
     std::atomic<S> s;
     s.store(S());
     std::atomic<int> i;
     i.store(5);
-    
+    Parent p;
+    // Let the child node know what its parent is.
+    p.child.parent = &p;
+
     return 0; // Set break point at this line.
 }