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. } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
