[Lldb-commits] [lldb] 51d8c59 - [lldb] Don't model std::atomic as a transparent data structure in the data formatter
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 18 02:22:34 PST 2020
Author: Raphael Isemann
Date: 2020-02-18T11:22:12+01:00
New Revision: 51d8c598331b25568b38691575c39729ae81a059
URL: https://github.com/llvm/llvm-project/commit/51d8c598331b25568b38691575c39729ae81a059
DIFF: https://github.com/llvm/llvm-project/commit/51d8c598331b25568b38691575c39729ae81a059.diff
LOG: [lldb] Don't model std::atomic as a transparent data structure in the data formatter
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
Reviewers: JDevlieghere, jingham, shafik
Reviewed By: shafik
Subscribers: shafik, christof, jfb, abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D74310
Added:
Modified:
lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/main.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
index b045c95c076a..45d4322e93d7 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "LibCxxAtomic.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
using namespace lldb;
using namespace lldb_private;
@@ -100,8 +101,6 @@ class LibcxxStdAtomicSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
size_t GetIndexOfChildWithName(ConstString name) override;
- lldb::ValueObjectSP GetSyntheticValue() override;
-
private:
ValueObject *m_real_child;
};
@@ -127,26 +126,20 @@ bool lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
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 *
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py
index 48e77c1a885d..908a2d0e3722 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py
@@ -42,13 +42,19 @@ def test(self):
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")
@@ -58,3 +64,9 @@ def test(self):
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"])
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/main.cpp
index 516331efdde5..a73e8d778259 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/atomic/main.cpp
@@ -8,6 +8,18 @@
#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;
@@ -19,7 +31,11 @@ int main ()
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.
}
More information about the lldb-commits
mailing list